Skip to content

Use levelbuilder-controlled course status for deprecation #66987

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

Open
wants to merge 1 commit into
base: ben/deprecate-csp-courses
Choose a base branch
from

Conversation

bencodeorg
Copy link
Contributor

@bencodeorg bencodeorg commented Jul 9, 2025

Switches from using the is_deprecated boolean flag (a serialized property on the Unit model) to the levelbuilder-controlled Course published state to determine whether to show the "deprecated course" page to users.

Prior to merging this, we'd want to:

Testing story

I tested manually that marking a Course as deprecated resulted in not being able to navigate to the course page (eg, /courses/csp-2019), a unit overview page (eg, /courses/csp-2019/units/1), or a script level page (eg, /courses/csp-2019/units/1/lessons/1/levels/1).

@@ -64,7 +64,7 @@ def show

# For deprecated courses, show deprecated course page
# if we haven't already redirected to a newer version of the course or the teacher dashboard.
if @unit_group.default_units.all?(&:is_deprecated)
if @unit_group.default_units.all?(&:deprecated?)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This may be circular -- should we just check the Course's deprecation status directly? This comment is confusing me (I think the comment doesn't match the code):

# If a script is in a unit group, use that unit group's published state. If not, use the script's published_state
# If both are null, the script is in_development
def get_published_state
published_state || unit_group&.published_state || Curriculum::SharedCourseConstants::PUBLISHED_STATE.in_development
end

@@ -209,10 +210,12 @@ def destroy

def edit
# Deprecated scripts should not be edited.
if @script.is_deprecated
if @script.deprecated?
Copy link
Contributor Author

@bencodeorg bencodeorg Jul 9, 2025

Choose a reason for hiding this comment

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

This currently makes it so that once a course is marked as deprecated on levelbuilder, levelbuilders will immediately not be able to view the edit page for any Unit within the course.

We don't have the same condition on the edit page for the Course itself (maybe we should?), so the /courses/csp-2017/edit page is still viewable (but /courses/csp-2017 is not).

Not sure how strongly we feel about making these uneditable / want to come up with some other mechanism to prevent edits.

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.

1 participant