From 121afd0c5fcdac96fcb41f943c685bc422f2cd6c Mon Sep 17 00:00:00 2001 From: Nathan Sprenkle Date: Mon, 2 Aug 2021 11:06:08 -0400 Subject: [PATCH] refactor: new logic for when to enable Gradebook bulk management (#28080) Formerly, bulk management was enabled by sending feature feature toggle to Gradebook which also checked for the presence of a master's track to enable the feature. This default enables for all courses with a master's track and also allows selective enabling for courses with the feature flag. --- lms/djangoapps/grades/api.py | 2 +- lms/djangoapps/grades/config/waffle.py | 6 +-- .../grades/rest_api/v1/gradebook_views.py | 16 ++++++-- .../rest_api/v1/tests/test_gradebook_views.py | 39 ++++++++++++++++++- 4 files changed, 54 insertions(+), 9 deletions(-) diff --git a/lms/djangoapps/grades/api.py b/lms/djangoapps/grades/api.py index ed0a82c08d..9f14e33945 100644 --- a/lms/djangoapps/grades/api.py +++ b/lms/djangoapps/grades/api.py @@ -16,7 +16,7 @@ from common.djangoapps.track.event_transaction_utils import create_new_event_tra from lms.djangoapps.grades import constants, context, course_data, events # Grades APIs that should NOT belong within the Grades subsystem # TODO move Gradebook to be an external feature outside of core Grades -from lms.djangoapps.grades.config.waffle import gradebook_can_see_bulk_management, is_writable_gradebook_enabled +from lms.djangoapps.grades.config.waffle import gradebook_bulk_management_enabled, is_writable_gradebook_enabled # Public Grades Factories from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory from lms.djangoapps.grades.models_api import * diff --git a/lms/djangoapps/grades/config/waffle.py b/lms/djangoapps/grades/config/waffle.py index da5c0ae80a..1ca42ff7b8 100644 --- a/lms/djangoapps/grades/config/waffle.py +++ b/lms/djangoapps/grades/config/waffle.py @@ -150,10 +150,8 @@ def is_writable_gradebook_enabled(course_key): return waffle_flags()[WRITABLE_GRADEBOOK].is_enabled(course_key) -def gradebook_can_see_bulk_management(course_key): +def gradebook_bulk_management_enabled(course_key): """ - Returns whether bulk management features should be visible for the given course. - - (provided that course contains a masters track, as of this writing) + Returns whether bulk management features should be specially enabled for a given course. """ return waffle_flags()[BULK_MANAGEMENT].is_enabled(course_key) diff --git a/lms/djangoapps/grades/rest_api/v1/gradebook_views.py b/lms/djangoapps/grades/rest_api/v1/gradebook_views.py index eb32de8c1d..a064e759af 100644 --- a/lms/djangoapps/grades/rest_api/v1/gradebook_views.py +++ b/lms/djangoapps/grades/rest_api/v1/gradebook_views.py @@ -21,7 +21,7 @@ from rest_framework.response import Response from rest_framework.views import APIView from common.djangoapps.student.auth import has_course_author_access -from common.djangoapps.student.models import CourseEnrollment, CourseAccessRole +from common.djangoapps.student.models import CourseAccessRole, CourseEnrollment, CourseMode from common.djangoapps.student.roles import BulkRoleCache from common.djangoapps.track.event_transaction_utils import ( create_new_event_transaction_id, @@ -35,7 +35,7 @@ from lms.djangoapps.grades.api import CourseGradeFactory, clear_prefetched_cours from lms.djangoapps.grades.api import constants as grades_constants from lms.djangoapps.grades.api import context as grades_context from lms.djangoapps.grades.api import events as grades_events -from lms.djangoapps.grades.api import gradebook_can_see_bulk_management as can_see_bulk_management +from lms.djangoapps.grades.api import gradebook_bulk_management_enabled from lms.djangoapps.grades.api import is_writable_gradebook_enabled, prefetch_course_and_subsection_grades from lms.djangoapps.grades.course_data import CourseData from lms.djangoapps.grades.grade_utils import are_grades_frozen @@ -56,6 +56,7 @@ from lms.djangoapps.grades.subsection_grade_factory import SubsectionGradeFactor from lms.djangoapps.grades.tasks import recalculate_subsection_grade_v3 from lms.djangoapps.program_enrollments.api import get_external_key_by_user_and_course from openedx.core.djangoapps.course_groups import cohorts +from openedx.core.djangoapps.enrollments.api import get_course_enrollment_details from openedx.core.djangoapps.util.forms import to_bool from openedx.core.lib.api.view_utils import ( DeveloperErrorViewMixin, @@ -281,10 +282,19 @@ class CourseGradingView(BaseCourseView): 'assignment_types': self._get_assignment_types(course), 'subsections': self._get_subsections(course, graded_only), 'grades_frozen': are_grades_frozen(course_key), - 'can_see_bulk_management': can_see_bulk_management(course_key), + 'can_see_bulk_management': self.can_see_bulk_management(course_key), } return Response(results) + def can_see_bulk_management(self, course_key): + """ + Whether or not to show bulk management for this course. Currently, if a course has a + master's track or is enabled with the grades.bulk_management course waffle flag. + """ + course_modes = get_course_enrollment_details(str(course_key), include_expired=True).get('course_modes', []) + course_has_masters_track = any((course_mode['slug'] == CourseMode.MASTERS for course_mode in course_modes)) + return course_has_masters_track or gradebook_bulk_management_enabled(course_key) + def _get_assignment_types(self, course): """ Helper function that returns a serialized dict of assignment types diff --git a/lms/djangoapps/grades/rest_api/v1/tests/test_gradebook_views.py b/lms/djangoapps/grades/rest_api/v1/tests/test_gradebook_views.py index f9e196c2c4..b0bc83369e 100644 --- a/lms/djangoapps/grades/rest_api/v1/tests/test_gradebook_views.py +++ b/lms/djangoapps/grades/rest_api/v1/tests/test_gradebook_views.py @@ -32,7 +32,7 @@ from common.djangoapps.student.tests.factories import InstructorFactory from common.djangoapps.student.tests.factories import StaffFactory from lms.djangoapps.certificates.data import CertificateStatuses from lms.djangoapps.certificates.models import GeneratedCertificate -from lms.djangoapps.grades.config.waffle import WRITABLE_GRADEBOOK, waffle_flags +from lms.djangoapps.grades.config.waffle import BULK_MANAGEMENT, WRITABLE_GRADEBOOK, waffle_flags from lms.djangoapps.grades.constants import GradeOverrideFeatureEnum from lms.djangoapps.grades.course_data import CourseData from lms.djangoapps.grades.course_grade import CourseGrade @@ -245,6 +245,43 @@ class CourseGradingViewTest(SharedModuleStoreTestCase, APITestCase): expected_data['grades_frozen'] = True assert expected_data == resp.data + @patch('lms.djangoapps.grades.rest_api.v1.gradebook_views.get_course_enrollment_details') + def test_can_see_bulk_management_non_masters(self, mock_course_enrollment_details): + # Given a course without a master's track + mock_course_enrollment_details.return_value = {'course_modes': [{'slug': 'not-masters'}]} + + # When getting course grading view + self.client.login(username=self.staff.username, password=self.password) + resp = self.client.get(self.get_url(self.course_key)) + + # Course staff should not be shown bulk management controls + assert resp.status_code == status.HTTP_200_OK + assert resp.data['can_see_bulk_management'] is False + + @patch('lms.djangoapps.grades.rest_api.v1.gradebook_views.get_course_enrollment_details') + def test_can_see_bulk_management_masters(self, mock_course_enrollment_details): + # Given a course with a master's track + mock_course_enrollment_details.return_value = {'course_modes': [{'slug': 'not-masters'}, {'slug': 'masters'}]} + + # When getting course grading view + self.client.login(username=self.staff.username, password=self.password) + resp = self.client.get(self.get_url(self.course_key)) + + # Course staff should be shown bulk management controls (default on for master's track courses) + assert resp.status_code == status.HTTP_200_OK + assert resp.data['can_see_bulk_management'] is True + + @override_waffle_flag(waffle_flags()[BULK_MANAGEMENT], active=True) + def test_can_see_bulk_management_force_enabled(self): + # Given a course without (or with) a master's track where bulk management is enabled with the config flag + # When getting course grading view + self.client.login(username=self.staff.username, password=self.password) + resp = self.client.get(self.get_url(self.course_key)) + + # # Course staff should be able to see bulk management + assert resp.status_code == status.HTTP_200_OK + assert resp.data['can_see_bulk_management'] is True + class GradebookViewTestBase(GradeViewTestMixin, APITestCase): """