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

feat: warn when relative dates are past due and can't be shifted #34366

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

DanielVZ96
Copy link
Contributor

Description

When the course_experience.relative_dates_disable_reset flag is set to true in self paced courses, warnings that otherwise allowed learners to shift deadlines don't appear. This PRs shows warnings in this case, with the sole difference of not allowing to shift dates.

Useful information to include:

  • Which edX user roles will this change impact?: Learner

Testing instructions

  1. Add the following Waffle Flags (with Everyone: Yes):

    1. studio.custom_relative_dates
    2. course_experience.relative_dates
    3. course_experience.relative_dates_disable_reset
  2. Go to Course_Date_Signals -> Self paced relative dates configs and add a config with Enabled: Yes.

  3. Create a new course in Studio.

  4. Go to Settings -> Schedule & Details and set the pacing to Self-Paced. Click "Save Changes".

  5. Set a past Course Start Date. Click "Save Changes".

  6. Create a new subsection in the course. Mark it as graded as "Homework" and set "Due in" to 1 week.

  7. Create a Problem Block (e.g., Checkboxes) in the subsection and publish it.

  8. Log in as an audit user and enroll yourself in a course.

  9. As an admin, go to Schedules -> Schedules and find the schedule for the audit user in this new course. Change the Start date to a year ago and Save.

  10. As an audit user, visit the Problem and check that there's a warning about not being able to shift the relative dates (see screenshot bellow), that the Submit button can no longer be enabled, and that should be a "Past due" pill near the due date above this Problem.

  11. You should also see these deadlines on the Course Outline page.

  12. Set the course_experience.relative_dates_disable_reset to No

  13. Revisit the Problem with the audit user. Check that the warning now lets you shift the relative dates. (second screenshot)

Screenshots

course_experience.relative_dates_disable_reset set to false:
image

course_experience.relative_dates_disable_reset set to true (old behavior, but still good to check):
image

Not past due date:
image

Private-ref: BB-8542

@openedx-webhooks
Copy link

Thanks for the pull request, @DanielVZ96! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Mar 14, 2024
@DanielVZ96 DanielVZ96 requested a review from Agrendalath March 14, 2024 01:13
@DanielVZ96 DanielVZ96 force-pushed the dvz/rel-date-alert branch 2 times, most recently from 68a296a to b55444e Compare March 14, 2024 03:09
@@ -164,3 +178,22 @@ def _make_reset_deadlines_cta(cls, xblock, category, is_learning_mfe=False):
}

return cta_data

@classmethod
def _make_warn_deadlines_cta(cls, xblock, category, is_learning_mfe=False):
Copy link
Member

Choose a reason for hiding this comment

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

We don't use any of these attributes. Technically, we could return this right after checking if RELATIVE_DATES_DISABLE_RESET_FLAG.is_enabled(course_key): instead of making separate helper functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea!

openedx/features/course_experience/utils.py Show resolved Hide resolved
lms/templates/problem.html Outdated Show resolved Hide resolved
openedx/features/course_experience/utils.py Outdated Show resolved Hide resolved
Copy link
Member

@Agrendalath Agrendalath left a comment

Choose a reason for hiding this comment

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

👍

  • I tested this: checked that the warning is displayed correctly
  • I read through the code
  • I checked for accessibility issues: n/a
  • Includes documentation: n/a
  • Added to the Code Drift project board (for backports)

@DanielVZ96
Copy link
Contributor Author

Tests failed with: Error: Failed to CreateArtifact: Received non-retryable error: Failed request: (409) Conflict: an artifact with this name already exists on the workflow run

@Agrendalath Agrendalath merged commit b98cbd4 into openedx:master Jun 10, 2024
46 checks passed
@Agrendalath Agrendalath deleted the dvz/rel-date-alert branch June 10, 2024 20:55
@openedx-webhooks
Copy link

@DanielVZ96 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

@edx-pipeline-bot
Copy link
Contributor

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

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open-source-contribution PR author is not from Axim or 2U
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants