diff --git a/.annotation_safe_list.yml b/.annotation_safe_list.yml index 0c1d6f6..62eaaa7 100644 --- a/.annotation_safe_list.yml +++ b/.annotation_safe_list.yml @@ -7,11 +7,6 @@ # fake_app_2.FakeModel2: # ".. choice_annotation:": foo, bar, baz -learning_paths.HistoricalLearningPathEnrollment: - ".. pii": "The email field is not retired to allow future learners to enroll." - ".. pii_types": "email_address" - ".. pii_retirement": "retained" - admin.LogEntry: ".. no_pii:": "This model has no PII" auth.Group: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8133fa7..3fb61de 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,8 +15,8 @@ jobs: strategy: matrix: os: [ubuntu-latest] - python-version: ['3.11'] - toxenv: [quality, docs, pii_check, django42] + python-version: [3.11, 3.12] + toxenv: [django42, django52, quality, docs, pii_check] steps: - uses: actions/checkout@v3 diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 4d740bf..156b91c 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -16,6 +16,25 @@ Unreleased * +0.3.4 - 2025-08-02 +****************** + +Added +===== + +* Bulk unenrollment API. +* Enrollment audit model that tracks the enrollment state transitions. +* Allow specifying time commitment. +* Allow duplicating Learning Paths in the Django admin interface. + +Changed +======= + +* The Learning Paths API includes start and end dates for its steps. +* Return enrollment date in the API instead of a boolean. +* Allow specifying any text for the duration. +* Make the skill level optional. + 0.3.3 - 2025-05-23 ****************** diff --git a/Makefile b/Makefile index 221eb9c..b30a6b0 100644 --- a/Makefile +++ b/Makefile @@ -32,18 +32,11 @@ docs: ## generate Sphinx HTML documentation, including API docs tox -e docs $(BROWSER)docs/_build/html/index.html -# See the `requirements/constraints.txt` file for an explanation. -COMMON_CONSTRAINTS_TXT=requirements/common_constraints.txt -.PHONY: $(COMMON_CONSTRAINTS_TXT) -$(COMMON_CONSTRAINTS_TXT): - wget -O "$(@)" https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt || touch "$(@)" - sed -i 's/^django-simple-history==3.0.0/# &/g' "$(@)" - # Define PIP_COMPILE_OPTS=-v to get more information during make upgrade. PIP_COMPILE = pip-compile --upgrade $(PIP_COMPILE_OPTS) upgrade: export CUSTOM_COMPILE_COMMAND=make upgrade -upgrade: $(COMMON_CONSTRAINTS_TXT) ## update the requirements/*.txt files with the latest packages satisfying requirements/*.in +upgrade: ## update the requirements/*.txt files with the latest packages satisfying requirements/*.in pip install -qr requirements/pip-tools.txt # Make sure to compile files after any other files they include! $(PIP_COMPILE) --allow-unsafe -o requirements/pip.txt requirements/pip.in diff --git a/learning_paths/__init__.py b/learning_paths/__init__.py index 4373120..b701867 100644 --- a/learning_paths/__init__.py +++ b/learning_paths/__init__.py @@ -2,4 +2,4 @@ Learning Paths plugin. """ -__version__ = "0.3.3" +__version__ = "0.3.4" diff --git a/learning_paths/admin.py b/learning_paths/admin.py index 94d9578..16342f4 100644 --- a/learning_paths/admin.py +++ b/learning_paths/admin.py @@ -2,17 +2,25 @@ Django Admin for learning_paths. """ +import os + from django import forms -from django.contrib import admin, auth +from django.contrib import admin, auth, messages from django.core.exceptions import ValidationError +from django.core.files.base import ContentFile from django.db import transaction +from django.http import HttpResponseRedirect +from django.urls import reverse from django.utils.translation import gettext_lazy as _ +from django_object_actions import DjangoObjectActions, action from .compat import get_course_keys_with_outlines from .models import ( AcquiredSkill, LearningPath, LearningPathEnrollment, + LearningPathEnrollmentAllowed, + LearningPathEnrollmentAudit, LearningPathGradingCriteria, LearningPathStep, RequiredSkill, @@ -103,6 +111,7 @@ class LearningPathGradingCriteriaInline(admin.TabularInline): """Inline Admin for Learning path grading criteria.""" model = LearningPathGradingCriteria + verbose_name = "Certificate Criteria" class BulkEnrollUsersForm(forms.ModelForm): @@ -135,7 +144,8 @@ def clean_usernames(self): return users -class LearningPathAdmin(admin.ModelAdmin): +@admin.register(LearningPath) +class LearningPathAdmin(DjangoObjectActions, admin.ModelAdmin): """Admin for Learning Path.""" model = LearningPath @@ -149,7 +159,7 @@ class LearningPathAdmin(admin.ModelAdmin): "key", "display_name", "level", - "duration_in_days", + "duration", "invite_only", ) list_filter = ("invite_only",) @@ -162,6 +172,8 @@ class LearningPathAdmin(admin.ModelAdmin): LearningPathGradingCriteriaInline, ] + change_actions = ("duplicate_learning_path",) + def get_readonly_fields(self, request, obj=None): """Make key read-only only for existing objects.""" if obj: # Editing an existing object. @@ -175,19 +187,142 @@ def save_related(self, request, form, formsets, change): for user in form.cleaned_data["usernames"]: LearningPathEnrollment.objects.get_or_create(user=user, learning_path=form.instance) + @action(label="Duplicate Learning Path", description="Create a copy of this Learning Path") + def duplicate_learning_path(self, request, obj: LearningPath) -> HttpResponseRedirect: + """Duplicate the learning path with a new unique key.""" + base_new_key = f"{str(obj.key)}_copy" + new_key = base_new_key + counter = 1 + + while LearningPath.objects.filter(key=new_key).exists(): + new_key = f"{base_new_key}_{counter}" + counter += 1 + with transaction.atomic(): + new_learning_path = LearningPath( + key=new_key, + display_name=f"{obj.display_name} (Copy)", + subtitle=obj.subtitle, + description=obj.description, + level=obj.level, + duration=obj.duration, + time_commitment=obj.time_commitment, + sequential=obj.sequential, + invite_only=obj.invite_only, + ) + + if obj.image: + with obj.image.open("rb") as original_file: + image_content = original_file.read() + + original_filename = os.path.basename(obj.image.name) + new_learning_path.image.save(original_filename, ContentFile(image_content), save=False) + + new_learning_path.save() + + new_learning_path.refresh_from_db() + new_learning_path.grading_criteria.required_completion = obj.grading_criteria.required_completion + new_learning_path.grading_criteria.required_grade = obj.grading_criteria.required_grade + new_learning_path.grading_criteria.save() + + for step in obj.steps.all(): + step.pk = None + step.learning_path = new_learning_path + step.save() + + for skill in obj.requiredskill_set.all(): + skill.pk = None + skill.learning_path = new_learning_path + skill.save() + + for skill in obj.acquiredskill_set.all(): + skill.pk = None + skill.learning_path = new_learning_path + skill.save() + + messages.success(request, f"Learning path duplicated successfully. New key: {new_key}") + return HttpResponseRedirect(reverse("admin:learning_paths_learningpath_change", args=[new_learning_path.pk])) + + +@admin.register(Skill) class SkillAdmin(admin.ModelAdmin): """Admin for Learning Path generic skill.""" model = Skill +class EnrollmentAuditInline(admin.TabularInline): + """Inline admin for LearningPathEnrollmentAudit records.""" + + model = LearningPathEnrollmentAudit + fk_name = "enrollment" + extra = 0 + exclude = ["enrollment_allowed"] + readonly_fields = [ + "state_transition", + "enrolled_by", + "reason", + "org", + "role", + "created", + ] + + def has_add_permission(self, request, obj=None): + """Disable manual creation of audit records.""" + return False + + def has_delete_permission(self, request, obj=None): + """Disable deletion of audit records.""" + return False + + +class EnrollmentAllowedAuditInline(admin.TabularInline): + """Inline admin for LearningPathEnrollmentAudit records related to enrollment allowed.""" + + model = LearningPathEnrollmentAudit + fk_name = "enrollment_allowed" + extra = 0 + exclude = ["enrollment"] + readonly_fields = [ + "state_transition", + "enrolled_by", + "reason", + "org", + "role", + "created", + ] + + def has_add_permission(self, request, obj=None): + """Disable manual creation of audit records.""" + return False + + def has_delete_permission(self, request, obj=None): + """Disable deletion of audit records.""" + return False + + +@admin.register(LearningPathEnrollment) class EnrolledUsersAdmin(admin.ModelAdmin): """Admin for Learning Path enrollment.""" model = LearningPathEnrollment raw_id_fields = ("user",) autocomplete_fields = ["learning_path"] + inlines = [EnrollmentAuditInline] + + list_display = [ + "id", + "user", + "learning_path", + "is_active", + "created", + ] + + list_filter = [ + "learning_path__key", + "created", + "is_active", + ] search_fields = [ "id", @@ -197,6 +332,103 @@ class EnrolledUsersAdmin(admin.ModelAdmin): ] -admin.site.register(LearningPath, LearningPathAdmin) -admin.site.register(Skill, SkillAdmin) -admin.site.register(LearningPathEnrollment, EnrolledUsersAdmin) +@admin.register(LearningPathEnrollmentAllowed) +class EnrollmentAllowedAdmin(admin.ModelAdmin): + """Admin configuration for LearningPathEnrollmentAllowed model.""" + + list_display = [ + "id", + "email", + "get_user", + "learning_path", + "created", + ] + + list_filter = [ + "learning_path", + "created", + ] + + search_fields = [ + "email", + "user__username", + "user__email", + "learning_path__key", + ] + + readonly_fields = [ + "user", + "created", + "modified", + ] + + inlines = [EnrollmentAllowedAuditInline] + + def get_user(self, obj): + """Get the associated user, if any.""" + return obj.user.username if obj.user else "-" + + get_user.short_description = "User" + + +@admin.register(LearningPathEnrollmentAudit) +class EnrollmentAuditAdmin(admin.ModelAdmin): + """Admin configuration for LearningPathEnrollmentAudit model.""" + + list_display = [ + "id", + "state_transition", + "enrolled_by", + "get_enrollee", + "get_learning_path", + "created", + "org", + "role", + ] + + list_filter = [ + "state_transition", + "created", + "org", + "role", + ] + + search_fields = [ + "enrolled_by__username", + "enrolled_by__email", + "enrollment__user__username", + "enrollment__user__email", + "enrollment_allowed__email", + "enrollment__learning_path__key", + "enrollment_allowed__learning_path__key", + "reason", + ] + + readonly_fields = [ + "enrollment", + "enrollment_allowed", + "enrolled_by", + "state_transition", + "created", + "modified", + ] + + def get_enrollee(self, obj): + """Get the enrollee (user or email).""" + if obj.enrollment: + return obj.enrollment.user.username + elif obj.enrollment_allowed: + return obj.enrollment_allowed.user.username if obj.enrollment_allowed.user else obj.enrollment_allowed.email + return "-" + + get_enrollee.short_description = "Enrollee" + + def get_learning_path(self, obj): + """Get the learning path title.""" + if obj.enrollment: + return obj.enrollment.learning_path.key + elif obj.enrollment_allowed: + return obj.enrollment_allowed.learning_path.key + return "-" + + get_learning_path.short_description = "Learning Path" diff --git a/learning_paths/api/v1/serializers.py b/learning_paths/api/v1/serializers.py index ee40e15..580819e 100644 --- a/learning_paths/api/v1/serializers.py +++ b/learning_paths/api/v1/serializers.py @@ -97,7 +97,7 @@ class LearningPathGradeSerializer(serializers.Serializer): class LearningPathStepSerializer(serializers.ModelSerializer): class Meta: model = LearningPathStep - fields = ["order", "course_key", "due_date", "weight"] + fields = ["order", "course_key", "course_dates", "weight"] class LearningPathListSerializer(serializers.ModelSerializer): @@ -105,7 +105,7 @@ class LearningPathListSerializer(serializers.ModelSerializer): steps = LearningPathStepSerializer(many=True, read_only=True) required_completion = serializers.FloatField(source="grading_criteria.required_completion", read_only=True) - is_enrolled = serializers.SerializerMethodField() + enrollment_date = serializers.SerializerMethodField() invite_only = serializers.BooleanField() image = serializers.ImageField(read_only=True) @@ -118,17 +118,17 @@ class Meta: "sequential", "steps", "required_completion", - "is_enrolled", + "enrollment_date", "invite_only", ] - def get_is_enrolled(self, obj): + def get_enrollment_date(self, obj): """ Check if the current user is enrolled in this learning path. """ - if hasattr(obj, "is_enrolled"): - return obj.is_enrolled - return False + if hasattr(obj, "enrollment_date"): + return obj.enrollment_date + return None class SkillSerializer(serializers.ModelSerializer): @@ -174,7 +174,8 @@ class Meta(LearningPathListSerializer.Meta): "subtitle", "description", "level", - "duration_in_days", + "duration", + "time_commitment", "required_skills", "acquired_skills", ] @@ -183,4 +184,4 @@ class Meta(LearningPathListSerializer.Meta): class LearningPathEnrollmentSerializer(serializers.ModelSerializer): class Meta: model = LearningPathEnrollment - fields = ("user", "learning_path", "is_active", "enrolled_at") + fields = ("user", "learning_path", "is_active", "created") diff --git a/learning_paths/api/v1/tests/test_serializers.py b/learning_paths/api/v1/tests/test_serializers.py index d45feec..a9c58a4 100644 --- a/learning_paths/api/v1/tests/test_serializers.py +++ b/learning_paths/api/v1/tests/test_serializers.py @@ -12,7 +12,6 @@ from learning_paths.tests.factories import ( LearningPathEnrollmentFactory, LearningPathFactory, - UserFactory, ) @@ -105,26 +104,24 @@ def test_list_serializer(): "sequential": False, "steps": [], "required_completion": 0.8, - "is_enrolled": False, + "enrollment_date": None, } assert dict(serializer.data) == expected @pytest.mark.django_db @pytest.mark.parametrize("is_enrolled", [True, False], ids=["enrolled", "not_enrolled"]) -def test_list_serializer_enrollment(is_enrolled): +def test_list_serializer_enrollment(user, learning_path, is_enrolled): """ - Tests LearningPathListSerializer shows is_enrolled as True when a user is enrolled. + Tests LearningPathListSerializer shows enrollment_date when a user is enrolled. """ - user = UserFactory() - learning_path = LearningPathFactory(invite_only=False) - LearningPathEnrollmentFactory(user=user, learning_path=learning_path, is_active=is_enrolled) + enrollment = LearningPathEnrollmentFactory(user=user, learning_path=learning_path, is_active=is_enrolled) # Get the annotated learning path with the enrollment status. learning_path = learning_path.__class__.objects.get_paths_visible_to_user(user).get(key=learning_path.key) serializer = LearningPathListSerializer(learning_path) - assert serializer.data["is_enrolled"] is is_enrolled + assert serializer.data["enrollment_date"] == (enrollment.created if is_enrolled else None) @pytest.mark.django_db @@ -142,11 +139,12 @@ def test_detail_serializer(): "invite_only": True, "level": "", "sequential": False, - "duration_in_days": None, + "duration": "", + "time_commitment": "", "required_skills": [], "acquired_skills": [], "steps": [], - "is_enrolled": False, + "enrollment_date": None, "required_completion": 0.8, } serializer = LearningPathDetailSerializer(learning_path) diff --git a/learning_paths/api/v1/tests/test_views.py b/learning_paths/api/v1/tests/test_views.py index 312fe5b..c7b00ad 100644 --- a/learning_paths/api/v1/tests/test_views.py +++ b/learning_paths/api/v1/tests/test_views.py @@ -1,6 +1,6 @@ # pylint: disable=missing-module-docstring,missing-class-docstring,redefined-outer-name,unused-argument from datetime import datetime, timezone -from unittest.mock import PropertyMock, patch +from unittest.mock import patch import pytest from django.test import override_settings @@ -19,10 +19,11 @@ from learning_paths.models import ( LearningPathEnrollment, LearningPathEnrollmentAllowed, - LearningPathStep, + LearningPathEnrollmentAudit, ) from learning_paths.tests.factories import ( AcquiredSkillFactory, + LearningPathEnrollmentAllowedFactory, LearningPathEnrollmentFactory, LearningPathFactory, LearningPathStepFactory, @@ -36,11 +37,6 @@ def api_client(): return APIClient() -@pytest.fixture -def user(): - return UserFactory() - - @pytest.fixture def staff_user(): return UserFactory(is_staff=True) @@ -69,16 +65,6 @@ def superuser_client(api_client, superuser): return api_client -@pytest.fixture -def learning_path(): - return LearningPathFactory.create(invite_only=False) - - -@pytest.fixture -def learning_path_with_invite_only(): - return LearningPathFactory.create() - - @pytest.fixture def learning_paths(): return LearningPathFactory.create_batch(5, invite_only=False) @@ -114,16 +100,6 @@ def learning_paths_with_steps(): # pylint: disable=missing-function-docstring return learning_paths -@pytest.fixture -def active_enrollment(user, learning_path): - return LearningPathEnrollmentFactory(user=user, learning_path=learning_path, is_active=True) - - -@pytest.fixture -def inactive_enrollment(user, learning_path): - return LearningPathEnrollmentFactory(user=user, learning_path=learning_path, is_active=False) - - @pytest.mark.django_db class TestLearningPathAsProgram: @@ -226,9 +202,11 @@ def test_invite_only_learning_path_grade_404(self, authenticated_client, learnin class TestLearningPathViewSet: @pytest.fixture(autouse=True) - def setup_mock_due_date(self): - due_date = datetime(2025, 1, 1, tzinfo=timezone.utc) - with patch("learning_paths.models.get_course_due_date", return_value=due_date): + def setup_mock_course_dates(self): + """Mock course dates that are retrieved from edx-platform.""" + start_date = datetime(2024, 1, 1, tzinfo=timezone.utc) + end_date = datetime(2025, 1, 1, tzinfo=timezone.utc) + with patch("learning_paths.models.get_course_dates", return_value=(start_date, end_date)): yield def test_learning_path_list(self, authenticated_client, learning_paths_with_steps): @@ -241,31 +219,28 @@ def test_learning_path_list(self, authenticated_client, learning_paths_with_step assert "key" in first_item assert "display_name" in first_item assert "steps" in first_item - assert "is_enrolled" in first_item - assert first_item["is_enrolled"] is False + assert "enrollment_date" in first_item + assert first_item["enrollment_date"] is None def test_learning_path_retrieve(self, authenticated_client, learning_paths_with_steps): """Test that the retrieve endpoint returns the details of a learning path.""" lp = learning_paths_with_steps[0] url = reverse("learning-path-detail", args=[lp.key]) - fake_due_date = datetime(2025, 1, 1, tzinfo=timezone.utc) - - with patch.object(LearningPathStep, "due_date", new_callable=PropertyMock) as mock_due_date: - mock_due_date.return_value = fake_due_date - response = authenticated_client.get(url) - assert response.status_code == status.HTTP_200_OK - assert "steps" in response.data - assert "required_skills" in response.data - assert "acquired_skills" in response.data - assert "is_enrolled" in response.data - assert response.data["is_enrolled"] is False - - if response.data["steps"]: - first_step = response.data["steps"][0] - assert "order" in first_step - assert "course_key" in first_step - assert "due_date" in first_step - assert "weight" in first_step + + response = authenticated_client.get(url) + assert response.status_code == status.HTTP_200_OK + assert "steps" in response.data + assert "required_skills" in response.data + assert "acquired_skills" in response.data + assert "enrollment_date" in response.data + assert response.data["enrollment_date"] is None + + if response.data["steps"]: + first_step = response.data["steps"][0] + assert "order" in first_step + assert "course_key" in first_step + assert "course_dates" in first_step + assert "weight" in first_step def test_invalid_learning_path_key_returns_404(self, authenticated_client): """Test that an invalid learning path key format returns a 404 response.""" @@ -289,8 +264,8 @@ def test_learning_path_list_with_enrollment(self, authenticated_client, active_e assert response.status_code == status.HTTP_200_OK assert len(response.data) == 1 first_item = response.data[0] - assert "is_enrolled" in first_item - assert first_item["is_enrolled"] is True + assert "enrollment_date" in first_item + assert first_item["enrollment_date"] is not None def test_learning_path_retrieve_with_enrollment(self, authenticated_client, active_enrollment, learning_path, user): """Test that the retrieve endpoint returns the details of a learning path with enrollment status.""" @@ -298,8 +273,8 @@ def test_learning_path_retrieve_with_enrollment(self, authenticated_client, acti response = authenticated_client.get(url) assert response.status_code == status.HTTP_200_OK - assert "is_enrolled" in response.data - assert response.data["is_enrolled"] is True + assert "enrollment_date" in response.data + assert response.data["enrollment_date"] is not None def test_learning_path_retrieve_with_inactive_enrollment( self, authenticated_client, inactive_enrollment, learning_path, user @@ -309,8 +284,8 @@ def test_learning_path_retrieve_with_inactive_enrollment( response = authenticated_client.get(url) assert response.status_code == status.HTTP_200_OK - assert "is_enrolled" in response.data - assert response.data["is_enrolled"] is False + assert "enrollment_date" in response.data + assert response.data["enrollment_date"] is None def test_invite_only_learning_paths_hidden_from_non_enrolled_users( self, authenticated_client, learning_path_with_invite_only, learning_path @@ -614,25 +589,14 @@ class TestBulkEnrollAPI: def bulk_enroll_url(self): return "/api/learning_paths/v1/enrollments/bulk-enroll/" - @pytest.fixture - def setup_users_and_paths(self): # pylint: disable=missing-function-docstring - learning_path1 = LearningPathFactory() - learning_path2 = LearningPathFactory() - user1 = UserFactory(email="user1@example.com") - user2 = UserFactory(email="user2@example.com") - return { - "learning_path1": learning_path1, - "learning_path2": learning_path2, - "user1": user1, - "user2": user2, - } - - def test_bulk_enrollment_success(self, staff_client, bulk_enroll_url, setup_users_and_paths): - """Test bulk enrollment creates enrollments and enrollment allowed objects.""" - data = setup_users_and_paths + def test_bulk_enrollment_success(self, staff_client, staff_user, bulk_enroll_url): + """Test bulk enrollment creates enrollments and enrollment allowed objects and their audits.""" payload = { - "learning_paths": f"{data['learning_path1'].key},{data['learning_path2'].key}", - "emails": "user1@example.com,user2@example.com,new_user@example.com", + "learning_paths": f"{LearningPathFactory().key},{LearningPathFactory().key}", + "emails": f"{UserFactory().email},{UserFactory().email},new_user@example.com", + "reason": "TestReason", + "org": "TestOrg", + "role": "TestRole", } response = staff_client.post(bulk_enroll_url, payload) @@ -642,22 +606,38 @@ def test_bulk_enrollment_success(self, staff_client, bulk_enroll_url, setup_user # (1 non-existing user x 2 learning paths) assert response.data["enrollment_allowed_created"] == 2 - # Check learning path enrollments - assert LearningPathEnrollment.objects.filter(user=data["user1"]).count() == 2 - assert LearningPathEnrollment.objects.filter(user=data["user2"]).count() == 2 + all_audits = list(LearningPathEnrollmentAudit.objects.all()) + assert len(all_audits) == 6 + + for audit in all_audits: + if audit.enrollment: + assert audit.state_transition == LearningPathEnrollmentAudit.UNENROLLED_TO_ENROLLED + else: + assert audit.state_transition == LearningPathEnrollmentAudit.UNENROLLED_TO_ALLOWEDTOENROLL + + assert audit.enrolled_by == staff_user + assert audit.reason == payload["reason"] + assert audit.org == payload["org"] + assert audit.role == payload["role"] + + def test_bulk_enrollment_updates_existing_enrollment_allowed(self, staff_client, bulk_enroll_url, learning_path): + """Test bulk enrollment updates existing enrollment allowed records.""" + email = "new_user@example.com" + existing_allowed = LearningPathEnrollmentAllowed.objects.create(email=email, learning_path=learning_path) + + payload = { + "learning_paths": learning_path.key, + "emails": email, + "reason": "TestReason", + } + response = staff_client.post(bulk_enroll_url, payload) - # Check learning path enrollment allowed - assert set( - LearningPathEnrollmentAllowed.objects.filter(learning_path=data["learning_path1"]).values_list( - "email", flat=True - ) - ) == {"new_user@example.com"} + assert response.status_code == status.HTTP_201_CREATED + assert response.data["enrollment_allowed_created"] == 0 - assert set( - LearningPathEnrollmentAllowed.objects.filter(learning_path=data["learning_path2"]).values_list( - "email", flat=True - ) - ) == {"new_user@example.com"} + audit = existing_allowed.audit.get() + assert audit.state_transition == LearningPathEnrollmentAudit.UNENROLLED_TO_ALLOWEDTOENROLL + assert audit.reason == payload["reason"] def test_bulk_enrollment_with_invalid_learning_path(self, staff_client, bulk_enroll_url): """Test bulk enrollment with invalid learning path creates no enrollments.""" @@ -671,12 +651,11 @@ def test_bulk_enrollment_with_invalid_learning_path(self, staff_client, bulk_enr assert response.data["enrollments_created"] == 0 assert response.data["enrollment_allowed_created"] == 0 - def test_bulk_enrollment_with_invalid_and_valid_emails(self, staff_client, bulk_enroll_url, setup_users_and_paths): + def test_bulk_enrollment_with_invalid_and_valid_emails(self, staff_client, bulk_enroll_url, user, learning_path): """Test bulk enrollment with invalid email only creates enrollments for valid emails.""" - data = setup_users_and_paths payload = { - "learning_paths": f"{data['learning_path1'].key}", - "emails": "user1@example.com,invalid_email", + "learning_paths": learning_path.key, + "emails": f"{user.email},invalid_email", } response = staff_client.post(bulk_enroll_url, payload) @@ -685,40 +664,42 @@ def test_bulk_enrollment_with_invalid_and_valid_emails(self, staff_client, bulk_ assert response.data["enrollment_allowed_created"] == 0 # Check learning path enrollment for valid user - assert LearningPathEnrollment.objects.filter(user=data["user1"], learning_path=data["learning_path1"]).exists() + assert LearningPathEnrollment.objects.filter(user=user, learning_path=learning_path).exists() # Check enrollment allowed for invalid email doesn't exist assert not LearningPathEnrollmentAllowed.objects.filter( - email="invalid_email", learning_path=data["learning_path1"] + email="invalid_email", learning_path=learning_path ).exists() - def test_bulk_enrollment_unauthenticated(self, api_client, bulk_enroll_url, user, setup_users_and_paths): - """Test unauthenticated and non-staff users receive 403 for bulk enrollment.""" - data = setup_users_and_paths + @pytest.mark.parametrize("http_method", ["post", "delete"]) + def test_bulk_operation_unauthenticated_and_non_staff( # pylint: disable=too-many-positional-arguments + self, api_client, bulk_enroll_url, user, learning_path, http_method + ): + """Test unauthenticated and non-staff users receive 403 for bulk operations (enroll/unenroll).""" payload = { - "learning_paths": f"{data['learning_path1'].key}", - "emails": "user1@example.com", + "learning_paths": learning_path.key, + "emails": user.email, } # Unauthenticated - response = api_client.post(bulk_enroll_url, payload) + request_method_func = getattr(api_client, http_method) + response = request_method_func(bulk_enroll_url, payload) assert response.status_code == status.HTTP_403_FORBIDDEN # Non-staff user api_client.force_authenticate(user=user) - response = api_client.post(bulk_enroll_url, payload) + request_method_func = getattr(api_client, http_method) + response = request_method_func(bulk_enroll_url, payload) assert response.status_code == status.HTTP_403_FORBIDDEN - def test_bulk_enrollment_returned_counts_reflect_only_new_ones( - self, staff_client, bulk_enroll_url, setup_users_and_paths + def test_bulk_enrollment_returned_counts_reflect_only_new_ones( # pylint: disable=too-many-positional-arguments + self, staff_client, staff_user, bulk_enroll_url, user, learning_path, active_enrollment ): - """Test bulk enrollment counts only reflect new enrollments.""" - data = setup_users_and_paths - LearningPathEnrollmentFactory(learning_path=data["learning_path1"], user=data["user1"], is_active=True) - + """Test bulk enrollment counts only reflect new enrollments and creates ENROLLED_TO_ENROLLED audit.""" payload = { - "learning_paths": f"{data['learning_path1'].key}", - "emails": data["user1"].email, + "learning_paths": learning_path.key, + "emails": user.email, + "reason": "TestReason", } response = staff_client.post(bulk_enroll_url, payload) @@ -727,14 +708,21 @@ def test_bulk_enrollment_returned_counts_reflect_only_new_ones( assert response.data["enrollments_created"] == 0 assert response.data["enrollment_allowed_created"] == 0 - def test_re_enrollment(self, staff_client, bulk_enroll_url, setup_users_and_paths): - """Test bulk enrollment reactivates inactive enrollments.""" - data = setup_users_and_paths - LearningPathEnrollmentFactory(learning_path=data["learning_path1"], user=data["user1"], is_active=False) + active_enrollment.refresh_from_db() + assert active_enrollment.is_active is True + + latest_audit = active_enrollment.audit.last() + assert latest_audit.state_transition == LearningPathEnrollmentAudit.ENROLLED_TO_ENROLLED + assert latest_audit.enrolled_by == staff_user + assert latest_audit.reason == payload["reason"] + # pylint: disable=too-many-positional-arguments + def test_re_enrollment(self, staff_client, staff_user, bulk_enroll_url, user, learning_path, inactive_enrollment): + """Test bulk enrollment reactivates inactive enrollments and creates a correct audit.""" payload = { - "learning_paths": f"{data['learning_path1'].key}", - "emails": data["user1"].email, + "learning_paths": learning_path.key, + "emails": user.email, + "reason": "TestReason", } response = staff_client.post(bulk_enroll_url, payload) @@ -742,6 +730,112 @@ def test_re_enrollment(self, staff_client, bulk_enroll_url, setup_users_and_path assert response.status_code == status.HTTP_201_CREATED assert response.data["enrollments_created"] == 1 + inactive_enrollment.refresh_from_db() + assert inactive_enrollment.is_active is True + + latest_audit = inactive_enrollment.audit.last() + assert latest_audit.state_transition == LearningPathEnrollmentAudit.UNENROLLED_TO_ENROLLED + assert latest_audit.enrolled_by == staff_user + assert latest_audit.reason == payload["reason"] + + def test_bulk_enrollment_allowed_re_enrollment(self, staff_client, bulk_enroll_url, learning_path): + """Test that bulk enrollment re-activates an inactive LearningPathEnrollmentAllowed.""" + email = "new_user@example.com" + allowed = LearningPathEnrollmentAllowedFactory(email=email, learning_path=learning_path, is_active=False) + payload = {"learning_paths": learning_path.key, "emails": email} + + response = staff_client.post(bulk_enroll_url, payload) + assert response.status_code == status.HTTP_201_CREATED + assert response.data["enrollment_allowed_created"] == 1 + assert LearningPathEnrollmentAllowed.objects.count() == 1 + + allowed.refresh_from_db() + assert allowed.is_active + + # Bulk unenrollment tests + + def test_unenroll_success(self, staff_client, staff_user, bulk_enroll_url): + """Test bulk unenrollment deactivates enrollments and enrollment allowed records and creates audit records.""" + user1 = UserFactory() + user2 = UserFactory() + user3 = UserFactory() # Not enrolled, should not be affected. + lp1 = LearningPathFactory() + lp2 = LearningPathFactory() + + enrollments = [ + LearningPathEnrollmentFactory(user=user1, learning_path=lp1), + LearningPathEnrollmentFactory(user=user1, learning_path=lp2), + LearningPathEnrollmentFactory(user=user2, learning_path=lp1), + LearningPathEnrollmentFactory(user=user2, learning_path=lp2, is_active=False), + ] + + allowed_records = [ + LearningPathEnrollmentAllowedFactory(email="new_user@example.com", learning_path=lp1, is_active=True), + LearningPathEnrollmentAllowedFactory(email="inactive@example.com", learning_path=lp2, is_active=False), + ] + + payload = { + "learning_paths": f"{lp1.key},{lp2.key}", + "emails": f"{user1.email},{user2.email},{user3.email},new_user@example.com,inactive@example.com,invalid", + "reason": "TestReason", + "org": "TestOrg", + "role": "TestRole", + } + + response = staff_client.delete(bulk_enroll_url, payload) + assert response.status_code == status.HTTP_204_NO_CONTENT + assert response.data["enrollments_unenrolled"] == 3 + assert response.data["enrollment_allowed_deactivated"] == 1 + + # 4 enrollments + 3 unenrollments + 1 inactive update + 2 allowed records + assert LearningPathEnrollmentAudit.objects.count() == 10 + + for i, enrollment in enumerate(enrollments): + enrollment.refresh_from_db() + assert not enrollment.is_active + + audit = enrollment.audit.last() + if i < 3: + assert audit.state_transition == LearningPathEnrollmentAudit.ENROLLED_TO_UNENROLLED + else: + assert audit.state_transition == LearningPathEnrollmentAudit.UNENROLLED_TO_UNENROLLED + assert audit.enrolled_by == staff_user + assert audit.reason == payload["reason"] + assert audit.org == payload["org"] + assert audit.role == payload["role"] + + for i, allowed_record in enumerate(allowed_records): + allowed_record.refresh_from_db() + assert not allowed_record.is_active + assert allowed_record.audit.count() == 1 + + audit = allowed_record.audit.last() + if i == 0: + assert audit.state_transition == LearningPathEnrollmentAudit.ALLOWEDTOENROLL_TO_UNENROLLED + else: + assert audit.state_transition == LearningPathEnrollmentAudit.UNENROLLED_TO_UNENROLLED + assert audit.enrolled_by == staff_user + assert audit.reason == payload["reason"] + assert audit.org == payload["org"] + assert audit.role == payload["role"] + + def test_unenroll_with_invalid_learning_path(self, staff_client, bulk_enroll_url): + """Test bulk unenrollment with invalid learning path creates no changes.""" + payload = {"learning_paths": "invalid-path-key", "emails": "user1@example.com"} + response = staff_client.delete(bulk_enroll_url, payload) + + assert response.status_code == status.HTTP_204_NO_CONTENT + assert response.data["enrollments_unenrolled"] == 0 + + def test_unenroll_empty_parameters(self, staff_client, bulk_enroll_url): + """Test bulk unenrollment with empty or missing parameters doesn't affect existing enrollments.""" + response1 = staff_client.delete(bulk_enroll_url, {"learning_paths": "", "emails": ""}) + response2 = staff_client.delete(bulk_enroll_url, {}) + + for response in [response1, response2]: + assert response.status_code == status.HTTP_204_NO_CONTENT + assert response.data["enrollments_unenrolled"] == 0 + @pytest.mark.django_db class TestLearningPathCourseEnrollment: diff --git a/learning_paths/api/v1/views.py b/learning_paths/api/v1/views.py index 4c3800f..a66d20c 100644 --- a/learning_paths/api/v1/views.py +++ b/learning_paths/api/v1/views.py @@ -3,12 +3,12 @@ """ import logging -from datetime import datetime, timezone from django.conf import settings from django.contrib.auth import get_user_model from django.core.exceptions import ObjectDoesNotExist, PermissionDenied, ValidationError from django.core.validators import validate_email +from django.db.models import QuerySet from django.shortcuts import get_object_or_404 from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey @@ -16,6 +16,7 @@ from rest_framework.exceptions import NotFound, ParseError from rest_framework.pagination import PageNumberPagination from rest_framework.permissions import IsAdminUser, IsAuthenticated +from rest_framework.request import Request from rest_framework.response import Response from rest_framework.views import APIView @@ -33,6 +34,7 @@ LearningPath, LearningPathEnrollment, LearningPathEnrollmentAllowed, + LearningPathEnrollmentAudit, ) from .filters import AdminOrSelfFilterBackend @@ -239,7 +241,6 @@ def post(self, request, learning_path_key_str: str): return Response({"detail": "Enrollment exists."}, status=status.HTTP_409_CONFLICT) enrollment.is_active = True - enrollment.enrolled_at = datetime.now(timezone.utc) enrollment.save() return Response(LearningPathEnrollmentSerializer(enrollment).data) @@ -295,13 +296,53 @@ class ListEnrollmentsView(generics.ListAPIView): class BulkEnrollView(APIView): """ - Bulk enrollment API for LearningPathEnrollment. - + Bulk enrollment/unenrollment API for LearningPathEnrollment. """ permission_classes = [IsAdminUser] - def post(self, request, *args, **kwargs): + @staticmethod + def _process_input_data(request: Request) -> tuple[list[str], list[str]]: + """Extract and validate input data from request.""" + data = request.data + learning_paths_keys = data.get("learning_paths", "").split(",") + emails = data.get("emails", "").split(",") + + return learning_paths_keys, emails + + @staticmethod + def _validate_learning_paths(learning_paths_keys: list[str]) -> QuerySet[LearningPath]: + """Validate learning path keys and return valid ones.""" + valid_learning_paths_keys = [] + for key in learning_paths_keys: + try: + LearningPathKey.from_string(key) + valid_learning_paths_keys.append(key) + except InvalidKeyError: + logger.warning("BulkEnrollView: Invalid learning path key: %s", key) + + return LearningPath.objects.filter(key__in=valid_learning_paths_keys) + + @staticmethod + def _create_audit_data(request: Request, state_transition: str) -> dict[str, str]: + """Create audit data dictionary.""" + return { + "enrolled_by": request.user, + "reason": request.data.get("reason", ""), + "org": request.data.get("org", ""), + "role": request.data.get("role", ""), + "state_transition": state_transition, + } + + def _setup_bulk_operation(self, request: Request) -> tuple[QuerySet[LearningPath], QuerySet[User], list[str]]: + """Common setup for bulk operations.""" + learning_paths_keys, emails = self._process_input_data(request) + learning_paths = self._validate_learning_paths(learning_paths_keys) + existing_users = User.objects.filter(email__in=emails) + + return learning_paths, existing_users, emails + + def post(self, request: Request, *args, **kwargs) -> Response: """ Bulk Enroll learners in Learning Paths. @@ -312,11 +353,17 @@ def post(self, request, *args, **kwargs): { "learning_paths": "learning_path_1,learning_path_2", - "emails": "user_1@example.com,user_2@example.com" + "emails": "user_1@example.com,user_2@example.com", + "reason": "Bulk enrollment for new cohort", + "org": "organization_name", + "role": "student" } `learning_paths` (str): A comma separated list of learning path IDs. `emails` (str): A comma separated list of email addresses. + `reason` (str, optional): Reason for enrollment, used for audit. + `org` (str, optional): Organization identifier, used for audit. + `role` (str, optional): User role, used for audit. * For existing users, it creates a new LearningPathEnrollment record, automatically enrolling them in the learning path. It also creates a LearningPathAllowed record @@ -325,42 +372,30 @@ def post(self, request, *args, **kwargs): with just the email address, allowing them to get enrolled when they register. """ - data = request.data - learning_paths_keys = data.get("learning_paths", "").split(",") - emails = data.get("emails", "").split(",") - - valid_learning_paths_keys = [] - for key in learning_paths_keys: - try: - LearningPathKey.from_string(key) - valid_learning_paths_keys.append(key) - except InvalidKeyError: - logger.warning("BulkEnrollView: Invalid learning path key: %s", key) - - learning_paths = LearningPath.objects.filter(key__in=valid_learning_paths_keys) - - existing_users = User.objects.filter(email__in=emails) + learning_paths, existing_users, emails = self._setup_bulk_operation(request) non_existing_emails = set(emails) - set(u.email for u in existing_users) enrollments_created = [] enrollment_allowed_created = [] for learning_path in learning_paths: - # Create LearningPathEnrollment for existing users for user in existing_users: enrollment = LearningPathEnrollment.objects.filter(user=user, learning_path=learning_path).first() enrolled_now = False - if not enrollment: - enrollment = LearningPathEnrollment( - user=user, - learning_path=learning_path, - ) - enrolled_now = True - if not enrollment.is_active: - enrollment.is_active = True - enrollment.enrolled_at = datetime.now(timezone.utc) + audit_data = self._create_audit_data(request, LearningPathEnrollmentAudit.UNENROLLED_TO_ENROLLED) + if enrollment: + if not enrollment.is_active: + enrollment.is_active = True + enrolled_now = True + else: + audit_data["state_transition"] = LearningPathEnrollmentAudit.ENROLLED_TO_ENROLLED + else: + enrollment = LearningPathEnrollment(user=user, learning_path=learning_path) enrolled_now = True + + # Set enrollment audit data that will be used by the post_save receiver. + enrollment._audit = audit_data # pylint: disable=protected-access enrollment.save() if enrolled_now: enrollments_created.append(enrollment) @@ -375,9 +410,14 @@ def post(self, request, *args, **kwargs): allowed, created = LearningPathEnrollmentAllowed.objects.get_or_create( email=email, learning_path=learning_path ) - if created: + if created or (not allowed.user and not allowed.is_active): + allowed.is_active = True enrollment_allowed_created.append(allowed) + audit_data = self._create_audit_data(request, LearningPathEnrollmentAudit.UNENROLLED_TO_ALLOWEDTOENROLL) + allowed._audit = audit_data # pylint: disable=protected-access + allowed.save() + return Response( { "enrollments_created": len(enrollments_created), @@ -386,6 +426,84 @@ def post(self, request, *args, **kwargs): status=status.HTTP_201_CREATED, ) + def delete(self, request, *args, **kwargs) -> Response: + """ + Bulk Unenroll learners from Learning Paths. + + The "bulk unenroll" API provides a way for the staff to unenroll multiple learners + from multiple learning paths at once. + + Example payload:: + + { + "learning_paths": "learning_path_1,learning_path_2", + "emails": "user_1@example.com,user_2@example.com", + "reason": "End of semester cleanup", + "org": "organization_name", + "role": "student" + } + + `learning_paths` (str): A comma separated list of learning path IDs. + `emails` (str): A comma separated list of email addresses. + `reason` (str, optional): Reason for unenrollment, used for audit. + `org` (str, optional): Organization identifier, used for audit. + `role` (str, optional): User role, used for audit. + + * For existing users, it deactivates their LearningPathEnrollment records. + * For emails with active LearningPathEnrollmentAllowed records, it deactivates those records. + + """ + learning_paths, existing_users, emails = self._setup_bulk_operation(request) + + enrollments_unenrolled = [] + enrollment_allowed_deactivated = [] + + for learning_path in learning_paths: + for user in existing_users: + enrollment = LearningPathEnrollment.objects.filter(user=user, learning_path=learning_path).first() + + if enrollment: + if enrollment.is_active: + state_transition = LearningPathEnrollmentAudit.ENROLLED_TO_UNENROLLED + enrollment.is_active = False + enrollments_unenrolled.append(enrollment) + else: + state_transition = LearningPathEnrollmentAudit.UNENROLLED_TO_UNENROLLED + audit_data = self._create_audit_data(request, state_transition) + enrollment._audit = audit_data # pylint: disable=protected-access + enrollment.save() + + for email in emails: + try: + validate_email(email) + except ValidationError: + logger.warning("BulkEnrollView: Invalid email: %s", email) + continue + + enrollment_allowed = LearningPathEnrollmentAllowed.objects.filter( + email=email, + learning_path=learning_path, + ).first() + + if enrollment_allowed: + if enrollment_allowed.is_active: + state_transition = LearningPathEnrollmentAudit.ALLOWEDTOENROLL_TO_UNENROLLED + enrollment_allowed.is_active = False + enrollment_allowed_deactivated.append(enrollment_allowed) + else: + state_transition = LearningPathEnrollmentAudit.UNENROLLED_TO_UNENROLLED + audit_data = self._create_audit_data(request, state_transition) + enrollment_allowed._audit = audit_data # pylint: disable=protected-access + enrollment_allowed.save() + + return Response( + { + "enrollments_unenrolled": len(enrollments_unenrolled), + "enrollment_allowed_deactivated": len(enrollment_allowed_deactivated), + }, + status=status.HTTP_204_NO_CONTENT, + ) + class LearningPathCourseEnrollmentView(APIView): """API View to enroll a user in a course that's part of a learning path.""" @@ -399,7 +517,7 @@ def _get_enrolled_learning_path(self, learning_path_key_str: str) -> LearningPat :raises: Http404 if the learning path is not found or the user does not have access. """ return get_object_or_404( - LearningPath.objects.get_paths_visible_to_user(self.request.user).filter(is_enrolled=True), + LearningPath.objects.get_paths_visible_to_user(self.request.user).filter(enrollment_date__isnull=False), key=learning_path_key_str, ) diff --git a/learning_paths/compat.py b/learning_paths/compat.py index adb6902..f514f01 100644 --- a/learning_paths/compat.py +++ b/learning_paths/compat.py @@ -46,18 +46,16 @@ def get_course_keys_with_outlines() -> list[CourseKey]: return course_keys_with_outlines() -def get_course_due_date(course_key: CourseKey) -> datetime | None: - """ - Retrieve course end date. - """ +def get_course_dates(course_key: CourseKey) -> tuple[datetime | None, datetime | None]: + """Retrieve course start and end dates.""" # pylint: disable=import-outside-toplevel, import-error from openedx.core.djangoapps.content.course_overviews.models import CourseOverview try: overview = CourseOverview.objects.get(id=course_key) - return overview.end + return overview.start, overview.end except CourseOverview.DoesNotExist: - return None + return None, None def enroll_user_in_course(user: AbstractBaseUser, course_key: CourseKey) -> bool: diff --git a/learning_paths/conftest.py b/learning_paths/conftest.py index e1a9205..4d6c773 100644 --- a/learning_paths/conftest.py +++ b/learning_paths/conftest.py @@ -1,8 +1,46 @@ """Pytest fixtures.""" +# pylint: disable=redefined-outer-name + import pytest from django.test import override_settings +from learning_paths.tests.factories import ( + LearningPathEnrollmentFactory, + LearningPathFactory, + UserFactory, +) + + +@pytest.fixture +def user(): + """Create a single user for testing.""" + return UserFactory() + + +@pytest.fixture +def learning_path(): + """Create a single learning path for testing.""" + return LearningPathFactory(invite_only=False) + + +@pytest.fixture +def learning_path_with_invite_only(): + """Create a learning path that is invite-only.""" + return LearningPathFactory() + + +@pytest.fixture +def active_enrollment(user, learning_path): + """Create an active enrollment for the user in the learning path.""" + return LearningPathEnrollmentFactory(user=user, learning_path=learning_path, is_active=True) + + +@pytest.fixture +def inactive_enrollment(user, learning_path): + """Create an inactive enrollment for the user in the learning path.""" + return LearningPathEnrollmentFactory(user=user, learning_path=learning_path, is_active=False) + @pytest.fixture def temp_media(tmpdir): diff --git a/learning_paths/migrations/0006_enrollment_models.py b/learning_paths/migrations/0006_enrollment_models.py index 821b3b5..f403650 100644 --- a/learning_paths/migrations/0006_enrollment_models.py +++ b/learning_paths/migrations/0006_enrollment_models.py @@ -2,10 +2,7 @@ from django.conf import settings from django.db import migrations, models -import django.db.models.deletion import django.utils.timezone -import model_utils.fields -import simple_history.models class Migration(migrations.Migration): @@ -27,30 +24,6 @@ class Migration(migrations.Migration): name='is_active', field=models.BooleanField(default=True, help_text='Indicates if the learner is enrolled or not in the Learning Path'), ), - migrations.CreateModel( - name='HistoricalLearningPathEnrollment', - fields=[ - ('id', models.IntegerField(auto_created=True, blank=True, db_index=True, verbose_name='ID')), - ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')), - ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')), - ('is_active', models.BooleanField(default=True, help_text='Indicates if the learner is enrolled or not in the Learning Path')), - ('enrolled_at', models.DateTimeField(blank=True, editable=False, help_text='Timestamp of enrollment or un-enrollment. To be explicitly set when performing a learner enrollment.')), - ('history_id', models.AutoField(primary_key=True, serialize=False)), - ('history_date', models.DateTimeField(db_index=True)), - ('history_change_reason', models.CharField(max_length=100, null=True)), - ('history_type', models.CharField(choices=[('+', 'Created'), ('~', 'Changed'), ('-', 'Deleted')], max_length=1)), - ('history_user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to=settings.AUTH_USER_MODEL)), - ('learning_path', models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='learning_paths.learningpath')), - ('user', models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to=settings.AUTH_USER_MODEL)), - ], - options={ - 'verbose_name': 'historical learning path enrollment', - 'verbose_name_plural': 'historical learning path enrollments', - 'ordering': ('-history_date', '-history_id'), - 'get_latest_by': ('history_date', 'history_id'), - }, - bases=(simple_history.models.HistoricalChanges, models.Model), - ), migrations.CreateModel( name='LearningPathEnrollmentAllowed', fields=[ diff --git a/learning_paths/migrations/0013_enrollment_audit.py b/learning_paths/migrations/0013_enrollment_audit.py new file mode 100644 index 0000000..95e0175 --- /dev/null +++ b/learning_paths/migrations/0013_enrollment_audit.py @@ -0,0 +1,169 @@ +# Generated by Django 4.2.20 on 2025-05-26 16:31 + +from django.conf import settings +from django.db import migrations, models +import django.utils.timezone +import model_utils.fields + + +def delete_historical_model_if_exists(apps, schema_editor): + """Delete HistoricalLearningPathEnrollment model if it exists.""" + try: + HistoricalLearningPathEnrollment = apps.get_model( + "learning_paths", "HistoricalLearningPathEnrollment" + ) + schema_editor.delete_model(HistoricalLearningPathEnrollment) + except LookupError: + # Model doesn't exist in Django's registry, skip deletion + pass + + +def reverse_delete_historical_model(apps, schema_editor): + """Reverse operation - this is a no-op since we cannot recreate the model.""" + pass + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("learning_paths", "0012_alter_learningpath_subtitle"), + ] + + operations = [ + migrations.RunPython( + delete_historical_model_if_exists, reverse_delete_historical_model + ), + migrations.AddField( + model_name="learningpathenrollmentallowed", + name="created", + field=model_utils.fields.AutoCreatedField( + default=django.utils.timezone.now, + editable=False, + verbose_name="created", + ), + ), + migrations.AddField( + model_name="learningpathenrollmentallowed", + name="modified", + field=model_utils.fields.AutoLastModifiedField( + default=django.utils.timezone.now, + editable=False, + verbose_name="modified", + ), + ), + migrations.AlterField( + model_name="learningpathenrollmentallowed", + name="email", + field=models.EmailField(db_index=True, max_length=254), + ), + migrations.CreateModel( + name="LearningPathEnrollmentAudit", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "created", + model_utils.fields.AutoCreatedField( + default=django.utils.timezone.now, + editable=False, + verbose_name="created", + ), + ), + ( + "modified", + model_utils.fields.AutoLastModifiedField( + default=django.utils.timezone.now, + editable=False, + verbose_name="modified", + ), + ), + ( + "state_transition", + models.CharField( + choices=[ + ( + "from unenrolled to allowed to enroll", + "from unenrolled to allowed to enroll", + ), + ( + "from allowed to enroll to enrolled", + "from allowed to enroll to enrolled", + ), + ("from enrolled to enrolled", "from enrolled to enrolled"), + ( + "from enrolled to unenrolled", + "from enrolled to unenrolled", + ), + ( + "from unenrolled to enrolled", + "from unenrolled to enrolled", + ), + ( + "from allowed to enroll to unenrolled", + "from allowed to enroll to unenrolled", + ), + ( + "from unenrolled to unenrolled", + "from unenrolled to unenrolled", + ), + ("N/A", "N/A"), + ], + default="N/A", + max_length=255, + ), + ), + ("reason", models.TextField(blank=True)), + ("org", models.CharField(blank=True, db_index=True, max_length=255)), + ("role", models.CharField(blank=True, max_length=255)), + ( + "enrolled_by", + models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="learning_path_audit", + to=settings.AUTH_USER_MODEL, + ), + ), + ( + "enrollment", + models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="audit", + to="learning_paths.learningpathenrollment", + ), + ), + ( + "enrollment_allowed", + models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="audit", + to="learning_paths.learningpathenrollmentallowed", + ), + ), + ], + options={ + "abstract": False, + }, + ), + migrations.AddField( + model_name="learningpathenrollmentallowed", + name="is_active", + field=models.BooleanField( + db_index=True, default=True, help_text="Indicates if the enrollment allowance is active" + ), + ), + migrations.RemoveField( + model_name="learningpathenrollment", + name="enrolled_at", + ), + ] diff --git a/learning_paths/migrations/0014_remove_learningpath_duration_in_days_and_more.py b/learning_paths/migrations/0014_remove_learningpath_duration_in_days_and_more.py new file mode 100644 index 0000000..a9903fa --- /dev/null +++ b/learning_paths/migrations/0014_remove_learningpath_duration_in_days_and_more.py @@ -0,0 +1,59 @@ +# Generated by Django 4.2.20 on 2025-07-14 13:10 + +from django.db import migrations, models + + +def transfer_duration_data(apps, schema_editor): + """Transfer duration_in_days values to duration field with '{x} days' format.""" + LearningPath = apps.get_model("learning_paths", "LearningPath") + + for learning_path in LearningPath.objects.filter(duration_in_days__isnull=False): + learning_path.duration = f"{learning_path.duration_in_days} days" + learning_path.save() + + +def reverse_transfer_duration_data(apps, schema_editor): + """Reverse operation: extract numeric values from duration field back to duration_in_days.""" + LearningPath = apps.get_model("learning_paths", "LearningPath") + + for learning_path in LearningPath.objects.filter(duration__endswith=" days"): + try: + days_str = learning_path.duration.replace(" days", "") + learning_path.duration_in_days = int(days_str) + learning_path.save() + except ValueError: + # Skip entries that don't match the expected format. + pass + + +class Migration(migrations.Migration): + dependencies = [ + ("learning_paths", "0013_enrollment_audit"), + ] + + operations = [ + migrations.AddField( + model_name="learningpath", + name="duration", + field=models.CharField( + blank=True, + help_text="Approximate time it should take to complete this Learning Path. Example: '10 Weeks'.", + max_length=255, + ), + ), + migrations.AddField( + model_name="learningpath", + name="time_commitment", + field=models.CharField( + blank=True, help_text="Approximate time commitment. Example: '4-6 hours/week'.", max_length=255 + ), + ), + migrations.RunPython( + transfer_duration_data, + reverse_transfer_duration_data, + ), + migrations.RemoveField( + model_name="learningpath", + name="duration_in_days", + ), + ] diff --git a/learning_paths/migrations/0015_make_skill_level_optional.py b/learning_paths/migrations/0015_make_skill_level_optional.py new file mode 100644 index 0000000..d272524 --- /dev/null +++ b/learning_paths/migrations/0015_make_skill_level_optional.py @@ -0,0 +1,27 @@ +# Generated by Django 4.2.23 on 2025-07-22 22:28 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("learning_paths", "0014_remove_learningpath_duration_in_days_and_more"), + ] + + operations = [ + migrations.AlterField( + model_name="acquiredskill", + name="level", + field=models.PositiveIntegerField( + blank=True, help_text="The skill level associated with this course.", null=True + ), + ), + migrations.AlterField( + model_name="requiredskill", + name="level", + field=models.PositiveIntegerField( + blank=True, help_text="The skill level associated with this course.", null=True + ), + ), + ] diff --git a/learning_paths/models.py b/learning_paths/models.py index d735929..d644f86 100644 --- a/learning_paths/models.py +++ b/learning_paths/models.py @@ -5,22 +5,21 @@ import logging import os import uuid -from datetime import datetime, timedelta +from datetime import datetime from uuid import uuid4 from django.contrib import auth from django.core.exceptions import ValidationError from django.core.validators import MaxValueValidator, MinValueValidator from django.db import models -from django.db.models import Exists, OuterRef, Q +from django.db.models import OuterRef, Q from django.utils.translation import gettext_lazy as _ from model_utils import FieldTracker from model_utils.models import TimeStampedModel from opaque_keys.edx.django.models import CourseKeyField -from simple_history.models import HistoricalRecords from slugify import slugify -from .compat import get_course_due_date, get_user_course_grade +from .compat import get_course_dates, get_user_course_grade from .keys import LearningPathKeyField log = logging.getLogger(__name__) @@ -39,27 +38,29 @@ class LearningPathManager(models.Manager): def get_paths_visible_to_user(self, user: User) -> models.QuerySet: """ - Return only learning paths that should be visible to the given user with enrollment status. + Return only learning paths that should be visible to the given user with an enrollment date. For staff users: all learning paths. For non-staff: non-invite-only paths or invite-only paths they're enrolled in. - Each learning path in the queryset is annotated with `is_enrolled` indicating - whether the user has an active enrollment in that learning path. + Each learning path in the queryset is annotated with `enrollment_date` indicating + the date when the user enrolled in that learning path (None if not enrolled). + Results are ordered by enrollment date (the most recent first), with non-enrolled paths at the end. """ queryset = self.get_queryset() - # Annotate each path with whether the user is enrolled. - enrollment_exists = LearningPathEnrollment.objects.filter( + # Annotate each path with the enrollment date. + enrollment_subquery = LearningPathEnrollment.objects.filter( learning_path=OuterRef("pk"), user=user, is_active=True - ) - queryset = queryset.annotate(is_enrolled=Exists(enrollment_exists)) + ).values("created")[:1] + queryset = queryset.annotate(enrollment_date=models.Subquery(enrollment_subquery)) # Apply visibility filtering based on the user role. if not user.is_staff: - queryset = queryset.filter(Q(invite_only=False) | Q(is_enrolled=True)) + queryset = queryset.filter(Q(invite_only=False) | Q(enrollment_date__isnull=False)) - return queryset + # Order by enrollment date (the most recent first), with null values at the end. + return queryset.order_by(models.F("enrollment_date").desc(nulls_last=True)) class LearningPath(TimeStampedModel): @@ -103,18 +104,22 @@ def _learning_path_image_upload_path(self, filename: str) -> str: subtitle = models.TextField(blank=True) description = models.TextField(blank=True) image = models.ImageField( - upload_to=_learning_path_image_upload_path, + upload_to=_learning_path_image_upload_path, # type: ignore blank=True, null=True, verbose_name=_("Image"), help_text=_("Image representing this Learning Path."), ) level = models.CharField(max_length=255, blank=True, choices=LEVEL_CHOICES) - duration_in_days = models.PositiveIntegerField( + duration = models.CharField( + max_length=255, blank=True, - null=True, - verbose_name=_("Duration (days)"), - help_text=_("Approximate time (in days) it should take to complete this Learning Path."), + help_text=_("Approximate time it should take to complete this Learning Path. Example: '10 Weeks'."), + ) + time_commitment = models.CharField( + max_length=255, + blank=True, + help_text=_("Approximate time commitment. Example: '4-6 hours/week'."), ) sequential = models.BooleanField( default=False, @@ -135,6 +140,11 @@ def _learning_path_image_upload_path(self, filename: str) -> str: objects = LearningPathManager() + steps: "models.Manager[LearningPathStep]" + requiredskill_set: "models.Manager[RequiredSkill]" + acquiredskill_set: "models.Manager[AcquiredSkill]" + grading_criteria: "LearningPathGradingCriteria" + def __str__(self): """User-friendly string representation of this model.""" return str(self.key) @@ -204,9 +214,9 @@ class Meta: ) @property - def due_date(self) -> datetime | None: + def course_dates(self) -> tuple[datetime | None, datetime | None]: """Retrieve the due date for this course.""" - return get_course_due_date(self.course_key) + return get_course_dates(self.course_key) def __str__(self): """User-friendly string representation of this model.""" @@ -249,7 +259,11 @@ class Meta: learning_path = models.ForeignKey(LearningPath, on_delete=models.CASCADE) skill = models.ForeignKey(Skill, on_delete=models.CASCADE) - level = models.PositiveIntegerField(help_text=_("The skill level associated with this course.")) + level = models.PositiveIntegerField( + blank=True, + null=True, + help_text=_("The skill level associated with this course."), + ) def __str__(self): """User-friendly string representation of this model.""" @@ -290,26 +304,12 @@ class Meta: default=True, help_text=_("Indicates if the learner is enrolled or not in the Learning Path"), ) - enrolled_at = models.DateTimeField( - auto_now_add=True, - help_text=_( - "Timestamp of enrollment or un-enrollment. To be explicitly set when performing a learner enrollment." - ), - ) - - history = HistoricalRecords() + tracker = FieldTracker(fields=["is_active"]) def __str__(self): """User-friendly string representation of this model.""" return "{}: {}".format(self.user, self.learning_path) - @property - def estimated_end_date(self): - """Estimated end date of the learning path.""" - if self.learning_path.duration_in_days is None: - return None - return self.created + timedelta(days=self.learning_path.duration_in_days) - class LearningPathGradingCriteria(models.Model): """ @@ -354,7 +354,7 @@ def calculate_grade(self, user): return weighted_sum / total_weight if total_weight > 0 else 0.0 -class LearningPathEnrollmentAllowed(models.Model): +class LearningPathEnrollmentAllowed(TimeStampedModel): """ Represents an allowed enrollment in a learning path for a user email. @@ -371,10 +371,76 @@ class Meta: unique_together = ("email", "learning_path") - email = models.EmailField() + email = models.EmailField(db_index=True) learning_path = models.ForeignKey(LearningPath, on_delete=models.CASCADE) user = models.ForeignKey(User, on_delete=models.CASCADE, blank=True, null=True) + is_active = models.BooleanField( + default=True, + db_index=True, + help_text=_("Indicates if the enrollment allowance is active"), + ) def __str__(self): """User-friendly string representation of this model.""" - return f"LearningPathEnrollmentAllowed for {self.user.username} in {self.learning_path.display_name}" + return f"LearningPathEnrollmentAllowed for {self.email} in {self.learning_path.key}" + + +class LearningPathEnrollmentAudit(TimeStampedModel): + """ + Audit model for tracking changes to learning path enrollments. + + .. no_pii: + """ + + # State transition constants (copied from edx-platform to maintain consistency) + UNENROLLED_TO_ALLOWEDTOENROLL = "from unenrolled to allowed to enroll" + ALLOWEDTOENROLL_TO_ENROLLED = "from allowed to enroll to enrolled" + ENROLLED_TO_ENROLLED = "from enrolled to enrolled" + ENROLLED_TO_UNENROLLED = "from enrolled to unenrolled" + UNENROLLED_TO_ENROLLED = "from unenrolled to enrolled" + ALLOWEDTOENROLL_TO_UNENROLLED = "from allowed to enroll to unenrolled" + UNENROLLED_TO_UNENROLLED = "from unenrolled to unenrolled" + DEFAULT_TRANSITION_STATE = "N/A" + + TRANSITION_STATES = ( + (UNENROLLED_TO_ALLOWEDTOENROLL, UNENROLLED_TO_ALLOWEDTOENROLL), + (ALLOWEDTOENROLL_TO_ENROLLED, ALLOWEDTOENROLL_TO_ENROLLED), + (ENROLLED_TO_ENROLLED, ENROLLED_TO_ENROLLED), + (ENROLLED_TO_UNENROLLED, ENROLLED_TO_UNENROLLED), + (UNENROLLED_TO_ENROLLED, UNENROLLED_TO_ENROLLED), + (ALLOWEDTOENROLL_TO_UNENROLLED, ALLOWEDTOENROLL_TO_UNENROLLED), + (UNENROLLED_TO_UNENROLLED, UNENROLLED_TO_UNENROLLED), + (DEFAULT_TRANSITION_STATE, DEFAULT_TRANSITION_STATE), + ) + + enrolled_by = models.ForeignKey(User, on_delete=models.CASCADE, null=True, related_name="learning_path_audit") + enrollment = models.ForeignKey( + LearningPathEnrollment, + on_delete=models.CASCADE, + null=True, + related_name="audit", + ) + enrollment_allowed = models.ForeignKey( + LearningPathEnrollmentAllowed, + on_delete=models.CASCADE, + null=True, + related_name="audit", + ) + state_transition = models.CharField(max_length=255, choices=TRANSITION_STATES, default=DEFAULT_TRANSITION_STATE) + reason = models.TextField(blank=True) + org = models.CharField(max_length=255, blank=True, db_index=True) + role = models.CharField(max_length=255, blank=True) + + def __str__(self): + """User-friendly string representation of this model.""" + enrollee = "unknown" + learning_path = "unknown" + + if self.enrollment: + enrollee = self.enrollment.user + learning_path = self.enrollment.learning_path.key + elif self.enrollment_allowed: + enrollee = self.enrollment_allowed.user or self.enrollment_allowed.email + learning_path = self.enrollment_allowed.learning_path.key + + return f"{self.state_transition} for {enrollee} in {learning_path}" diff --git a/learning_paths/receivers.py b/learning_paths/receivers.py index 841ae9b..8fa662e 100644 --- a/learning_paths/receivers.py +++ b/learning_paths/receivers.py @@ -1,13 +1,23 @@ """Django signal handler for learning paths plugin.""" +# pylint: disable=unused-argument + import logging -from learning_paths.models import LearningPathEnrollment, LearningPathEnrollmentAllowed +from django.db import IntegrityError +from django.db.models.signals import post_save +from django.dispatch import receiver + +from learning_paths.models import ( + LearningPathEnrollment, + LearningPathEnrollmentAllowed, + LearningPathEnrollmentAudit, +) logger = logging.getLogger(__name__) -def process_pending_enrollments(sender, instance, created, **kwargs): # pylint: disable=unused-argument +def process_pending_enrollments(sender, instance, created, **kwargs): """ Process pending enrollments after a user instance has been created. @@ -29,18 +39,101 @@ def process_pending_enrollments(sender, instance, created, **kwargs): # pylint: return logger.info("[LearningPaths] Processing pending enrollments for user %s", instance) - pending_enrollments = LearningPathEnrollmentAllowed.objects.filter(email=instance.email).all() - - enrollments = [] + pending_enrollments = LearningPathEnrollmentAllowed.objects.filter(email=instance.email, is_active=True) + enrollments_created = 0 for entry in pending_enrollments: - entry.user = instance - entry.save() + try: + audit_data = { + "enrolled_by": instance, + "state_transition": LearningPathEnrollmentAudit.ALLOWEDTOENROLL_TO_ENROLLED, + } + if last_allowed_audit := entry.audit.order_by("-created").first(): + for field in ["reason", "org", "role"]: + audit_data[field] = getattr(last_allowed_audit, field, "") + + enrollment = LearningPathEnrollment(learning_path=entry.learning_path, user=instance) + enrollment._audit = audit_data # pylint: disable=protected-access + enrollment.save() + enrollments_created += 1 + + # Link existing audits from the "allowed to enroll" entry to the new enrollment. + entry.audit.update(enrollment=enrollment) + + except IntegrityError: # pragma: no cover + logger.info( + "[LearningPaths] Enrollment already exists for user %s in the learning path %s", + instance, + entry.learning_path.key, + ) + finally: + entry.is_active = False + entry.user = instance + entry.save() - enrollments.append(LearningPathEnrollment(learning_path=entry.learning_path, user=instance)) - new_enrollments = LearningPathEnrollment.objects.bulk_create(enrollments) logger.info( "[LearningPaths] Processed %d pending Learning Path enrollments for user %s.", + enrollments_created, instance, - len(new_enrollments), ) + + +def _create_enrollment_audit(instance: LearningPathEnrollment | LearningPathEnrollmentAllowed, audit_data: dict): + """Create an audit record for the given instance with the provided audit data.""" + # If a previous audit exists, copy over missing fields + previous_audit = instance.audit.order_by("-created").first() + if previous_audit: + for field in ["reason", "org", "role"]: + if not audit_data.get(field): + audit_data[field] = getattr(previous_audit, field) + + instance.audit.create( + state_transition=audit_data.get("state_transition"), + enrolled_by=audit_data.get("enrolled_by"), + reason=audit_data.get("reason", ""), + org=audit_data.get("org", ""), + role=audit_data.get("role", ""), + ) + + +@receiver(post_save, sender=LearningPathEnrollment) +def create_enrollment_audit(sender, instance, created, **kwargs): + """Create audit records when LearningPathEnrollment is saved.""" + audit_data = getattr(instance, "_audit", {}) + + # Determine state transition if not provided + if "state_transition" not in audit_data: + if created: + audit_data["state_transition"] = LearningPathEnrollmentAudit.UNENROLLED_TO_ENROLLED + elif instance.is_active and not instance.tracker.previous("is_active"): + audit_data["state_transition"] = LearningPathEnrollmentAudit.UNENROLLED_TO_ENROLLED + elif not instance.is_active and instance.tracker.previous("is_active"): + audit_data["state_transition"] = LearningPathEnrollmentAudit.ENROLLED_TO_UNENROLLED + elif instance.is_active and instance.tracker.previous("is_active"): + audit_data["state_transition"] = LearningPathEnrollmentAudit.ENROLLED_TO_ENROLLED + elif not instance.is_active and not instance.tracker.previous("is_active"): + audit_data["state_transition"] = LearningPathEnrollmentAudit.UNENROLLED_TO_UNENROLLED + else: # pragma: no cover + # No relevant state change. This should not happen. + audit_data["state_transition"] = LearningPathEnrollmentAudit.DEFAULT_TRANSITION_STATE + + _create_enrollment_audit(instance, audit_data) + + +@receiver(post_save, sender=LearningPathEnrollmentAllowed) +def create_enrollment_allowed_audit(sender, instance, created, **kwargs): + """Create audit records when LearningPathEnrollmentAllowed is saved.""" + # The audit data can be missing in the following scenarios: + # 1. The instance is created with `get_or_create`, so we want to provide this data later. + # 2. The instance is updated when the user creates an account. In this case, the audit record is already created for + # the enrollment record, so we do not need to create it here. + if not (audit_data := getattr(instance, "_audit", {})): + return + + audit_data.setdefault("state_transition", LearningPathEnrollmentAudit.UNENROLLED_TO_ALLOWEDTOENROLL) + + audit_data["state_transition"] = audit_data.get( + "state_transition", LearningPathEnrollmentAudit.UNENROLLED_TO_ALLOWEDTOENROLL + ) + + _create_enrollment_audit(instance, audit_data) diff --git a/learning_paths/tests/factories.py b/learning_paths/tests/factories.py index 67d606e..951d8c6 100644 --- a/learning_paths/tests/factories.py +++ b/learning_paths/tests/factories.py @@ -1,6 +1,4 @@ # pylint: disable=missing-module-docstring,missing-class-docstring -from datetime import datetime, timezone - import factory from django.contrib import auth from factory.fuzzy import FuzzyText @@ -11,6 +9,7 @@ LearningPath, LearningPathEnrollment, LearningPathEnrollmentAllowed, + LearningPathEnrollmentAudit, LearningPathGradingCriteria, LearningPathStep, RequiredSkill, @@ -92,7 +91,22 @@ class Meta: level = factory.Faker("random_int", min=1, max=5) -class LearningPathEnrollmentFactory(factory.django.DjangoModelFactory): +class AuditAttributeMixin: + """Mixin for factory classes that need to handle the _audit attribute before saving.""" + + @classmethod + def _create(cls, model_class, *args, **kwargs): + """A custom create method to handle _audit attribute before the first save.""" + audit_data = kwargs.pop("_audit", None) + instance = model_class(*args, **kwargs) + if audit_data is not None: + instance._audit = audit_data # pylint: disable=protected-access + + instance.save() + return instance + + +class LearningPathEnrollmentFactory(AuditAttributeMixin, factory.django.DjangoModelFactory): """ Factory for LearningPathEnrollment model. """ @@ -100,13 +114,12 @@ class LearningPathEnrollmentFactory(factory.django.DjangoModelFactory): user = factory.SubFactory(UserFactory) learning_path = factory.SubFactory(LearningPathFactory) is_active = True - enrolled_at = factory.LazyFunction(lambda: datetime.now(timezone.utc)) class Meta: model = LearningPathEnrollment -class LearningPathEnrollmentAllowedFactory(factory.django.DjangoModelFactory): +class LearningPathEnrollmentAllowedFactory(AuditAttributeMixin, factory.django.DjangoModelFactory): """ Factory for LearningPathEnrollmentAllowed model. """ @@ -114,6 +127,24 @@ class LearningPathEnrollmentAllowedFactory(factory.django.DjangoModelFactory): email = factory.Faker("email") learning_path = factory.SubFactory(LearningPathFactory) user = None + is_active = True class Meta: model = LearningPathEnrollmentAllowed + + +class LearningPathEnrollmentAuditFactory(factory.django.DjangoModelFactory): + """ + Factory for LearningPathEnrollmentAudit model. + """ + + enrolled_by = factory.SubFactory(UserFactory) + enrollment = None + enrollment_allowed = None + state_transition = LearningPathEnrollmentAudit.DEFAULT_TRANSITION_STATE + reason = factory.Faker("sentence") + org = factory.Faker("company") + role = factory.Faker("job") + + class Meta: + model = LearningPathEnrollmentAudit diff --git a/learning_paths/tests/test_models.py b/learning_paths/tests/test_models.py index 6152b30..6a789bd 100644 --- a/learning_paths/tests/test_models.py +++ b/learning_paths/tests/test_models.py @@ -13,7 +13,16 @@ from slugify import slugify from learning_paths.keys import LearningPathKey -from learning_paths.models import LearningPath +from learning_paths.models import ( + LearningPath, + LearningPathEnrollmentAllowed, + LearningPathEnrollmentAudit, +) + +from .factories import ( + LearningPathEnrollmentAllowedFactory, + LearningPathEnrollmentAuditFactory, +) @pytest.fixture @@ -31,7 +40,8 @@ def learning_path(learning_path_key): subtitle="Test Subtitle", description="Test description", level="intermediate", - duration_in_days=30, + duration="30 days", + time_commitment="4-6 hours/week", sequential=True, ) @@ -134,3 +144,63 @@ def test_grading_criteria_auto_creation(self, learning_path): assert criteria is not None assert criteria.required_completion == 0.80 assert criteria.required_grade == 0.75 + + +@pytest.mark.django_db +class TestLearningPathEnrollmentAllowed: + """Tests for the LearningPathEnrollmentAllowed model.""" + + @pytest.mark.parametrize("user_provided", [True, False]) + def test_string_representation(self, user, learning_path, user_provided): + """Test the string representation.""" + user_kwarg = {"user": user} if user_provided else {} + allowed = LearningPathEnrollmentAllowedFactory(email=user.email, learning_path=learning_path, **user_kwarg) + expected_str = f"LearningPathEnrollmentAllowed for {user.email} in {learning_path.key}" + assert str(allowed) == expected_str + + def test_is_active_defaults_to_true(self, learning_path): + """Test that is_active field defaults to True.""" + allowed = LearningPathEnrollmentAllowed(email="test@example.com", learning_path=learning_path) + assert allowed.is_active is True + + +@pytest.mark.django_db +class TestLearningPathEnrollmentAudit: + """Tests for the LearningPathEnrollmentAudit model.""" + + def test_string_representation_with_enrollment(self, user, learning_path, active_enrollment): + """Test the string representation when linked to a LearningPathEnrollment.""" + audit = active_enrollment.audit.get() + expected_str = ( + f"{LearningPathEnrollmentAudit.UNENROLLED_TO_ENROLLED} for {user.username} in {learning_path.key}" + ) + assert str(audit) == expected_str + + def test_string_representation_with_enrollment_allowed_and_user(self, user, learning_path): + """Test the string representation when linked to LearningPathEnrollmentAllowed with a user.""" + enrollment_allowed = LearningPathEnrollmentAllowedFactory( + user=user, email=user.email, learning_path=learning_path + ) + enrollment_allowed._audit = {"reason": "TestReason"} + enrollment_allowed.save() + audit = enrollment_allowed.audit.get() + expected_str = ( + f"{LearningPathEnrollmentAudit.UNENROLLED_TO_ALLOWEDTOENROLL} for {user.username} in {learning_path.key}" + ) + assert str(audit) == expected_str + + def test_string_representation_with_enrollment_allowed_no_user(self, learning_path): + """Test the string representation when linked to LearningPathEnrollmentAllowed without a user (email only).""" + email = "new_user@example.com" + enrollment_allowed = LearningPathEnrollmentAllowedFactory(email=email, learning_path=learning_path) + enrollment_allowed._audit = {"reason": "TestReason"} + enrollment_allowed.save() + audit = enrollment_allowed.audit.get() + expected_str = f"{LearningPathEnrollmentAudit.UNENROLLED_TO_ALLOWEDTOENROLL} for {email} in {learning_path.key}" + assert str(audit) == expected_str + + def test_string_representation_no_enrollment_or_allowed(self): + """Test the string representation when no enrollment or enrollment_allowed is linked.""" + audit = LearningPathEnrollmentAuditFactory() + expected_str = f"{LearningPathEnrollmentAudit.DEFAULT_TRANSITION_STATE} for unknown in unknown" + assert str(audit) == expected_str diff --git a/learning_paths/tests/test_receivers.py b/learning_paths/tests/test_receivers.py index c5a65f6..f81a2a6 100644 --- a/learning_paths/tests/test_receivers.py +++ b/learning_paths/tests/test_receivers.py @@ -4,11 +4,16 @@ import pytest from django.contrib.auth import get_user_model -from learning_paths.models import LearningPathEnrollment, LearningPathEnrollmentAllowed +from learning_paths.models import ( + LearningPathEnrollment, + LearningPathEnrollmentAllowed, + LearningPathEnrollmentAudit, +) from learning_paths.receivers import process_pending_enrollments from .factories import ( LearningPathEnrollmentAllowedFactory, + LearningPathEnrollmentFactory, LearningPathFactory, UserFactory, ) @@ -34,9 +39,17 @@ def test_process_pending_enrollments_with_pending_enrollments(user_email, learni GIVEN that there are LearningPathEnrollmentAllowed objects for an email WHEN the process_pending_enrollments signal handler is triggered THEN actual enrollment objects are created for the user + AND audit records are created for the new enrollments + AND audit data is preserved from the last "allowed to enroll" audit + AND existing audits are linked to the new enrollment + AND enrollment allowed records are deactivated and linked to the user """ - pending_entry_1 = LearningPathEnrollmentAllowedFactory(email=user_email, learning_path=learning_paths[0]) + initial_audit_payload = {"role": "TestRole"} + pending_entry_1 = LearningPathEnrollmentAllowedFactory( + email=user_email, learning_path=learning_paths[0], _audit=initial_audit_payload + ) pending_entry_2 = LearningPathEnrollmentAllowedFactory(email=user_email, learning_path=learning_paths[1]) + pending_entry_2.audit.all().delete() # Test that the enrollment can be created without an allowed audit. user = UserFactory(email=user_email) process_pending_enrollments(sender=User, instance=user, created=True) @@ -45,6 +58,8 @@ def test_process_pending_enrollments_with_pending_enrollments(user_email, learni pending_entry_2.refresh_from_db() assert pending_entry_1.user == user assert pending_entry_2.user == user + assert pending_entry_1.is_active is False + assert pending_entry_2.is_active is False enrollments = LearningPathEnrollment.objects.all() assert len(enrollments) == 2 @@ -52,6 +67,51 @@ def test_process_pending_enrollments_with_pending_enrollments(user_email, learni assert enrollments[0].learning_path == pending_entry_1.learning_path assert enrollments[1].learning_path == pending_entry_2.learning_path + assert LearningPathEnrollmentAudit.objects.count() == 3 + for enrollment in enrollments: + audit = enrollment.audit.last() + assert audit.state_transition == LearningPathEnrollmentAudit.ALLOWEDTOENROLL_TO_ENROLLED + assert audit.enrolled_by == user + assert audit.reason == "" + assert audit.org == "" + + assert enrollments[0].audit.count() == 2 + assert enrollments[0].audit.first().enrollment_allowed == pending_entry_1 + assert enrollments[0].audit.last().role == initial_audit_payload["role"] + + assert enrollments[1].audit.count() == 1 + assert enrollments[1].audit.get().role == "" + + +@pytest.mark.django_db +def test_process_pending_enrollments_only_processes_active_records(user_email, learning_paths): + """ + GIVEN that there are both active and inactive LearningPathEnrollmentAllowed objects for an email + WHEN the process_pending_enrollments signal handler is triggered + THEN only active enrollment allowed records are processed + """ + active_entry = LearningPathEnrollmentAllowedFactory( + email=user_email, learning_path=learning_paths[0], is_active=True + ) + inactive_entry = LearningPathEnrollmentAllowedFactory( + email=user_email, learning_path=learning_paths[1], is_active=False + ) + + user = UserFactory(email=user_email) + process_pending_enrollments(sender=User, instance=user, created=True) + + enrollments = LearningPathEnrollment.objects.all() + assert len(enrollments) == 1 + assert enrollments[0].learning_path == active_entry.learning_path + + active_entry.refresh_from_db() + assert active_entry.is_active is False + assert active_entry.user == user + + inactive_entry.refresh_from_db() + assert inactive_entry.is_active is False + assert inactive_entry.user is None + @pytest.mark.django_db def test_process_pending_enrollments_when_no_pending_enrollments(user_email): @@ -84,3 +144,140 @@ def test_process_pending_enrollments_when_not_created(user_email): enrollments = LearningPathEnrollment.objects.all() assert len(enrollments) == 0 + + +# pylint: disable=protected-access +@pytest.mark.django_db +class TestEnrollmentAuditReceivers: + """Tests for the audit record creation signals.""" + + @pytest.mark.parametrize( + "expected_state,expected_values", + [ + ( + LearningPathEnrollmentAudit.UNENROLLED_TO_ENROLLED, + {"enrolled_by": "user", "reason": "TestReason", "org": "TestOrg", "role": "Tester"}, + ), + ( + LearningPathEnrollmentAudit.UNENROLLED_TO_ENROLLED, + {"enrolled_by": None, "reason": "", "org": "", "role": ""}, + ), + ], + ids=["with_audit_data", "without_audit_data"], + ) + def test_create_enrollment_audit_new_enrollment(self, user, learning_path, expected_state, expected_values): + """Test audit creation for a new enrollment with or without explicit audit data.""" + if expected_values["reason"]: + expected_values["enrolled_by"] = user + enrollment = LearningPathEnrollmentFactory.create( + user=user, learning_path=learning_path, _audit=expected_values + ) + else: + enrollment = LearningPathEnrollmentFactory.create(user=user, learning_path=learning_path) + + audit = enrollment.audit.get() + assert audit.state_transition == expected_state + for field, value in expected_values.items(): + assert getattr(audit, field) == value + + @pytest.mark.parametrize( + "initial_state,new_state,expected_transition,audit_data", + [ + (False, True, LearningPathEnrollmentAudit.UNENROLLED_TO_ENROLLED, {"reason": "Reactivation"}), + (True, False, LearningPathEnrollmentAudit.ENROLLED_TO_UNENROLLED, {"reason": "Deactivation"}), + (True, True, LearningPathEnrollmentAudit.ENROLLED_TO_ENROLLED, {"reason": "Updated reason"}), + (False, False, LearningPathEnrollmentAudit.UNENROLLED_TO_UNENROLLED, {"reason": "Still inactive"}), + ], + ids=["reactivate", "deactivate", "no_state_change_active", "no_state_change_inactive"], + ) + def test_create_enrollment_audit_state_changes( # pylint: disable=too-many-positional-arguments + self, user, learning_path, initial_state, new_state, expected_transition, audit_data + ): + """Test audit creation when the enrollment state changes.""" + enrollment = LearningPathEnrollmentFactory.create( + user=user, learning_path=learning_path, is_active=initial_state + ) + + enrollment.is_active = new_state + enrollment._audit = {"enrolled_by": user, **audit_data} + enrollment.save() + + assert enrollment.audit.count() == 2 + audit = enrollment.audit.last() + assert audit.state_transition == expected_transition + assert audit.enrolled_by == user + assert audit.reason == audit_data["reason"] + + @pytest.mark.parametrize( + "updated_audit,expected_values", + [ + ( + {}, + {"reason": "TestReason", "org": "TestOrg", "role": "TestRole"}, + ), + ( + {"reason": "", "org": "", "role": ""}, + {"reason": "TestReason", "org": "TestOrg", "role": "TestRole"}, + ), + ( + {"reason": "New Reason", "org": "New Org", "role": "New Role"}, + {"reason": "New Reason", "org": "New Org", "role": "New Role"}, + ), + ], + ids=["preserve_metadata", "preserve_over_empty_strings", "use_provided_metadata"], + ) + def test_create_enrollment_audit_metadata_handling(self, user, learning_path, updated_audit, expected_values): + """Test how metadata is preserved or updated in audit records.""" + initial_audit_payload = {"enrolled_by": user, "reason": "TestReason", "org": "TestOrg", "role": "TestRole"} + enrollment = LearningPathEnrollmentFactory.create( + user=user, learning_path=learning_path, _audit=initial_audit_payload + ) + + another_user = UserFactory() + enrollment.is_active = False + enrollment._audit = {"enrolled_by": another_user, **updated_audit} + enrollment.save() + + assert enrollment.audit.count() == 2 + latest_audit = enrollment.audit.last() + + assert latest_audit.state_transition == LearningPathEnrollmentAudit.ENROLLED_TO_UNENROLLED + assert latest_audit.enrolled_by == another_user + for field, value in expected_values.items(): + assert getattr(latest_audit, field) == value + + def test_create_enrollment_allowed_audit_with_audit_data(self, user, learning_path): + """Test audit creation for a new LearningPathEnrollmentAllowed with explicit audit data.""" + audit_payload = {"enrolled_by": user, "reason": "TestReason", "org": "TestOrg", "role": "TestRole"} + enrollment_allowed = LearningPathEnrollmentAllowedFactory.create( + email="testallow@example.com", learning_path=learning_path, _audit=audit_payload + ) + audit = enrollment_allowed.audit.get() + assert audit.state_transition == LearningPathEnrollmentAudit.UNENROLLED_TO_ALLOWEDTOENROLL + for field, value in audit_payload.items(): + assert getattr(audit, field) == value + + def test_create_enrollment_allowed_audit_without_audit_data_is_skipped(self, learning_path, user_email): + """Test that no audit is created for LearningPathEnrollmentAllowed if _audit is not present.""" + enrollment_allowed = LearningPathEnrollmentAllowedFactory.create(email=user_email, learning_path=learning_path) + assert not enrollment_allowed.audit.exists() + + def test_create_enrollment_allowed_audit_metadata_handling(self, user, learning_path, user_email): + """Test that LearningPathEnrollmentAllowed audit records handle metadata correctly.""" + initial_payload = {"enrolled_by": user, "reason": "TestReason", "org": "TestOrg"} + enrollment_allowed = LearningPathEnrollmentAllowedFactory.create( + email=user_email, learning_path=learning_path, _audit=initial_payload + ) + + another_user = UserFactory() + enrollment_allowed._audit = {"enrolled_by": another_user, "reason": "", "role": "TestRole"} + enrollment_allowed.save() + + assert enrollment_allowed.audit.count() == 2 + latest_audit = enrollment_allowed.audit.last() + + assert latest_audit.enrolled_by == another_user + assert latest_audit.reason == initial_payload["reason"] + assert latest_audit.org == initial_payload["org"] + assert latest_audit.role == "TestRole" + assert latest_audit.state_transition == LearningPathEnrollmentAudit.UNENROLLED_TO_ALLOWEDTOENROLL diff --git a/requirements/base.in b/requirements/base.in index e3f62e5..c0163a4 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -4,10 +4,10 @@ Django # Web application framework django-model-utils +django-object-actions # Add action buttons to Django admin djangorestframework edx-django-utils edx-opaque-keys openedx-atlas openedx-completion-aggregator # Required for fetching course completion -django-simple-history pillow # Required for the ImageField diff --git a/requirements/base.txt b/requirements/base.txt index 99b337e..f7d75c5 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -53,7 +53,7 @@ cryptography==45.0.5 # pyjwt django==4.2.23 # via - # -c requirements/common_constraints.txt + # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/base.in # django-config-models # django-crum @@ -88,12 +88,10 @@ django-model-utils==5.0.0 # edx-celeryutils # edx-completion # openedx-completion-aggregator +django-object-actions==5.0.0 + # via -r requirements/base.in django-redis==6.0.0 # via edx-event-routing-backends -django-simple-history==3.4.0 - # via - # -c requirements/constraints.txt - # -r requirements/base.in django-waffle==5.0.0 # via # edx-django-utils @@ -261,7 +259,7 @@ tzdata==2025.2 # via kombu urllib3==2.2.3 # via - # -c requirements/common_constraints.txt + # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # requests vine==5.1.0 # via diff --git a/requirements/common_constraints.txt b/requirements/common_constraints.txt deleted file mode 100644 index bc3ba09..0000000 --- a/requirements/common_constraints.txt +++ /dev/null @@ -1,31 +0,0 @@ -# A central location for most common version constraints -# (across edx repos) for pip-installation. -# -# Similar to other constraint files this file doesn't install any packages. -# It specifies version constraints that will be applied if a package is needed. -# When pinning something here, please provide an explanation of why it is a good -# idea to pin this package across all edx repos, Ideally, link to other information -# that will help people in the future to remove the pin when possible. -# Writing an issue against the offending project and linking to it here is good. -# -# Note: Changes to this file will automatically be used by other repos, referencing -# this file from Github directly. It does not require packaging in edx-lint. - -# using LTS django version -Django<5.0 - -# elasticsearch>=7.14.0 includes breaking changes in it which caused issues in discovery upgrade process. -# elastic search changelog: https://www.elastic.co/guide/en/enterprise-search/master/release-notes-7.14.0.html -# See https://github.com/openedx/edx-platform/issues/35126 for more info -elasticsearch<7.14.0 - -# django-simple-history>3.0.0 adds indexing and causes a lot of migrations to be affected -# django-simple-history==3.0.0 - -# Cause: https://github.com/openedx/edx-lint/issues/458 -# This can be unpinned once https://github.com/openedx/edx-lint/issues/459 has been resolved. -pip<24.3 - -# Cause: https://github.com/openedx/edx-lint/issues/475 -# This can be unpinned once https://github.com/openedx/edx-lint/issues/476 has been resolved. -urllib3<2.3.0 diff --git a/requirements/constraints.txt b/requirements/constraints.txt index dd8c899..d91704b 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -9,9 +9,4 @@ # linking to it here is good. # Common constraints for edx repos --c common_constraints.txt - -# django-simple-history has been pinned on the edx-platform to version 3.4.0 -# since the platform was updated to Django 4.2 -# Ref: https://github.com/openedx/edx-platform/commit/e40a01c7ccfcc853e5be8cc25bdaa0d14248a270#diff-86d5fe588ff2fc7dccb1f4cdd8019d4473146536e88d7a9ede946ea962a91acb -django-simple-history==3.4.0 +-c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt diff --git a/requirements/dev.txt b/requirements/dev.txt index e1a69d9..937573a 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -128,7 +128,7 @@ distlib==0.3.9 # virtualenv django==4.2.23 # via - # -c requirements/common_constraints.txt + # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/quality.txt # django-config-models # django-crum @@ -169,14 +169,12 @@ django-model-utils==5.0.0 # edx-celeryutils # edx-completion # openedx-completion-aggregator +django-object-actions==5.0.0 + # via -r requirements/quality.txt django-redis==6.0.0 # via # -r requirements/quality.txt # edx-event-routing-backends -django-simple-history==3.4.0 - # via - # -c requirements/constraints.txt - # -r requirements/quality.txt django-waffle==5.0.0 # via # -r requirements/quality.txt @@ -548,7 +546,7 @@ tzdata==2025.2 # kombu urllib3==2.2.3 # via - # -c requirements/common_constraints.txt + # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/quality.txt # requests vine==5.1.0 diff --git a/requirements/doc.txt b/requirements/doc.txt index bdfeec1..65f7423 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -102,7 +102,7 @@ cryptography==45.0.5 # secretstorage django==4.2.23 # via - # -c requirements/common_constraints.txt + # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/test.txt # django-config-models # django-crum @@ -142,14 +142,12 @@ django-model-utils==5.0.0 # edx-celeryutils # edx-completion # openedx-completion-aggregator +django-object-actions==5.0.0 + # via -r requirements/test.txt django-redis==6.0.0 # via # -r requirements/test.txt # edx-event-routing-backends -django-simple-history==3.4.0 - # via - # -c requirements/constraints.txt - # -r requirements/test.txt django-waffle==5.0.0 # via # -r requirements/test.txt @@ -530,7 +528,7 @@ tzdata==2025.2 # kombu urllib3==2.2.3 # via - # -c requirements/common_constraints.txt + # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/test.txt # requests # twine diff --git a/requirements/pip.txt b/requirements/pip.txt index 08a4e26..e109db2 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -10,7 +10,7 @@ wheel==0.45.1 # The following packages are considered to be unsafe in a requirements file: pip==24.2 # via - # -c /home/runner/work/learning-paths-plugin/learning-paths-plugin/requirements/common_constraints.txt + # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/pip.in setuptools==80.9.0 # via -r requirements/pip.in diff --git a/requirements/quality.txt b/requirements/quality.txt index 2f350c8..a977aa6 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -98,7 +98,7 @@ dill==0.4.0 # via pylint django==4.2.23 # via - # -c requirements/common_constraints.txt + # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/test.txt # django-config-models # django-crum @@ -138,14 +138,12 @@ django-model-utils==5.0.0 # edx-celeryutils # edx-completion # openedx-completion-aggregator +django-object-actions==5.0.0 + # via -r requirements/test.txt django-redis==6.0.0 # via # -r requirements/test.txt # edx-event-routing-backends -django-simple-history==3.4.0 - # via - # -c requirements/constraints.txt - # -r requirements/test.txt django-waffle==5.0.0 # via # -r requirements/test.txt @@ -461,7 +459,7 @@ tzdata==2025.2 # kombu urllib3==2.2.3 # via - # -c requirements/common_constraints.txt + # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/test.txt # requests vine==5.1.0 diff --git a/requirements/test.txt b/requirements/test.txt index e66e2b6..2b0f637 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -85,7 +85,7 @@ cryptography==45.0.5 # django-fernet-fields-v2 # pyjwt # via - # -c requirements/common_constraints.txt + # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/base.txt # django-config-models # django-crum @@ -125,14 +125,12 @@ django-model-utils==5.0.0 # edx-celeryutils # edx-completion # openedx-completion-aggregator +django-object-actions==5.0.0 + # via -r requirements/base.txt django-redis==6.0.0 # via # -r requirements/base.txt # edx-event-routing-backends -django-simple-history==3.4.0 - # via - # -c requirements/constraints.txt - # -r requirements/base.txt django-waffle==5.0.0 # via # -r requirements/base.txt @@ -405,7 +403,7 @@ tzdata==2025.2 # kombu urllib3==2.2.3 # via - # -c requirements/common_constraints.txt + # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/base.txt # requests vine==5.1.0 diff --git a/tox.ini b/tox.ini index c916b21..5225ce9 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py311-django{42} +envlist = py{311,312}-django{42,52},quality,docs,pii_check [doc8] ; D001 = Line too long @@ -37,17 +37,18 @@ norecursedirs = .* docs requirements site-packages [testenv] deps = django42: Django>=4.2,<4.3 + django52: Django>=5.2,<5.3 -r{toxinidir}/requirements/test.txt commands = python manage.py check python manage.py makemigrations --check --dry-run --verbosity 3 - pytest {posargs} + pytest {posargs:learning_paths/} [testenv:docs] setenv = DJANGO_SETTINGS_MODULE = test_settings PYTHONPATH = {toxinidir} - # Adding the option here instead of as a default in the docs Makefile because that Makefile is generated by shpinx. + # Adding the option here instead of as a default in the docs Makefile because that Makefile is generated by sphinx. SPHINXOPTS = -W allowlist_externals = make