Also, @gonyeahialam just to confirm: from a user's perspective, what happens after they submit the form, assuming the submission is valid? Should we send the user to Special:InvitationList and show the pending state (T364802)?

On second thought, this task really is blocked. The patch above is blocked on the creation of the schema (T366354). There's also one more patch I need to write which is blocked on both the creation of the schema, and the validation (T365629).

Resolving this, and eagerly awaiting to learn how I managed to screw something up in some new fun way.

@ifried One more question. I assume we want to also display an error if the invitation list name is empty, or it contains only spaces. I wrote the following message for this:

In T368830#9939471, @matmarex wrote:

This came up in the past in the context of on-wiki message overrides adding not just lists, but also tables and stuff.

• T230229: With two messages in Abusefilter errorbox, second box is indented
• T303060: Status::getWikiText should not be used for EditFilterMergedContentHook failures

In T368821#9939464, @matmarex wrote:

Maybe that means that nothing important relies on it, and it could be changed?

I think we're not too far away. Adding the constructor is maybe a small price we can pay. The one thing I would like for phan to do is to better handle the union type when the template type isn't specified (i.e., just Status instead of Status<Something>). Like resolving the templated type to mixed (or equivalent polymorphic type). This would let us drop the |mixed and get all the benefits when using the full Status<T> syntax, while retaining the existing behaviour for plain Status. The problem is, I doubt that's going to happen given the state of upstream.

Is this a fresh bug report? I'm unable to reproduce this, see screenshots below.

In T365629#9936537, @ifried wrote:

• Language corrected to present tense. Thanks for the catch!
• Yes, it should be "article" if there is only article.

In T357782#9934597, @kostajh wrote:

I am not sure if AbuseFilter makes sense for this use case

Also wondering about

In T365629#9919233, @ifried wrote:

I have updated the AC to have just one error message for all types of invalid article titles, and we will include the list of articles below. This way, we have a more simplified implementation that does not require logic for specific types of title errors, while still giving the user helpful information on which articles have errors in the titles.

Classic PEBKAC -- the host was changed as part of T345566. Docs updated.

In T368617#9930694, @Daimona wrote:

What I still need to understand is why phan didn't flag the problem.

I have a fix for this and will push it shortly. What I still need to understand is why phan didn't flag the problem.

I found out that the event causing the exception is https://meta.wikimedia.beta.wmflabs.org/wiki/Special:EventDetails/119, whose organizer's account has been deleted. I still have to reproduce the actual exception though, and T368620 certainly doesn't help.

This work is now complete, and since it's not testable, I'm closing the task.

@vaughnwalters Sorry, there have been CI issues earlier today and the page didn't get merged. I also just made a couple more tweaks to the patch, hence moving this back to code review.

In T365629#9913760, @Daimona wrote:

There are more reasons why a title can be invalid. MW provides a set of error messages with more information about each case, see list. Could we just reuse this? Note, if doing so, we would have to display one error per page (instead of a single error with a list of affected pages). I think this might be fine though, given the average number of errors?

I have reordered the AC so that it's easier to see which criteria refer to the event page, and which to the worklist.

@gonyeahialam Two questions about the image:

• Is there a repository for these where we can copy the SVG from? Copying directly from Figma gives a hefty 4MB SVG, but I assume there's an optimized version somewhere.
• Since the image represents a written document, we need a flipped version of it for RTL languages. I would assume that the flipped version can be found where this image lives.

A few questions/thoughts:

• The AC say that the title of the page would be "Invitation lists", but the prototype has "My invitation lists". Which one is correct?
• T365068 has been closed as duplicate. Should we just copy the relevant parts from the AC of T356679, for what concerns feature enabled vs disabled and logged out vs not authorized vs authorized?
• The explanatory text and button label are also different, so I'd like to confirm whether we should go with what's in the AC:

In T356679#9910863, @vaughnwalters wrote:

1. The generate button should be full width on mobile

Hi @Wargo, are you still planning to work on this? Please let me know if you need any help.

I just realized that this task is now in "Ready for development", but I think the AC are incomplete. Of the validations I listed in T365629#9889879, I only see a message for the "page must exist" one, but not for invalid titles, or for pages outside of the mainspace.

In T366354#9904630, @cmelo wrote:

@Daimona, yes, I removed the primary key from ce_invitation_list_users because of what you mention, and I also think it is not needed:
"I think we'll only need primary keys and the indices on invitation list IDs"

@cmelo I just realized that the ce_invitation_list_users table does not currently have a primary key.

In T365629#9900683, @ifried wrote:

@gonyeahialam & @Daimona: For the event page title, can we only display the event pages in which the user is an organizer (when the dropdown appears during the time that they are filling in the field)?

Hey @ifried, I have two minor questions that came up in code review.

1. Should the two notices ("This wiki does not have invitation lists enabled" and "You are not allowed to use invitation lists") end with a full stop? This isn't very clear in the AC.
2. What should happen if a logged-out user visits the Special:GenerateInvitationList page and the feature is not enabled? Should we tell them that the feature is disabled, or should we ask them to login, and then tell them that it's disabled? I think the first, but I'd like to hear your thoughts on this.

I think a way to test this would be to 1) confirm that there's a substantial reduction in the number of queries, especially when showing many events (can be quickly seen in the debug toolbar), and 2) verify that the information shown is still correct.

Thank you. Is this ready for DBA review then?

In T365629#9890374, @gonyeahialam wrote:

With the component we are going to use for this, where they have to select form a dropdown that appears as they type, is it still possible to input the wrong event page? @Daimona

I was going through this again, and I have a few questions/thoughts:

• AIUI, "worklist" is just the list of articles. "Invitation list" is the list of users to invite, and we're also using this term to identify the combination of worklists + users. But then, shouldn't ce_worklists actually be called ce_invitation_lists? ce_worklists_articles is maybe fine, though I'm unsure about the plural in "worklists". ce_worklists_users would presumably be ce_invitation_list_users?
• Who should be able to view a given invitation list? I thought it would only be the person who created the list itself (not the event). If so, we should store the creator of the list in the ce_worklists table, else we don't know who each list belongs to.
• Thinking about indices. In SpecialMyInvitationLists, we'll need to filter by user and, optionally, wiki. So, I think we'll need a composite index on user + wiki (in this order). I don't think the application needs an index on the wiki alone, but unsure about whether it's needed for analytics. The other indices seem sufficient (I think we'll only need primary keys and the indices on invitation list IDs).
• There's also the page ID question that was discussed in code review, which should go with the DBA review.