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.

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: allow timed exams for providers that are no longer valid #1237

Merged
merged 4 commits into from
Oct 3, 2024

Conversation

alangsto
Copy link
Contributor

@alangsto alangsto commented Oct 2, 2024

Description:

If a proctoring backend is removed, timed exams should render as expected. This PR also fixes a bug where the instructor dashboard was not available for courses that had a proctoring provider that was no longer valid.

JIRA:

COSMO-515
COSMO-513

Pre-Merge Checklist:

  • Updated the version number in edx_proctoring/__init__.py and package.json if these changes are to be released.
  • Described your changes in CHANGELOG.rst
  • Confirmed Github reports all automated tests/checks are passing.
  • Approved by at least one additional reviewer.

Post-Merge:

  • Create a tag matching the new version number.

@alangsto alangsto force-pushed the alangsto/fix_timed_exams branch 3 times, most recently from 5325ae7 to ba4bc5d Compare October 2, 2024 18:54
@alangsto alangsto force-pushed the alangsto/fix_timed_exams branch from ba4bc5d to 6151238 Compare October 2, 2024 18:55
@@ -2943,7 +2943,7 @@ def get_student_view(user_id, course_id, content_id,
is_proctored_exam = exam['is_proctored'] and not exam['is_practice_exam']
is_timed_exam = not exam['is_proctored'] and not exam['is_practice_exam']

exam_backend = get_backend_provider(name=exam['backend'])
exam_backend = get_backend_provider(exam=exam)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If an exam is passed in, as opposed to the provider name, a timed exam will always return "None".

if get_backend_provider(name=exam.backend).has_dashboard:
return True
try:
if get_backend_provider(name=exam.backend).has_dashboard:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't want this case to throw an error because we still want the instructor dashboard to load for courses that still have software secure chosen.

backend = get_backend_provider(name=exam['backend'])
backend.on_exam_saved(exam)
try:
backend = get_backend_provider(name=exam['backend'])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

courses with software secure should be allowed to switch from a proctored to a timed exam. We don't want this code to throw an error in that case

instance.id,
instance.proctored_exam.backend,
)
if instance.proctored_exam.is_proctored:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only remove from the backend if it is a proctored exam, so this does fix a bug where instructors wouldn't be able to delete a timed exam.

[Question] For the case of PSI, do we care if removing attempts blows up?

@alangsto alangsto force-pushed the alangsto/fix_timed_exams branch 5 times, most recently from 2c5ae05 to 33df82a Compare October 3, 2024 17:04
@alangsto alangsto force-pushed the alangsto/fix_timed_exams branch from 33df82a to 2c1033d Compare October 3, 2024 17:12
Copy link
Contributor

@schenedx schenedx left a comment

Choose a reason for hiding this comment

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

👍

@alangsto alangsto merged commit 5384a26 into master Oct 3, 2024
53 of 55 checks passed
@alangsto alangsto deleted the alangsto/fix_timed_exams branch October 3, 2024 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants