Initial attempt at grouped allowance endpoint #892
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added grouped allowance function within views and the endpoint within urls.py. Added test cases to test_views.py.
schenedx
requested changes
Jun 21, 2021
edx_proctoring/views.py
Outdated
| for allowance in all_allowances: | ||
| serialied_allowance = ProctoredExamStudentAllowanceSerializer(allowance).data | ||
| user_id = serialied_allowance['user']['id'] | ||
| if user_id in grouped_allowances: |
There was a problem hiding this comment.
Use setdefault function here would help simplify these 4 lines
|
|
||
| **Expected Response** | ||
| HTTP GET: | ||
| The response will contain a dictionary with the allowances of a course grouped by student. |
There was a problem hiding this comment.
#NIT can you provide an example response here? It makes the comments on this function much easier to read.
edx_proctoring/tests/test_views.py
Outdated
| grouped_allowances = response_data['grouped_allowances'] | ||
| self.assertEqual(len(grouped_allowances), 3) | ||
| # Check that all users allowances are inputted correctly | ||
| first_user = str(user_list[0].id) |
There was a problem hiding this comment.
Could you write a function that would compare the returned dictionary with the expected dictionary? We have assertDictEqual function to help make sure all the elements are correct.
| self.assertEqual(len(grouped_allowances[first_user]), 2) | ||
| self.assertNotEqual(grouped_allowances[first_user][0], grouped_allowances[first_user][1]) | ||
|
|
||
| def test_get_grouped_allowances_non_staff(self): |
There was a problem hiding this comment.
You are missing a test case where the user is a course staff, not a global staff. For example, see https://github.com/edx/edx-proctoring/blob/f1ebee0814ac45f7b11d030df4991f4ccea082bd/edx_proctoring/tests/test_views.py#L473
Added comments/refactored code to be more concise/readable. Adjust test case to be more thorough.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added grouped allowance function within views and the endpoint within urls.py. Added test cases to test_views.py.
Description:
Describe in a couple of sentences how this pull request modifies the repository.
JIRA:
MST-845
Pre-Merge Checklist:
edx_proctoring/__init__.pyandpackage.jsonif these changes are to be released.CHANGELOG.rstPost-Merge: