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

Catalog card should show Start Anytime if the course is archived #2359

Merged
merged 9 commits into from
Aug 26, 2024

Conversation

annagav
Copy link
Contributor

@annagav annagav commented Aug 23, 2024

What are the relevant tickets?

Fix #2357

Description (What does it do?)

This PR changes the front end to use a single source of is_archived and makes the catalog display "Start Anytime" even when the course is not `is_self_paced=True'
Is archived is used in many places in the frontend, but it's value is not dynamic, so it's better to compute it in the backend.

Screenshots (if appropriate):

Screenshot 2024-08-23 at 6 26 39 PM

How can this be tested?

Copy link
Contributor

@JenniWhitman JenniWhitman left a comment

Choose a reason for hiding this comment

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

Not sure what the verdict is on enrollment_end date - I asked a question below. Once we get that conclusion, we can approve

def get_is_archived(self, instance):
return (
instance.is_enrollable
and instance.enrollment_end is None
Copy link
Contributor

Choose a reason for hiding this comment

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

In the get_archived_courseruns function, we defined enrollment end as able to be null or future. Here we're ONLY defining it as None. Is this the case? Should the other function be updated? Should this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, that's a mistake.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that condition is not needed there at all.

Copy link
Contributor

@JenniWhitman JenniWhitman left a comment

Choose a reason for hiding this comment

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

Good to go

@annagav annagav merged commit ade4025 into main Aug 26, 2024
7 checks passed
@annagav annagav deleted the ag/start_anytime branch August 26, 2024 16:44
@odlbot odlbot mentioned this pull request Aug 26, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Catalog shows "Started November 9th" when course is actually archived
2 participants