Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion openedx/core/djangoapps/course_groups/cohorts.py
Original file line number Diff line number Diff line change
Expand Up @@ -319,14 +319,16 @@ 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.

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
Expand All @@ -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)


Expand Down
43 changes: 43 additions & 0 deletions openedx/core/djangoapps/course_groups/tests/test_api_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
20 changes: 17 additions & 3 deletions openedx/core/djangoapps/course_groups/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
Loading