From aaa1e27549a764c7a9872ec8711765b675b309a4 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Mon, 14 Apr 2025 13:39:53 -0400 Subject: [PATCH 1/7] feat: Update settings and make migrations. --- backend/.gitignore | 3 ++ backend/sample_plugin/apps.py | 2 +- .../sample_plugin/migrations/0001_initial.py | 39 +++++++++++++++++++ backend/sample_plugin/migrations/__init__.py | 0 backend/test_settings.py | 1 + 5 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 backend/sample_plugin/migrations/0001_initial.py create mode 100644 backend/sample_plugin/migrations/__init__.py diff --git a/backend/.gitignore b/backend/.gitignore index 0e5aa7f..c5cb788 100644 --- a/backend/.gitignore +++ b/backend/.gitignore @@ -63,3 +63,6 @@ docs/sample_plugin.*.rst # Private requirements requirements/private.in requirements/private.txt + +# Test cruft +default.db diff --git a/backend/sample_plugin/apps.py b/backend/sample_plugin/apps.py index 2b21f84..dce5fdf 100644 --- a/backend/sample_plugin/apps.py +++ b/backend/sample_plugin/apps.py @@ -9,5 +9,5 @@ class SamplePluginConfig(AppConfig): """ Configuration for the sample_plugin Django application. """ - + default_auto_field = 'django.db.models.BigAutoField' name = 'sample_plugin' diff --git a/backend/sample_plugin/migrations/0001_initial.py b/backend/sample_plugin/migrations/0001_initial.py new file mode 100644 index 0000000..55591b9 --- /dev/null +++ b/backend/sample_plugin/migrations/0001_initial.py @@ -0,0 +1,39 @@ +# Generated by Django 4.2.20 on 2025-04-14 12:39 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import opaque_keys.edx.django.models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='CourseArchiveStatus', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('course_id', opaque_keys.edx.django.models.CourseKeyField(db_index=True, help_text='The unique identifier for the course.', max_length=255)), + ('is_archived', models.BooleanField(db_index=True, default=False, help_text='Whether the course is archived.')), + ('archive_date', models.DateTimeField(blank=True, help_text='The date and time when the course was archived.', null=True)), + ('created_at', models.DateTimeField(auto_now_add=True)), + ('updated_at', models.DateTimeField(auto_now=True)), + ('user', models.ForeignKey(help_text='The user who this archive status is for.', on_delete=django.db.models.deletion.CASCADE, related_name='course_archive_statuses', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'verbose_name': 'Course Archive Status', + 'verbose_name_plural': 'Course Archive Statuses', + 'ordering': ['-updated_at'], + }, + ), + migrations.AddConstraint( + model_name='coursearchivestatus', + constraint=models.UniqueConstraint(fields=('course_id', 'user'), name='unique_user_course_archive_status'), + ), + ] diff --git a/backend/sample_plugin/migrations/__init__.py b/backend/sample_plugin/migrations/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/backend/test_settings.py b/backend/test_settings.py index c8ffc82..cc1f8aa 100644 --- a/backend/test_settings.py +++ b/backend/test_settings.py @@ -57,6 +57,7 @@ def root(*args): 'OPTIONS': { 'context_processors': [ 'django.contrib.auth.context_processors.auth', # this is required for admin + 'django.template.context_processors.request', # this is also required for admin navigation sidebar 'django.contrib.messages.context_processors.messages', # this is required for admin ], }, From 448235529a02a7a187aa031887c8f93f0b829289 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Tue, 15 Apr 2025 11:04:55 -0400 Subject: [PATCH 2/7] feat: Add bits needed to make a django app plugin. This adds the plugin entry points as well as relevant settings and updates the testing setup to load the plugin using the plugin system so that we're testing that things load correctly. --- backend/sample_plugin/apps.py | 64 +++++++- backend/sample_plugin/settings/common.py | 14 ++ backend/sample_plugin/settings/production.py | 15 ++ backend/sample_plugin/settings/test.py | 16 ++ backend/sample_plugin/signals.py | 15 ++ backend/setup.py | 8 + backend/test_settings.py | 130 +++++++++------ backend/tests/test_api.py | 164 +++++++++---------- backend/tests/test_models.py | 30 ++-- backend/tests/test_plugin_integration.py | 23 +++ backend/tests/urls.py | 11 ++ 11 files changed, 338 insertions(+), 152 deletions(-) create mode 100644 backend/sample_plugin/settings/common.py create mode 100644 backend/sample_plugin/settings/production.py create mode 100644 backend/sample_plugin/settings/test.py create mode 100644 backend/sample_plugin/signals.py create mode 100644 backend/tests/test_plugin_integration.py create mode 100644 backend/tests/urls.py diff --git a/backend/sample_plugin/apps.py b/backend/sample_plugin/apps.py index dce5fdf..e5ac9a3 100644 --- a/backend/sample_plugin/apps.py +++ b/backend/sample_plugin/apps.py @@ -3,11 +3,69 @@ """ from django.apps import AppConfig +from edx_django_utils.plugins.constants import PluginSettings, PluginSignals, PluginURLs class SamplePluginConfig(AppConfig): + # pylint: disable=line-too-long """ Configuration for the sample_plugin Django application. - """ - default_auto_field = 'django.db.models.BigAutoField' - name = 'sample_plugin' + + See https://github.com/openedx/edx-django-utils/blob/master/edx_django_utils/plugins/docs/how_tos/how_to_create_a_plugin_app.rst#manual-setup + for more details and examples. + """ # noqa: + + default_auto_field = "django.db.models.BigAutoField" + name = "sample_plugin" + plugin_app = { + "url_config": { + "lms.djangoapp": { + PluginURLs.NAMESPACE: "sample_plugin", + PluginURLs.REGEX: r"^sample-plugin/", + PluginURLs.RELATIVE_PATH: "urls", + }, + "cms.djangoapp": { + PluginURLs.NAMESPACE: "sample_plugin", + PluginURLs.REGEX: r"^sample-plugin/", + PluginURLs.RELATIVE_PATH: "urls", + }, + }, + PluginSettings.CONFIG: { + "lms.djangoapp": { + "common": { + PluginURLs.RELATIVE_PATH: "settings.common", + }, + "test": { + PluginURLs.RELATIVE_PATH: "settings.test", + }, + "production": { + PluginURLs.RELATIVE_PATH: "settings.production", + }, + }, + "cms.djangoapp": { + "common": { + PluginURLs.RELATIVE_PATH: "settings.common", + }, + "test": { + PluginURLs.RELATIVE_PATH: "settings.test", + }, + "production": { + PluginURLs.RELATIVE_PATH: "settings.production", + }, + }, + }, + PluginSignals.CONFIG: { + "lms.djangoapp": { + PluginURLs.RELATIVE_PATH: "signals", + PluginSignals.RECEIVERS: [ + # Signals handlers can be registered here + ], + }, + "cms.djangoapp": { + PluginURLs.RELATIVE_PATH: "signals", + PluginSignals.RECEIVERS: [ + # Signals handlers can be registered here + ], + }, + }, + } diff --git a/backend/sample_plugin/settings/common.py b/backend/sample_plugin/settings/common.py new file mode 100644 index 0000000..904cf8c --- /dev/null +++ b/backend/sample_plugin/settings/common.py @@ -0,0 +1,14 @@ +""" +Common settings for the sample_plugin application. +""" + + +def plugin_settings(settings): + """ + Add plugin settings to main settings object. + + Args: + settings (dict): Django settings object + """ + pass + # settings.FOO = 'bar' diff --git a/backend/sample_plugin/settings/production.py b/backend/sample_plugin/settings/production.py new file mode 100644 index 0000000..cca1b71 --- /dev/null +++ b/backend/sample_plugin/settings/production.py @@ -0,0 +1,15 @@ +""" +Production settings for the sample_plugin application. +""" +from sample_plugin.settings.common import plugin_settings as common_settings + + +def plugin_settings(settings): + """ + Set up production-specific settings. + + Args: + settings (dict): Django settings object + """ + # Apply common settings + common_settings(settings) diff --git a/backend/sample_plugin/settings/test.py b/backend/sample_plugin/settings/test.py new file mode 100644 index 0000000..b9fc50b --- /dev/null +++ b/backend/sample_plugin/settings/test.py @@ -0,0 +1,16 @@ +""" +Test settings for the sample_plugin application. +""" +from sample_plugin.settings.common import plugin_settings as common_settings + + +def plugin_settings(settings): + """ + Set up test-specific settings. + + Args: + settings (dict): Django settings object + """ + + # Apply common settings + common_settings(settings) diff --git a/backend/sample_plugin/signals.py b/backend/sample_plugin/signals.py new file mode 100644 index 0000000..5f99125 --- /dev/null +++ b/backend/sample_plugin/signals.py @@ -0,0 +1,15 @@ +""" +Signal handlers for the sample_plugin application. +""" + +# Signal handlers can be defined here if needed +# For example: +# from django.dispatch import receiver +# from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_LMS_CRITICAL +# +# @receiver(USER_RETIRE_LMS_CRITICAL) +# def _handle_user_retirement(sender, **kwargs): +# """ +# Handle user retirement actions for this app. +# """ +# pass \ No newline at end of file diff --git a/backend/setup.py b/backend/setup.py index 0832abc..28ca885 100755 --- a/backend/setup.py +++ b/backend/setup.py @@ -157,4 +157,12 @@ def is_requirement(line): 'Programming Language :: Python :: 3', 'Programming Language :: Python :: 3.12', ], + entry_points={ + 'lms.djangoapp': [ + 'sample_plugin = sample_plugin.apps:SamplePluginConfig', + ], + 'cms.djangoapp': [ + 'sample_plugin = sample_plugin.apps:SamplePluginConfig', + ], + }, ) diff --git a/backend/test_settings.py b/backend/test_settings.py index cc1f8aa..88894fc 100644 --- a/backend/test_settings.py +++ b/backend/test_settings.py @@ -7,6 +7,9 @@ from os.path import abspath, dirname, join +from edx_django_utils.plugins.plugin_apps import get_plugin_apps +from edx_django_utils.plugins.plugin_settings import add_plugins + def root(*args): """ @@ -16,70 +19,103 @@ def root(*args): DATABASES = { - 'default': { - 'ENGINE': 'django.db.backends.sqlite3', - 'NAME': 'default.db', - 'USER': '', - 'PASSWORD': '', - 'HOST': '', - 'PORT': '', + "default": { + "ENGINE": "django.db.backends.sqlite3", + "NAME": "default.db", + "USER": "", + "PASSWORD": "", + "HOST": "", + "PORT": "", } } -INSTALLED_APPS = ( - 'django.contrib.admin', - 'django.contrib.auth', - 'django.contrib.contenttypes', - 'django.contrib.messages', - 'django.contrib.sessions', - 'rest_framework', - 'django_filters', - 'sample_plugin', -) +# Plugin Settings +ENABLE_PLUGINS = True +# Define both contexts for reference, but we'll only use one for testing +PLUGIN_CONTEXTS = ["lms.djangoapp", "cms.djangoapp"] +# We only use the LMS context for testing as the plugin is configured similarly for both +# Could use CMS context instead by changing the index to 1 + +# Base INSTALLED_APPS before plugin discovery +INSTALLED_APPS = [ + "django.contrib.admin", + "django.contrib.auth", + "django.contrib.contenttypes", + "django.contrib.messages", + "django.contrib.sessions", + "rest_framework", + "django_filters", + "edx_django_utils.plugins", + "django_extensions", +] + +# Dynamically add plugin apps - only using the LMS context for simplicity +plugin_apps = get_plugin_apps(PLUGIN_CONTEXTS[0]) +INSTALLED_APPS.extend(plugin_apps) LOCALE_PATHS = [ - root('sample_plugin', 'conf', 'locale'), + root("sample_plugin", "conf", "locale"), ] -ROOT_URLCONF = 'sample_plugin.urls' +ROOT_URLCONF = "tests.urls" -SECRET_KEY = 'insecure-secret-key' +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", + "django.contrib.auth.middleware.AuthenticationMiddleware", + "django.contrib.messages.middleware.MessageMiddleware", ) -TEMPLATES = [{ - 'BACKEND': 'django.template.backends.django.DjangoTemplates', - 'APP_DIRS': False, - 'OPTIONS': { - 'context_processors': [ - 'django.contrib.auth.context_processors.auth', # this is required for admin - 'django.template.context_processors.request', # this is also required for admin navigation sidebar - 'django.contrib.messages.context_processors.messages', # this is required for admin - ], - }, -}] +TEMPLATES = [ + { + "BACKEND": "django.template.backends.django.DjangoTemplates", + "APP_DIRS": False, + "OPTIONS": { + "context_processors": [ + "django.contrib.auth.context_processors.auth", # this is required for admin + "django.template.context_processors.request", # this is also required for admin navigation sidebar + "django.contrib.messages.context_processors.messages", # this is required for admin + ], + }, + } +] REST_FRAMEWORK = { - 'DEFAULT_PERMISSION_CLASSES': [ - 'rest_framework.permissions.IsAuthenticated', + "DEFAULT_PERMISSION_CLASSES": [ + "rest_framework.permissions.IsAuthenticated", ], - 'DEFAULT_AUTHENTICATION_CLASSES': [ - 'rest_framework.authentication.SessionAuthentication', + "DEFAULT_AUTHENTICATION_CLASSES": [ + "rest_framework.authentication.SessionAuthentication", ], - 'DEFAULT_FILTER_BACKENDS': [ - 'django_filters.rest_framework.DjangoFilterBackend', - 'rest_framework.filters.OrderingFilter', + "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_CLASSES": [ + "rest_framework.throttling.AnonRateThrottle", + "rest_framework.throttling.UserRateThrottle", ], - 'DEFAULT_THROTTLE_RATES': { - 'anon': '20/hour', - 'user': '100/hour', + "DEFAULT_THROTTLE_RATES": { + "anon": "20/hour", + "user": "100/hour", + }, +} + +LOGGING = { + "version": 1, + "disable_existing_loggers": False, + "handlers": { + "console": { + "class": "logging.StreamHandler", + }, + }, + "root": { + "handlers": ["console"], + "level": "DEBUG", }, } +# Apply plugin settings - must be done after base settings are defined +# Only using the LMS context for simplicity +# Third parameter is the settings_type which should match the keys in settings_config +add_plugins(__name__, PLUGIN_CONTEXTS[0], "test") diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index e42e862..bbfbe61 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -30,9 +30,7 @@ def user(): Create and return a test user. """ return User.objects.create_user( - username='testuser', - email='testuser@example.com', - password='password123' + username="testuser", email="testuser@example.com", password="password123" ) @@ -42,9 +40,7 @@ def another_user(): Create and return another test user. """ return User.objects.create_user( - username='anotheruser', - email='anotheruser@example.com', - password='password123' + username="anotheruser", email="anotheruser@example.com", password="password123" ) @@ -54,10 +50,10 @@ 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 + username="staffuser", + email="staffuser@example.com", + password="password123", + is_staff=True, ) @@ -66,7 +62,7 @@ def course_key(): """ Create and return a test course key. """ - return CourseKey.from_string('course-v1:edX+DemoX+Demo_Course') + return CourseKey.from_string("course-v1:edX+DemoX+Demo_Course") @pytest.fixture @@ -75,26 +71,30 @@ 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 + 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): +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') + url = reverse("sample_plugin: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 + 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 @@ -102,35 +102,35 @@ def test_list_course_archive_status_unauthenticated(api_client): """ Test that an unauthenticated user cannot list course archive statuses. """ - url = reverse('course-archive-status-list') + url = reverse("sample_plugin: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): +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 + course_id=course_key, user=user, is_archived=False ) CourseArchiveStatus.objects.create( - course_id=CourseKey.from_string('course-v1:edX+DemoX+Demo_Course2'), + course_id=CourseKey.from_string("course-v1:edX+DemoX+Demo_Course2"), user=another_user, - is_archived=True + is_archived=True, ) api_client.force_authenticate(user=staff_user) - url = reverse('course-archive-status-list') + url = reverse("sample_plugin:course-archive-status-list") response = api_client.get(url) assert response.status_code == status.HTTP_200_OK - assert response.data['count'] == 2 + assert response.data["count"] == 2 @pytest.mark.django_db @@ -139,62 +139,56 @@ 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') + url = reverse("sample_plugin: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 + 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) + 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): +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') + url = reverse("sample_plugin: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): +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') + url = reverse("sample_plugin: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 + 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 @@ -203,15 +197,15 @@ 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') + url = reverse( + "sample_plugin: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 + assert response.data["is_archived"] is True + assert response.data["archive_date"] is not None # Verify in database course_archive_status.refresh_from_db() @@ -225,7 +219,9 @@ 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]) + url = reverse( + "sample_plugin:course-archive-status-detail", args=[course_archive_status.id] + ) response = api_client.delete(url) assert response.status_code == status.HTTP_204_NO_CONTENT @@ -233,31 +229,35 @@ def test_delete_course_archive_status(api_client, user, course_archive_status): @pytest.mark.django_db -def test_cannot_update_other_user_course_archive_status(api_client, another_user, course_archive_status): +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') + url = reverse( + "sample_plugin: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): +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') + url = reverse( + "sample_plugin: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["is_archived"] is True diff --git a/backend/tests/test_models.py b/backend/tests/test_models.py index ef0bd78..331598f 100644 --- a/backend/tests/test_models.py +++ b/backend/tests/test_models.py @@ -20,9 +20,7 @@ def user(): Create and return a test user. """ return User.objects.create_user( - username='testuser', - email='testuser@example.com', - password='password123' + username="testuser", email="testuser@example.com", password="password123" ) @@ -32,10 +30,10 @@ 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 + username="staffuser", + email="staffuser@example.com", + password="password123", + is_staff=True, ) @@ -44,7 +42,7 @@ def course_key(): """ Create and return a test course key. """ - return CourseKey.from_string('course-v1:edX+DemoX+Demo_Course') + return CourseKey.from_string("course-v1:edX+DemoX+Demo_Course") @pytest.mark.django_db @@ -53,9 +51,7 @@ 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 + course_id=course_key, user=user, is_archived=False ) assert course_archive_status.pk is not None @@ -73,17 +69,13 @@ def test_course_archive_status_uniqueness(user, course_key): Test that a CourseArchiveStatus must be unique per user and course_id. """ CourseArchiveStatus.objects.create( - course_id=course_key, - user=user, - is_archived=False + 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 + course_id=course_key, user=user, is_archived=True ) @@ -93,9 +85,7 @@ 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 + course_id=course_key, user=user, is_archived=True ) expected_str = f"{course_key} - {user.username} - Archived" diff --git a/backend/tests/test_plugin_integration.py b/backend/tests/test_plugin_integration.py new file mode 100644 index 0000000..765b9d0 --- /dev/null +++ b/backend/tests/test_plugin_integration.py @@ -0,0 +1,23 @@ +""" +Tests to verify the plugin is discoverable and loaded correctly. +""" + +from django.apps import apps +from django.conf import settings + + +def test_app_is_installed(): + """ + Test that the plugin app is installed in Django. + + This confirms that the plugin entrypoints are correct and that the + plugin tooling was able to correctly load the plugin and add the app to + INSTALLED_APPS + + """ + assert "sample_plugin.apps.SamplePluginConfig" in settings.INSTALLED_APPS + assert apps.get_app_config("sample_plugin") is not None + + +# We don't do a test for the URLs because the namespaced urls which should be auto registered are tested in the +# test_api.py tests. diff --git a/backend/tests/urls.py b/backend/tests/urls.py new file mode 100644 index 0000000..ae59772 --- /dev/null +++ b/backend/tests/urls.py @@ -0,0 +1,11 @@ +""" +A URL Conf for testing. + +We don't add the sample plugin URLs here because they should be added automatically by the plugin interface. +""" + +from edx_django_utils.plugins import get_plugin_url_patterns + +urlpatterns = [] + +urlpatterns.extend(get_plugin_url_patterns("lms.djangoapp")) From 30f542600e6ab51b27869332c346cc9417ca4c0e Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Tue, 15 Apr 2025 16:01:25 -0400 Subject: [PATCH 3/7] build: Add the edx-django-extensions and django_extensions libraries. We use edx-django-extensions to load the plugin tooling for testing and we used django_extensions as a helpful development tool to list urls or make use of the various other useful management commands it provides. --- backend/requirements/dev.txt | 38 +++++++++++++++++++++++++++++++ backend/requirements/doc.txt | 39 ++++++++++++++++++++++++++++++-- backend/requirements/quality.txt | 38 +++++++++++++++++++++++++++++++ backend/requirements/test.in | 2 ++ backend/requirements/test.txt | 27 +++++++++++++++++++++- 5 files changed, 141 insertions(+), 3 deletions(-) diff --git a/backend/requirements/dev.txt b/backend/requirements/dev.txt index 16f714d..13ccbfe 100644 --- a/backend/requirements/dev.txt +++ b/backend/requirements/dev.txt @@ -21,6 +21,10 @@ cachetools==5.5.2 # via # -r requirements/ci.txt # tox +cffi==1.17.1 + # via + # -r requirements/quality.txt + # pynacl chardet==5.2.0 # via # -r requirements/ci.txt @@ -32,6 +36,7 @@ click==8.1.8 # -r requirements/quality.txt # click-log # code-annotations + # edx-django-utils # edx-lint # pip-tools click-log==0.4.0 @@ -64,17 +69,33 @@ 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-crum + # django-extensions # django-filter + # django-waffle # djangorestframework + # edx-django-utils # edx-i18n-tools +django-crum==0.7.9 + # via + # -r requirements/quality.txt + # edx-django-utils +django-extensions==4.1 + # via -r requirements/quality.txt django-filter==25.1 # via -r requirements/quality.txt +django-waffle==4.2.0 + # via + # -r requirements/quality.txt + # edx-django-utils djangorestframework==3.16.0 # via -r requirements/quality.txt dnspython==2.7.0 # via # -r requirements/quality.txt # pymongo +edx-django-utils==7.4.0 + # via -r requirements/quality.txt edx-i18n-tools==1.7.0 # via -r requirements/dev.in edx-lint==5.6.0 @@ -113,6 +134,10 @@ mccabe==0.7.0 # via # -r requirements/quality.txt # pylint +newrelic==10.9.0 + # via + # -r requirements/quality.txt + # edx-django-utils openedx-atlas==0.7.0 # via -r requirements/quality.txt packaging==24.2 @@ -148,8 +173,16 @@ pluggy==1.5.0 # tox polib==1.2.0 # via edx-i18n-tools +psutil==7.0.0 + # via + # -r requirements/quality.txt + # edx-django-utils pycodestyle==2.13.0 # via -r requirements/quality.txt +pycparser==2.22 + # via + # -r requirements/quality.txt + # cffi pydocstyle==6.3.0 # via -r requirements/quality.txt pygments==2.19.1 @@ -178,6 +211,10 @@ pymongo==4.12.0 # via # -r requirements/quality.txt # edx-opaque-keys +pynacl==1.5.0 + # via + # -r requirements/quality.txt + # edx-django-utils pyproject-api==1.9.0 # via # -r requirements/ci.txt @@ -221,6 +258,7 @@ stevedore==5.4.1 # via # -r requirements/quality.txt # code-annotations + # edx-django-utils # edx-opaque-keys text-unidecode==1.3 # via diff --git a/backend/requirements/doc.txt b/backend/requirements/doc.txt index 4c8ae0d..55e7b32 100644 --- a/backend/requirements/doc.txt +++ b/backend/requirements/doc.txt @@ -23,13 +23,17 @@ build==1.2.2.post1 certifi==2025.1.31 # via requests cffi==1.17.1 - # via cryptography + # via + # -r requirements/test.txt + # cryptography + # pynacl charset-normalizer==3.4.1 # via requests click==8.1.8 # via # -r requirements/test.txt # code-annotations + # edx-django-utils code-annotations==2.3.0 # via -r requirements/test.txt coverage[toml]==7.8.0 @@ -42,10 +46,24 @@ 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-crum + # django-extensions # django-filter + # django-waffle # djangorestframework + # edx-django-utils +django-crum==0.7.9 + # via + # -r requirements/test.txt + # edx-django-utils +django-extensions==4.1 + # via -r requirements/test.txt django-filter==25.1 # via -r requirements/test.txt +django-waffle==4.2.0 + # via + # -r requirements/test.txt + # edx-django-utils djangorestframework==3.16.0 # via -r requirements/test.txt dnspython==2.7.0 @@ -61,6 +79,8 @@ docutils==0.21.2 # readme-renderer # restructuredtext-lint # sphinx +edx-django-utils==7.4.0 + # via -r requirements/test.txt edx-opaque-keys==2.12.0 # via -r requirements/test.txt id==1.5.0 @@ -102,6 +122,10 @@ more-itertools==10.6.0 # via # jaraco-classes # jaraco-functools +newrelic==10.9.0 + # via + # -r requirements/test.txt + # edx-django-utils nh3==0.2.21 # via readme-renderer openedx-atlas==0.7.0 @@ -122,8 +146,14 @@ pluggy==1.5.0 # via # -r requirements/test.txt # pytest +psutil==7.0.0 + # via + # -r requirements/test.txt + # edx-django-utils pycparser==2.22 - # via cffi + # via + # -r requirements/test.txt + # cffi pydata-sphinx-theme==0.15.4 # via sphinx-book-theme pygments==2.19.1 @@ -138,6 +168,10 @@ pymongo==4.12.0 # via # -r requirements/test.txt # edx-opaque-keys +pynacl==1.5.0 + # via + # -r requirements/test.txt + # edx-django-utils pyproject-hooks==1.2.0 # via build pytest==8.3.5 @@ -209,6 +243,7 @@ stevedore==5.4.1 # -r requirements/test.txt # code-annotations # doc8 + # edx-django-utils # edx-opaque-keys text-unidecode==1.3 # via diff --git a/backend/requirements/quality.txt b/backend/requirements/quality.txt index 3d93970..8820e77 100644 --- a/backend/requirements/quality.txt +++ b/backend/requirements/quality.txt @@ -12,11 +12,16 @@ astroid==3.3.9 # via # pylint # pylint-celery +cffi==1.17.1 + # via + # -r requirements/test.txt + # pynacl click==8.1.8 # via # -r requirements/test.txt # click-log # code-annotations + # edx-django-utils # edx-lint click-log==0.4.0 # via edx-lint @@ -34,16 +39,32 @@ 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-crum + # django-extensions # django-filter + # django-waffle # djangorestframework + # edx-django-utils +django-crum==0.7.9 + # via + # -r requirements/test.txt + # edx-django-utils +django-extensions==4.1 + # via -r requirements/test.txt django-filter==25.1 # via -r requirements/test.txt +django-waffle==4.2.0 + # via + # -r requirements/test.txt + # edx-django-utils djangorestframework==3.16.0 # via -r requirements/test.txt dnspython==2.7.0 # via # -r requirements/test.txt # pymongo +edx-django-utils==7.4.0 + # via -r requirements/test.txt edx-lint==5.6.0 # via -r requirements/quality.in edx-opaque-keys==2.12.0 @@ -66,6 +87,10 @@ markupsafe==3.0.2 # jinja2 mccabe==0.7.0 # via pylint +newrelic==10.9.0 + # via + # -r requirements/test.txt + # edx-django-utils openedx-atlas==0.7.0 # via -r requirements/test.txt packaging==24.2 @@ -82,8 +107,16 @@ pluggy==1.5.0 # via # -r requirements/test.txt # pytest +psutil==7.0.0 + # via + # -r requirements/test.txt + # edx-django-utils pycodestyle==2.13.0 # via -r requirements/quality.in +pycparser==2.22 + # via + # -r requirements/test.txt + # cffi pydocstyle==6.3.0 # via -r requirements/quality.in pylint==3.3.6 @@ -104,6 +137,10 @@ pymongo==4.12.0 # via # -r requirements/test.txt # edx-opaque-keys +pynacl==1.5.0 + # via + # -r requirements/test.txt + # edx-django-utils pytest==8.3.5 # via # -r requirements/test.txt @@ -133,6 +170,7 @@ stevedore==5.4.1 # via # -r requirements/test.txt # code-annotations + # edx-django-utils # edx-opaque-keys text-unidecode==1.3 # via diff --git a/backend/requirements/test.in b/backend/requirements/test.in index 6797160..60eb0f4 100644 --- a/backend/requirements/test.in +++ b/backend/requirements/test.in @@ -6,3 +6,5 @@ pytest-cov # pytest extension for code coverage statistics pytest-django # pytest extension for better Django support code-annotations # provides commands used by the pii_check make target. +edx-django-utils # Open edX Django utilities, needed for plugin testing +django_extensions # Helpful dev commans including 'show_urls' diff --git a/backend/requirements/test.txt b/backend/requirements/test.txt index 82418dc..9606320 100644 --- a/backend/requirements/test.txt +++ b/backend/requirements/test.txt @@ -8,8 +8,12 @@ asgiref==3.8.1 # via # -r requirements/base.txt # django +cffi==1.17.1 + # via pynacl click==8.1.8 - # via code-annotations + # via + # code-annotations + # edx-django-utils code-annotations==2.3.0 # via -r requirements/test.in coverage[toml]==7.8.0 @@ -17,16 +21,28 @@ 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-crum + # django-extensions # django-filter + # django-waffle # djangorestframework + # edx-django-utils +django-crum==0.7.9 + # via edx-django-utils +django-extensions==4.1 + # via -r requirements/test.in django-filter==25.1 # via -r requirements/base.txt +django-waffle==4.2.0 + # via edx-django-utils djangorestframework==3.16.0 # via -r requirements/base.txt dnspython==2.7.0 # via # -r requirements/base.txt # pymongo +edx-django-utils==7.4.0 + # via -r requirements/test.in edx-opaque-keys==2.12.0 # via -r requirements/base.txt iniconfig==2.1.0 @@ -35,6 +51,8 @@ jinja2==3.1.6 # via code-annotations markupsafe==3.0.2 # via jinja2 +newrelic==10.9.0 + # via edx-django-utils openedx-atlas==0.7.0 # via -r requirements/base.txt packaging==24.2 @@ -45,10 +63,16 @@ pbr==6.1.1 # stevedore pluggy==1.5.0 # via pytest +psutil==7.0.0 + # via edx-django-utils +pycparser==2.22 + # via cffi pymongo==4.12.0 # via # -r requirements/base.txt # edx-opaque-keys +pynacl==1.5.0 + # via edx-django-utils pytest==8.3.5 # via # pytest-cov @@ -69,6 +93,7 @@ stevedore==5.4.1 # via # -r requirements/base.txt # code-annotations + # edx-django-utils # edx-opaque-keys text-unidecode==1.3 # via python-slugify From 4d5bfaf76654d4450f0e611af449988bd6d65543 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Tue, 15 Apr 2025 16:03:01 -0400 Subject: [PATCH 4/7] feat: Add the migration for the new model. --- .../sample_plugin/migrations/0001_initial.py | 65 +++++++++++++++---- 1 file changed, 52 insertions(+), 13 deletions(-) diff --git a/backend/sample_plugin/migrations/0001_initial.py b/backend/sample_plugin/migrations/0001_initial.py index 55591b9..79a97d3 100644 --- a/backend/sample_plugin/migrations/0001_initial.py +++ b/backend/sample_plugin/migrations/0001_initial.py @@ -16,24 +16,63 @@ class Migration(migrations.Migration): operations = [ migrations.CreateModel( - name='CourseArchiveStatus', + name="CourseArchiveStatus", fields=[ - ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('course_id', opaque_keys.edx.django.models.CourseKeyField(db_index=True, help_text='The unique identifier for the course.', max_length=255)), - ('is_archived', models.BooleanField(db_index=True, default=False, help_text='Whether the course is archived.')), - ('archive_date', models.DateTimeField(blank=True, help_text='The date and time when the course was archived.', null=True)), - ('created_at', models.DateTimeField(auto_now_add=True)), - ('updated_at', models.DateTimeField(auto_now=True)), - ('user', models.ForeignKey(help_text='The user who this archive status is for.', on_delete=django.db.models.deletion.CASCADE, related_name='course_archive_statuses', to=settings.AUTH_USER_MODEL)), + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "course_id", + opaque_keys.edx.django.models.CourseKeyField( + db_index=True, + help_text="The unique identifier for the course.", + max_length=255, + ), + ), + ( + "is_archived", + models.BooleanField( + db_index=True, + default=False, + help_text="Whether the course is archived.", + ), + ), + ( + "archive_date", + models.DateTimeField( + blank=True, + help_text="The date and time when the course was archived.", + null=True, + ), + ), + ("created_at", models.DateTimeField(auto_now_add=True)), + ("updated_at", models.DateTimeField(auto_now=True)), + ( + "user", + models.ForeignKey( + help_text="The user who this archive status is for.", + on_delete=django.db.models.deletion.CASCADE, + related_name="course_archive_statuses", + to=settings.AUTH_USER_MODEL, + ), + ), ], options={ - 'verbose_name': 'Course Archive Status', - 'verbose_name_plural': 'Course Archive Statuses', - 'ordering': ['-updated_at'], + "verbose_name": "Course Archive Status", + "verbose_name_plural": "Course Archive Statuses", + "ordering": ["-updated_at"], }, ), migrations.AddConstraint( - model_name='coursearchivestatus', - constraint=models.UniqueConstraint(fields=('course_id', 'user'), name='unique_user_course_archive_status'), + model_name="coursearchivestatus", + constraint=models.UniqueConstraint( + fields=("course_id", "user"), name="unique_user_course_archive_status" + ), ), ] From c655fbcd45c9df520fa236372541aae9bb6f025a Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Tue, 15 Apr 2025 16:03:24 -0400 Subject: [PATCH 5/7] style: Run `black` on the rest of the project. --- backend/sample_plugin/__init__.py | 2 +- backend/sample_plugin/models.py | 14 +++---- backend/sample_plugin/serializers.py | 17 ++++---- backend/sample_plugin/signals.py | 2 +- backend/sample_plugin/urls.py | 9 ++++- backend/sample_plugin/views.py | 58 +++++++++++++++++----------- backend/tests/test_api.py | 26 ++++++++++--- 7 files changed, 80 insertions(+), 48 deletions(-) diff --git a/backend/sample_plugin/__init__.py b/backend/sample_plugin/__init__.py index 5de8b3f..8ff2f9e 100644 --- a/backend/sample_plugin/__init__.py +++ b/backend/sample_plugin/__init__.py @@ -2,4 +2,4 @@ A sample backend plugin for the Open edX Platform. """ -__version__ = '0.1.0' +__version__ = "0.1.0" diff --git a/backend/sample_plugin/models.py b/backend/sample_plugin/models.py index 6dd31e6..240b1c4 100644 --- a/backend/sample_plugin/models.py +++ b/backend/sample_plugin/models.py @@ -1,6 +1,7 @@ """ 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 @@ -16,28 +17,26 @@ class CourseArchiveStatus(models.Model): """ course_id = CourseKeyField( - max_length=255, - db_index=True, - help_text="The unique identifier for the course." + 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." + 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." + 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." + help_text="The date and time when the course was archived.", ) created_at = models.DateTimeField(auto_now_add=True) @@ -61,7 +60,6 @@ class Meta: # Ensure combination of course_id and user is unique constraints = [ models.UniqueConstraint( - fields=['course_id', 'user'], - name='unique_user_course_archive_status' + fields=["course_id", "user"], name="unique_user_course_archive_status" ) ] diff --git a/backend/sample_plugin/serializers.py b/backend/sample_plugin/serializers.py index 99d2180..9562a4b 100644 --- a/backend/sample_plugin/serializers.py +++ b/backend/sample_plugin/serializers.py @@ -1,6 +1,7 @@ """ Serializers for the sample_plugin app. """ + from rest_framework import serializers from sample_plugin.models import CourseArchiveStatus @@ -18,12 +19,12 @@ class Meta: model = CourseArchiveStatus fields = [ - 'id', - 'course_id', - 'user', - 'is_archived', - 'archive_date', - 'created_at', - 'updated_at', + "id", + "course_id", + "user", + "is_archived", + "archive_date", + "created_at", + "updated_at", ] - read_only_fields = ['id', 'created_at', 'updated_at', 'archive_date'] + read_only_fields = ["id", "created_at", "updated_at", "archive_date"] diff --git a/backend/sample_plugin/signals.py b/backend/sample_plugin/signals.py index 5f99125..46c8ff3 100644 --- a/backend/sample_plugin/signals.py +++ b/backend/sample_plugin/signals.py @@ -12,4 +12,4 @@ # """ # Handle user retirement actions for this app. # """ -# pass \ No newline at end of file +# pass diff --git a/backend/sample_plugin/urls.py b/backend/sample_plugin/urls.py index 5a86a03..7f64ce0 100644 --- a/backend/sample_plugin/urls.py +++ b/backend/sample_plugin/urls.py @@ -1,6 +1,7 @@ """ URLs for sample_plugin. """ + from django.urls import include, path from rest_framework.routers import DefaultRouter @@ -8,9 +9,13 @@ # Create a router and register our viewsets with it router = DefaultRouter() -router.register(r'course-archive-status', CourseArchiveStatusViewSet, basename='course-archive-status') +router.register( + r"course-archive-status", + CourseArchiveStatusViewSet, + basename="course-archive-status", +) # The API URLs are now determined automatically by the router urlpatterns = [ - path('api/v1/', include(router.urls)), + path("api/v1/", include(router.urls)), ] diff --git a/backend/sample_plugin/views.py b/backend/sample_plugin/views.py index 3c0c51c..f93f901 100644 --- a/backend/sample_plugin/views.py +++ b/backend/sample_plugin/views.py @@ -1,6 +1,7 @@ """ Views for the sample_plugin app. """ + import logging from django.utils import timezone @@ -51,7 +52,7 @@ class CourseArchiveStatusPagination(PageNumberPagination): """ page_size = 20 - page_size_query_param = 'page_size' + page_size_query_param = "page_size" max_page_size = 100 @@ -60,7 +61,7 @@ class CourseArchiveStatusThrottle(UserRateThrottle): Throttle for the CourseArchiveStatus API. """ - rate = '60/minute' + rate = "60/minute" class CourseArchiveStatusViewSet(viewsets.ModelViewSet): @@ -78,9 +79,16 @@ class CourseArchiveStatusViewSet(viewsets.ModelViewSet): 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'] + 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): """ @@ -95,7 +103,7 @@ def get_queryset(self): self._validate_query_params() # Always use select_related to avoid N+1 queries - base_queryset = CourseArchiveStatus.objects.select_related('user') + base_queryset = CourseArchiveStatus.objects.select_related("user") if user.is_staff or user.is_superuser: return base_queryset @@ -108,12 +116,12 @@ 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') + 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 + self.request.user.username, ) raise ValidationError({"course_id": "Invalid course ID format."}) @@ -140,20 +148,24 @@ def perform_create(self, serializer): data = serializer.validated_data.copy() # Set user to requesting user if not specified - if 'user' not in data: - data['user'] = self.request.user + 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): + 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 + data["user"].username, + ) + raise PermissionDenied( + "You do not have permission to create records for other users." ) - 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() + if data.get("is_archived", False): + data["archive_date"] = timezone.now() # Create the record instance = serializer.save(**data) @@ -163,7 +175,7 @@ def perform_create(self, serializer): "CourseArchiveStatus created: course_id=%s, user=%s, is_archived=%s", instance.course_id, instance.user.username, - instance.is_archived + instance.is_archived, ) return instance @@ -178,13 +190,13 @@ def perform_update(self, serializer): data = serializer.validated_data.copy() # Handle archive_date if is_archived changes - if 'is_archived' in data: + 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 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 + elif not data["is_archived"] and instance.is_archived: + data["archive_date"] = None # Update the record updated_instance = serializer.save(**data) @@ -194,7 +206,7 @@ def perform_update(self, serializer): "CourseArchiveStatus updated: course_id=%s, user=%s, is_archived=%s", updated_instance.course_id, updated_instance.user.username, - updated_instance.is_archived + updated_instance.is_archived, ) return updated_instance @@ -208,7 +220,7 @@ def perform_destroy(self, instance): "CourseArchiveStatus deleted: course_id=%s, user=%s, by=%s", instance.course_id, instance.user.username, - self.request.user.username + self.request.user.username, ) # Delete the instance diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index bbfbe61..1ccc2d1 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -30,7 +30,9 @@ def user(): Create and return a test user. """ return User.objects.create_user( - username="testuser", email="testuser@example.com", password="password123" + username="testuser", + email="testuser@example.com", + password="password123", ) @@ -40,7 +42,9 @@ def another_user(): Create and return another test user. """ return User.objects.create_user( - username="anotheruser", email="anotheruser@example.com", password="password123" + username="anotheruser", + email="anotheruser@example.com", + password="password123", ) @@ -140,7 +144,11 @@ def test_create_course_archive_status(api_client, user, course_key): """ api_client.force_authenticate(user=user) url = reverse("sample_plugin:course-archive-status-list") - data = {"course_id": str(course_key), "user": user.id, "is_archived": True} + 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 @@ -166,7 +174,11 @@ def test_create_course_archive_status_for_another_user( """ api_client.force_authenticate(user=user) url = reverse("sample_plugin:course-archive-status-list") - data = {"course_id": str(course_key), "user": another_user.id, "is_archived": True} + 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 @@ -181,7 +193,11 @@ def test_staff_create_course_archive_status_for_another_user( """ api_client.force_authenticate(user=staff_user) url = reverse("sample_plugin:course-archive-status-list") - data = {"course_id": str(course_key), "user": user.id, "is_archived": True} + 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 From 094f94a133383a82cb15f2887f1ac48ee679f9b4 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Tue, 15 Apr 2025 16:04:59 -0400 Subject: [PATCH 6/7] build: Install local editable after installing requirements. I would add the local project to dev.in but that results in the compiled output including a fully qualified path to my local version of the project right now which is not helpful. Adding `-e .` to `dev.in` puts a `-e file://my/local/path/to/sample-plugin` in the dev.txt file which is not helpful. --- backend/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/backend/Makefile b/backend/Makefile index 043e138..5ec7066 100644 --- a/backend/Makefile +++ b/backend/Makefile @@ -67,6 +67,8 @@ piptools: ## install pinned version of pip-compile and pip-sync requirements: clean_tox piptools ## install development environment requirements pip-sync -q requirements/dev.txt requirements/private.* + # So that the plugin entrypoints are installed and loaded correctly. + pip install -e . test: clean ## run tests in the current virtualenv pytest From 57dd3719a4df06658ad53a7b49d2217382d8fcaa Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Tue, 15 Apr 2025 16:07:39 -0400 Subject: [PATCH 7/7] test: Run all the tests when you call `tox` --- backend/.gitignore | 1 + backend/tox.ini | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/.gitignore b/backend/.gitignore index c5cb788..80a788a 100644 --- a/backend/.gitignore +++ b/backend/.gitignore @@ -66,3 +66,4 @@ requirements/private.txt # Test cruft default.db +pii_report diff --git a/backend/tox.ini b/backend/tox.ini index a820fd0..cde63c0 100644 --- a/backend/tox.ini +++ b/backend/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py312-django{42} +envlist = py312-django{42},docs,quality,pii_check [doc8] ; D001 = Line too long