Skip to content

Commit 8f27032

Browse files
fix: Add group_id and user_partition_id support to Cohorts API v1
1 parent fd644d4 commit 8f27032

File tree

3 files changed

+328
-26
lines changed

3 files changed

+328
-26
lines changed

docs/lms-openapi.yaml

Lines changed: 95 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -629,10 +629,9 @@ paths:
629629
parameters: []
630630
responses:
631631
'200':
632-
description: ''
632+
description: Successful response with cohort details.
633633
schema:
634-
type: object
635-
properties: {}
634+
$ref: '#/definitions/Cohort'
636635
tags:
637636
- cohorts
638637
post:
@@ -643,32 +642,26 @@ paths:
643642
in: body
644643
required: true
645644
schema:
646-
type: object
647-
properties: {}
645+
$ref: '#/definitions/CohortCreate'
648646
responses:
649-
'201':
650-
description: ''
647+
'200':
648+
description: Successful response with created cohort details.
651649
schema:
652-
type: object
653-
properties: {}
650+
$ref: '#/definitions/Cohort'
654651
tags:
655652
- cohorts
656653
patch:
657654
operationId: cohorts_v1_courses_cohorts_partial_update
658-
description: Endpoint to update a cohort name and/or assignment type.
655+
description: Endpoint to update a cohort name, assignment type, and/or content group association.
659656
parameters:
660657
- name: data
661658
in: body
662659
required: true
663660
schema:
664-
type: object
665-
properties: {}
661+
$ref: '#/definitions/CohortUpdate'
666662
responses:
667-
'200':
668-
description: ''
669-
schema:
670-
type: object
671-
properties: {}
663+
'204':
664+
description: Successful update, no content returned.
672665
tags:
673666
- cohorts
674667
parameters:
@@ -11040,6 +11033,91 @@ paths:
1104011033
required: true
1104111034
type: string
1104211035
definitions:
11036+
Cohort:
11037+
description: A cohort representation
11038+
type: object
11039+
properties:
11040+
id:
11041+
title: ID
11042+
description: The integer identifier for a cohort.
11043+
type: integer
11044+
name:
11045+
title: Name
11046+
description: The string identifier for a cohort.
11047+
type: string
11048+
user_count:
11049+
title: User Count
11050+
description: The number of students in the cohort.
11051+
type: integer
11052+
assignment_type:
11053+
title: Assignment Type
11054+
description: The assignment type ("manual" or "random").
11055+
type: string
11056+
enum:
11057+
- manual
11058+
- random
11059+
user_partition_id:
11060+
title: User Partition ID
11061+
description: The integer identifier of the UserPartition (content group configuration).
11062+
type: integer
11063+
x-nullable: true
11064+
group_id:
11065+
title: Group ID
11066+
description: The integer identifier of the specific group in the partition.
11067+
type: integer
11068+
x-nullable: true
11069+
CohortCreate:
11070+
description: Request body for creating a new cohort
11071+
required:
11072+
- name
11073+
- assignment_type
11074+
type: object
11075+
properties:
11076+
name:
11077+
title: Name
11078+
description: The string identifier for a cohort.
11079+
type: string
11080+
minLength: 1
11081+
assignment_type:
11082+
title: Assignment Type
11083+
description: The assignment type ("manual" or "random").
11084+
type: string
11085+
enum:
11086+
- manual
11087+
- random
11088+
user_partition_id:
11089+
title: User Partition ID
11090+
description: The integer identifier of the UserPartition (content group configuration). Required if group_id is specified.
11091+
type: integer
11092+
group_id:
11093+
title: Group ID
11094+
description: The integer identifier of the specific group in the partition.
11095+
type: integer
11096+
CohortUpdate:
11097+
description: Request body for updating a cohort. At least one of name, assignment_type, or group_id must be provided.
11098+
type: object
11099+
properties:
11100+
name:
11101+
title: Name
11102+
description: The string identifier for a cohort.
11103+
type: string
11104+
minLength: 1
11105+
assignment_type:
11106+
title: Assignment Type
11107+
description: The assignment type ("manual" or "random").
11108+
type: string
11109+
enum:
11110+
- manual
11111+
- random
11112+
user_partition_id:
11113+
title: User Partition ID
11114+
description: The integer identifier of the UserPartition (content group configuration). Required if group_id is specified (non-null).
11115+
type: integer
11116+
group_id:
11117+
title: Group ID
11118+
description: The integer identifier of the specific group in the partition. Set to null to remove the content group association.
11119+
type: integer
11120+
x-nullable: true
1104311121
PermissionValidation:
1104411122
description: The permissions to validate
1104511123
required:

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

Lines changed: 151 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@
1515
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order
1616
from xmodule.modulestore.tests.factories import ToyCourseFactory # lint-amnesty, pylint: disable=wrong-import-order
1717

18-
from .. import cohorts
19-
from .helpers import CohortFactory
18+
from openedx.core.djangoapps.course_groups import cohorts
19+
from openedx.core.djangoapps.course_groups.views import link_cohort_to_partition_group
20+
from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
2021

2122
USERNAME = 'honor'
2223
USER_MAIL = 'honor@example.com'
@@ -458,3 +459,151 @@ def test_add_users_csv(self, is_staff, payload, status):
458459
response = self.client.post(path=path,
459460
data={'uploaded-file': file_pointer})
460461
assert response.status_code == status
462+
463+
def test_post_cohort_with_group_id(self):
464+
"""
465+
Test creating a cohort with group_id and user_partition_id.
466+
"""
467+
path = reverse('api_cohorts:cohort_handler', kwargs={'course_key_string': self.course_str})
468+
self.client.login(username=self.staff_user.username, password=self.password)
469+
470+
payload = json.dumps({
471+
'name': 'TestCohort',
472+
'assignment_type': 'manual',
473+
'group_id': 1,
474+
'user_partition_id': 50
475+
})
476+
response = self.client.post(path=path, data=payload, content_type='application/json')
477+
assert response.status_code == 200
478+
479+
data = json.loads(response.content.decode('utf-8'))
480+
assert data['name'] == 'TestCohort'
481+
assert data['assignment_type'] == 'manual'
482+
assert data['group_id'] == 1
483+
assert data['user_partition_id'] == 50
484+
assert data['user_count'] == 0
485+
assert 'id' in data
486+
487+
def test_post_cohort_with_group_id_missing_partition_id(self):
488+
"""
489+
Test that creating a cohort with group_id but without user_partition_id returns an error.
490+
"""
491+
path = reverse('api_cohorts:cohort_handler', kwargs={'course_key_string': self.course_str})
492+
self.client.login(username=self.staff_user.username, password=self.password)
493+
494+
payload = json.dumps({
495+
'name': 'TestCohort',
496+
'assignment_type': 'manual',
497+
'group_id': 1
498+
})
499+
response = self.client.post(path=path, data=payload, content_type='application/json')
500+
assert response.status_code == 400
501+
502+
data = json.loads(response.content.decode('utf-8'))
503+
assert data['developer_message'] == 'If group_id is specified, user_partition_id must also be specified.'
504+
assert data['error_code'] == 'missing-user-partition-id'
505+
506+
def test_patch_cohort_set_group_id(self):
507+
"""
508+
Test updating a cohort to set group_id and user_partition_id.
509+
"""
510+
cohort = cohorts.add_cohort(self.course_key, "TestCohort", "manual")
511+
path = reverse(
512+
'api_cohorts:cohort_handler',
513+
kwargs={'course_key_string': self.course_str, 'cohort_id': cohort.id}
514+
)
515+
self.client.login(username=self.staff_user.username, password=self.password)
516+
517+
payload = json.dumps({
518+
'group_id': 2,
519+
'user_partition_id': 50
520+
})
521+
response = self.client.patch(path=path, data=payload, content_type='application/json')
522+
assert response.status_code == 204
523+
524+
# Verify by fetching the cohort
525+
response = self.client.get(path=path)
526+
data = json.loads(response.content.decode('utf-8'))
527+
assert data['id'] == cohort.id
528+
assert data['name'] == 'TestCohort'
529+
assert data['assignment_type'] == 'manual'
530+
assert data['group_id'] == 2
531+
assert data['user_partition_id'] == 50
532+
533+
def test_patch_cohort_remove_group_id(self):
534+
"""
535+
Test updating a cohort to remove the group_id association by setting it to null.
536+
"""
537+
cohort = cohorts.add_cohort(self.course_key, "TestCohort", "manual")
538+
link_cohort_to_partition_group(cohort, 50, 1)
539+
540+
path = reverse(
541+
'api_cohorts:cohort_handler',
542+
kwargs={'course_key_string': self.course_str, 'cohort_id': cohort.id}
543+
)
544+
self.client.login(username=self.staff_user.username, password=self.password)
545+
546+
# Verify the cohort has a group_id
547+
response = self.client.get(path=path)
548+
data = json.loads(response.content.decode('utf-8'))
549+
assert data['id'] == cohort.id
550+
assert data['name'] == 'TestCohort'
551+
assert data['group_id'] == 1
552+
assert data['user_partition_id'] == 50
553+
554+
# Remove the group_id by setting it to null
555+
payload = json.dumps({'group_id': None})
556+
response = self.client.patch(path=path, data=payload, content_type='application/json')
557+
assert response.status_code == 204
558+
559+
# Verify the group_id was removed but other fields unchanged
560+
response = self.client.get(path=path)
561+
data = json.loads(response.content.decode('utf-8'))
562+
assert data['id'] == cohort.id
563+
assert data['name'] == 'TestCohort'
564+
assert data['assignment_type'] == 'manual'
565+
assert data['group_id'] is None
566+
assert data['user_partition_id'] is None
567+
568+
def test_patch_cohort_with_group_id_missing_partition_id(self):
569+
"""
570+
Test that updating a cohort with group_id but without user_partition_id returns an error.
571+
"""
572+
cohort = cohorts.add_cohort(self.course_key, "TestCohort", "manual")
573+
path = reverse(
574+
'api_cohorts:cohort_handler',
575+
kwargs={'course_key_string': self.course_str, 'cohort_id': cohort.id}
576+
)
577+
self.client.login(username=self.staff_user.username, password=self.password)
578+
579+
payload = json.dumps({'group_id': 2})
580+
response = self.client.patch(path=path, data=payload, content_type='application/json')
581+
assert response.status_code == 400
582+
583+
data = json.loads(response.content.decode('utf-8'))
584+
assert data['developer_message'] == 'If group_id is specified, user_partition_id must also be specified.'
585+
assert data['error_code'] == 'missing-user-partition-id'
586+
587+
def test_patch_cohort_with_name_only(self):
588+
"""
589+
Test that PATCH with only name is now valid (previously required assignment_type too).
590+
"""
591+
cohort = cohorts.add_cohort(self.course_key, "OldName", "manual")
592+
path = reverse(
593+
'api_cohorts:cohort_handler',
594+
kwargs={'course_key_string': self.course_str, 'cohort_id': cohort.id}
595+
)
596+
self.client.login(username=self.staff_user.username, password=self.password)
597+
598+
payload = json.dumps({'name': 'NewName'})
599+
response = self.client.patch(path=path, data=payload, content_type='application/json')
600+
assert response.status_code == 204
601+
602+
# Verify the name was updated and other fields unchanged
603+
response = self.client.get(path=path)
604+
data = json.loads(response.content.decode('utf-8'))
605+
assert data['id'] == cohort.id
606+
assert data['name'] == 'NewName'
607+
assert data['assignment_type'] == 'manual'
608+
assert data['group_id'] is None
609+
assert data['user_partition_id'] is None

0 commit comments

Comments
 (0)