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: due date reminder #32826

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

navinkarkera
Copy link
Contributor

@navinkarkera navinkarkera commented Jul 24, 2023

Description

This PR adds a command to send reminder emails for self-paced courses with relative due dates enabled. The command takes in a argument due-in which allows us to send reminders for subsections due within specified number of days.

Testing instructions

  • Setup master devstack
  • Enable SelfPacedRelativeDateConfig globally.
  • Add waffle flag studio.custom_relative_dates to allow adding custom relative dates to graded subsections.
  • Start LMS and CMS using make {lms,cms}-up.
  • Run make lms-migrate to add enqueue and deliver columns for new schedule.
  • Open Schedules config and add an entry for localhost:18000 site and enable all flags.
  • Go to demo course in studio and update start date to some date in past (around 2 months) and change pacing to self-paced (for this you might need to update start_date to some date in future and revert back).
  • Login to LMS using edx user and enroll in the demo course.
  • Go to schedules and update edx user's start_date to some recent date within the past one week.
  • Open LMS shell using make lms-shell and run below command, play with date and due-in parameter and see the output. It should print email with subsections having due date falling within the days passed via due-in parameter.
    python manage.py lms send_course_due_date_reminders localhost:18000 --due-in 7
  • Also, it should only send email if SelfPacedRelativeDateConfig is enabled and if course pacing is set to self-paced.
  • Re-run the command by updating Due in field for a graded subsection:

image

Example output:

------- EMAIL -------
To: edx@example.com
From: Demonstration Course
Subject: Important course due dates coming up
Body:
Hi there! We wanted to remind you of some important due dates coming up for your course, Demonstration Course, that you need to be aware of. The due dates are coming up in the following subsections:


  * Lesson 1 - Getting Started (28 Jul 2023)

  * Homework - Question Styles (07 Aug 2023)

  * Lesson 2 - Let's Get Interactive! (02 Aug 2023)

  * Homework - Labs and Demos (02 Aug 2023)

  * Homework - Essays (02 Aug 2023)

  * Lesson 3 - Be Social (08 Aug 2023)

  * Homework - Find Your Study Buddy (08 Aug 2023)

  * More Ways to Connect (08 Aug 2023)


Happy learning!
-------  END  -------

Deadline

"None" if there's no rush, or provide a specific date or event (and reason) if there is one.

Other information

Doubts:

  • Why is access_duration limited to max duration of 18 weeks?
  • Copied relative week due date logic from course_date_signals handler. It sets minimum of 1 week for each section. Not sure if it is valid.
  • Some tests in schedules app are not running due to invalid condition in skip check. Many tests fail as soon as we fix the condition, we probably cannot fix it now but creating a issue might help.
    # Currently it is set to.
    'openedx.core.djangoapps.schedules.apps.SchedulesConfig' in settings.INSTALLED_APPS
    # Should be below as it is set this way in common settings.
    'openedx.core.djangoapps.schedules' in settings.INSTALLED_APPS

Private-ref: BB-7407

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jul 24, 2023
@openedx-webhooks
Copy link

openedx-webhooks commented Jul 24, 2023

Thanks for the pull request, @navinkarkera! 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.

This is currently a draft pull request. When it is ready for our review and all tests are green, click "Ready for Review", or remove "WIP" from the title, as appropriate.

@navinkarkera navinkarkera force-pushed the navin/due-date-reminder branch 4 times, most recently from 5593a64 to bfd2e01 Compare July 27, 2023 18:25
@navinkarkera navinkarkera requested a review from Agrendalath July 27, 2023 18:25
@navinkarkera navinkarkera marked this pull request as ready for review July 27, 2023 18:26
@navinkarkera navinkarkera changed the title wip: feat: due date reminder feat: due date reminder Jul 27, 2023
@Agrendalath Agrendalath force-pushed the navin/due-date-reminder branch from bfd2e01 to fd4f0e6 Compare August 29, 2023 14:22
openedx/core/djangoapps/content/course_overviews/models.py Outdated Show resolved Hide resolved
openedx/core/djangoapps/schedules/models.py Outdated Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
openedx/core/djangoapps/schedules/tasks.py Outdated Show resolved Hide resolved
openedx/core/djangoapps/schedules/tasks.py Show resolved Hide resolved
openedx/core/djangoapps/schedules/content_highlights.py Outdated Show resolved Hide resolved
openedx/core/djangoapps/schedules/content_highlights.py Outdated Show resolved Hide resolved
openedx/core/djangoapps/schedules/content_highlights.py Outdated Show resolved Hide resolved
openedx/core/djangoapps/schedules/content_highlights.py Outdated Show resolved Hide resolved
openedx/core/djangoapps/schedules/resolvers.py Outdated Show resolved Hide resolved
@navinkarkera navinkarkera force-pushed the navin/due-date-reminder branch 4 times, most recently from d414237 to 0fd30c9 Compare September 1, 2023 12:05
@navinkarkera navinkarkera force-pushed the navin/due-date-reminder branch 2 times, most recently from 7884b4f to 70fc549 Compare September 4, 2023 11:44
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.

@navinkarkera, this works great! I'll convert this to a draft for now so we can test it on Staging.

@Agrendalath Agrendalath marked this pull request as draft September 7, 2023 18:23
@Agrendalath Agrendalath force-pushed the navin/due-date-reminder branch from 89f92d1 to c21378b Compare September 7, 2023 18:25
@navinkarkera navinkarkera force-pushed the navin/due-date-reminder branch 2 times, most recently from fc17ad0 to 5816a03 Compare September 12, 2023 12:55
Adds a command to send reminder emails for self-paced courses with relative
due dates.
@gabor-boros gabor-boros force-pushed the navin/due-date-reminder branch from 5816a03 to 8f213ac Compare October 1, 2023 07:35
@open-craft-grove
Copy link

Sandbox destroy request received.

@mphilbrick211
Copy link

Hi @navinkarkera and @Agrendalath! Just checking to see if this is still in progress?

@Agrendalath
Copy link
Member

@mphilbrick211, this has already been implemented, but we must pass it through the client's QA. This should be resumed around March. It would be great to get your product review in the meantime.

@mphilbrick211 mphilbrick211 added the product review PR requires product review before merging label Dec 12, 2023
@jmakowski1123
Copy link

Thanks @Agrendalath . With the info provided here, this seems like a nice addition. I'd like to be able to see the whole workflow in action, from adding the due date in the graded subsection to creating and sending an email that pulls the query. Is it possible to play with it in your sandbox?

@Agrendalath
Copy link
Member

@jmakowski1123, thanks for checking. Of course, we will create a sandbox once we move forward with the implementation after the initial QA. We're discussing adding more features, like opting out of these emails for individual courses.
However, we had some reprioritizations with the client, so we are still trying to determine when to resume this PR.

@jmakowski1123
Copy link

Ok, let's keep this on hold then until it becomes a priority, and then can continue reviewing once you have specs for the new features.

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 product review PR requires product review before merging
Projects
Status: Product Review
Development

Successfully merging this pull request may close these issues.

6 participants