-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: add view for allowance creation #291
Conversation
b3bc7e6
to
4d20e9b
Compare
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
02a8463
to
0ce433a
Compare
|
||
def test_invalid_post(self): | ||
""" | ||
Test that 400 response is returned if serializer is invalid |
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.
Can you add a test where none of the expected query strings are provided? No exam_id
, no user_id
, and no extra_time_mins
? What about test that some querystrings are missing?
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.
👍
eb0f619
to
516f42c
Compare
if allowance.get('username') | ||
else User.objects.get(email=allowance['email']) | ||
), | ||
exam=Exam.objects.get(id=allowance['exam_id']), |
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.
Normally I'd say don't query inside a loop but it's probably okay here. May be a waste of complexity optimizing it. It could be worth at least commenting that we expect the number of allowances to be reasonably small.
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.
👍
516f42c
to
f12bceb
Compare
f12bceb
to
f3c4d89
Compare
JIRA: COSMO-369
Description: This PR adds the ability to create one or more allowances via a POST request.
Merge checklist:
Post merge: