-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
initial commit of membership endpoint #20
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the memberships idea to the API description. I had some feedback on the repeated schema descriptions - these can be combined to help keep things consistent. The data is very similar between the membership types so there's no need to describe separate payloads.
type: float | ||
example: 100 | ||
PatronMembership: | ||
ription: Benefits of a patron membership |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ription: Benefits of a patron membership | |
description: Benefits of a patron membership |
- Patron | ||
- Benefactor | ||
example: Family | ||
FamilyMembership: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These three schemas named *Membership
are identical in structure. To ensure consistency, they should be one schema.
description: Annual cost of a membership | ||
type: float | ||
example: 1000 | ||
BuyFamilyMembershipRequest: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These identically-structured schemas Buy*MembershipRequest
should be a single schema since the requests will all the be same structure of payload, with different details included. Please combine them.
- membershipLevel | ||
- startDate | ||
BuyFamilyMembershipResponse: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Combine the Buy*MembershipResponses
into a single schema unless some membership types have distinctly different structures. The goal of re-using schemas in an API is to ensure consistency - with these duplicated schemas, it would be easy to change one, or miss changing one, and introduce inconsistency. Since the structure is the same, they should use the same schema.
@@ -628,6 +859,24 @@ components: | |||
- date: "2023-09-22" | |||
timeOpen: "10:00" | |||
timeClose: "16:00" | |||
BuyFamilyMembershipExample: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple examples are great to illustrate the different expected data fields when the structures are the same. Thanks for adding these (no changes needed, I'm just happy to see the examples)!
Closing due to inactivity |
What/Why/How?
Initial pass at creating a membership system with multiple tiers for museum patrons.
issue #17
Submitted for feedback!
Reference
Testing
Screenshots (optional)
Check yourself
Security