Skip to content

Commit

Permalink
feat!: hide courses in /courses based on catalog visibility
Browse files Browse the repository at this point in the history
Previously, courses were always displayed on the LMS /courses page,
independently of their catalog visibility attribute. This meant that
even with visibility="none" courses were being displayed. This was very
counter-intuitive.

With this change, courses are displayed only when their visibility is
set to "both".

This change is flagged as breaking because it has the potential to
affect course catalogs in existing platforms.

To test this change, go to http(s)://LMS/courses as an anonymous user.
Pick a visible course and go to its "advanced settings" in the studio.
Set "Course visibility in catalog" to "about" or "none". Then, clear the
cache with the following command:

    ./manage.py lms shell -c "from django.core.cache import cache; cache.clear()"

Open the /courses page again: the course should no longer be visible.

Close openedx/wg-build-test-release#330
  • Loading branch information
regisb authored and sarina committed Oct 16, 2024
1 parent d8303a1 commit bf862d8
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 2 deletions.
24 changes: 24 additions & 0 deletions lms/djangoapps/branding/tests/test_page.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.course_block import CATALOG_VISIBILITY_ABOUT, CATALOG_VISIBILITY_NONE

FEATURES_WITH_STARTDATE = settings.FEATURES.copy()
FEATURES_WITH_STARTDATE['DISABLE_START_DATES'] = False
Expand Down Expand Up @@ -201,6 +202,20 @@ def setUp(self):
display_name='Tech Beta Course',
emit_signals=True,
)
self.course_with_none_visibility = CourseFactory.create(
org='MITx',
number='1003',
catalog_visibility=CATALOG_VISIBILITY_NONE,
display_name='Course with "none" catalog visibility',
emit_signals=True,
)
self.course_with_about_visibility = CourseFactory.create(
org='MITx',
number='1003',
catalog_visibility=CATALOG_VISIBILITY_ABOUT,
display_name='Course with "about" catalog visibility',
emit_signals=True,
)
self.factory = RequestFactory()

@patch('common.djangoapps.student.views.management.render_to_response', RENDER_MOCK)
Expand Down Expand Up @@ -300,6 +315,15 @@ def test_course_cards_sorted_by_start_date_disabled(self):
assert context['courses'][1].id == self.starting_earlier.id
assert context['courses'][2].id == self.course_with_default_start_date.id

@patch('lms.djangoapps.courseware.views.views.render_to_response', RENDER_MOCK)
def test_invisible_courses_are_not_displayed(self):
response = self.client.get(reverse('courses'))
((_template, context), _) = RENDER_MOCK.call_args # pylint: disable=unpacking-non-sequence

rendered_ids = [course.id for course in context["courses"]]
assert self.course_with_none_visibility.id not in rendered_ids
assert self.course_with_about_visibility.id not in rendered_ids


class IndexPageProgramsTests(SiteMixin, ModuleStoreTestCase):
"""
Expand Down
11 changes: 9 additions & 2 deletions lms/djangoapps/courseware/views/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@
from rest_framework.throttling import UserRateThrottle
from token_utils.api import unpack_token_for
from web_fragments.fragment import Fragment
from xmodule.course_block import COURSE_VISIBILITY_PUBLIC, COURSE_VISIBILITY_PUBLIC_OUTLINE
from xmodule.course_block import (
COURSE_VISIBILITY_PUBLIC,
COURSE_VISIBILITY_PUBLIC_OUTLINE,
CATALOG_VISIBILITY_CATALOG_AND_ABOUT,
)
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
from xmodule.tabs import CourseTabList
Expand Down Expand Up @@ -288,7 +292,10 @@ def courses(request):
course_discovery_meanings = getattr(settings, 'COURSE_DISCOVERY_MEANINGS', {})
set_default_filter = ENABLE_COURSE_DISCOVERY_DEFAULT_LANGUAGE_FILTER.is_enabled()
if not settings.FEATURES.get('ENABLE_COURSE_DISCOVERY'):
courses_list = get_courses(request.user)
courses_list = get_courses(
request.user,
filter_={"catalog_visibility": CATALOG_VISIBILITY_CATALOG_AND_ABOUT},
)

if configuration_helpers.get_value("ENABLE_COURSE_SORTING_BY_START_DATE",
settings.FEATURES["ENABLE_COURSE_SORTING_BY_START_DATE"]):
Expand Down

0 comments on commit bf862d8

Please sign in to comment.