-
Notifications
You must be signed in to change notification settings - Fork 9
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
Taxonomy view/management REST APIs #63
Taxonomy view/management REST APIs #63
Conversation
Thanks for the pull request, @rpenido! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
This comment was marked as outdated.
This comment was marked as outdated.
adfcbbd
to
7c93cd2
Compare
@pomegranited Updated this PR. I think we are ready to review here! |
2b6fd16
to
1b1b59a
Compare
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.
Hi @rpenido this is looking really good!
I've suggested two really minor changes, but this is ready for upstream review once they're addressed.
👍
- I tested this by running the dev server:
# Run migrations python3 manage.py migrate # Create a superuser python3 manage.py createsuperuser # and used the shell to create non-superusers # Run the dev server python3 manage.py runserver # Login using Django Admin (non-staff users can login, they just can't see Django Admin) # http://127.0.0.1:8000/admin/ # REST API is here: # http://127.0.0.1:8000/tagging/rest_api/v1/
- I read through the code
-
I checked for accessibility issuesN/A - Includes documentation
- Commit structure follows OEP-0051
ff5dda3
to
4172286
Compare
4172286
to
d532b0c
Compare
Thanks for applying those changes @rpenido ! This is ready for @ormsbee and/or @bradenmacdonald 's review now. |
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.
From a quick look, this looks fine. Just had two questions. I don't need to review this again; @pomegranited can give final approval and merge.
@pomegranited I made the changes with new commits: 2ccae75 and cec362d. That way, I think it is easier to review. Let me know if I should squash the commits, or you can do it yourself. Thank you! |
e2aeb9a
to
3548151
Compare
Description
This PR adds support for creating, viewing, updating and deleting Taxonomies via REST APIs, with the permissions applied accordingly.
Supporting Information
Testing instructions
To do before merge