Skip to content

Alert banner for proctoring settings error - #24960

Merged
zacharis278 merged 6 commits into
masterfrom
zhancock/proctoring-settings-error
Sep 17, 2020
Merged

Alert banner for proctoring settings error#24960
zacharis278 merged 6 commits into
masterfrom
zhancock/proctoring-settings-error

Conversation

@zacharis278

@zacharis278 zacharis278 commented Sep 10, 2020

Copy link
Copy Markdown
Contributor

MST-359

  • Adds message to course overview and advanced settings prompting users to fix invalid configuration
  • Cleaned up dependency between the HTML settings view and email validation since it was only needed for rollout.

@edx/masters-devs-cosmonauts

@zacharis278

zacharis278 commented Sep 10, 2020

Copy link
Copy Markdown
Contributor Author

Advanced Settings

image

Course Outline

image

@zacharis278
zacharis278 marked this pull request as ready for review September 14, 2020 13:20

@schenedx schenedx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I want to see new unit test for the changed behavior

self.assertIn('proctoring_escalation_email', test_model)

@override_settings(
PROCTORING_BACKENDS={

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

While it's nice to remove no longer used unit tests, but I believe you should add unit test for the added warning messages on course settings.

@alangsto alangsto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree with Simon's comment about the unit tests, and also had a small suggestion for wording!

Comment thread cms/templates/course_outline.html Outdated
Comment thread cms/templates/settings_advanced.html Outdated
@zacharis278

Copy link
Copy Markdown
Contributor Author

jenkins run python

@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests. There were no failures.

@schenedx schenedx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@zacharis278
zacharis278 merged commit 33f6d77 into master Sep 17, 2020
@zacharis278
zacharis278 deleted the zhancock/proctoring-settings-error branch September 17, 2020 18:15
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

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.

5 participants