From a4f1e4551ecf256d6407da40a10a4c0422609597 Mon Sep 17 00:00:00 2001 From: Johan Castiblanco Date: Thu, 5 Sep 2024 15:46:51 -0500 Subject: [PATCH 1/5] feat: remove staff access for catalog views This is controversial, but simple. To do this in a monkey patch is a bigger task and also with more work. Is different due the change is in a nested function. Anyway the alternative is to monkeyPatch the _dispatch method and handle there. But its risky and also that method manage other permission with different ways. --- lms/djangoapps/courseware/access.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index f71f2daec5bb..e8bff5d1b1da 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -394,7 +394,6 @@ def can_see_in_catalog(): """ return ( _has_catalog_visibility(courselike, CATALOG_VISIBILITY_CATALOG_AND_ABOUT) - or _has_staff_access_to_descriptor(user, courselike, courselike.id) ) @function_trace('can_see_about_page') From 0edd14230be619044f1814edb3b19b6cb77cc87b Mon Sep 17 00:00:00 2001 From: Johan Castiblanco Date: Thu, 5 Sep 2024 17:32:48 -0500 Subject: [PATCH 2/5] fix: update tests for staff in see_in_catalog --- lms/djangoapps/courseware/tests/test_access.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index 45d36ad0a7f8..617c58d83508 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -611,7 +611,7 @@ def test__catalog_visibility(self): ) assert not access._has_access_course(user, 'see_in_catalog', course) assert access._has_access_course(user, 'see_about_page', course) - assert access._has_access_course(staff, 'see_in_catalog', course) + assert not access._has_access_course(staff, 'see_in_catalog', course) assert access._has_access_course(staff, 'see_about_page', course) # Now set visibility to none, which means neither in catalog nor about pages @@ -621,7 +621,7 @@ def test__catalog_visibility(self): ) assert not access._has_access_course(user, 'see_in_catalog', course) assert not access._has_access_course(user, 'see_about_page', course) - assert access._has_access_course(staff, 'see_in_catalog', course) + assert not access._has_access_course(staff, 'see_in_catalog', course) assert access._has_access_course(staff, 'see_about_page', course) @patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True, 'MILESTONES_APP': True}) From 35bce5824db53a2d65f3b610780885f8c33f3bc8 Mon Sep 17 00:00:00 2001 From: Johan Castiblanco Date: Thu, 12 Sep 2024 08:58:20 -0500 Subject: [PATCH 3/5] feat: add flag to manage staff see in catalog --- lms/djangoapps/courseware/access.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index e8bff5d1b1da..f17aa6257f3c 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -394,6 +394,10 @@ def can_see_in_catalog(): """ return ( _has_catalog_visibility(courselike, CATALOG_VISIBILITY_CATALOG_AND_ABOUT) + or ( + _has_staff_access_to_descriptor(user, courselike, courselike.id) + and getattr(settings, "STAFF_CAN_SEE_IN_CATALOG", False) + ) ) @function_trace('can_see_about_page') From bd7c87e66b5d487d8281856dc1710f7fbdd853b1 Mon Sep 17 00:00:00 2001 From: Johan Castiblanco Date: Thu, 12 Sep 2024 10:57:03 -0500 Subject: [PATCH 4/5] Revert "fix: update tests for staff in see_in_catalog" This reverts commit 1fbf18cd67d5ce51cb82ef7743bec6c3e53f341e. --- lms/djangoapps/courseware/tests/test_access.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index 617c58d83508..45d36ad0a7f8 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -611,7 +611,7 @@ def test__catalog_visibility(self): ) assert not access._has_access_course(user, 'see_in_catalog', course) assert access._has_access_course(user, 'see_about_page', course) - assert not access._has_access_course(staff, 'see_in_catalog', course) + assert access._has_access_course(staff, 'see_in_catalog', course) assert access._has_access_course(staff, 'see_about_page', course) # Now set visibility to none, which means neither in catalog nor about pages @@ -621,7 +621,7 @@ def test__catalog_visibility(self): ) assert not access._has_access_course(user, 'see_in_catalog', course) assert not access._has_access_course(user, 'see_about_page', course) - assert not access._has_access_course(staff, 'see_in_catalog', course) + assert access._has_access_course(staff, 'see_in_catalog', course) assert access._has_access_course(staff, 'see_about_page', course) @patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True, 'MILESTONES_APP': True}) From a29df930a89a0b9e58d69689a863d2d334d79962 Mon Sep 17 00:00:00 2001 From: Johan Castiblanco Date: Thu, 12 Sep 2024 10:57:35 -0500 Subject: [PATCH 5/5] feat: keep custom test with default beahviour --- lms/djangoapps/courseware/access.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index f17aa6257f3c..de92d3895ffb 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -396,7 +396,7 @@ def can_see_in_catalog(): _has_catalog_visibility(courselike, CATALOG_VISIBILITY_CATALOG_AND_ABOUT) or ( _has_staff_access_to_descriptor(user, courselike, courselike.id) - and getattr(settings, "STAFF_CAN_SEE_IN_CATALOG", False) + and getattr(settings, "STAFF_CAN_SEE_IN_CATALOG", True) ) )