[Product Pull Request] Main Page Course Listing

[jmakowski1123]

For Contributing Author:

This is the Primary Product Ticket for the following community contribution: Make course description editable in certificates

Checklist prior to undergoing Product Review:

The following information is required in order for Product Managers to be able to review your pull request:

• Explanation of the problem being solved
• Description of how users will be impacted, and which users will be impacted
• Screenshots or video showing the functionality or fix, before and after
• Reproduction steps and/or testing steps

Only if necessary:

• If necessary, links to corresponding configuration changes
• If necessary, links to corresponding enablement changes, particularly waffle/toggle status details

Related PRs

• Add the default enrollment start date on course creation edx-platform#30954

For Product Manager doing the review:

What criteria should be analyzed from Product to approve a PR?

• The problem being solved by the feature or fix is clear.
• There is clarity on how the change or fix will impact the end user.
• It is clear that the change will not negatively impact users or other areas of the platform.
• The change is implemented comprehensively.
• Any changes to UI use the current, standard Paragon Design System: https://paragon-openedx.netlify.app/

[github-actions]

Thanks for your submission, @openedx/open-edx-project-managers will review shortly.

[jmakowski1123]

Original information from the PR:

Problem

Course is visible in the main page right after creation. So anonymous users can see them and access course about page for the courses without valid data (e.g. they will see default course overview)

Root cause

When courses list filtering processed it checks the see_exists permission for the anonymous user.
Actually, see_exists means can_load OR can_enroll.

can_load is False in our case because course start is in the future.

But can_enroll returns True because course's enrollment_start and enrollment_end dates are blank:

enrollment_start = courselike.enrollment_start or datetime.min.replace(tzinfo=UTC)
enrollment_end = courselike.enrollment_end or datetime.max.replace(tzinfo=UTC)
if enrollment_start < now < enrollment_end:
    debug("Allow: in enrollment period")
    return ACCESS_GRANTED

Fix

Set the enrollment_start the same as a course start by default

Known Issues:

• this requires the refactor: add enrollment start openedx-demo-course#38 to be merged. Otherwise, it will break the clean (first) deployment while trying to enroll audit and verified users in the Demo Course

Notes

• this fix will work for the course catalog page when the course discovery disabled (FEATURES['ENABLE_COURSE_DISCOVERY'] = False)
• I wounder why DEFAULT_START_DATE is hardcoded (1 Jan 2030)? How and when is it updated?

[jmakowski1123]

Questions that need clarification before this can undergo product review:

• This is the original problem statement: "Course is visible in the main page right after creation. So anonymous users can see them and access course about page for the courses without valid data (e.g. they will see default course overview)." A few things to clarify: Main page means the learner-facing course catalog page? And anonymous users means any learner who is browsing courses in the course catalog?

• With this proposed change, when does the course become visible? On the first day that the enrollment period opens?

• I imagine there are circumstances where instances administrators want a course to be visible in the catalog, even if the enrollment period is not yet open. Is my interpretation correct, that this proposed change would prevent this from being possible?

[dyudyunov]

Hello @jmakowski1123

A few things to clarify: Main page means the learner-facing course catalog page? And anonymous users mean any learner who is browsing courses in the course catalog?

The main page - is the index page of the LMS (e.g. https://www.edx.org/). The /courses (aka "Course Catalog" or "Discover New" or "Find Courses") page has the same problem if the FEATURES['ENABLE_COURSE_DISCOVERY'] = False. Anonymous user - is the user without an active login session (by default - only anonymous users can see the index (main) page)

With this proposed change, when does the course become visible? On the first day that the enrollment period opens?

Yes. Enrollment start - is a DateTime, so if enrollment is today and the now > enrollment start time - course will be visible

I imagine there are circumstances where instances administrators want a course to be visible in the catalog, even if the enrollment period is not yet open. Is my interpretation correct, that this proposed change would prevent this from being possible?

In the context of the main (index) page and course catalog without course discovery enabled - yes, but it could be changed using the combination of the course settings:

• the course_experience.pre_start_access course waffle flag is enabled
• course's visible_to_staff_only attribute is False (default)
• course prerequisites are satisfied (always True for the staff or anonymous)
• the course is not expired

In the context of the course catalog with course discovery enabled - the page already works as we expect. Courses with the enrollment start in the future are not shown. This is also controlled by the SEARCH_SKIP_ENROLLMENT_START_DATE_FILTERING setting.

[ProductRyan]

@boulevardofdef - I believe that this issue is referring to the Course Home page in discovery... can you take a look?

[boulevardofdef]

@ProductRyan @jmakowski1123 @dyudyunov First off, the edX homepage/main page (www.edx.org) does not automatically display any courses. All the courses you see there are manually placed, usually via our content management system, Contentful, but sometimes via a code change. We define UUIDs in Contentful, and the template then makes the calls to course-discovery that it needs to display course data. Are we saying here that if this PR is merged, Contentful would no longer be able to fetch the data for the defined courses? Or does that not matter?

[dyudyunov]

I would like to clarify some moments:

• As far as I see from the @boulevardofdef comment, edx uses the redirect to the marketing site's MKTG_URLS['ROOT'] URL but I might be wrong here. Anyway, edx's approach is different and my fix seems to not affect it because course discovery search has its own filtering and excluding mechanisms
• the index (main) page doesn't call the course discovery but gathers the courses from the CourseOverviews Django model directly. The fix is aimed at filtering the courses visible to the user and that’s all. I don't change the way courses are gathered (i.e. searched) but only filter for them

[mphilbrick211]

@boulevardofdef any update on this this? Just checking in :)

[boulevardofdef]

@mphilbrick211 I had to go over this one a few times! If I'm reading @dyudyunov correctly, it seems like this change would not affect the functionality on the marketing site, which makes this just fine with me. But please correct me if my assumption is wrong on this.

[mphilbrick211]

@mphilbrick211 I had to go over this one a few times! If I'm reading @dyudyunov correctly, it seems like this change would not affect the functionality on the marketing site, which makes this just fine with me. But please correct me if my assumption is wrong on this.

Thanks, @boulevardofdef! @dyudyunov please confirm and then we'll move the PR along. Thanks!

[dyudyunov]

@boulevardofdef @mphilbrick211
Confirmed! 🙂

[mariajgrimaldi]

Hi there: @jmakowski1123 @mphilbrick211
The fix for this ticket underwent product review, and it was approved according to this comment: openedx/edx-platform#30954 (comment)
So I'm wondering, should we close this ticket?

[itsjeyd]

@jmakowski1123 @mphilbrick211 It looks like this is done; the PR for it (openedx/edx-platform#30954) has been merged. Can we close this ticket and move it to Shipped on the roadmap?