Skip to content

Commit f1ce202

Browse files
fix: Refactor URLs and tests to core
1 parent d5ab5c9 commit f1ce202

6 files changed

Lines changed: 44 additions & 25 deletions

File tree

lms/djangoapps/instructor/docs/references/instructor-v2-content-groups-spec.yaml renamed to openedx/core/djangoapps/course_groups/rest_api/docs/content-groups-api-v2-spec.yaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
swagger: '2.0'
22
info:
3-
title: Instructor Dashboard Content Groups API v2
3+
title: Content Groups API v2
44
version: 2.0.0
55
description: |
66
REST API for managing content group configurations.
@@ -42,7 +42,7 @@ parameters:
4242
description: The ID of the content group configuration
4343

4444
paths:
45-
/api/instructor/v2/courses/{course_id}/group_configurations:
45+
/api/cohorts/v2/courses/{course_id}/group_configurations:
4646
get:
4747
tags:
4848
- Content Groups
@@ -69,7 +69,7 @@ paths:
6969
404:
7070
description: Course not found
7171

72-
/api/instructor/v2/courses/{course_id}/group_configurations/{configuration_id}:
72+
/api/cohorts/v2/courses/{course_id}/group_configurations/{configuration_id}:
7373
get:
7474
tags:
7575
- Content Groups
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
"""
2+
Tests for Content Groups REST API v2.
3+
"""

lms/djangoapps/instructor/tests/views/test_group_configurations_v2.py renamed to openedx/core/djangoapps/course_groups/rest_api/tests/test_views.py

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
"""
2-
Tests for Content Groups (Group Configurations) V2 API in Instructor Dashboard.
2+
Tests for Content Groups REST API v2.
33
"""
44
from unittest.mock import patch
55

@@ -11,11 +11,13 @@
1111
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
1212
from xmodule.modulestore.tests.factories import CourseFactory
1313
from openedx.core.djangoapps.course_groups.constants import COHORT_SCHEME
14+
from openedx.core.djangolib.testing.utils import skip_unless_lms
1415

1516

17+
@skip_unless_lms
1618
class GroupConfigurationsListViewTestCase(ModuleStoreTestCase):
1719
"""
18-
Tests for GET /api/instructor/v2/courses/{course_id}/group_configurations
20+
Tests for GET /api/cohorts/v2/courses/{course_id}/group_configurations
1921
"""
2022

2123
def setUp(self):
@@ -28,14 +30,13 @@ def setUp(self):
2830
def _get_url(self, course_id=None):
2931
"""Helper to get the list URL"""
3032
course_id = course_id or str(self.course.id)
31-
return f'/api/instructor/v2/courses/{course_id}/group_configurations'
33+
return f'/api/cohorts/v2/courses/{course_id}/group_configurations'
3234

3335
@patch('lms.djangoapps.instructor.permissions.InstructorPermission.has_permission')
3436
def test_list_content_groups_returns_json(self, mock_perm):
3537
"""Verify endpoint returns JSON with correct structure"""
3638
mock_perm.return_value = True
3739

38-
# Create content groups
3940
self.course.user_partitions = [
4041
UserPartition(
4142
id=50,
@@ -50,10 +51,8 @@ def test_list_content_groups_returns_json(self, mock_perm):
5051
]
5152
self.update_course(self.course, self.user.id)
5253

53-
# Get endpoint
5454
response = self.api_client.get(self._get_url())
5555

56-
# Verify response
5756
self.assertEqual(response.status_code, status.HTTP_200_OK)
5857
self.assertEqual(response['Content-Type'], 'application/json')
5958

@@ -62,7 +61,6 @@ def test_list_content_groups_returns_json(self, mock_perm):
6261
self.assertIn('should_show_enrollment_track', data)
6362
self.assertIn('should_show_experiment_groups', data)
6463

65-
# Verify content groups
6664
configs = data['all_group_configurations']
6765
self.assertEqual(len(configs), 1)
6866
self.assertEqual(configs[0]['scheme'], COHORT_SCHEME)
@@ -73,7 +71,6 @@ def test_list_content_groups_filters_non_cohort_partitions(self, mock_perm):
7371
"""Verify only cohort-scheme partitions are returned"""
7472
mock_perm.return_value = True
7573

76-
# Create mixed partitions
7774
self.course.user_partitions = [
7875
UserPartition(
7976
id=50,
@@ -87,7 +84,7 @@ def test_list_content_groups_filters_non_cohort_partitions(self, mock_perm):
8784
name='Experiment Groups',
8885
description='Random experiment groups',
8986
groups=[Group(id=1, name='Group B')],
90-
scheme_id='random' # Not cohort scheme
87+
scheme_id='random'
9188
),
9289
]
9390
self.update_course(self.course, self.user.id)
@@ -97,7 +94,6 @@ def test_list_content_groups_filters_non_cohort_partitions(self, mock_perm):
9794
data = response.json()
9895
configs = data['all_group_configurations']
9996

100-
# Should only return cohort-scheme partition
10197
self.assertEqual(len(configs), 1)
10298
self.assertEqual(configs[0]['id'], 50)
10399
self.assertEqual(configs[0]['scheme'], COHORT_SCHEME)
@@ -107,20 +103,18 @@ def test_list_auto_creates_empty_content_group_if_none_exists(self, mock_perm):
107103
"""Verify empty content group is auto-created when none exists"""
108104
mock_perm.return_value = True
109105

110-
# Don't add any partitions
111106
response = self.api_client.get(self._get_url())
112107

113108
data = response.json()
114109
configs = data['all_group_configurations']
115110

116-
# Should have auto-created empty content group
117111
self.assertEqual(len(configs), 1)
118112
self.assertEqual(configs[0]['scheme'], COHORT_SCHEME)
119113
self.assertEqual(len(configs[0]['groups']), 0)
120114

121115
def test_list_requires_authentication(self):
122116
"""Verify endpoint requires authentication"""
123-
client = APIClient() # Unauthenticated
117+
client = APIClient()
124118
response = client.get(self._get_url())
125119
self.assertIn(response.status_code, [status.HTTP_401_UNAUTHORIZED, status.HTTP_403_FORBIDDEN])
126120

@@ -129,14 +123,14 @@ def test_list_invalid_course_key_returns_400(self, mock_perm):
129123
"""Verify invalid course key returns 400"""
130124
mock_perm.return_value = True
131125

132-
# Use a course key that matches URL pattern but is invalid when parsed
133-
response = self.api_client.get('/api/instructor/v2/courses/course-v1:org/course/run/group_configurations')
134-
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
126+
response = self.api_client.get('/api/cohorts/v2/courses/course-v1:invalid+course+key/group_configurations')
127+
self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
135128

136129

130+
@skip_unless_lms
137131
class GroupConfigurationDetailViewTestCase(ModuleStoreTestCase):
138132
"""
139-
Tests for GET /api/instructor/v2/courses/{course_id}/group_configurations/{id}
133+
Tests for GET /api/cohorts/v2/courses/{course_id}/group_configurations/{id}
140134
"""
141135

142136
def setUp(self):
@@ -146,7 +140,6 @@ def setUp(self):
146140
self.course = CourseFactory.create()
147141
self.api_client.force_authenticate(user=self.user)
148142

149-
# Create a test configuration
150143
self.course.user_partitions = [
151144
UserPartition(
152145
id=50,
@@ -163,7 +156,7 @@ def setUp(self):
163156

164157
def _get_url(self, configuration_id=50):
165158
"""Helper to get detail URL"""
166-
return f'/api/instructor/v2/courses/{self.course.id}/group_configurations/{configuration_id}'
159+
return f'/api/cohorts/v2/courses/{self.course.id}/group_configurations/{configuration_id}'
167160

168161
@patch('lms.djangoapps.instructor.permissions.InstructorPermission.has_permission')
169162
def test_get_configuration_details(self, mock_perm):
@@ -181,6 +174,7 @@ def test_get_configuration_details(self, mock_perm):
181174
self.assertEqual(len(data['groups']), 2)
182175

183176

177+
@skip_unless_lms
184178
class ContentGroupsPermissionsTestCase(ModuleStoreTestCase):
185179
"""
186180
Tests for permission checking
@@ -194,7 +188,7 @@ def setUp(self):
194188

195189
def _get_url(self):
196190
"""Helper to get list URL"""
197-
return f'/api/instructor/v2/courses/{self.course.id}/group_configurations'
191+
return f'/api/cohorts/v2/courses/{self.course.id}/group_configurations'
198192

199193
@patch('lms.djangoapps.instructor.permissions.InstructorPermission.has_permission')
200194
def test_staff_user_can_access(self, mock_perm):
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
"""
2+
Content Groups REST API v2 URLs
3+
"""
4+
from django.urls import re_path
5+
6+
from openedx.core.constants import COURSE_ID_PATTERN
7+
from openedx.core.djangoapps.course_groups.rest_api import views
8+
9+
urlpatterns = [
10+
re_path(
11+
fr'^v2/courses/{COURSE_ID_PATTERN}/group_configurations$',
12+
views.GroupConfigurationsListView.as_view(),
13+
name='group_configurations_list'
14+
),
15+
re_path(
16+
fr'^v2/courses/{COURSE_ID_PATTERN}/group_configurations/(?P<configuration_id>\d+)$',
17+
views.GroupConfigurationDetailView.as_view(),
18+
name='group_configurations_detail'
19+
),
20+
]

openedx/core/djangoapps/course_groups/rest_api/views.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ def get(self, request, course_id):
8787
"should_show_enrollment_track": False,
8888
"should_show_experiment_groups": True,
8989
"context_course": None,
90-
"group_configuration_url": f"/api/instructor/v2/courses/{course_id}/group_configurations",
90+
"group_configuration_url": f"/api/cohorts/v2/courses/{course_id}/group_configurations",
9191
"course_outline_url": f"/api/contentstore/v1/courses/{course_id}",
9292
}
9393

openedx/core/djangoapps/course_groups/urls.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55

66
from django.conf import settings
7-
from django.urls import re_path
7+
from django.urls import include, re_path
88

99
import lms.djangoapps.instructor.views.api
1010
import openedx.core.djangoapps.course_groups.views
@@ -38,4 +38,6 @@
3838
lms.djangoapps.instructor.views.api.CohortCSV.as_view(),
3939
name='cohort_users_csv',
4040
),
41+
# v2 Content Groups API
42+
re_path(r'', include('openedx.core.djangoapps.course_groups.rest_api.urls')),
4143
]

0 commit comments

Comments
 (0)