Page MenuHomePhabricator
Create Task
Maniphest T319455

API to switch between public/private registration [HIGH PRIORITY]
Closed, ResolvedPublic
Actions

Description

On our last engineering meeting, we decided that we will use the register/unregister API.

Acceptance criteria

  • Add a new parameter to the register/unregister API , "is_private", if it is true set the registration as private, if false set it as public.
  • Document this parameter on mw.org
  • Given a participant changes its registration from private to public
    • Then we must update the "cep_private" to false on "ce_participants"
  • Given a participant changes its registration from public to private
    • Then we must update the "cep_private" to true on "ce_participants"

Details

ProjectBranchLines +/-Subject
mediawiki/extensions/CampaignEventsmaster+316 -92Add support for private registrations
Customize query in gerrit

Related Objects
Search...

Event Timeline

ifried created this task.Oct 5 2022, 5:50 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 5 2022, 5:50 PM
Daimona added a subscriber: Daimona.Oct 5 2022, 9:46 PM

Preliminary note: since the REST API framework does not support PATCH requests, it's probably easier to include it in the existing endpoint.

cmelo added a subscriber: cmelo.Oct 10 2022, 1:22 PM
In T319455#8288901, @Daimona wrote:

Preliminary note: since the REST API framework does not support PATCH requests, it's probably easier to include it in the existing endpoint.

Sounds good to me, do we have a ticket for that? If not, we can update this one, and add the acceptance criteria.

Daimona added a comment.Oct 10 2022, 1:39 PM

I think we can just use this one.

Daimona added a parent task: T316405: [EPIC] Allow participant option of joining event privately.Oct 10 2022, 1:40 PM
cmelo updated the task description. (Show Details)Oct 13 2022, 12:20 PM
cmelo added a subscriber: MHorsey-WMF.Oct 13 2022, 12:22 PM

Hi @Daimona and @MHorsey-WMF I updated the acceptance criteria, please let me know what you think.

Daimona updated the task description. (Show Details)Oct 13 2022, 12:41 PM
Daimona updated the task description. (Show Details)
Daimona added a comment.Oct 13 2022, 12:44 PM
In T319455#8314068, @cmelo wrote:

Hi @Daimona and @MHorsey-WMF I updated the acceptance criteria, please let me know what you think.

LGTM, I've reformatted it a bit and removed the part about the parameter not being passed. Since this is a PUT endpoint, the client is required to provide a full representation of the resource, and "private" needs to be part of that representation.

The alternative would be to create a PATCH endpoint, but that's not possible due to the reasons already discussed.

Daimona added a comment.Oct 13 2022, 12:47 PM
In T319455#8314123, @Daimona wrote:
In T319455#8314068, @cmelo wrote:

Hi @Daimona and @MHorsey-WMF I updated the acceptance criteria, please let me know what you think.

LGTM, I've reformatted it a bit and removed the part about the parameter not being passed. Since this is a PUT endpoint, the client is required to provide a full representation of the resource, and "private" needs to be part of that representation.

To clarify, I think we could make the parameter optional and still respect the REST specification, but the correct implementation would be:

  • Given the register/unregister API is called and the private parameter is not passed
    • cep_private is set to a default value that is not dependent on the previous state, so always true or always false

Which I don't think is a good idea.

cmelo added a comment.Oct 13 2022, 1:30 PM
In T319455#8314124, @Daimona wrote:
In T319455#8314123, @Daimona wrote:
In T319455#8314068, @cmelo wrote:

Hi @Daimona and @MHorsey-WMF I updated the acceptance criteria, please let me know what you think.

LGTM, I've reformatted it a bit and removed the part about the parameter not being passed. Since this is a PUT endpoint, the client is required to provide a full representation of the resource, and "private" needs to be part of that representation.

To clarify, I think we could make the parameter optional and still respect the REST specification, but the correct implementation would be:

  • Given the register/unregister API is called and the private parameter is not passed
    • cep_private is set to a default value that is not dependent on the previous state, so always true or always false

Which I don't think is a good idea.

Thanks @Daimona I totally agree.

Daimona updated the task description. (Show Details)Oct 13 2022, 1:38 PM
cmelo updated the task description. (Show Details)Oct 13 2022, 7:37 PM
Daimona assigned this task to MHorsey-WMF.Oct 17 2022, 2:01 PM
Daimona edited projects, added Campaign-Tools (Campaign-Tools-Current-Sprint); removed Campaign-Tools.
Daimona updated the task description. (Show Details)
Daimona moved this task from Needs Input / Design in progress to Code Review/Tech Input 💬 on the Campaign-Tools (Campaign-Tools-Current-Sprint) board.
gerritbot added a comment.Oct 18 2022, 10:06 AM

Change 839633 had a related patch set uploaded (by Mhorsey; author: Mhorsey):

[mediawiki/extensions/CampaignEvents@master] Add support for private registrations

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

gerritbot added a project: Patch-For-Review.Oct 18 2022, 10:06 AM
MHorsey-WMF added a comment.Oct 26 2022, 2:28 PM

@Daimona @cmelo - ESLINT does not like is_private and demands camel case. I have implemented it as such. Thoughts?

Daimona added a comment.Oct 26 2022, 2:43 PM
In T319455#8346038, @MHorsey-WMF wrote:

@Daimona @cmelo - ESLINT does not like is_private and demands camel case. I have implemented it as such. Thoughts?

eslint is stupid, unfortunately, like all linters... It disallows snake_case in general, and doesn't understand when it's used for API calls. But snake_case is allowed in API parameters, so the eslint warning should be suppressed. There are some examples of that, e.g. here.

gerritbot added a comment.Oct 28 2022, 12:20 PM

Change 839633 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Add support for private registrations

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

MHorsey-WMF mentioned this in rUCAM235c6da74e38: Add support for private registrations.Oct 28 2022, 12:21 PM
Maintenance_bot removed a project: Patch-For-Review.Oct 28 2022, 12:30 PM
Daimona moved this task from Code Review/Tech Input 💬 to QA 🐛 on the Campaign-Tools (Campaign-Tools-Current-Sprint) board.Oct 28 2022, 1:23 PM
Daimona mentioned this in T318121: Add support for private registration to the backend code.
• vyuen moved this task from Backlog to V1 (MVP) on the Campaign-Registration board.Oct 31 2022, 4:45 PM
• vyuen changed the task status from Open to In Progress.Oct 31 2022, 4:47 PM
vaughnwalters added a subscriber: vaughnwalters.Nov 1 2022, 3:48 PM

There is overlap between this ticket and T318121, so notes are included there

vaughnwalters closed this task as Resolved.Nov 1 2022, 9:39 PM
vaughnwalters moved this task from QA 🐛 to Done 🏁 on the Campaign-Tools (Campaign-Tools-Current-Sprint) board.
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL