From 5c4cf7ab3f16039ae147c8fc2134a96fa68c1a55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Behmo?= Date: Mon, 14 Oct 2024 07:53:41 +0200 Subject: [PATCH] feat!: hide courses in /courses based on catalog visibility 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 https://github.com/openedx/wg-build-test-release/issues/330 --- lms/djangoapps/branding/tests/test_page.py | 24 ++++++++++++++++++++++ lms/djangoapps/courseware/views/views.py | 11 ++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/branding/tests/test_page.py b/lms/djangoapps/branding/tests/test_page.py index 9904f79f644f..46f10bd6923a 100644 --- a/lms/djangoapps/branding/tests/test_page.py +++ b/lms/djangoapps/branding/tests/test_page.py @@ -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 @@ -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) @@ -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): """ diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 79a52db8a0b6..1c57b23d9b11 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -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 @@ -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"]):