-
Notifications
You must be signed in to change notification settings - Fork 200
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
expired onboarding exam messaging fix #997
Conversation
327fb3c
to
055f611
Compare
Codecov ReportBase: 84.13% // Head: 84.20% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #997 +/- ##
==========================================
+ Coverage 84.13% 84.20% +0.06%
==========================================
Files 264 264
Lines 4519 4539 +20
Branches 1158 1166 +8
==========================================
+ Hits 3802 3822 +20
Misses 697 697
Partials 20 20
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
2cae16b
to
29dd049
Compare
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.
👍
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.
One small comment fix, but otherwise the logic looks good to me
@@ -77,11 +83,19 @@ function ProctoringInfoPanel({ intl }) { | |||
return borderClass; | |||
} | |||
|
|||
function isExpired(dateString) { | |||
// Returns true if the expiration date is within 28 days |
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.
Should update this comment for expired, not expiring soon
Currently expiring soon is displayed 28 days before expiration and forever afterwards. Adds an actual expired state for after. Also clarifies the expring soon message which assumed other course, that was not necessarily true. Also updates the take action lines when you do not have valid onboarding to make sure they appear for everything not currently valid or in process, and updates the submitted process lines to not appear for expired statuses.
29dd049
to
d63ebe2
Compare
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.
LGTM
- code review
- code execution
Currently expiring soon is displayed 28 days before expiration and forever afterwards. Adds an actual expired state for after. Also clarifies the expring soon message which assumed other course, that was not necessarily true. Also updates the take action lines when you do not have valid onboarding to make sure they appear for everything not currently valid or in process, and updates the submitted process lines to not appear for expired statuses.
Currently expiring soon is displayed 28 days before expiration and forever afterwards. Adds an actual expired state for after.
Also clarifies the expring soon message which assumed other course, that was not necessarily true.
Also updates the take action lines when you do not have valid onboarding to make sure they appear for everything not currently valid or in process, and updates the submitted process lines to not appear for expired statuses.
This also requires expiration time updates provided in openedx/edx-proctoring#1088 to make sure that expiration time properly comes along with more exam states.