diff --git a/openedx/core/djangoapps/course_groups/cohorts.py b/openedx/core/djangoapps/course_groups/cohorts.py index 5bded799fdb9..a90ee4cb7b18 100644 --- a/openedx/core/djangoapps/course_groups/cohorts.py +++ b/openedx/core/djangoapps/course_groups/cohorts.py @@ -319,7 +319,7 @@ def migrate_cohort_settings(course): return cohort_settings -def get_course_cohorts(course=None, course_id=None, assignment_type=None): +def get_course_cohorts(course=None, course_id=None, assignment_type=None, ordering='asc'): """ Get a list of all the cohorts in the given course. This will include auto cohorts, regardless of whether or not the auto cohorts include any users. @@ -327,6 +327,8 @@ def get_course_cohorts(course=None, course_id=None, assignment_type=None): Arguments: course: the course for which cohorts should be returned assignment_type: cohort assignment type + ordering: sort direction for results by name. Use 'desc' for descending order. + Any other value (including the default 'asc') results in ascending order. Returns: A list of CourseUserGroup objects. Empty if there are no cohorts. Does @@ -343,6 +345,11 @@ def get_course_cohorts(course=None, course_id=None, assignment_type=None): group_type=CourseUserGroup.COHORT ) query_set = query_set.filter(cohort__assignment_type=assignment_type) if assignment_type else query_set + ordering = ordering.lower() + if ordering not in ('asc', 'desc'): + raise ValueError(f"Invalid ordering value '{ordering}'. Must be 'asc' or 'desc'.") + sort_field = '-name' if ordering == 'desc' else 'name' + query_set = query_set.order_by(sort_field) return list(query_set) diff --git a/openedx/core/djangoapps/course_groups/tests/test_api_views.py b/openedx/core/djangoapps/course_groups/tests/test_api_views.py index d357bbc35fc7..46d6a3399a0b 100644 --- a/openedx/core/djangoapps/course_groups/tests/test_api_views.py +++ b/openedx/core/djangoapps/course_groups/tests/test_api_views.py @@ -583,6 +583,49 @@ def test_patch_cohort_with_group_id_missing_partition_id(self): assert data['developer_message'] == 'If group_id is specified, user_partition_id must also be specified.' assert data['error_code'] == 'missing-user-partition-id' + def test_get_cohorts_default_ordering(self): + """ + Test that cohorts are returned in ascending alphabetical order by default. + """ + cohorts.add_cohort(self.course_key, "Zebra", "manual") + cohorts.add_cohort(self.course_key, "Alpha", "manual") + cohorts.add_cohort(self.course_key, "Mango", "manual") + + path = reverse('api_cohorts:cohort_handler', kwargs={'course_key_string': self.course_str}) + self.client.login(username=self.staff_user.username, password=self.password) + response = self.client.get(path=path) + + assert response.status_code == 200 + names = [c['name'] for c in response.json()] + assert names == ['Alpha', 'Mango', 'Zebra'] + + def test_get_cohorts_desc_ordering(self): + """ + Test that cohorts are returned in descending alphabetical order when ordering=desc. + """ + cohorts.add_cohort(self.course_key, "Zebra", "manual") + cohorts.add_cohort(self.course_key, "Alpha", "manual") + cohorts.add_cohort(self.course_key, "Mango", "manual") + + path = reverse('api_cohorts:cohort_handler', kwargs={'course_key_string': self.course_str}) + self.client.login(username=self.staff_user.username, password=self.password) + response = self.client.get(path=path, data={'ordering': 'desc'}) + + assert response.status_code == 200 + names = [c['name'] for c in response.json()] + assert names == ['Zebra', 'Mango', 'Alpha'] + + def test_get_cohorts_invalid_ordering(self): + """ + Test that an invalid ordering value returns a 400 error. + """ + path = reverse('api_cohorts:cohort_handler', kwargs={'course_key_string': self.course_str}) + self.client.login(username=self.staff_user.username, password=self.password) + response = self.client.get(path=path, data={'ordering': 'invalid'}) + + assert response.status_code == 400 + assert response.json().get('error_code') == 'invalid-ordering-value' + def test_patch_cohort_with_name_only(self): """ Test that PATCH with only name is now valid (previously required assignment_type too). diff --git a/openedx/core/djangoapps/course_groups/views.py b/openedx/core/djangoapps/course_groups/views.py index c4775e4be6ca..4f63335cb8a6 100644 --- a/openedx/core/djangoapps/course_groups/views.py +++ b/openedx/core/djangoapps/course_groups/views.py @@ -532,11 +532,18 @@ class CohortHandler(DeveloperErrorViewMixin, APIPermissions): **Example Requests**: - GET /api/cohorts/v1/courses/{course_id}/cohorts - POST /api/cohorts/v1/courses/{course_id}/cohorts + GET /api/cohorts/v1/courses/{course_id}/cohorts/ + GET /api/cohorts/v1/courses/{course_id}/cohorts/?ordering=asc + GET /api/cohorts/v1/courses/{course_id}/cohorts/?ordering=desc + POST /api/cohorts/v1/courses/{course_id}/cohorts/ GET /api/cohorts/v1/courses/{course_id}/cohorts/{cohort_id} PATCH /api/cohorts/v1/courses/{course_id}/cohorts/{cohort_id} + **GET Query Parameters** + + * ordering (optional): Sort direction for the cohort list by name. Accepted values are + "asc" (ascending, default) and "desc" (descending). Returns HTTP 400 for invalid values. + **POST Request Values** * name (required): The string identifier for a cohort. @@ -575,7 +582,14 @@ def get(self, request, course_key_string, cohort_id=None): """ course_key, course = _get_course_with_access(request, course_key_string, 'load') if not cohort_id: - all_cohorts = cohorts.get_course_cohorts(course) + ordering = request.query_params.get('ordering', 'asc').lower() + if ordering not in ('asc', 'desc'): + raise self.api_error( + status.HTTP_400_BAD_REQUEST, + 'Invalid ordering value. Must be "asc" or "desc".', + 'invalid-ordering-value' + ) + all_cohorts = cohorts.get_course_cohorts(course, ordering=ordering) paginator = NamespacedPageNumberPagination() paginator.max_page_size = MAX_PAGE_SIZE page = paginator.paginate_queryset(all_cohorts, request)