Skip to content
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.

Sign up for GitHub

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

fix: grant course staff exam access tokens #359

Merged
merged 3 commits into from
Feb 13, 2025
Merged

fix: grant course staff exam access tokens #359

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
39b2793
fix: grant course staff exam access tokens
ilee2u Feb 13, 2025
9dd1cc5
test: refine tests
ilee2u Feb 13, 2025
0879fa7
chore: update docstring
ilee2u Feb 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions edx_exams/apps/api/v1/tests/test_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,26 @@ def test_access_no_due_date(self, attempt_status, response_status):
if response_status == 200:
self.assert_valid_exam_access_token(response, self.user, no_due_date_exam)

def test_access_user_is_course_staff(self):
"""
Verify the endpoint grants access for an exam
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you agree with my suggestion below, please update the docstring and the test.

with no due date, if started exam attempt.
"""
allowed_time_limit_mins = self.exam.time_limit_mins
start_time = timezone.now() - timedelta(minutes=allowed_time_limit_mins/2)
ExamAttempt.objects.create(
user=self.user,
exam=self.exam,
attempt_number=1,
status='started',
start_time=start_time,
allowed_time_limit_mins=allowed_time_limit_mins
)

response = self.get_exam_access(self.user, self.url)
self.assertEqual(200, response.status_code)
self.assert_valid_exam_access_token(response, self.user, self.exam)

def test_access_not_granted_if_hide_after_due(self):
"""
Verify the endpoint does not grant access for past-due exam
Expand Down
4 changes: 4 additions & 0 deletions edx_exams/apps/api/v1/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,10 @@ def get_response(cls, exam, user):
elif exam.due_date is not None and timezone.now() >= exam.due_date:
grant_access, response_status = True, status.HTTP_200_OK

# If user is course staff, then grant them access
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what we want, but I think we should have this before line 369. On the frontend, access to the contents of the exam will always be granted to staff members. Because of this, they never see any exam functionality and will not be able to start an exam attempt. Therefore, we can short circuit early and give them access immediately if they are a staff member - no need to look at exam attempts. This makes the code easier to reason about. Does that match your understanding?

elif user.has_course_staff_permission(exam.course_id):
grant_access, response_status = True, status.HTTP_200_OK

if grant_access:
log.info('Creating exam access token')
access_token = sign_token_for(user.lms_user_id, expiration_window, claims)
Expand Down
Loading