-
Notifications
You must be signed in to change notification settings - Fork 1
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
Adding is_manager
feild
#99
base: main
Are you sure you want to change the base?
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.
Implementation looks good but this seems to include the changes from #98.
My bad, I was making this change while on that branch and switched to this one with the changes. Just fixed it |
Added the logic for the |
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.
LGTM.
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.
Actually... please add a test for the view function change.
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'm going to be super picky and say that I'd prefer a linear history for the migrations. Rather than merging you can just delete the migrations added in this PR and makemigrations
again and you should end up with a single migration.
Description
Adding is_manager field
Fixes #70 and #72 (issue)
I have added the new field to the model
Type of change
Key checklist
python -m pytest
)python -m sphinx -b html docs docs/build
)pre-commit run --all-files
)Further checks