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
6 changes: 6 additions & 0 deletions lms/djangoapps/ccx/api/v2/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,3 +101,9 @@ class CreateCCXRequestSerializer(serializers.Serializer): # pylint: disable=abs
"""Validate the `create_ccx` POST body: `{ "name": str }`."""

name = serializers.CharField(max_length=255, allow_blank=False, trim_whitespace=True)


class RemoveScheduleRequestSerializer(serializers.Serializer): # pylint: disable=abstract-method
"""Validate the `remove_schedule` POST body: `{ "location": str }`."""

location = serializers.CharField(allow_blank=False, trim_whitespace=True)
154 changes: 154 additions & 0 deletions lms/djangoapps/ccx/api/v2/tests/test_schedule_views.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
"""
Tests for the CCX Coach API v2 schedule endpoints.
"""

from ccx_keys.locator import CCXLocator
from django.test.utils import override_settings
from django.urls import reverse
from rest_framework import status
from rest_framework.test import APIClient

from common.djangoapps.student.tests.factories import UserFactory
from lms.djangoapps.ccx.models import CcxFieldOverride
from lms.djangoapps.ccx.tests.utils import CcxTestCase


class ScheduleTestMixin:
"""Shared setup for the schedule endpoint tests."""

endpoint_name = None

def setUp(self):
super().setUp()
self.make_coach()
self.ccx = self.make_ccx()
self.ccx_key = CCXLocator.from_course_locator(self.course.id, str(self.ccx.id))
self.api_client = APIClient()
self.api_client.force_authenticate(user=self.coach)

def _url(self, course_id):
return reverse(f'ccx_coach_api_v2:{self.endpoint_name}', kwargs={'course_id': str(course_id)})


@override_settings(CUSTOM_COURSES_EDX=True)
class CCXCoachV2ScheduleGetViewTest(ScheduleTestMixin, CcxTestCase):
"""Tests for `GET /api/ccx_coach/v2/courses/{ccxId}/schedule`."""

endpoint_name = 'schedule'

def test_returns_schedule_tree(self):
response = self.api_client.get(self._url(self.ccx_key))

assert response.status_code == status.HTTP_200_OK
# one node per master-course section, each with subsection children
assert len(response.data) == len(self.chapters)
first = response.data[0]
assert {'location', 'display_name', 'category', 'start', 'hidden'} <= set(first.keys())
assert 'children' in first

def test_master_course_id_rejected(self):
response = self.api_client.get(self._url(self.course.id))
assert response.status_code == status.HTTP_400_BAD_REQUEST

def test_non_coach_forbidden(self):
self.api_client.force_authenticate(user=UserFactory.create())
response = self.api_client.get(self._url(self.ccx_key))
assert response.status_code == status.HTTP_403_FORBIDDEN

def test_requires_authentication(self):
self.api_client.force_authenticate(user=None)
response = self.api_client.get(self._url(self.ccx_key))
assert response.status_code in (status.HTTP_401_UNAUTHORIZED, status.HTTP_403_FORBIDDEN)


@override_settings(CUSTOM_COURSES_EDX=True)
class CCXCoachV2RemoveScheduleViewTest(ScheduleTestMixin, CcxTestCase):
"""Tests for `POST /api/ccx_coach/v2/courses/{ccxId}/remove_schedule`."""

endpoint_name = 'remove_schedule'

def test_remove_hides_block_and_descendants(self):
location = str(self.chapters[0].location)

response = self.api_client.post(self._url(self.ccx_key), {'location': location}, format='json')

assert response.status_code == status.HTTP_200_OK
node = next(n for n in response.data if n['location'] == location)
assert node['hidden'] is True
for child in node.get('children', []):
assert child['hidden'] is True

def test_missing_location_returns_400(self):
response = self.api_client.post(self._url(self.ccx_key), {}, format='json')
assert response.status_code == status.HTTP_400_BAD_REQUEST

def test_unknown_location_returns_400(self):
bogus = str(self.course.id.make_usage_key('chapter', 'does_not_exist'))
response = self.api_client.post(self._url(self.ccx_key), {'location': bogus}, format='json')
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.data.get('error_code') == 'schedule_block_not_found'

def test_non_coach_forbidden(self):
self.api_client.force_authenticate(user=UserFactory.create())
response = self.api_client.post(
self._url(self.ccx_key), {'location': str(self.chapters[0].location)}, format='json'
)
assert response.status_code == status.HTTP_403_FORBIDDEN


@override_settings(CUSTOM_COURSES_EDX=True)
class CCXCoachV2SaveScheduleViewTest(ScheduleTestMixin, CcxTestCase):
"""Tests for `POST /api/ccx_coach/v2/courses/{ccxId}/save_schedule`."""

endpoint_name = 'save_schedule'

def test_save_hides_section_and_returns_payload(self):
location = str(self.chapters[0].location)
payload = [{'location': location, 'hidden': True, 'start': ''}]

response = self.api_client.post(self._url(self.ccx_key), payload, format='json')

assert response.status_code == status.HTTP_200_OK
assert 'schedule' in response.data
assert 'grading_policy' in response.data
node = next(n for n in response.data['schedule'] if n['location'] == location)
assert node['hidden'] is True

def test_invalid_payload_returns_400(self):
response = self.api_client.post(self._url(self.ccx_key), {'not': 'a list'}, format='json')
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.data.get('error_code') == 'invalid_schedule_payload'

def test_partial_failure_rolls_back_overrides(self):
"""
A payload that fails part-way through leaves no overrides applied.

The first entry is valid and would be written, the second references an
unknown block and raises. Because the view catches that exception to
return JSON, the ATOMIC_REQUESTS rollback is suppressed, so the save
runs inside an explicit atomic block. This guards that rollback.
"""
good_location = str(self.chapters[0].location)
bogus_location = str(self.course.id.make_usage_key('chapter', 'does_not_exist'))
payload = [
{'location': good_location, 'hidden': True, 'start': ''},
{'location': bogus_location, 'hidden': True, 'start': ''},
]

response = self.api_client.post(self._url(self.ccx_key), payload, format='json')

assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.data.get('error_code') == 'invalid_schedule_payload'
# The valid entry's override must not have been committed. (Note the CCX
# fixture itself creates an override on the course, so this assertion is
# scoped to the section touched by this payload.)
assert not CcxFieldOverride.objects.filter(
ccx=self.ccx, location=self.chapters[0].location
).exists()

def test_unknown_location_returns_json_400(self):
bogus = str(self.course.id.make_usage_key('chapter', 'does_not_exist'))
payload = [{'location': bogus, 'hidden': True, 'start': ''}]
response = self.api_client.post(self._url(self.ccx_key), payload, format='json')
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.data.get('error_code') == 'invalid_schedule_payload'
15 changes: 15 additions & 0 deletions lms/djangoapps/ccx/api/v2/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,19 @@
views.CreateCCXView.as_view(),
name='create_ccx',
),
re_path(
fr'^courses/{settings.COURSE_ID_PATTERN}/schedule$',
views.CCXScheduleView.as_view(),
name='schedule',
),
re_path(
fr'^courses/{settings.COURSE_ID_PATTERN}/save_schedule$',
views.SaveScheduleView.as_view(),
name='save_schedule',
),
re_path(
fr'^courses/{settings.COURSE_ID_PATTERN}/remove_schedule$',
views.RemoveScheduleView.as_view(),
name='remove_schedule',
),
]
154 changes: 149 additions & 5 deletions lms/djangoapps/ccx/api/v2/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
`DeveloperErrorViewMixin`, JWT/session auth) and reuse existing CCX logic.
"""

import json
import logging

from ccx_keys.locator import CCXLocator
Expand All @@ -25,9 +26,20 @@

from lms.djangoapps.ccx.api.v0.views import get_valid_course
from lms.djangoapps.ccx.api.v2.permissions import IsCCXCoach
from lms.djangoapps.ccx.api.v2.serializers import CCXCoachMetadataSerializer, CreateCCXRequestSerializer
from lms.djangoapps.ccx.utils import create_ccx_course, get_ccx_for_coach
from lms.djangoapps.ccx.api.v2.serializers import (
CCXCoachMetadataSerializer,
CreateCCXRequestSerializer,
RemoveScheduleRequestSerializer,
)
from lms.djangoapps.ccx.utils import (
create_ccx_course,
get_ccx_for_coach,
get_ccx_schedule,
remove_block_from_ccx_schedule,
save_ccx_schedule,
)
from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin
from openedx.core.lib.courses import get_course_by_id

log = logging.getLogger(__name__)

Expand All @@ -46,15 +58,31 @@ def _error_response(error_code, http_status, field_errors=None):
return Response(payload, status=http_status)


def _resolve_ccx_course(course_id):
"""
Resolve a CCX course id to `(master_course, ccx, error_response)`.

`master_course` is the master :class:`CourseBlock` (loaded with full
depth for schedule traversal) and `ccx` is the
:class:`CustomCourseForEdX`. On failure, `error_response` is a DRF
`Response` and the first two values are `None`.
"""
ccx, ccx_key, error_code, http_status = get_valid_course(course_id, is_ccx=True)
if error_code:
return None, None, _error_response(error_code, http_status)
master_course = get_course_by_id(ccx_key.to_course_locator(), depth=None)
return master_course, ccx, None


class CCXCoachMetadataView(DeveloperErrorViewMixin, APIView):
"""
Return CCX Coach metadata for a master course or CCX course.

**Example Request**
*Example Request*

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: do we need these changes? this would make these italic instead of bold (if this works like markdown...)


GET /api/ccx_coach/v2/courses/{course_id|ccx_course_id}/metadata

**Response Values**
*Response Values*

{
"course_id": "course-v1:edX+DemoX+Demo_Course",
Expand Down Expand Up @@ -103,7 +131,7 @@ class CreateCCXView(DeveloperErrorViewMixin, APIView):
"""
Create a CCX course for a master course and return its metadata payload.

**Example Request**
*Example Request*

POST /api/ccx_coach/v2/courses/{course_id}/create_ccx
{ "name": "My CCX" }
Expand Down Expand Up @@ -158,3 +186,119 @@ def post(self, request, course_id):

data = {'master_course_key': master_course_key, 'ccx_course_key': ccx_course_key}
return Response(CCXCoachMetadataSerializer(data).data, status=status.HTTP_201_CREATED)


class CCXScheduleView(DeveloperErrorViewMixin, APIView):
"""
Return the CCX schedule for a CCX course.

*Example Request*

GET /api/ccx_coach/v2/courses/{ccx_course_id}/schedule

*Response Values*

A JSON array of schedule blocks (sections -> subsections -> units), each
with `location`, `display_name`, `category`, `start`, optional
`due`, `hidden` and optional `children`. This mirrors the legacy
`ccx_schedule` output.
"""

authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser)
permission_classes = (IsAuthenticated, IsCCXCoach)

def get(self, request, course_id):
"""Return the CCX schedule for the given CCX course id."""
master_course, ccx, error_response = _resolve_ccx_course(course_id)
if error_response:
return error_response
return Response(get_ccx_schedule(master_course, ccx), status=status.HTTP_200_OK)


class SaveScheduleView(DeveloperErrorViewMixin, APIView):
"""
Apply an edited schedule tree to a CCX course.

*Example Request*

POST /api/ccx_coach/v2/courses/{ccx_course_id}/save_schedule
[ { "location": "...", "hidden": false, "start": "...", "due": "...", "children": [...] }, ... ]

*Response Values*

{ "schedule": [...], "grading_policy": "<json string>" }

Mirrors the legacy `save_ccx` behavior (including automatic grading-policy
adjustment) but with DRF/JWT auth and JSON in/out.
"""

authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser)
permission_classes = (IsAuthenticated, IsCCXCoach)

def post(self, request, course_id):
"""Save the supplied schedule tree to the CCX course."""
master_course, ccx, error_response = _resolve_ccx_course(course_id)
if error_response:
return error_response

schedule_data = request.data
if not isinstance(schedule_data, list):
return _error_response('invalid_schedule_payload', status.HTTP_400_BAD_REQUEST)

try:
# Explicit atomic block: the exception is caught below and converted
# into a response, which would otherwise let the ATOMIC_REQUESTS
# transaction commit. `save_ccx_schedule` writes overrides as it
# walks the tree, so a failure part-way through would leave the
# schedule half-applied. Exiting via the exception rolls it back.
with transaction.atomic():
schedule, policy = save_ccx_schedule(master_course, ccx, schedule_data)
except (KeyError, ValueError, TypeError):
# Unknown block location, missing required keys, or malformed dates
# in the payload. Return a structured JSON error rather than a 500.
return _error_response('invalid_schedule_payload', status.HTTP_400_BAD_REQUEST)

return Response(
{'schedule': schedule, 'grading_policy': json.dumps(policy, indent=4)},
status=status.HTTP_200_OK,
)


class RemoveScheduleView(DeveloperErrorViewMixin, APIView):
"""
Remove a block (and its descendants) from a CCX schedule.

*Example Request*

POST /api/ccx_coach/v2/courses/{ccx_course_id}/remove_schedule
{ "location": "block-v1:edX+DemoX+Demo_Course+type@chapter+block@week1" }

Hides the block and its descendants and clears their start/due overrides,
then returns the updated schedule (same shape as the schedule endpoint) so
the client can refresh in a single call.
"""

authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser)
permission_classes = (IsAuthenticated, IsCCXCoach)

def post(self, request, course_id):
"""Remove the block identified by `location` from the CCX schedule."""
master_course, ccx, error_response = _resolve_ccx_course(course_id)
if error_response:
return error_response

request_serializer = RemoveScheduleRequestSerializer(data=request.data)
request_serializer.is_valid(raise_exception=True)
location = request_serializer.validated_data['location']

try:
# Explicit atomic block, for the same reason as the save endpoint:
# the caught exception suppresses the ATOMIC_REQUESTS rollback, and
# `remove_block_from_ccx_schedule` clears overrides as it walks the
# block's descendants.
with transaction.atomic():
schedule = remove_block_from_ccx_schedule(ccx, master_course, location)
except ValueError:
return _error_response('schedule_block_not_found', status.HTTP_400_BAD_REQUEST)

return Response(schedule, status=status.HTTP_200_OK)
Loading
Loading