diff --git a/lms/djangoapps/ccx/api/v2/serializers.py b/lms/djangoapps/ccx/api/v2/serializers.py index ec3b8f0508de..72fcb88cf7c5 100644 --- a/lms/djangoapps/ccx/api/v2/serializers.py +++ b/lms/djangoapps/ccx/api/v2/serializers.py @@ -9,8 +9,14 @@ from django.utils.translation import gettext_lazy as _ from rest_framework import serializers +from lms.djangoapps.grades.api import is_writable_gradebook_enabled +from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers + log = logging.getLogger(__name__) +# Tab that depends on the writable gradebook being available. +STUDENT_GRADES_TAB_ID = 'student_grades' + # CCX Coach navigation tabs, in display order. Each entry is # `(tab_id, title, sort_order)`. `tab_id` values must match the route # segments registered by the Instructor Dashboard MFE `ccxCoachConfig` so @@ -24,6 +30,36 @@ ) +def is_student_grades_tab_available(ccx_course_key): + """ + Return whether the Student Grades tab should be offered for this CCX. + + The tab is backed by the writable gradebook, which requires both the + ``grades.writable_gradebook`` waffle flag and a configured + ``WRITABLE_GRADEBOOK_URL``. This mirrors the check the Instructor Dashboard + uses to decide whether to expose its gradebook link + (``instructor.views.serializers_v2.get_gradebook_url``), including reading the + URL through site configuration so per-site overrides are honored. + + The flag is evaluated against the **CCX** key rather than the master course + key, because that is the key the gradebook endpoints are called with. A + course-scoped override on the master course does not apply to the CCX key, so + checking the CCX key keeps the tab's presence an accurate predictor of + whether the gradebook will actually work. + + Arguments: + ccx_course_key (CCXLocator): the CCX course key. + + Returns: + bool + """ + gradebook_url = configuration_helpers.get_value( + 'WRITABLE_GRADEBOOK_URL', + getattr(settings, 'WRITABLE_GRADEBOOK_URL', None), + ) + return bool(gradebook_url) and is_writable_gradebook_enabled(ccx_course_key) + + def build_ccx_coach_tab_url(ccx_course_key, tab_id): """ Build a CCX Coach MFE tab URL from `CCX_COACH_MICROFRONTEND_URL`. @@ -82,10 +118,20 @@ def get_ccx_course_id(self, data): return str(ccx_course_key) if ccx_course_key else '' def get_tabs(self, data): - """The CCX Coach tabs, or an empty list when no CCX exists.""" + """ + The CCX Coach tabs, or an empty list when no CCX exists. + + The Student Grades tab is omitted when the writable gradebook is not + available for this CCX, so the MFE does not render a tab that cannot + work. Remaining tabs keep their original ``sort_order`` values, leaving a + gap in the sequence — the MFE orders by ``sort_order``, so gaps are + harmless. + """ ccx_course_key = data.get('ccx_course_key') if not ccx_course_key: return [] + + include_student_grades = is_student_grades_tab_available(ccx_course_key) return [ { 'tab_id': tab_id, @@ -94,6 +140,7 @@ def get_tabs(self, data): 'sort_order': sort_order, } for tab_id, title, sort_order in CCX_COACH_TABS + if include_student_grades or tab_id != STUDENT_GRADES_TAB_ID ] diff --git a/lms/djangoapps/ccx/api/v2/tests/test_views.py b/lms/djangoapps/ccx/api/v2/tests/test_views.py index bf6cea3cba0c..9d9fd081c2c1 100644 --- a/lms/djangoapps/ccx/api/v2/tests/test_views.py +++ b/lms/djangoapps/ccx/api/v2/tests/test_views.py @@ -7,6 +7,7 @@ from ccx_keys.locator import CCXLocator from django.test.utils import override_settings from django.urls import reverse +from edx_toggles.toggles.testutils import override_waffle_flag from rest_framework import status from rest_framework.test import APIClient @@ -16,8 +17,10 @@ from lms.djangoapps.ccx.models import CustomCourseForEdX from lms.djangoapps.ccx.overrides import get_override_for_ccx, override_field_for_ccx from lms.djangoapps.ccx.tests.utils import CcxTestCase +from lms.djangoapps.grades.config.waffle import WRITABLE_GRADEBOOK CCX_COACH_MFE_URL = 'http://localhost:2003/ccx-coach' +WRITABLE_GRADEBOOK_URL = 'http://localhost:1994/gradebook' @override_settings(CUSTOM_COURSES_EDX=True) @@ -42,7 +45,10 @@ def test_master_course_without_ccx_returns_empty(self): assert response.data['ccx_course_id'] == '' assert response.data['tabs'] == [] - @override_settings(CCX_COACH_MICROFRONTEND_URL=CCX_COACH_MFE_URL) + @override_waffle_flag(WRITABLE_GRADEBOOK, active=True) + @override_settings( + CCX_COACH_MICROFRONTEND_URL=CCX_COACH_MFE_URL, WRITABLE_GRADEBOOK_URL=WRITABLE_GRADEBOOK_URL + ) def test_master_course_with_ccx_returns_tabs(self): """When the coach has a CCX, the master id resolves to it (legacy behavior).""" ccx = self.make_ccx() @@ -55,7 +61,10 @@ def test_master_course_with_ccx_returns_tabs(self): assert response.data['ccx_course_id'] == str(ccx_key) self._assert_tabs(response.data['tabs'], ccx_key) - @override_settings(CCX_COACH_MICROFRONTEND_URL=CCX_COACH_MFE_URL) + @override_waffle_flag(WRITABLE_GRADEBOOK, active=True) + @override_settings( + CCX_COACH_MICROFRONTEND_URL=CCX_COACH_MFE_URL, WRITABLE_GRADEBOOK_URL=WRITABLE_GRADEBOOK_URL + ) def test_ccx_course_id_returns_tabs(self): """Passing the CCX id directly resolves and returns its tabs.""" ccx = self.make_ccx() @@ -78,6 +87,8 @@ def _assert_tabs(self, tabs, ccx_key): assert set(tab.keys()) == {'tab_id', 'title', 'url', 'sort_order'} assert tab['url'] == f'/ccx-coach/{ccx_key}/{tab["tab_id"]}' + @override_waffle_flag(WRITABLE_GRADEBOOK, active=True) + @override_settings(WRITABLE_GRADEBOOK_URL=WRITABLE_GRADEBOOK_URL) def test_tabs_without_mfe_url_setting(self): """With the MFE URL unset, tabs are still returned as relative links.""" self.make_ccx() @@ -87,6 +98,50 @@ def test_tabs_without_mfe_url_setting(self): assert len(response.data['tabs']) == 4 assert all(tab['url'].startswith('/') for tab in response.data['tabs']) + # -- Student Grades tab depends on writable gradebook availability ------ + + @override_settings( + CCX_COACH_MICROFRONTEND_URL=CCX_COACH_MFE_URL, WRITABLE_GRADEBOOK_URL=WRITABLE_GRADEBOOK_URL + ) + def test_student_grades_tab_omitted_when_gradebook_flag_off(self): + """Without the writable-gradebook flag the tab is not advertised.""" + self.make_ccx() + + response = self.api_client.get(self._url(self.course.id)) + + assert response.status_code == status.HTTP_200_OK + tab_ids = [tab['tab_id'] for tab in response.data['tabs']] + assert tab_ids == ['enrollments', 'schedule', 'grading_policy'] + # Remaining tabs keep their original sort_order values (gap at 30). + assert [tab['sort_order'] for tab in response.data['tabs']] == [10, 20, 40] + + @override_waffle_flag(WRITABLE_GRADEBOOK, active=True) + @override_settings( + CCX_COACH_MICROFRONTEND_URL=CCX_COACH_MFE_URL, WRITABLE_GRADEBOOK_URL=None + ) + def test_student_grades_tab_omitted_when_gradebook_url_unset(self): + """An enabled flag is not enough; the gradebook URL must be configured.""" + self.make_ccx() + + response = self.api_client.get(self._url(self.course.id)) + + assert response.status_code == status.HTTP_200_OK + tab_ids = [tab['tab_id'] for tab in response.data['tabs']] + assert 'student_grades' not in tab_ids + + @override_waffle_flag(WRITABLE_GRADEBOOK, active=True) + @override_settings( + CCX_COACH_MICROFRONTEND_URL=CCX_COACH_MFE_URL, WRITABLE_GRADEBOOK_URL=WRITABLE_GRADEBOOK_URL + ) + def test_student_grades_tab_present_when_gradebook_available(self): + """With both signals present the tab is advertised.""" + self.make_ccx() + + response = self.api_client.get(self._url(self.course.id)) + + assert response.status_code == status.HTTP_200_OK + assert 'student_grades' in [tab['tab_id'] for tab in response.data['tabs']] + def test_nonexistent_master_course_returns_404(self): response = self.api_client.get(self._url('course-v1:edX+Missing+Missing')) assert response.status_code == status.HTTP_404_NOT_FOUND @@ -127,7 +182,10 @@ def setUp(self): def _url(self, course_id): return reverse('ccx_coach_api_v2:create_ccx', kwargs={'course_id': str(course_id)}) - @override_settings(CCX_COACH_MICROFRONTEND_URL=CCX_COACH_MFE_URL) + @override_waffle_flag(WRITABLE_GRADEBOOK, active=True) + @override_settings( + CCX_COACH_MICROFRONTEND_URL=CCX_COACH_MFE_URL, WRITABLE_GRADEBOOK_URL=WRITABLE_GRADEBOOK_URL + ) def test_create_returns_full_payload_not_redirect(self): """Create returns 201 with the full metadata payload, not a 302 redirect.""" response = self.api_client.post(self._url(self.course.id), {'name': 'My CCX'}, format='json')