Sage-Bionetworks / rocc-schemas

OpenAPI specification of the Registry of Open Community Challenges
Apache License 2.0
0 stars 0 forks source link

Update Challenge Organizer schemas #192

Closed tschaffter closed 3 years ago

tschaffter commented 3 years ago

I noticed that the schemas needs some modifications while implementing the API service. Each proposed changes is added in a separate comment so they can be up-voted/down-voted individually.

@rrchai Moving ahead with the implementation of the changes proposed below but would still like to know what you think of them.

tschaffter commented 3 years ago

Replace PageOfChallengeOrganizers by ChallengeOrganizerList

This would not be the first as we already have ArrayOfTopics - which I would rename ListOfTopics because I like the shorter name. "List" are suitable when the number of items is expected to be limited (e.g. < 50). This is applies to the number of organizers of a challenge. I initially thought that we would support a search of organizers, but now I actually think that the search will return user objects and so PageOfUsers can be reused. We could also add back PageOfChallengeOrganizers in addition of ListOfChallengeOrganizers if needed.

Beside the shorter name, a List is easier to implement as there is no extra query parameters like offset and limit and less metadata like the URL to the next page.

If the Page container is preferred, then add missing query parameters offset and limit.

tschaffter commented 3 years ago

Rename ListOfChallengeOrganizers.users to ListOfChallengeOrganizers.challengeOrganizers

In the current implementation, the name of the property that contain the array of organizers queries is users. This is because I envisioned that organizers would be User objects. However, ChallengeOrganizer is now a thing out of the necessity of capturing additional information about the organizer, like its role.

The long name maybe frightening, but I recommend that we stick to the convention of naming the property according to the type of objects contained in the array.

tschaffter commented 3 years ago

Rename ChallengeOrganizer.organizerRoles to ChallengeOrganizer.roles

I initially thought that the organizer may have Challenge management roles, like being an admin and simple user and being able to manage the challenge object. Naming the property organizerRoles was a way to prevent role confusion. However I think that giving access to users to manage the challenge should be done separately from the organizer list because organizers don't necessarily have an account (ChallengeOrganizer.login may be null).

Adopting the shorter name is preferred for now. We can revisit this if needed.

rrchai commented 3 years ago

They all make sense to me and I agree on these changes.