-
-
Notifications
You must be signed in to change notification settings - Fork 210
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
Add an endpoint to get all donors with flairs #2988
base: master
Are you sure you want to change the base?
Conversation
Hello @amCap1712! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2024-10-24 17:21:23 UTC |
@crossdomain | ||
@ratelimit() | ||
def all_flairs(): | ||
""" Get flairs (including expired ones) for all users """ |
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.
I think we need more details in this endpoint documentation. What does this endpoint return? Does the caller need to care about expired flairs?
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.
fixed.
@@ -27,6 +27,8 @@ def update_flair(): | |||
if "flair" not in request.json: | |||
raise APIBadRequest("Missing flair") | |||
|
|||
# todo: add flair validation |
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.
Does this need to be done now or add a ticket?
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.
this needs to be done (if at all) after the structure of the flairs has been decided.
No description provided.