From 6b6524577a57b9d33679163686ad0f2987a83bb9 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Mon, 14 Apr 2025 10:30:19 -0400 Subject: [PATCH 1/3] build: Add requirements for model and APIs Add DRF and django-filters to be able to add an API and add edx-opaque-keys so we can have a course_id model. --- backend/requirements/base.in | 4 +++- backend/requirements/base.txt | 21 +++++++++++++++++++++ backend/requirements/dev.txt | 21 +++++++++++++++++++++ backend/requirements/doc.txt | 19 +++++++++++++++++++ backend/requirements/quality.txt | 21 +++++++++++++++++++++ backend/requirements/test.txt | 29 +++++++++++++++++++++++++++-- 6 files changed, 112 insertions(+), 3 deletions(-) diff --git a/backend/requirements/base.in b/backend/requirements/base.in index 9f4002e..ffce419 100644 --- a/backend/requirements/base.in +++ b/backend/requirements/base.in @@ -2,6 +2,8 @@ -c constraints.txt Django # Web application framework - +djangorestframework # REST API framework +django-filter # Filtering for DRF +edx-opaque-keys # Open edX CourseKeyField openedx-atlas diff --git a/backend/requirements/base.txt b/backend/requirements/base.txt index 055b87f..825f50e 100644 --- a/backend/requirements/base.txt +++ b/backend/requirements/base.txt @@ -10,7 +10,28 @@ django==4.2.20 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/base.in + # django-filter + # djangorestframework +django-filter==25.1 + # via -r requirements/base.in +djangorestframework==3.16.0 + # via -r requirements/base.in +dnspython==2.7.0 + # via pymongo +edx-opaque-keys==2.12.0 + # via -r requirements/base.in openedx-atlas==0.7.0 # via -r requirements/base.in +pbr==6.1.1 + # via stevedore +pymongo==4.12.0 + # via edx-opaque-keys sqlparse==0.5.3 # via django +stevedore==5.4.1 + # via edx-opaque-keys +typing-extensions==4.13.2 + # via edx-opaque-keys + +# The following packages are considered to be unsafe in a requirements file: +# setuptools diff --git a/backend/requirements/dev.txt b/backend/requirements/dev.txt index 94b0222..16f714d 100644 --- a/backend/requirements/dev.txt +++ b/backend/requirements/dev.txt @@ -64,11 +64,23 @@ django==4.2.20 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/quality.txt + # django-filter + # djangorestframework # edx-i18n-tools +django-filter==25.1 + # via -r requirements/quality.txt +djangorestframework==3.16.0 + # via -r requirements/quality.txt +dnspython==2.7.0 + # via + # -r requirements/quality.txt + # pymongo edx-i18n-tools==1.7.0 # via -r requirements/dev.in edx-lint==5.6.0 # via -r requirements/quality.txt +edx-opaque-keys==2.12.0 + # via -r requirements/quality.txt filelock==3.18.0 # via # -r requirements/ci.txt @@ -162,6 +174,10 @@ pylint-plugin-utils==0.8.2 # -r requirements/quality.txt # pylint-celery # pylint-django +pymongo==4.12.0 + # via + # -r requirements/quality.txt + # edx-opaque-keys pyproject-api==1.9.0 # via # -r requirements/ci.txt @@ -205,6 +221,7 @@ stevedore==5.4.1 # via # -r requirements/quality.txt # code-annotations + # edx-opaque-keys text-unidecode==1.3 # via # -r requirements/quality.txt @@ -215,6 +232,10 @@ tomlkit==0.13.2 # pylint tox==4.25.0 # via -r requirements/ci.txt +typing-extensions==4.13.2 + # via + # -r requirements/quality.txt + # edx-opaque-keys virtualenv==20.30.0 # via # -r requirements/ci.txt diff --git a/backend/requirements/doc.txt b/backend/requirements/doc.txt index 66b2d64..4c8ae0d 100644 --- a/backend/requirements/doc.txt +++ b/backend/requirements/doc.txt @@ -42,6 +42,16 @@ django==4.2.20 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/test.txt + # django-filter + # djangorestframework +django-filter==25.1 + # via -r requirements/test.txt +djangorestframework==3.16.0 + # via -r requirements/test.txt +dnspython==2.7.0 + # via + # -r requirements/test.txt + # pymongo doc8==1.1.2 # via -r requirements/doc.in docutils==0.21.2 @@ -51,6 +61,8 @@ docutils==0.21.2 # readme-renderer # restructuredtext-lint # sphinx +edx-opaque-keys==2.12.0 + # via -r requirements/test.txt id==1.5.0 # via twine idna==3.10 @@ -122,6 +134,10 @@ pygments==2.19.1 # readme-renderer # rich # sphinx +pymongo==4.12.0 + # via + # -r requirements/test.txt + # edx-opaque-keys pyproject-hooks==1.2.0 # via build pytest==8.3.5 @@ -193,6 +209,7 @@ stevedore==5.4.1 # -r requirements/test.txt # code-annotations # doc8 + # edx-opaque-keys text-unidecode==1.3 # via # -r requirements/test.txt @@ -201,7 +218,9 @@ twine==6.1.0 # via -r requirements/doc.in typing-extensions==4.13.2 # via + # -r requirements/test.txt # beautifulsoup4 + # edx-opaque-keys # pydata-sphinx-theme urllib3==2.2.3 # via diff --git a/backend/requirements/quality.txt b/backend/requirements/quality.txt index 0a45588..3d93970 100644 --- a/backend/requirements/quality.txt +++ b/backend/requirements/quality.txt @@ -34,8 +34,20 @@ django==4.2.20 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/test.txt + # django-filter + # djangorestframework +django-filter==25.1 + # via -r requirements/test.txt +djangorestframework==3.16.0 + # via -r requirements/test.txt +dnspython==2.7.0 + # via + # -r requirements/test.txt + # pymongo edx-lint==5.6.0 # via -r requirements/quality.in +edx-opaque-keys==2.12.0 + # via -r requirements/test.txt iniconfig==2.1.0 # via # -r requirements/test.txt @@ -88,6 +100,10 @@ pylint-plugin-utils==0.8.2 # via # pylint-celery # pylint-django +pymongo==4.12.0 + # via + # -r requirements/test.txt + # edx-opaque-keys pytest==8.3.5 # via # -r requirements/test.txt @@ -117,12 +133,17 @@ stevedore==5.4.1 # via # -r requirements/test.txt # code-annotations + # edx-opaque-keys text-unidecode==1.3 # via # -r requirements/test.txt # python-slugify tomlkit==0.13.2 # via pylint +typing-extensions==4.13.2 + # via + # -r requirements/test.txt + # edx-opaque-keys # The following packages are considered to be unsafe in a requirements file: # setuptools diff --git a/backend/requirements/test.txt b/backend/requirements/test.txt index 01c6455..82418dc 100644 --- a/backend/requirements/test.txt +++ b/backend/requirements/test.txt @@ -17,6 +17,18 @@ coverage[toml]==7.8.0 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/base.txt + # django-filter + # djangorestframework +django-filter==25.1 + # via -r requirements/base.txt +djangorestframework==3.16.0 + # via -r requirements/base.txt +dnspython==2.7.0 + # via + # -r requirements/base.txt + # pymongo +edx-opaque-keys==2.12.0 + # via -r requirements/base.txt iniconfig==2.1.0 # via pytest jinja2==3.1.6 @@ -28,9 +40,15 @@ openedx-atlas==0.7.0 packaging==24.2 # via pytest pbr==6.1.1 - # via stevedore + # via + # -r requirements/base.txt + # stevedore pluggy==1.5.0 # via pytest +pymongo==4.12.0 + # via + # -r requirements/base.txt + # edx-opaque-keys pytest==8.3.5 # via # pytest-cov @@ -48,9 +66,16 @@ sqlparse==0.5.3 # -r requirements/base.txt # django stevedore==5.4.1 - # via code-annotations + # via + # -r requirements/base.txt + # code-annotations + # edx-opaque-keys text-unidecode==1.3 # via python-slugify +typing-extensions==4.13.2 + # via + # -r requirements/base.txt + # edx-opaque-keys # The following packages are considered to be unsafe in a requirements file: # setuptools From 429f2bc53c34fe83af95fff77bbc94c89285986c Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Mon, 14 Apr 2025 10:33:30 -0400 Subject: [PATCH 2/3] feat: Add a sample model and rest API. We add a sample CourseArchiveStatus model and a rest API to access it. The plan is for this model to be used by the frontend plugin eventually to modify the course dashboard view. --- CLAUDE.md | 21 +++ backend/sample_plugin/models.py | 62 +++++++ backend/sample_plugin/serializers.py | 28 +++ backend/sample_plugin/urls.py | 14 +- backend/sample_plugin/views.py | 211 +++++++++++++++++++++ backend/test_settings.py | 25 ++- backend/tests/__init__.py | 0 backend/tests/test_api.py | 263 +++++++++++++++++++++++++++ backend/tests/test_models.py | 102 ++++++++++- 9 files changed, 718 insertions(+), 8 deletions(-) create mode 100644 CLAUDE.md create mode 100644 backend/sample_plugin/serializers.py create mode 100644 backend/sample_plugin/views.py create mode 100644 backend/tests/__init__.py create mode 100644 backend/tests/test_api.py diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..0b1bd39 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,21 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Build/Lint/Test Commands +- Backend testing: `cd backend && pytest` or `cd backend && make test` +- Run a single test: `cd backend && pytest tests/test_models.py::test_placeholder` +- Quality checks: `cd backend && make quality` +- Install requirements: `cd backend && make requirements` +- Compile requirements: `cd backend && make compile-requirements` + +## Code Style Guidelines +- Python: Follow PEP 8 with max line length of 120 +- Use isort for import sorting +- Document classes and functions with docstrings +- Type hints are encouraged but not required +- Error handling should use appropriate exceptions with descriptive messages +- Use pytest for testing, with descriptive test function names +- Frontend uses React and follows standard JSX conventions + +Always run `make quality` before creating a PR to ensure consistent code style. diff --git a/backend/sample_plugin/models.py b/backend/sample_plugin/models.py index c24d125..af3c33d 100644 --- a/backend/sample_plugin/models.py +++ b/backend/sample_plugin/models.py @@ -1,3 +1,65 @@ """ Database models for sample_plugin. """ +from django.contrib.auth import get_user_model +from django.db import models +from opaque_keys.edx.django.models import CourseKeyField + + +class CourseArchiveStatus(models.Model): + """ + Model to track the archive status of a course. + + Stores information about whether a course has been archived and when it was archived. + + .. no_pii: This model does not store PII directly, only references to users via foreign keys. + """ + + course_id = CourseKeyField( + max_length=255, + db_index=True, + help_text="The unique identifier for the course." + ) + + user = models.ForeignKey( + get_user_model(), + on_delete=models.CASCADE, + related_name="course_archive_statuses", + help_text="The user who this archive status is for." + ) + + is_archived = models.BooleanField( + default=False, + db_index=True, # Add index for performance on this frequently filtered field + help_text="Whether the course is archived." + ) + + archive_date = models.DateTimeField( + null=True, + blank=True, + help_text="The date and time when the course was archived." + ) + + created_at = models.DateTimeField(auto_now_add=True) + updated_at = models.DateTimeField(auto_now=True) + + def __str__(self): + """ + Return a string representation of the course archive status. + """ + return f"{self.course_id} - {self.user.username} - {'Archived' if self.is_archived else 'Not Archived'}" + + class Meta: + """ + Meta options for the CourseArchiveStatus model. + """ + verbose_name = "Course Archive Status" + verbose_name_plural = "Course Archive Statuses" + ordering = ["-updated_at"] + # Ensure combination of course_id and user is unique + constraints = [ + models.UniqueConstraint( + fields=['course_id', 'user'], + name='unique_user_course_archive_status' + ) + ] diff --git a/backend/sample_plugin/serializers.py b/backend/sample_plugin/serializers.py new file mode 100644 index 0000000..27fac92 --- /dev/null +++ b/backend/sample_plugin/serializers.py @@ -0,0 +1,28 @@ +""" +Serializers for the sample_plugin app. +""" +from rest_framework import serializers + +from sample_plugin.models import CourseArchiveStatus + + +class CourseArchiveStatusSerializer(serializers.ModelSerializer): + """ + Serializer for the CourseArchiveStatus model. + """ + + class Meta: + """ + Meta class for CourseArchiveStatusSerializer. + """ + model = CourseArchiveStatus + fields = [ + 'id', + 'course_id', + 'user', + 'is_archived', + 'archive_date', + 'created_at', + 'updated_at', + ] + read_only_fields = ['id', 'created_at', 'updated_at', 'archive_date'] \ No newline at end of file diff --git a/backend/sample_plugin/urls.py b/backend/sample_plugin/urls.py index 19f2c2c..5a86a03 100644 --- a/backend/sample_plugin/urls.py +++ b/backend/sample_plugin/urls.py @@ -1,10 +1,16 @@ """ URLs for sample_plugin. """ -from django.urls import re_path # pylint: disable=unused-import -from django.views.generic import TemplateView # pylint: disable=unused-import +from django.urls import include, path +from rest_framework.routers import DefaultRouter +from sample_plugin.views import CourseArchiveStatusViewSet + +# Create a router and register our viewsets with it +router = DefaultRouter() +router.register(r'course-archive-status', CourseArchiveStatusViewSet, basename='course-archive-status') + +# The API URLs are now determined automatically by the router urlpatterns = [ - # TODO: Fill in URL patterns and views here. - # re_path(r'', TemplateView.as_view(template_name="sample_plugin/base.html")), + path('api/v1/', include(router.urls)), ] diff --git a/backend/sample_plugin/views.py b/backend/sample_plugin/views.py new file mode 100644 index 0000000..6131869 --- /dev/null +++ b/backend/sample_plugin/views.py @@ -0,0 +1,211 @@ +""" +Views for the sample_plugin app. +""" +import logging +from django.utils import timezone +from django_filters.rest_framework import DjangoFilterBackend +from rest_framework import filters, permissions, viewsets +from rest_framework.exceptions import PermissionDenied, ValidationError +from rest_framework.pagination import PageNumberPagination +from rest_framework.throttling import UserRateThrottle, AnonRateThrottle + +from sample_plugin.models import CourseArchiveStatus +from sample_plugin.serializers import CourseArchiveStatusSerializer + + +logger = logging.getLogger(__name__) + + +class IsOwnerOrStaffSuperuser(permissions.BasePermission): + """ + Custom permission to only allow owners of an object or staff/superusers to view or edit it. + """ + + def has_permission(self, request, view): + """ + Return True if permission is granted to the view. + """ + # Allow authenticated users to list and create + return request.user and request.user.is_authenticated + + def has_object_permission(self, request, view, obj): + """ + Return True if permission is granted to the object. + """ + # Allow if the object belongs to the requesting user + if obj.user == request.user: + return True + + # Allow staff users and superusers + if request.user.is_staff or request.user.is_superuser: + return True + + return False + + +class CourseArchiveStatusPagination(PageNumberPagination): + """ + Pagination class for CourseArchiveStatus. + """ + page_size = 20 + page_size_query_param = 'page_size' + max_page_size = 100 + + +class CourseArchiveStatusThrottle(UserRateThrottle): + """ + Throttle for the CourseArchiveStatus API. + """ + rate = '60/minute' + + +class CourseArchiveStatusViewSet(viewsets.ModelViewSet): + """ + API viewset for CourseArchiveStatus. + + Allows users to view their own course archive statuses and staff/superusers to view all. + Pagination is applied with a default page size of 20 (max 100). + Filtering is available on course_id, user, and is_archived fields. + Ordering is available on all fields. + """ + serializer_class = CourseArchiveStatusSerializer + permission_classes = [IsOwnerOrStaffSuperuser] + pagination_class = CourseArchiveStatusPagination + throttle_classes = [CourseArchiveStatusThrottle, AnonRateThrottle] + filter_backends = [DjangoFilterBackend, filters.OrderingFilter] + filterset_fields = ['course_id', 'user', 'is_archived'] + ordering_fields = ['course_id', 'user', 'is_archived', 'archive_date', 'created_at', 'updated_at'] + ordering = ['-updated_at'] + + def get_queryset(self): + """ + Return the queryset for this viewset. + + Regular users can only see their own records. + Staff and superusers can see all records but with optimized queries. + """ + user = self.request.user + + # Validate query parameters to prevent injection + self._validate_query_params() + + # Always use select_related to avoid N+1 queries + base_queryset = CourseArchiveStatus.objects.select_related('user') + + if user.is_staff or user.is_superuser: + return base_queryset + + # Regular users only see their own records + return base_queryset.filter(user=user) + + def _validate_query_params(self): + """ + Validate query parameters to prevent injection. + """ + # Example validation for course_id format + course_id = self.request.query_params.get('course_id') + if course_id and not self._is_valid_course_id(course_id): + logger.warning( + "Invalid course_id in request: %s, user: %s", + course_id, + self.request.user.username + ) + raise ValidationError({"course_id": "Invalid course ID format."}) + + def _is_valid_course_id(self, course_id): + """ + Check if the course_id is in a valid format. + + This is a basic implementation - in production, you might use a more + sophisticated validator from the edx-platform. + """ + try: + from opaque_keys.edx.keys import CourseKey + CourseKey.from_string(course_id) + return True + except: + return False + + def perform_create(self, serializer): + """ + Perform creation of a new CourseArchiveStatus. + + Sets the user to the requesting user if not specified. + Sets archive_date and archived_by if is_archived is True. + """ + data = serializer.validated_data.copy() + + # Set user to requesting user if not specified + if 'user' not in data: + data['user'] = self.request.user + # Only allow staff/superusers to create records for other users + elif data['user'] != self.request.user and not (self.request.user.is_staff or self.request.user.is_superuser): + logger.warning( + "Permission denied: User %s tried to create a record for user %s", + self.request.user.username, + data['user'].username + ) + raise PermissionDenied("You do not have permission to create records for other users.") + + # Set archive_date if is_archived is True + if data.get('is_archived', False): + data['archive_date'] = timezone.now() + + # Create the record + instance = serializer.save(**data) + + # Log at debug level for normal operation + logger.debug( + "CourseArchiveStatus created: course_id=%s, user=%s, is_archived=%s", + instance.course_id, + instance.user.username, + instance.is_archived + ) + + return instance + + def perform_update(self, serializer): + """ + Perform update of an existing CourseArchiveStatus. + + Updates archive_date and archived_by if is_archived changes. + """ + instance = serializer.instance + data = serializer.validated_data.copy() + + # Handle archive_date if is_archived changes + if 'is_archived' in data: + # If changing from not archived to archived + if data['is_archived'] and not instance.is_archived: + data['archive_date'] = timezone.now() + # If changing from archived to not archived + elif not data['is_archived'] and instance.is_archived: + data['archive_date'] = None + + # Update the record + updated_instance = serializer.save(**data) + + # Log at debug level + logger.debug( + "CourseArchiveStatus updated: course_id=%s, user=%s, is_archived=%s", + updated_instance.course_id, + updated_instance.user.username, + updated_instance.is_archived + ) + + return updated_instance + + def perform_destroy(self, instance): + """ + Perform deletion of an existing CourseArchiveStatus. + """ + # Log at debug level before deletion + logger.debug( + "CourseArchiveStatus deleted: course_id=%s, user=%s, by=%s", + instance.course_id, + instance.user.username, + self.request.user.username + ) + + # Delete the instance + return super().perform_destroy(instance) diff --git a/backend/test_settings.py b/backend/test_settings.py index ededf85..c8ffc82 100644 --- a/backend/test_settings.py +++ b/backend/test_settings.py @@ -32,6 +32,8 @@ def root(*args): 'django.contrib.contenttypes', 'django.contrib.messages', 'django.contrib.sessions', + 'rest_framework', + 'django_filters', 'sample_plugin', ) @@ -44,9 +46,9 @@ def root(*args): SECRET_KEY = 'insecure-secret-key' MIDDLEWARE = ( + 'django.contrib.sessions.middleware.SessionMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', - 'django.contrib.sessions.middleware.SessionMiddleware', ) TEMPLATES = [{ @@ -59,3 +61,24 @@ def root(*args): ], }, }] + +REST_FRAMEWORK = { + 'DEFAULT_PERMISSION_CLASSES': [ + 'rest_framework.permissions.IsAuthenticated', + ], + 'DEFAULT_AUTHENTICATION_CLASSES': [ + 'rest_framework.authentication.SessionAuthentication', + ], + 'DEFAULT_FILTER_BACKENDS': [ + 'django_filters.rest_framework.DjangoFilterBackend', + 'rest_framework.filters.OrderingFilter', + ], + 'DEFAULT_THROTTLE_CLASSES': [ + 'rest_framework.throttling.AnonRateThrottle', + 'rest_framework.throttling.UserRateThrottle', + ], + 'DEFAULT_THROTTLE_RATES': { + 'anon': '20/hour', + 'user': '100/hour', + }, +} diff --git a/backend/tests/__init__.py b/backend/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py new file mode 100644 index 0000000..cda8974 --- /dev/null +++ b/backend/tests/test_api.py @@ -0,0 +1,263 @@ +#!/usr/bin/env python +""" +Tests for the `sample-plugin` REST API. +""" + +import pytest +from django.contrib.auth import get_user_model +from django.urls import reverse +from opaque_keys.edx.keys import CourseKey +from rest_framework import status +from rest_framework.test import APIClient + +from sample_plugin.models import CourseArchiveStatus + + +User = get_user_model() + + +@pytest.fixture +def api_client(): + """ + Return a REST framework API client. + """ + return APIClient() + + +@pytest.fixture +def user(): + """ + Create and return a test user. + """ + return User.objects.create_user( + username='testuser', + email='testuser@example.com', + password='password123' + ) + + +@pytest.fixture +def another_user(): + """ + Create and return another test user. + """ + return User.objects.create_user( + username='anotheruser', + email='anotheruser@example.com', + password='password123' + ) + + +@pytest.fixture +def staff_user(): + """ + Create and return a test staff user. + """ + return User.objects.create_user( + username='staffuser', + email='staffuser@example.com', + password='password123', + is_staff=True + ) + + +@pytest.fixture +def course_key(): + """ + Create and return a test course key. + """ + return CourseKey.from_string('course-v1:edX+DemoX+Demo_Course') + + +@pytest.fixture +def course_archive_status(user, course_key): + """ + Create and return a test course archive status. + """ + return CourseArchiveStatus.objects.create( + course_id=course_key, + user=user, + is_archived=False + ) + + +@pytest.mark.django_db +def test_list_course_archive_status_authenticated(api_client, user, course_archive_status): + """ + Test that an authenticated user can list their own course archive statuses. + """ + api_client.force_authenticate(user=user) + url = reverse('course-archive-status-list') + response = api_client.get(url) + + assert response.status_code == status.HTTP_200_OK + assert response.data['count'] == 1 + assert response.data['results'][0]['course_id'] == str(course_archive_status.course_id) + assert response.data['results'][0]['user'] == user.id + assert response.data['results'][0]['is_archived'] == course_archive_status.is_archived + + +@pytest.mark.django_db +def test_list_course_archive_status_unauthenticated(api_client, course_archive_status): + """ + Test that an unauthenticated user cannot list course archive statuses. + """ + url = reverse('course-archive-status-list') + response = api_client.get(url) + + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +def test_list_course_archive_status_staff_can_see_all(api_client, staff_user, user, another_user, course_key): + """ + Test that a staff user can list all course archive statuses. + """ + # Create archive statuses for both users + CourseArchiveStatus.objects.create( + course_id=course_key, + user=user, + is_archived=False + ) + CourseArchiveStatus.objects.create( + course_id=CourseKey.from_string('course-v1:edX+DemoX+Demo_Course2'), + user=another_user, + is_archived=True + ) + + api_client.force_authenticate(user=staff_user) + url = reverse('course-archive-status-list') + response = api_client.get(url) + + assert response.status_code == status.HTTP_200_OK + assert response.data['count'] == 2 + + +@pytest.mark.django_db +def test_create_course_archive_status(api_client, user, course_key): + """ + Test that a user can create a course archive status. + """ + api_client.force_authenticate(user=user) + url = reverse('course-archive-status-list') + data = { + 'course_id': str(course_key), + 'user': user.id, + 'is_archived': True + } + response = api_client.post(url, data, format='json') + + assert response.status_code == status.HTTP_201_CREATED + assert response.data['course_id'] == str(course_key) + assert response.data['user'] == user.id + assert response.data['is_archived'] is True + assert response.data['archive_date'] is not None + + # Verify in database + course_archive_status = CourseArchiveStatus.objects.get(course_id=course_key, user=user) + assert course_archive_status.is_archived is True + assert course_archive_status.archive_date is not None + + +@pytest.mark.django_db +def test_create_course_archive_status_for_another_user(api_client, user, another_user, course_key): + """ + Test that a regular user cannot create a course archive status for another user. + """ + api_client.force_authenticate(user=user) + url = reverse('course-archive-status-list') + data = { + 'course_id': str(course_key), + 'user': another_user.id, + 'is_archived': True + } + response = api_client.post(url, data, format='json') + + assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +def test_staff_create_course_archive_status_for_another_user(api_client, staff_user, user, course_key): + """ + Test that a staff user can create a course archive status for another user. + """ + api_client.force_authenticate(user=staff_user) + url = reverse('course-archive-status-list') + data = { + 'course_id': str(course_key), + 'user': user.id, + 'is_archived': True + } + response = api_client.post(url, data, format='json') + + assert response.status_code == status.HTTP_201_CREATED + assert response.data['course_id'] == str(course_key) + assert response.data['user'] == user.id + assert response.data['is_archived'] is True + assert response.data['archive_date'] is not None + + +@pytest.mark.django_db +def test_update_course_archive_status(api_client, user, course_archive_status): + """ + Test that a user can update their own course archive status. + """ + api_client.force_authenticate(user=user) + url = reverse('course-archive-status-detail', args=[course_archive_status.id]) + data = { + 'is_archived': True + } + response = api_client.patch(url, data, format='json') + + assert response.status_code == status.HTTP_200_OK + assert response.data['is_archived'] is True + assert response.data['archive_date'] is not None + + # Verify in database + course_archive_status.refresh_from_db() + assert course_archive_status.is_archived is True + assert course_archive_status.archive_date is not None + + +@pytest.mark.django_db +def test_delete_course_archive_status(api_client, user, course_archive_status): + """ + Test that a user can delete their own course archive status. + """ + api_client.force_authenticate(user=user) + url = reverse('course-archive-status-detail', args=[course_archive_status.id]) + response = api_client.delete(url) + + assert response.status_code == status.HTTP_204_NO_CONTENT + assert CourseArchiveStatus.objects.filter(id=course_archive_status.id).count() == 0 + + +@pytest.mark.django_db +def test_cannot_update_other_user_course_archive_status(api_client, another_user, course_archive_status): + """ + Test that a user cannot update another user's course archive status. + """ + api_client.force_authenticate(user=another_user) + url = reverse('course-archive-status-detail', args=[course_archive_status.id]) + data = { + 'is_archived': True + } + response = api_client.patch(url, data, format='json') + + assert response.status_code == status.HTTP_404_NOT_FOUND + + +@pytest.mark.django_db +def test_staff_can_update_other_user_course_archive_status(api_client, staff_user, course_archive_status): + """ + Test that a staff user can update another user's course archive status. + """ + api_client.force_authenticate(user=staff_user) + url = reverse('course-archive-status-detail', args=[course_archive_status.id]) + data = { + 'is_archived': True + } + response = api_client.patch(url, data, format='json') + + assert response.status_code == status.HTTP_200_OK + assert response.data['is_archived'] is True diff --git a/backend/tests/test_models.py b/backend/tests/test_models.py index 5b66289..eff7355 100644 --- a/backend/tests/test_models.py +++ b/backend/tests/test_models.py @@ -4,10 +4,106 @@ """ import pytest +from django.contrib.auth import get_user_model +from django.db.utils import IntegrityError +from opaque_keys.edx.django.models import CourseKeyField +from opaque_keys.edx.keys import CourseKey +from sample_plugin.models import CourseArchiveStatus -@pytest.mark.skip(reason="Placeholder to allow pytest to succeed before real tests are in place.") -def test_placeholder(): + +User = get_user_model() + + +@pytest.fixture +def user(): + """ + Create and return a test user. + """ + return User.objects.create_user( + username='testuser', + email='testuser@example.com', + password='password123' + ) + + +@pytest.fixture +def staff_user(): + """ + Create and return a test staff user. + """ + return User.objects.create_user( + username='staffuser', + email='staffuser@example.com', + password='password123', + is_staff=True + ) + + +@pytest.fixture +def course_key(): + """ + Create and return a test course key. + """ + return CourseKey.from_string('course-v1:edX+DemoX+Demo_Course') + + +@pytest.mark.django_db +def test_course_archive_status_creation(user, course_key): + """ + Test that a CourseArchiveStatus can be created with valid data. + """ + course_archive_status = CourseArchiveStatus.objects.create( + course_id=course_key, + user=user, + is_archived=False + ) + + assert course_archive_status.pk is not None + assert course_archive_status.course_id == course_key + assert course_archive_status.user == user + assert course_archive_status.is_archived is False + assert course_archive_status.archive_date is None + assert course_archive_status.created_at is not None + assert course_archive_status.updated_at is not None + + +@pytest.mark.django_db +def test_course_archive_status_uniqueness(user, course_key): + """ + Test that a CourseArchiveStatus must be unique per user and course_id. """ - TODO: Delete this test once there are real tests. + CourseArchiveStatus.objects.create( + course_id=course_key, + user=user, + is_archived=False + ) + + # Creating another with same user and course_id should raise an IntegrityError + with pytest.raises(IntegrityError): + CourseArchiveStatus.objects.create( + course_id=course_key, + user=user, + is_archived=True + ) + + +@pytest.mark.django_db +def test_course_archive_status_str_method(user, course_key): + """ + Test the string representation of CourseArchiveStatus. """ + course_archive_status = CourseArchiveStatus.objects.create( + course_id=course_key, + user=user, + is_archived=True + ) + + expected_str = f"{course_key} - {user.username} - Archived" + assert str(course_archive_status) == expected_str + + course_archive_status.is_archived = False + course_archive_status.save() + + expected_str = f"{course_key} - {user.username} - Not Archived" + assert str(course_archive_status) == expected_str From 6fafdde7d33ad47677b1329fe324cd43a0e4bc65 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Mon, 14 Apr 2025 11:06:11 -0400 Subject: [PATCH 3/3] style: Fix `tox -e quality` issues. --- backend/sample_plugin/models.py | 4 +++- backend/sample_plugin/serializers.py | 5 +++-- backend/sample_plugin/views.py | 12 ++++++++---- backend/tests/__init__.py | 0 backend/tests/test_api.py | 4 ++-- backend/tests/test_models.py | 3 +-- 6 files changed, 17 insertions(+), 11 deletions(-) delete mode 100644 backend/tests/__init__.py diff --git a/backend/sample_plugin/models.py b/backend/sample_plugin/models.py index af3c33d..6dd31e6 100644 --- a/backend/sample_plugin/models.py +++ b/backend/sample_plugin/models.py @@ -11,7 +11,7 @@ class CourseArchiveStatus(models.Model): Model to track the archive status of a course. Stores information about whether a course has been archived and when it was archived. - + .. no_pii: This model does not store PII directly, only references to users via foreign keys. """ @@ -47,12 +47,14 @@ def __str__(self): """ Return a string representation of the course archive status. """ + # pylint: disable=no-member return f"{self.course_id} - {self.user.username} - {'Archived' if self.is_archived else 'Not Archived'}" class Meta: """ Meta options for the CourseArchiveStatus model. """ + verbose_name = "Course Archive Status" verbose_name_plural = "Course Archive Statuses" ordering = ["-updated_at"] diff --git a/backend/sample_plugin/serializers.py b/backend/sample_plugin/serializers.py index 27fac92..99d2180 100644 --- a/backend/sample_plugin/serializers.py +++ b/backend/sample_plugin/serializers.py @@ -10,11 +10,12 @@ class CourseArchiveStatusSerializer(serializers.ModelSerializer): """ Serializer for the CourseArchiveStatus model. """ - + class Meta: """ Meta class for CourseArchiveStatusSerializer. """ + model = CourseArchiveStatus fields = [ 'id', @@ -25,4 +26,4 @@ class Meta: 'created_at', 'updated_at', ] - read_only_fields = ['id', 'created_at', 'updated_at', 'archive_date'] \ No newline at end of file + read_only_fields = ['id', 'created_at', 'updated_at', 'archive_date'] diff --git a/backend/sample_plugin/views.py b/backend/sample_plugin/views.py index 6131869..3c0c51c 100644 --- a/backend/sample_plugin/views.py +++ b/backend/sample_plugin/views.py @@ -2,17 +2,19 @@ Views for the sample_plugin app. """ import logging + from django.utils import timezone from django_filters.rest_framework import DjangoFilterBackend +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey from rest_framework import filters, permissions, viewsets from rest_framework.exceptions import PermissionDenied, ValidationError from rest_framework.pagination import PageNumberPagination -from rest_framework.throttling import UserRateThrottle, AnonRateThrottle +from rest_framework.throttling import AnonRateThrottle, UserRateThrottle from sample_plugin.models import CourseArchiveStatus from sample_plugin.serializers import CourseArchiveStatusSerializer - logger = logging.getLogger(__name__) @@ -47,6 +49,7 @@ class CourseArchiveStatusPagination(PageNumberPagination): """ Pagination class for CourseArchiveStatus. """ + page_size = 20 page_size_query_param = 'page_size' max_page_size = 100 @@ -56,6 +59,7 @@ class CourseArchiveStatusThrottle(UserRateThrottle): """ Throttle for the CourseArchiveStatus API. """ + rate = '60/minute' @@ -68,6 +72,7 @@ class CourseArchiveStatusViewSet(viewsets.ModelViewSet): Filtering is available on course_id, user, and is_archived fields. Ordering is available on all fields. """ + serializer_class = CourseArchiveStatusSerializer permission_classes = [IsOwnerOrStaffSuperuser] pagination_class = CourseArchiveStatusPagination @@ -120,10 +125,9 @@ def _is_valid_course_id(self, course_id): sophisticated validator from the edx-platform. """ try: - from opaque_keys.edx.keys import CourseKey CourseKey.from_string(course_id) return True - except: + except InvalidKeyError: return False def perform_create(self, serializer): diff --git a/backend/tests/__init__.py b/backend/tests/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index cda8974..e42e862 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -1,4 +1,5 @@ #!/usr/bin/env python +# pylint: disable=redefined-outer-name """ Tests for the `sample-plugin` REST API. """ @@ -12,7 +13,6 @@ from sample_plugin.models import CourseArchiveStatus - User = get_user_model() @@ -98,7 +98,7 @@ def test_list_course_archive_status_authenticated(api_client, user, course_archi @pytest.mark.django_db -def test_list_course_archive_status_unauthenticated(api_client, course_archive_status): +def test_list_course_archive_status_unauthenticated(api_client): """ Test that an unauthenticated user cannot list course archive statuses. """ diff --git a/backend/tests/test_models.py b/backend/tests/test_models.py index eff7355..ef0bd78 100644 --- a/backend/tests/test_models.py +++ b/backend/tests/test_models.py @@ -1,4 +1,5 @@ #!/usr/bin/env python +# pylint: disable=redefined-outer-name """ Tests for the `sample-plugin` models module. """ @@ -6,12 +7,10 @@ import pytest from django.contrib.auth import get_user_model from django.db.utils import IntegrityError -from opaque_keys.edx.django.models import CourseKeyField from opaque_keys.edx.keys import CourseKey from sample_plugin.models import CourseArchiveStatus - User = get_user_model()