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: mark new courses as self-paced [BB-7401] #5

Merged
merged 2 commits into from
May 15, 2023

Conversation

Agrendalath
Copy link
Member

@Agrendalath Agrendalath commented May 12, 2023

This changes the new course to be self-paced instead of instructor-paced.

Testing instructions.

  1. Follow the steps added to the README.
  2. Create a new section-based course in the Django admin.
  3. Check that the course is self-paced (Studio -> Schedule & Details).
  4. If you want to check if the relative dates work as expected, you can use the instructions from feat: add Waffle Flag to disable resetting self-paced deadlines by learners openedx/edx-platform#32148. Please note that the Waffle flag to enforce these dates is currently broken in Nutmeg - you can use the branch from fix: use old-style Course Waffle flag for RELATIVE_DATES_DISABLE_RESET_FLAG edx-platform#539 (unless it's merged before you review this).

Author's notes

1. Adding relative due dates to sections is a manual step because we will need to change the grading policy for new courses to ensure that completing the section results in a 100% grade from the whole course. This is a more significant change, so it's outside of this ticket's scope.
2. Once the previous point is completed, we can mark the section as graded while creating the course. We can also allow setting the number of relative weeks from the admin form (if needed).

I thought about this a bit more and realized that I've been talking about sections, but the subsections are actually graded. I then tested the cms.djangoapps.models.settings.course_grading.update_section_grader_type method, and it turns out that it's possible to mark the whole section as graded. It's even displayed on the Course Outline page in Studio as such (without using this API there is no graded/ungraded indication there). However, it's impossible to modify this option on the Studio page, so we don't want to use this approach.
Then, I thought about the second thing - i.e., setting a custom grading policy in the newly created course. We can do this with the CourseGradingModel mentioned above, but there could be multiple subsections in the "Lesson", so we don't know if all of them should be graded (and if they should be graded equally). Therefore, keeping the grading settings as a manual step sounds logical until we obtain more requirements from the client.

We could set up a default relative date (Number of Relative Weeks Due By in the "Advanced Settings"; relative_weeks_due directly in the Course XBlock). However, it means that this date will be enforced for Problem blocks in ungraded sections, without informing learners about them anywhere (I've mentioned this in the README). Therefore, this would be a confusing behavior for both learners and course authors.

Merge checklist:
Check off if complete or not applicable:

  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Fixup commits are squashed away
  • Unit tests added/updated
  • Manual testing instructions provided
  • Noted any: Concerns, dependencies, migration issues, deadlines, tickets

@Agrendalath Agrendalath requested a review from Kelketek May 12, 2023 16:57
@Agrendalath Agrendalath self-assigned this May 12, 2023
@Agrendalath Agrendalath force-pushed the agrendalath/bb-7401_self-paced_courses branch from a6a8df3 to 497882c Compare May 12, 2023 18:19
Copy link
Member

@Kelketek Kelketek left a comment

Choose a reason for hiding this comment

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

@Agrendalath

👍

  • I tested this: Courses are self-paced, and relative due dates copy over.
  • I read through the code
  • I checked for accessibility issues
  • Includes documentation
  • I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository.

I added one little request for the README, and I had one question, but assuming that question doesn't yield an unsettling answer, you can go ahead and merge once you've made the README update.

README.rst Show resolved Hide resolved
README.rst Show resolved Hide resolved
@Agrendalath Agrendalath merged commit da4dda6 into main May 15, 2023
@Agrendalath Agrendalath deleted the agrendalath/bb-7401_self-paced_courses branch May 15, 2023 14:29
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