Change 881753 had a related patch set uploaded (by Cmelo; author: Cmelo):

[mediawiki/core@master] Add an option to be able filter users by group on UserMultSelectWidget

https://gerrit.wikimedia.org/r/881753

Change 881752 had a related patch set uploaded (by Cmelo; author: Cmelo):

[mediawiki/extensions/CampaignEvents@master] Add suport for multple organizers

https://gerrit.wikimedia.org/r/881752

Hey @cmelo , at this week's Design/Engineering meeting we decided to add a verification for users whose organizer right had been removed, so that they would no longer be able to administer event registrations they previously could. Do you think we should create a new task for the verification, or add to the acceptance criteria in this task?

Change 889124 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] Add support for multiple organizers to the API

https://gerrit.wikimedia.org/r/889124

Change 889129 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] Add support for multiple organizers to the frontend

https://gerrit.wikimedia.org/r/889129

@Daimona has caught some bugs; TBD whether the list is complete and whether they are release blocking:
T330362
T285252
T330353

Full list of related bugs:

• T330353: MenuTagMultiselectWidget duplicates card on click if allowDisplayInvalidTags is true - Noticeable user-visible effects, not too serious though
• T330362: "TypeError: Cannot read properties of null" when clicking invalid cards in MenuTagMultiselectWidget with allowDisplayInvalidTags=true - Probably not user-visible, but it would appear in the monitoring dashboards
• T285252: MenuTagMultiselectWidget removes selected item when tabbing away - User-visible, but pretty mild
• T330424: MenuTagMultiselectWidget unexpectedly removes card if invalid input is entered after the field loses focus - User-visible, also pretty mild

I think none of these are serious enough to block the release on their own, but they surely add up, and it's 4 of them... It's probably still fine to release as the MVP, but I'd like to have these fixed soon(TM).

Change 895234 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[operations/mediawiki-config@master] Enable $wgCampaignEventsEnableMultipleOrganizers on beta

https://gerrit.wikimedia.org/r/895234

Change 881752 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Add support for multiple organizers, backend only

https://gerrit.wikimedia.org/r/881752

Change 889129 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Add support for multiple organizers to the frontend

https://gerrit.wikimedia.org/r/889129

Change 895326 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] Fix crash in MWPermissionsLookup with anons

https://gerrit.wikimedia.org/r/895326

Change 895326 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Fix crash in MWPermissionsLookup with anons

https://gerrit.wikimedia.org/r/895326

Change 895234 merged by jenkins-bot:

[operations/mediawiki-config@master] Enable $wgCampaignEventsEnableMultipleOrganizers on beta

https://gerrit.wikimedia.org/r/895234

Change 889124 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Add support for multiple organizers to the API

https://gerrit.wikimedia.org/r/889124

Change 881753 abandoned by Cmelo:

[mediawiki/core@master] Add an option to be able filter users by group on UserMultSelectWidget

Reason:

https://gerrit.wikimedia.org/r/881753

@vaughnwalters We changed our implementation after T327470#8642549 was written, so the only expected bugs are:

• T285252 - Pressing tab while the field is focused will remove an item
• T331421 - If you have an event created by user A, add B as organizer, then go to Special:EditEventRegistration while logged in as B, you will see that the card with A is disabled. However, you can still remove A by highlighting their name in the dropdown menu of the field and pressing enter. Backend validation will still make the form submission fail.

Acceptance Criteria:

• ✅ Allow organizer to add other organizers to event registration by specifying their username
  • ✅ The text should read: "Add the usernames of other organizers to give them permission to manage this event."
    

• ✅ Allow organizer to remove organizer
  
  • ✅ There should be an area for the organizer to add in the usernames
• ✅ If a new organizer is added to an event, they have all basic organizer privileges, including being able to:
  • ✅ Edit event registration information
    
  • ✅ Cancel the registration of a participant
    
  • ✅ Open or close event registration
    
  • ✅ Delete event registration
    

• Should your username of the event creator be in the list?
  • ✅ Yes.
    
    
• Should you be able to remove yourself, as an event creator?
  • ✅ Every event should have at least 1 organizer. So you can only remove yourself if there is more than one organizer. If there is more than one organizer, you can remove yourself.
    

• If you're not the event creator, should you be able to remove the event creator?
  • ✅ No. The primary organizer name should be displayed separately and it should be non-editable by other organizers.
    
• Note that all organizers will have the same rights for V1, but they will have the option to have different rights in V2 (see T316138).
• 🔴 Update the API documentation for the new "set event organizers" endpoint.

Note: we need to check how the widget works when there's a large number of organizers.

• 🔴 ➡ Right now only 10 organizers can be added.

Design
🔴 When there is at least one username

• ➡ Placeholder text should be Add username

comp:

🔴 Warning message when the field is blank

• ➡ Placeholder text should be Add username
• ➡ Icon should be the yellow caution icon
• ➡ Copy in the error message should be Events need to have at least one organizer. Please add an organizer.

comp:

🔴 Error message when the user tries to the submit form with a blank field

• ➡ Placeholder text should be Add username
• ➡ Copy in the error message should be Events need to have at least one organizer. Please add an organizer.

comp:

Heya @cmelo A few notes after going through this, and i put a 🔴 and ➡ in the notes above, beside of the things I am talking about so it's hopefully easier to reference.

1. there is no documentation for this endpoint on the api doc - let me know whenever you get that added in as I would like to test the API as well

2. there is discrepancy between the comp and the build in the copy and icons if there are no organizers in the field, I made a few notes above about those.

3. Here is a bug I found:

There is an error when an organizer is either suppressed or deleted by central auth:

The user is correctly shown as deleted or suppressed, but then the organizer is no longer able to edit the event, and it displays the following internal error for suppressed user:

[ZBDHpMa7OaHm64f@SFLXygAAAAo] /wiki/Special:EditEventRegistration/118 MediaWiki\Extension\CampaignEvents\MWEntity\HiddenCentralUserException: Central ID 215879 belongs to a hidden user

screencap for Central ID 215879 belongs to a hidden user

and it displays the following internal error for deleted user:

[ZBDJTca7OaHm64f@SFLY7wAAAAQ] /wiki/Special:EditEventRegistration/118 MediaWiki\Extension\CampaignEvents\MWEntity\CentralUserNotFoundException: Central ID 215933 does not belong to any user

screencap of error for Central ID 215933 does not belong to any user

As this bug is fixed, I think it is worth considering special attention to the case if the creator of the event is deleted or suppressed, since that user under normal circumstances can not be removed by any other organizer of the event. So even if we wanted to not display deleted or suppressed users, or give organizers the ability to delete them, then there would possibly have to be some exception for the creator of the event, to allow them to be removed in this case.

Here is an example where I suppressed the creator. I added @cmelo as an organizer as well so you can see the error when you try to edit the registration:
https://en.wikipedia.beta.wmflabs.org/wiki/Special:EventDetails/209

4. Here is one more bug I found:

If you try to add anyone who can not be an organizer, it will quickly flash an error message, but it is too fast to be able to read, and but is still displays generic error message at the top of the page:

This is an example of trying to add a deleted user as organizer:
screencap:

Here is an example of trying to add a user that doesn't have permissions as an organizer:
screencap:

These helpful error messages should remain on the page so the user knows what the specific problem is

• 🔴 Update the API documentation for the new "set event organizers" endpoint.

Done, thanks!!! Update organizers API

Note: we need to check how the widget works when there's a large number of organizers.

• 🔴 ➡ Right now only 10 organizers can be added.

The limit of organizers per event is 10 for the MVP, I didn't find where we mention that, but I will add it to the acceptance criteria, thanks

@vaughnwalters @gonyeahialam @ldelench_wmf @VPuffetMichel

Change 899959 had a related patch set uploaded (by Cmelo; author: Cmelo):

[mediawiki/extensions/CampaignEvents@master] Change organizer filed place holder message

https://gerrit.wikimedia.org/r/899959

Change 899987 had a related patch set uploaded (by Cmelo; author: Cmelo):

[mediawiki/extensions/CampaignEvents@master] Fix - Icon should be the yellow caution icon Fix - Copy in the error message should be ...

https://gerrit.wikimedia.org/r/899987

🔴 Warning message when the field is blank

• ➡ Placeholder text should be Add username
• ➡ Icon should be the yellow caution icon

For consistence, I think the icon should be the same as when the user submits the form, also because this is trick to implement (I mean show one icon on live validation and another one after submitting the form), the JS is responsible for adding this message, so there is not a beauty way to know whether this is a live validation or not.

@vaughnwalters @gonyeahialam

Change 899987 abandoned by Cmelo:

[mediawiki/extensions/CampaignEvents@master] Fix - Icon should be the yellow caution icon Fix - Copy in the error message should be ...

Reason:

https://gerrit.wikimedia.org/r/899987

Change 899959 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Change organizer field placeholder message

https://gerrit.wikimedia.org/r/899959

Change 902181 had a related patch set uploaded (by Cmelo; author: Cmelo):

[mediawiki/extensions/CampaignEvents@master] Fix error when an organizer is deleted on edit event special page Do not display "deleted user" in case an organizer is deleted Display "This event doesn't have an organizer" in case there is no organizer

https://gerrit.wikimedia.org/r/902181

Change 902408 had a related patch set uploaded (by Cmelo; author: Cmelo):

[mediawiki/extensions/CampaignEvents@master] Change error messsage in case the user remove all organizers

https://gerrit.wikimedia.org/r/902408

Change 902181 had a related patch set uploaded (by Cmelo; author: Cmelo):

[mediawiki/extensions/CampaignEvents@master] Fix error when an organizer is deleted on edit event special page

https://gerrit.wikimedia.org/r/902181

Change 902181 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Fix error when an organizer is deleted on edit event special page

https://gerrit.wikimedia.org/r/902181

Change 902408 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Change error messsage in case the user remove all organizers

https://gerrit.wikimedia.org/r/902408

I think all issues reported by @vaughnwalters have been fixed, except for the flashing text issue that I'm unable to reproduce.

In T327470#8742610, @Daimona wrote:

I think all issues reported by @vaughnwalters have been fixed, except for the flashing text issue that I'm unable to reproduce.

@Daimona here is an event that I created and added you as organizer so you can test this:
https://en.wikipedia.beta.wmflabs.org/wiki/Special:EditEventRegistration/212

try adding this user as an organizer (you might have to delete the last two characters the 12 to be able to search correctly for it)
v13827615369812

here is what I am seeing:

and the quick flash that is showing is this:

Let me know if you are still having problems reproducing

Thanks, now I get it. I wasn't able to reproduce it locally, but it's perfectly reproducible on beta. The reason is that the configuration is different: my local is configured so that only users in a given user group can be organizers, whereas on beta everyone can be an organizer. This specific issue happens when a user has the needed organizer rights (and on beta, every logged-in user does), but fails some other permission check; in particular, I think this can only happen if they're blocked or do not have a central account. This is a known limitation, as the live validation is only meant to perform the simplest permission checks. The trouble is, when we setup the live validation, all errors on the field are erased. We shouldn't be doing that, we should merge and selectively remove errors instead. This is something I remember noticing in code review, but then we forgot about it. Let me try and fix it.

Change 904612 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] Add extra layer of validation for organizers when editing an event

https://gerrit.wikimedia.org/r/904612

Hmmmm this is a bit of a mess... The above can be done, but then the result becomes something between ugly and wrong when you start using the field (i.e., duplicated or out-of-date error messages). I tried a different approach.

Change 904612 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Add extra layer of validation for organizers when editing an event

https://gerrit.wikimedia.org/r/904612

Api checks:
✅ 400 Organizer who is not the event creator is not allowed to remove the event creator.

✅ 400 Can not add more than 10 organizers

✅ 400 If a user does not exist, an error displays that the user doesn't have the needed permissions to be an organizer. Technically this is correct, since the user does not exist they can not have the needed permissions to be an organizer.

✅ 403 if not allowed to edit event registration (logged in user is not an organizer of the event in this case):

✅ 404 if event does not exist:

Notes from defects found earlier that are now corrected:

✅ The flashing error text noted at T327470#8742742 is now fixed:

✅ Suppressed organizers no longer cause the page to have errors:

✅ Deleted organizers no longer cause the page to have errors:

✅ The error message is now correct. Tagging @gonyeahialam to also sign off on the UX of this.

A few notes about the API:
🔴 The api doc for this endpoint is incorrect. It has id and organizer_usernames listed as parameters, but organizer_usernames should be listed as request schema, and id needs to be in the request URL. As it currently is documented, when I follow the docs and add them as parameters, I get this:

but when I add organizer_usernames into the body as request schema, it works correctly (it doesn't need id in the request body, only in the URL:

So, the endpoint is working correctly, it is just the doc that needs updated to reflect this.

🔴 If I put some extra characters in the organizer_username, then it throws a 500 error. Should this 500 error be handled differently or maybe just be documented in the doc? @cmelo @Daimona

These are the only two remaining things, let me know what you think of the 500 error and when the docs are updated and then we can move this to design sign off.

Change 905765 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] Manually validate organizer usernames in SetOrganizersHandler

https://gerrit.wikimedia.org/r/905765

@vaughnwalters Thanks, I've already updated the API docs, and pushed a fix for the validation issue.

Change 905765 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Manually validate organizer usernames in SetOrganizersHandler

https://gerrit.wikimedia.org/r/905765

In T327470#8757365, @Daimona wrote:

@vaughnwalters Thanks, I've already updated the API docs, and pushed a fix for the validation issue.

✅ Api docs for this endpoint is correct now

✅ Validation issue for extra characters in the organizer_username is fixed