fix: Add group_id and user_partition_id support to Cohorts API v1 (#37982)

Author: brianjbuck-wgu

docs/lms-openapi.yaml

@@ -629,10 +629,9 @@ paths:
       parameters: []
       responses:
         '200':
-          description: ''
+          description: Successful response with cohort details.
           schema:
-            type: object
-            properties: {}
+            $ref: '#/definitions/Cohort'
       tags:
       - cohorts
     post:
@@ -643,32 +642,26 @@ paths:
         in: body
         required: true
         schema:
-          type: object
-          properties: {}
+          $ref: '#/definitions/CohortCreate'
       responses:
-        '201':
-          description: ''
+        '200':
+          description: Successful response with created cohort details.
           schema:
-            type: object
-            properties: {}
+            $ref: '#/definitions/Cohort'
       tags:
       - cohorts
     patch:
       operationId: cohorts_v1_courses_cohorts_partial_update
-      description: Endpoint to update a cohort name and/or assignment type.
+      description: Endpoint to update a cohort name, assignment type, and/or content group association.
       parameters:
       - name: data
         in: body
         required: true
         schema:
-          type: object
-          properties: {}
+          $ref: '#/definitions/CohortUpdate'
       responses:
-        '200':
-          description: ''
-          schema:
-            type: object
-            properties: {}
+        '204':
+          description: Successful update, no content returned.
       tags:
       - cohorts
     parameters:
@@ -11040,6 +11033,91 @@ paths:
       required: true
       type: string
 definitions:
+  Cohort:
+    description: A cohort representation
+    type: object
+    properties:
+      id:
+        title: ID
+        description: The integer identifier for a cohort.
+        type: integer
+      name:
+        title: Name
+        description: The string identifier for a cohort.
+        type: string
+      user_count:
+        title: User Count
+        description: The number of students in the cohort.
+        type: integer
+      assignment_type:
+        title: Assignment Type
+        description: The assignment type ("manual" or "random").
+        type: string
+        enum:
+        - manual
+        - random
+      user_partition_id:
+        title: User Partition ID
+        description: The integer identifier of the UserPartition (content group configuration).
+        type: integer
+        x-nullable: true
+      group_id:
+        title: Group ID
+        description: The integer identifier of the specific group in the partition.
+        type: integer
+        x-nullable: true
+  CohortCreate:
+    description: Request body for creating a new cohort
+    required:
+    - name
+    - assignment_type
+    type: object
+    properties:
+      name:
+        title: Name
+        description: The string identifier for a cohort.
+        type: string
+        minLength: 1
+      assignment_type:
+        title: Assignment Type
+        description: The assignment type ("manual" or "random").
+        type: string
+        enum:
+        - manual
+        - random
+      user_partition_id:
+        title: User Partition ID
+        description: The integer identifier of the UserPartition (content group configuration). Required if group_id is specified.
+        type: integer
+      group_id:
+        title: Group ID
+        description: The integer identifier of the specific group in the partition.
+        type: integer
+  CohortUpdate:
+    description: Request body for updating a cohort. At least one of name, assignment_type, or group_id must be provided.
+    type: object
+    properties:
+      name:
+        title: Name
+        description: The string identifier for a cohort.
+        type: string
+        minLength: 1
+      assignment_type:
+        title: Assignment Type
+        description: The assignment type ("manual" or "random").
+        type: string
+        enum:
+        - manual
+        - random
+      user_partition_id:
+        title: User Partition ID
+        description: The integer identifier of the UserPartition (content group configuration). Required if group_id is specified (non-null).
+        type: integer
+      group_id:
+        title: Group ID
+        description: The integer identifier of the specific group in the partition. Set to null to remove the content group association.
+        type: integer
+        x-nullable: true
   PermissionValidation:
     description: The permissions to validate
     required:

openedx/core/djangoapps/course_groups/tests/test_api_views.py

@@ -3,7 +3,6 @@
 """
 
 
-import json
 import tempfile
 
 import ddt
@@ -15,16 +14,17 @@
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase  # lint-amnesty, pylint: disable=wrong-import-order
 from xmodule.modulestore.tests.factories import ToyCourseFactory  # lint-amnesty, pylint: disable=wrong-import-order
 
-from .. import cohorts
-from .helpers import CohortFactory
+from openedx.core.djangoapps.course_groups import cohorts
+from openedx.core.djangoapps.course_groups.views import link_cohort_to_partition_group
+from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
 
 USERNAME = 'honor'
 USER_MAIL = 'honor@example.com'
 SETTINGS_PAYLOAD = '{"is_cohorted": true}'
 HANDLER_POST_PAYLOAD = '{"name":"Default","user_count":0,"assignment_type":"random","user_partition_id":null\
 ,"group_id":null}'
 HANDLER_PATCH_PAYLOAD = '{"name":"Default Group","group_id":null,"user_partition_id":null,"assignment_type":"random"}'
-ADD_USER_PAYLOAD = json.dumps({'users': [USER_MAIL, ]})
+ADD_USER_PAYLOAD = {'users': [USER_MAIL, ]}
 CSV_DATA = f'''email,cohort\n{USER_MAIL},DEFAULT'''
 
 
@@ -307,7 +307,7 @@ def test_list_users_in_cohort(self, is_staff, status):
         assert response.status_code == status
 
         if status == 200:
-            results = json.loads(response.content.decode('utf-8'))['results']
+            results = response.json()['results']
             expected_results = [{
                 'username': user.username,
                 'email': user.email,
@@ -406,7 +406,7 @@ def test_add_users_to_cohort_different_types_of_users(self):
             "invalid": ["foo@bar"],
             "present": ["user2"]
         }
-        assert json.loads(response.content.decode('utf-8')) == expected_response
+        assert response.json() == expected_response
 
     def test_remove_user_from_cohort_missing_username(self):
         """
@@ -458,3 +458,151 @@ def test_add_users_csv(self, is_staff, payload, status):
             response = self.client.post(path=path,
                                         data={'uploaded-file': file_pointer})
             assert response.status_code == status
+
+    def test_post_cohort_with_group_id(self):
+        """
+        Test creating a cohort with group_id and user_partition_id.
+        """
+        path = reverse('api_cohorts:cohort_handler', kwargs={'course_key_string': self.course_str})
+        self.client.login(username=self.staff_user.username, password=self.password)
+
+        payload = {
+            'name': 'TestCohort',
+            'assignment_type': 'manual',
+            'group_id': 1,
+            'user_partition_id': 50
+        }
+        response = self.client.post(path=path, data=payload, content_type='application/json')
+        assert response.status_code == 200
+
+        data = response.json()
+        assert data['name'] == 'TestCohort'
+        assert data['assignment_type'] == 'manual'
+        assert data['group_id'] == 1
+        assert data['user_partition_id'] == 50
+        assert data['user_count'] == 0
+        assert 'id' in data
+
+    def test_post_cohort_with_group_id_missing_partition_id(self):
+        """
+        Test that creating a cohort with group_id but without user_partition_id returns an 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)
+
+        payload = {
+            'name': 'TestCohort',
+            'assignment_type': 'manual',
+            'group_id': 1
+        }
+        response = self.client.post(path=path, data=payload, content_type='application/json')
+        assert response.status_code == 400
+
+        data = response.json()
+        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_patch_cohort_set_group_id(self):
+        """
+        Test updating a cohort to set group_id and user_partition_id.
+        """
+        cohort = cohorts.add_cohort(self.course_key, "TestCohort", "manual")
+        path = reverse(
+            'api_cohorts:cohort_handler',
+            kwargs={'course_key_string': self.course_str, 'cohort_id': cohort.id}
+        )
+        self.client.login(username=self.staff_user.username, password=self.password)
+
+        payload = {
+            'group_id': 2,
+            'user_partition_id': 50
+        }
+        response = self.client.patch(path=path, data=payload, content_type='application/json')
+        assert response.status_code == 204
+
+        # Verify by fetching the cohort
+        response = self.client.get(path=path)
+        data = response.json()
+        assert data['id'] == cohort.id
+        assert data['name'] == 'TestCohort'
+        assert data['assignment_type'] == 'manual'
+        assert data['group_id'] == 2
+        assert data['user_partition_id'] == 50
+
+    def test_patch_cohort_remove_group_id(self):
+        """
+        Test updating a cohort to remove the group_id association by setting it to null.
+        """
+        cohort = cohorts.add_cohort(self.course_key, "TestCohort", "manual")
+        link_cohort_to_partition_group(cohort, 50, 1)
+
+        path = reverse(
+            'api_cohorts:cohort_handler',
+            kwargs={'course_key_string': self.course_str, 'cohort_id': cohort.id}
+        )
+        self.client.login(username=self.staff_user.username, password=self.password)
+
+        # Verify the cohort has a group_id
+        response = self.client.get(path=path)
+        data = response.json()
+        assert data['id'] == cohort.id
+        assert data['name'] == 'TestCohort'
+        assert data['group_id'] == 1
+        assert data['user_partition_id'] == 50
+
+        # Remove the group_id by setting it to null
+        payload = {'group_id': None}
+        response = self.client.patch(path=path, data=payload, content_type='application/json')
+        assert response.status_code == 204
+
+        # Verify the group_id was removed but other fields unchanged
+        response = self.client.get(path=path)
+        data = response.json()
+        assert data['id'] == cohort.id
+        assert data['name'] == 'TestCohort'
+        assert data['assignment_type'] == 'manual'
+        assert data['group_id'] is None
+        assert data['user_partition_id'] is None
+
+    def test_patch_cohort_with_group_id_missing_partition_id(self):
+        """
+        Test that updating a cohort with group_id but without user_partition_id returns an error.
+        """
+        cohort = cohorts.add_cohort(self.course_key, "TestCohort", "manual")
+        path = reverse(
+            'api_cohorts:cohort_handler',
+            kwargs={'course_key_string': self.course_str, 'cohort_id': cohort.id}
+        )
+        self.client.login(username=self.staff_user.username, password=self.password)
+
+        payload = {'group_id': 2}
+        response = self.client.patch(path=path, data=payload, content_type='application/json')
+        assert response.status_code == 400
+
+        data = response.json()
+        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_patch_cohort_with_name_only(self):
+        """
+        Test that PATCH with only name is now valid (previously required assignment_type too).
+        """
+        cohort = cohorts.add_cohort(self.course_key, "OldName", "manual")
+        path = reverse(
+            'api_cohorts:cohort_handler',
+            kwargs={'course_key_string': self.course_str, 'cohort_id': cohort.id}
+        )
+        self.client.login(username=self.staff_user.username, password=self.password)
+
+        payload = {'name': 'NewName'}
+        response = self.client.patch(path=path, data=payload, content_type='application/json')
+        assert response.status_code == 204
+
+        # Verify the name was updated and other fields unchanged
+        response = self.client.get(path=path)
+        data = response.json()
+        assert data['id'] == cohort.id
+        assert data['name'] == 'NewName'
+        assert data['assignment_type'] == 'manual'
+        assert data['group_id'] is None
+        assert data['user_partition_id'] is None