From 3b51154699e42e70a4e5948bdeaac2803ef72c14 Mon Sep 17 00:00:00 2001 From: "kshitij.sobti" Date: Wed, 20 Nov 2024 18:11:37 +0530 Subject: [PATCH 1/2] feat: User agreements API for generic agreement records This change adds a new kind of generic user agreement that allows plugins or even the core platform to record a user's acknowledgement of an agreement. --- openedx/core/djangoapps/agreements/api.py | 55 ++++++++++-- openedx/core/djangoapps/agreements/data.py | 23 +++++ .../migrations/0006_useragreementrecord.py | 25 ++++++ openedx/core/djangoapps/agreements/models.py | 17 ++++ .../core/djangoapps/agreements/serializers.py | 12 ++- .../djangoapps/agreements/tests/test_api.py | 58 ++++++++++--- .../djangoapps/agreements/tests/test_views.py | 69 +++++++++++++-- openedx/core/djangoapps/agreements/urls.py | 5 +- openedx/core/djangoapps/agreements/views.py | 86 +++++++++++++++++-- 9 files changed, 318 insertions(+), 32 deletions(-) create mode 100644 openedx/core/djangoapps/agreements/migrations/0006_useragreementrecord.py diff --git a/openedx/core/djangoapps/agreements/api.py b/openedx/core/djangoapps/agreements/api.py index 2489cefb8533..11ad23dddd25 100644 --- a/openedx/core/djangoapps/agreements/api.py +++ b/openedx/core/djangoapps/agreements/api.py @@ -3,17 +3,15 @@ """ import logging +from datetime import datetime +from typing import Iterable, Optional from django.contrib.auth import get_user_model from django.core.exceptions import ObjectDoesNotExist from opaque_keys.edx.keys import CourseKey -from openedx.core.djangoapps.agreements.models import IntegritySignature -from openedx.core.djangoapps.agreements.models import LTIPIITool -from openedx.core.djangoapps.agreements.models import LTIPIISignature - -from .data import LTIToolsReceivingPIIData -from .data import LTIPIISignatureData +from .data import LTIPIISignatureData, LTIToolsReceivingPIIData, UserAgreementRecordData +from .models import IntegritySignature, LTIPIISignature, LTIPIITool, UserAgreementRecord log = logging.getLogger(__name__) User = get_user_model() @@ -240,3 +238,48 @@ def _user_signature_out_of_date(username, course_id): return False else: return user_lti_pii_signature_hash != course_lti_pii_tools_hash + + +def get_user_agreements(user: User) -> Iterable[UserAgreementRecordData]: + """ + Retrieves all the agreements that the specified user has acknowledged. + """ + for agreement_record in UserAgreementRecord.objects.filter(user=user): + yield UserAgreementRecordData.from_model(agreement_record) + + +def get_latest_user_agreement_record( + user: User, + agreement_type: str, + agreed_after: datetime = None, +) -> Optional[UserAgreementRecordData]: + """ + Retrieve the user agreement record for the specified user and agreement type. + + An agreement update timestamp can be provided to return a record only if it + was signed after that timestamp. + """ + try: + record_query = UserAgreementRecord.objects.filter( + user=user, + agreement_type=agreement_type, + ) + if agreed_after: + record_query = record_query.filter(timestamp__gte=agreed_after) + record = record_query.latest("timestamp") + return UserAgreementRecordData.from_model(record) + except UserAgreementRecord.DoesNotExist: + return None + + +def create_user_agreement_record(user: User, agreement_type: str) -> UserAgreementRecordData: + """ + Creates a user agreement record if one doesn't already exist, or updates existing + record to current timestamp. + """ + record = UserAgreementRecord.objects.create( + user=user, + agreement_type=agreement_type, + timestamp=datetime.now(), + ) + return UserAgreementRecordData.from_model(record) diff --git a/openedx/core/djangoapps/agreements/data.py b/openedx/core/djangoapps/agreements/data.py index 9d843c73cb04..01d83665c009 100644 --- a/openedx/core/djangoapps/agreements/data.py +++ b/openedx/core/djangoapps/agreements/data.py @@ -1,8 +1,13 @@ """ Public data structures for this app. """ +from dataclasses import dataclass +from datetime import datetime + import attr +from .models import UserAgreementRecord + @attr.s(frozen=True, auto_attribs=True) class LTIToolsReceivingPIIData: @@ -21,3 +26,21 @@ class LTIPIISignatureData: course_id: str lti_tools: str lti_tools_hash: str + + +@dataclass +class UserAgreementRecordData: + """ + Data for a single user agreement record. + """ + username: str + agreement_type: str + accepted_at: datetime + + @classmethod + def from_model(cls, model: UserAgreementRecord): + return UserAgreementRecordData( + username=model.user.username, + agreement_type=model.agreement_type, + accepted_at=model.timestamp, + ) diff --git a/openedx/core/djangoapps/agreements/migrations/0006_useragreementrecord.py b/openedx/core/djangoapps/agreements/migrations/0006_useragreementrecord.py new file mode 100644 index 000000000000..2e0985adb6de --- /dev/null +++ b/openedx/core/djangoapps/agreements/migrations/0006_useragreementrecord.py @@ -0,0 +1,25 @@ +# Generated by Django 4.2.16 on 2024-12-06 11:34 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('agreements', '0005_timestampedmodels'), + ] + + operations = [ + migrations.CreateModel( + name='UserAgreementRecord', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('agreement_type', models.CharField(max_length=255)), + ('timestamp', models.DateTimeField(auto_now_add=True)), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + ), + ] diff --git a/openedx/core/djangoapps/agreements/models.py b/openedx/core/djangoapps/agreements/models.py index 2672a4f47b24..2ceeeb98109f 100644 --- a/openedx/core/djangoapps/agreements/models.py +++ b/openedx/core/djangoapps/agreements/models.py @@ -70,3 +70,20 @@ class ProctoringPIISignature(TimeStampedModel): class Meta: app_label = 'agreements' + + +class UserAgreementRecord(models.Model): + """ + This model stores the agreements a user has accepted or acknowledged. + + Each record here represents a user agreeing to the agreement type represented + by `agreement_type` at a particular time. + + .. no_pii: + """ + user = models.ForeignKey(User, db_index=True, on_delete=models.CASCADE) + agreement_type = models.CharField(max_length=255) + timestamp = models.DateTimeField(auto_now_add=True) + + class Meta: + app_label = 'agreements' diff --git a/openedx/core/djangoapps/agreements/serializers.py b/openedx/core/djangoapps/agreements/serializers.py index 397a9ba61d54..6809052e4f2f 100644 --- a/openedx/core/djangoapps/agreements/serializers.py +++ b/openedx/core/djangoapps/agreements/serializers.py @@ -3,9 +3,10 @@ """ from rest_framework import serializers -from openedx.core.djangoapps.agreements.models import IntegritySignature, LTIPIISignature from openedx.core.lib.api.serializers import CourseKeyField +from .models import IntegritySignature, LTIPIISignature + class IntegritySignatureSerializer(serializers.ModelSerializer): """ @@ -31,3 +32,12 @@ class LTIPIISignatureSerializer(serializers.ModelSerializer): class Meta: model = LTIPIISignature fields = ('username', 'course_id', 'lti_tools', 'created_at') + + +class UserAgreementsSerializer(serializers.Serializer): + """ + Serializer for UserAgreementRecord model + """ + username = serializers.CharField(read_only=True) + agreement_type = serializers.CharField(read_only=True) + accepted_at = serializers.DateTimeField() diff --git a/openedx/core/djangoapps/agreements/tests/test_api.py b/openedx/core/djangoapps/agreements/tests/test_api.py index c66065789939..eb1e02956dc5 100644 --- a/openedx/core/djangoapps/agreements/tests/test_api.py +++ b/openedx/core/djangoapps/agreements/tests/test_api.py @@ -2,25 +2,29 @@ Tests for the Agreements API """ import logging +from datetime import datetime, timedelta +from django.test import TestCase +from opaque_keys.edx.keys import CourseKey from testfixtures import LogCapture from common.djangoapps.student.tests.factories import UserFactory -from openedx.core.djangoapps.agreements.api import ( +from openedx.core.djangolib.testing.utils import skip_unless_lms +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +from ..api import ( create_integrity_signature, + create_lti_pii_signature, + create_user_agreement_record, get_integrity_signature, get_integrity_signatures_for_course, + get_lti_pii_signature, get_pii_receiving_lti_tools, - create_lti_pii_signature, - get_lti_pii_signature + get_latest_user_agreement_record, + get_user_agreements ) -from openedx.core.djangolib.testing.utils import skip_unless_lms -from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order -from ..models import ( - LTIPIITool, -) -from opaque_keys.edx.keys import CourseKey +from ..models import LTIPIITool LOGGER_NAME = "openedx.core.djangoapps.agreements.api" @@ -186,3 +190,37 @@ def _assert_ltitools(self, lti_list): Helper function to assert the returned list has the correct tools """ self.assertEqual(self.lti_tools, lti_list) + + +@skip_unless_lms +class UserAgreementsTests(TestCase): + """ + Tests for the python APIs related to user agreements. + """ + def setUp(self): + self.user = UserFactory() + + def test_get_user_agreements(self, ): + result = list(get_user_agreements(self.user)) + assert len(result) == 0 + + record = create_user_agreement_record(self.user, 'test_type') + result = list(get_user_agreements(self.user)) + + assert len(result) == 1 + assert result[0].agreement_type == 'test_type' + assert result[0].username == self.user.username + assert result[0].accepted_at == record.accepted_at + + def test_get_user_agreement_record(self): + record = create_user_agreement_record(self.user, 'test_type') + result = get_latest_user_agreement_record(self.user, 'test_type') + + assert result == record + + result = get_latest_user_agreement_record(self.user, 'test_type', datetime.now() + timedelta(days=1)) + + assert result is None + + def tearDown(self): + self.user.delete() diff --git a/openedx/core/djangoapps/agreements/tests/test_views.py b/openedx/core/djangoapps/agreements/tests/test_views.py index 4c52e5853f05..61cc8661fb43 100644 --- a/openedx/core/djangoapps/agreements/tests/test_views.py +++ b/openedx/core/djangoapps/agreements/tests/test_views.py @@ -2,26 +2,28 @@ Tests for agreements views """ +import json from datetime import datetime, timedelta from unittest.mock import patch from django.conf import settings from django.urls import reverse -from rest_framework.test import APITestCase -from rest_framework import status from freezegun import freeze_time -import json +from rest_framework import status +from rest_framework.test import APITestCase -from common.djangoapps.student.tests.factories import UserFactory, AdminFactory from common.djangoapps.student.roles import CourseStaffRole -from openedx.core.djangoapps.agreements.api import ( +from common.djangoapps.student.tests.factories import AdminFactory, UserFactory +from openedx.core.djangolib.testing.utils import skip_unless_lms +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + +from ..api import ( create_integrity_signature, + create_user_agreement_record, get_integrity_signatures_for_course, get_lti_pii_signature ) -from openedx.core.djangolib.testing.utils import skip_unless_lms -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.tests.factories import CourseFactory # lint-amnesty, pylint: disable=wrong-import-order @skip_unless_lms @@ -289,3 +291,54 @@ def test_post_lti_pii_signature(self): signature = get_lti_pii_signature(self.user.username, self.course_id) self.assertEqual(signature.user.username, self.user.username) self.assertEqual(signature.lti_tools, self.lti_tools) + + +@skip_unless_lms +class UserAgreementsViewTests(APITestCase): + """ + Tests for the UserAgreementsView + """ + + def setUp(self): + self.user = UserFactory(username="testuser", password="password") + self.url = reverse('user_agreements', kwargs={'agreement_type': 'sample_agreement'}) + self.login() + + def login(self): + self.client.login(username="testuser", password="password") + + def test_get_user_agreement_record_no_data(self): + response = self.client.get(self.url) + assert response.status_code == status.HTTP_404_NOT_FOUND + + def test_get_user_agreement_record_invalid_date(self): + response = self.client.get(self.url, {'after': 'invalid_date'}) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + def test_get_user_agreement_record(self): + create_user_agreement_record(self.user, 'sample_agreement') + response = self.client.get(self.url) + assert response.status_code == status.HTTP_200_OK + assert 'accepted_at' in response.data + + response = self.client.get(self.url, {"after": str(datetime.now() + timedelta(days=1))}) + assert response.status_code == status.HTTP_404_NOT_FOUND + + def test_post_user_agreement(self): + with freeze_time("2024-11-21 12:00:00"): + response = self.client.post(self.url) + assert response.status_code == status.HTTP_201_CREATED + + self.login() + + response = self.client.get(self.url) + assert response.status_code == status.HTTP_200_OK + + response = self.client.get(self.url, {"after": "2024-11-21T13:00:00Z"}) + assert response.status_code == status.HTTP_404_NOT_FOUND + + response = self.client.post(self.url) + assert response.status_code == status.HTTP_201_CREATED + + response = self.client.get(self.url, {"after": "2024-11-21T13:00:00Z"}) + assert response.status_code == status.HTTP_200_OK diff --git a/openedx/core/djangoapps/agreements/urls.py b/openedx/core/djangoapps/agreements/urls.py index d9d009d65ac1..902f477a7087 100644 --- a/openedx/core/djangoapps/agreements/urls.py +++ b/openedx/core/djangoapps/agreements/urls.py @@ -3,9 +3,9 @@ """ from django.conf import settings -from django.urls import re_path +from django.urls import path, re_path -from .views import IntegritySignatureView, LTIPIISignatureView +from .views import IntegritySignatureView, LTIPIISignatureView, UserAgreementsView urlpatterns = [ re_path(r'^integrity_signature/{course_id}$'.format( @@ -14,4 +14,5 @@ re_path(r'^lti_pii_signature/{course_id}$'.format( course_id=settings.COURSE_ID_PATTERN ), LTIPIISignatureView.as_view(), name='lti_pii_signature'), + path("agreement/", UserAgreementsView.as_view(), name="user_agreements"), ] diff --git a/openedx/core/djangoapps/agreements/views.py b/openedx/core/djangoapps/agreements/views.py index cc928669ffdd..daf2ce09428c 100644 --- a/openedx/core/djangoapps/agreements/views.py +++ b/openedx/core/djangoapps/agreements/views.py @@ -2,21 +2,28 @@ Views served by the Agreements app """ +import edx_api_doc_tools as apidocs +from django import forms from django.conf import settings +from drf_yasg import openapi +from opaque_keys.edx.keys import CourseKey from rest_framework import status -from rest_framework.views import APIView -from rest_framework.response import Response from rest_framework.permissions import IsAuthenticated -from opaque_keys.edx.keys import CourseKey +from rest_framework.response import Response +from rest_framework.views import APIView from common.djangoapps.student import auth from common.djangoapps.student.roles import CourseStaffRole -from openedx.core.djangoapps.agreements.api import ( + +from .api import ( create_integrity_signature, create_lti_pii_signature, + create_user_agreement_record, get_integrity_signature, + get_latest_user_agreement_record ) -from openedx.core.djangoapps.agreements.serializers import IntegritySignatureSerializer, LTIPIISignatureSerializer +from .serializers import IntegritySignatureSerializer, LTIPIISignatureSerializer, UserAgreementsSerializer +from ...lib.api.view_utils import view_auth_classes def is_user_course_or_global_staff(user, course_id): @@ -159,3 +166,72 @@ def post(self, request, course_id): else: statusStr = status.HTTP_500_INTERNAL_SERVER_ERROR return Response(data=serializer.data, status=statusStr) + + +@view_auth_classes(is_authenticated=True) +class UserAgreementsView(APIView): + """ + Endpoint for the user agreements API. + """ + + class QueryFilterForm(forms.Form): + """ + Query parameters for the GET method. + """ + after = forms.DateTimeField(required=False) + + @apidocs.schema( + parameters=[ + apidocs.string_parameter( + 'agreement_type', + apidocs.ParameterLocation.PATH, + description="Agreement ID/Type", + ), + openapi.Parameter( + 'after', + apidocs.ParameterLocation.QUERY, + required=False, + type=openapi.TYPE_STRING, + format=openapi.FORMAT_DATETIME, + description="Return records after this date/time", + ), + ], + responses={ + 200: UserAgreementsSerializer, + 400: "Bad Request", + 404: "Not Found", + }, + ) + def get(self, request, agreement_type): + """ + Get a user's acknowledgement record for this agreement type. + """ + params = UserAgreementsView.QueryFilterForm(request.query_params) + if not params.is_valid(): + return Response(status=status.HTTP_400_BAD_REQUEST) + record = get_latest_user_agreement_record(request.user, agreement_type, params.cleaned_data.get('after')) + if record is None: + return Response(status=status.HTTP_404_NOT_FOUND) + serializer = UserAgreementsSerializer(record) + return Response(serializer.data) + + @apidocs.schema( + parameters=[ + apidocs.string_parameter( + 'agreement_type', + apidocs.ParameterLocation.PATH, + description="Agreement ID/Type", + ), + ], + responses={ + 200: UserAgreementsSerializer, + 400: "Bad Request", + }, + ) + def post(self, request, agreement_type): + """ + Marks a user's acknowledgement of this agreement type. + """ + record = create_user_agreement_record(request.user, agreement_type) + serializer = UserAgreementsSerializer(record) + return Response(serializer.data, status=status.HTTP_201_CREATED) From 1abd876769f8529d5fb39a4dddd11d2fbf4172ed Mon Sep 17 00:00:00 2001 From: "kshitij.sobti" Date: Tue, 27 Jan 2026 13:18:06 +0530 Subject: [PATCH 2/2] feat: user agreement api --- openedx/core/djangoapps/agreements/admin.py | 16 +++- openedx/core/djangoapps/agreements/api.py | 11 +-- openedx/core/djangoapps/agreements/data.py | 28 ++++++- ...caluseragreement_useragreement_and_more.py | 63 ++++++++++++++ ...agreementrecord_agreement_type_and_more.py | 38 +++++++++ openedx/core/djangoapps/agreements/models.py | 40 ++++++++- .../core/djangoapps/agreements/serializers.py | 14 +++- .../djangoapps/agreements/tests/test_api.py | 6 +- .../agreements/tests/test_models.py | 82 +++++++++++++++++++ openedx/core/djangoapps/agreements/urls.py | 9 +- openedx/core/djangoapps/agreements/views.py | 81 ++++++++++++++++-- 11 files changed, 361 insertions(+), 27 deletions(-) create mode 100644 openedx/core/djangoapps/agreements/migrations/0007_historicaluseragreement_useragreement_and_more.py create mode 100644 openedx/core/djangoapps/agreements/migrations/0008_remove_useragreementrecord_agreement_type_and_more.py create mode 100644 openedx/core/djangoapps/agreements/tests/test_models.py diff --git a/openedx/core/djangoapps/agreements/admin.py b/openedx/core/djangoapps/agreements/admin.py index 82e2427dff50..802672c7b978 100644 --- a/openedx/core/djangoapps/agreements/admin.py +++ b/openedx/core/djangoapps/agreements/admin.py @@ -3,7 +3,7 @@ """ from django.contrib import admin -from openedx.core.djangoapps.agreements.models import IntegritySignature +from openedx.core.djangoapps.agreements.models import IntegritySignature, UserAgreement from openedx.core.djangoapps.agreements.models import LTIPIITool from openedx.core.djangoapps.agreements.models import LTIPIISignature from openedx.core.djangoapps.agreements.models import ProctoringPIISignature @@ -62,3 +62,17 @@ class Meta: admin.site.register(ProctoringPIISignature, ProctoringPIISignatureAdmin) + + +class UserAgreementAdmin(admin.ModelAdmin): + """ + Admin for the UserAgreement Model + """ + + list_display = ('type', 'name', 'url', 'created', 'updated') + + class Meta: + model = UserAgreement + + +admin.site.register(UserAgreement, UserAgreementAdmin) diff --git a/openedx/core/djangoapps/agreements/api.py b/openedx/core/djangoapps/agreements/api.py index 11ad23dddd25..e7e7be23a055 100644 --- a/openedx/core/djangoapps/agreements/api.py +++ b/openedx/core/djangoapps/agreements/api.py @@ -240,18 +240,17 @@ def _user_signature_out_of_date(username, course_id): return user_lti_pii_signature_hash != course_lti_pii_tools_hash -def get_user_agreements(user: User) -> Iterable[UserAgreementRecordData]: +def get_user_agreement_records(user: User) -> Iterable[UserAgreementRecordData]: """ Retrieves all the agreements that the specified user has acknowledged. """ - for agreement_record in UserAgreementRecord.objects.filter(user=user): + for agreement_record in UserAgreementRecord.objects.filter(user=user).select_related("agreement"): yield UserAgreementRecordData.from_model(agreement_record) def get_latest_user_agreement_record( user: User, agreement_type: str, - agreed_after: datetime = None, ) -> Optional[UserAgreementRecordData]: """ Retrieve the user agreement record for the specified user and agreement type. @@ -262,10 +261,8 @@ def get_latest_user_agreement_record( try: record_query = UserAgreementRecord.objects.filter( user=user, - agreement_type=agreement_type, + agreement__type=agreement_type, ) - if agreed_after: - record_query = record_query.filter(timestamp__gte=agreed_after) record = record_query.latest("timestamp") return UserAgreementRecordData.from_model(record) except UserAgreementRecord.DoesNotExist: @@ -279,7 +276,7 @@ def create_user_agreement_record(user: User, agreement_type: str) -> UserAgreeme """ record = UserAgreementRecord.objects.create( user=user, - agreement_type=agreement_type, + agreement__type=agreement_type, timestamp=datetime.now(), ) return UserAgreementRecordData.from_model(record) diff --git a/openedx/core/djangoapps/agreements/data.py b/openedx/core/djangoapps/agreements/data.py index 01d83665c009..a725a5f49e3d 100644 --- a/openedx/core/djangoapps/agreements/data.py +++ b/openedx/core/djangoapps/agreements/data.py @@ -6,7 +6,7 @@ import attr -from .models import UserAgreementRecord +from .models import UserAgreementRecord, UserAgreement @attr.s(frozen=True, auto_attribs=True) @@ -28,6 +28,28 @@ class LTIPIISignatureData: lti_tools_hash: str + +@dataclass +class UserAgreementData: + """ + Data for a user agreement record. + """ + type: str + name: str + summary: str + text: str|None + url: str|None + + @classmethod + def from_model(cls, model: UserAgreement): + return UserAgreementData( + type=model.type, + name=model.name, + summary=model.summary, + text=model.text, + url=model.url + ) + @dataclass class UserAgreementRecordData: """ @@ -36,11 +58,13 @@ class UserAgreementRecordData: username: str agreement_type: str accepted_at: datetime + is_current: bool = True @classmethod def from_model(cls, model: UserAgreementRecord): return UserAgreementRecordData( username=model.user.username, - agreement_type=model.agreement_type, + agreement_type=model.agreement.type, accepted_at=model.timestamp, + is_current=model.agreement.updated < model.timestamp ) diff --git a/openedx/core/djangoapps/agreements/migrations/0007_historicaluseragreement_useragreement_and_more.py b/openedx/core/djangoapps/agreements/migrations/0007_historicaluseragreement_useragreement_and_more.py new file mode 100644 index 000000000000..13b4dce9c63d --- /dev/null +++ b/openedx/core/djangoapps/agreements/migrations/0007_historicaluseragreement_useragreement_and_more.py @@ -0,0 +1,63 @@ +# Generated by Django 5.2.10 on 2026-01-26 10:21 + +import django.db.models.deletion +import simple_history.models +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('agreements', '0006_useragreementrecord'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='HistoricalUserAgreement', + fields=[ + ('id', models.IntegerField(auto_created=True, blank=True, db_index=True, verbose_name='ID')), + ('type', models.CharField(db_index=True, max_length=255)), + ('name', models.CharField(help_text='Human-readable name for the agreement type. Will be displayed to users in alert to accept the agreement.', max_length=255)), + ('summary', models.TextField(help_text='Brief summary of the agreement content. Will be displayed to users in alert to accept the agreement.', max_length=1024)), + ('text', models.TextField(blank=True, help_text='Full text of the agreement. (Required if url is not provided)', null=True)), + ('url', models.URLField(blank=True, help_text='URL where the full agreement can be accessed. Will be used for "Learn More" link in alert to accept the agreement.', null=True)), + ('created', models.DateTimeField(blank=True, editable=False)), + ('updated', models.DateTimeField(help_text='Timestamp of the last update to this agreement. If changed users will be prompted to accept the agreement again.')), + ('history_id', models.AutoField(primary_key=True, serialize=False)), + ('history_date', models.DateTimeField()), + ('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)), + ], + options={ + 'verbose_name': 'historical user agreement', + 'verbose_name_plural': 'historical user agreements', + 'ordering': ('-history_date', '-history_id'), + 'get_latest_by': ('history_date', 'history_id'), + }, + bases=(simple_history.models.HistoricalChanges, models.Model), + ), + migrations.CreateModel( + name='UserAgreement', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('type', models.CharField(max_length=255, unique=True)), + ('name', models.CharField(help_text='Human-readable name for the agreement type. Will be displayed to users in alert to accept the agreement.', max_length=255)), + ('summary', models.TextField(help_text='Brief summary of the agreement content. Will be displayed to users in alert to accept the agreement.', max_length=1024)), + ('text', models.TextField(blank=True, help_text='Full text of the agreement. (Required if url is not provided)', null=True)), + ('url', models.URLField(blank=True, help_text='URL where the full agreement can be accessed. Will be used for "Learn More" link in alert to accept the agreement.', null=True)), + ('created', models.DateTimeField(auto_now_add=True)), + ('updated', models.DateTimeField(help_text='Timestamp of the last update to this agreement. If changed users will be prompted to accept the agreement again.')), + ], + options={ + 'constraints': [models.CheckConstraint(condition=models.Q(('text__isnull', False), ('url__isnull', False), _connector='OR'), name='agreement_has_text_or_url')], + }, + ), + migrations.AddField( + model_name='useragreementrecord', + name='agreement', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, related_name='records', to='agreements.useragreement'), + ), + ] diff --git a/openedx/core/djangoapps/agreements/migrations/0008_remove_useragreementrecord_agreement_type_and_more.py b/openedx/core/djangoapps/agreements/migrations/0008_remove_useragreementrecord_agreement_type_and_more.py new file mode 100644 index 000000000000..c6894ce7aac1 --- /dev/null +++ b/openedx/core/djangoapps/agreements/migrations/0008_remove_useragreementrecord_agreement_type_and_more.py @@ -0,0 +1,38 @@ +# Generated by Django 5.2.10 on 2026-01-26 10:22 + +import django.db.models.deletion +from django.db import migrations, models + + + +def migrate_agreement_type(apps, schema_editor): + UserAgreementRecord = apps.get_model('agreements', 'UserAgreementRecord') + UserAgreement = apps.get_model('agreements', 'UserAgreement') + for user_agreement_record in UserAgreementRecord.objects.all(): + user_agreement_record.agreement = UserAgreement.objects.get_or_create(type=user_agreement_record.agreement_type, defaults=dict(text='')) + + +def migrate_agreement_type_rev(apps, schema_editor): + UserAgreementRecord = apps.get_model('agreements', 'UserAgreementRecord') + for user_agreement_record in UserAgreementRecord.objects.all(): + user_agreement_record.agreement_type = user_agreement_record.agreement.type + + +class Migration(migrations.Migration): + + dependencies = [ + ('agreements', '0007_historicaluseragreement_useragreement_and_more'), + ] + + operations = [ + migrations.RunPython(migrate_agreement_type, migrate_agreement_type_rev), + migrations.RemoveField( + model_name='useragreementrecord', + name='agreement_type', + ), + migrations.AlterField( + model_name='useragreementrecord', + name='agreement', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='records', to='agreements.useragreement'), + ), + ] diff --git a/openedx/core/djangoapps/agreements/models.py b/openedx/core/djangoapps/agreements/models.py index 2ceeeb98109f..747ccedc1176 100644 --- a/openedx/core/djangoapps/agreements/models.py +++ b/openedx/core/djangoapps/agreements/models.py @@ -6,6 +6,7 @@ from django.db import models from model_utils.models import TimeStampedModel from opaque_keys.edx.django.models import CourseKeyField +from simple_history.models import HistoricalRecords User = get_user_model() @@ -72,6 +73,43 @@ class Meta: app_label = 'agreements' +class UserAgreement(models.Model): + """ + This model stores agreements that as user can accept that can gate certain + platform features. + + .. no_pii: + """ + type = models.CharField(max_length=255, unique=True) + name = models.CharField( + max_length=255, + help_text='Human-readable name for the agreement type. Will be displayed to users in alert to accept the agreement.', + ) + summary = models.TextField( + max_length=1024, + help_text='Brief summary of the agreement content. Will be displayed to users in alert to accept the agreement.', + ) + text = models.TextField( + help_text='Full text of the agreement. (Required if url is not provided)', + null=True, blank=True, + ) + url = models.URLField( + help_text='URL where the full agreement can be accessed. Will be used for "Learn More" link in alert to accept the agreement.', + null=True, blank=True, + ) + created = models.DateTimeField(auto_now_add=True) + updated = models.DateTimeField( + help_text='Timestamp of the last update to this agreement. If changed users will be prompted to accept the agreement again.') + history = HistoricalRecords() + + class Meta: + app_label = 'agreements' + constraints = [ + models.CheckConstraint(check=models.Q(text__isnull=False) | models.Q(url__isnull=False), + name='agreement_has_text_or_url') + ] + + class UserAgreementRecord(models.Model): """ This model stores the agreements a user has accepted or acknowledged. @@ -82,7 +120,7 @@ class UserAgreementRecord(models.Model): .. no_pii: """ user = models.ForeignKey(User, db_index=True, on_delete=models.CASCADE) - agreement_type = models.CharField(max_length=255) + agreement = models.ForeignKey(UserAgreement, on_delete=models.CASCADE, related_name='records') timestamp = models.DateTimeField(auto_now_add=True) class Meta: diff --git a/openedx/core/djangoapps/agreements/serializers.py b/openedx/core/djangoapps/agreements/serializers.py index 6809052e4f2f..9298ad0d0025 100644 --- a/openedx/core/djangoapps/agreements/serializers.py +++ b/openedx/core/djangoapps/agreements/serializers.py @@ -34,7 +34,19 @@ class Meta: fields = ('username', 'course_id', 'lti_tools', 'created_at') -class UserAgreementsSerializer(serializers.Serializer): +class UserAgreementSerializer(serializers.Serializer): + """ + Serializer for UserAgreement model + """ + type = serializers.CharField(read_only=True) + name = serializers.CharField(read_only=True) + summary = serializers.CharField(read_only=True) + text = serializers.CharField(read_only=True) + url = serializers.URLField(read_only=True) + updated = serializers.DateTimeField(read_only=True) + + +class UserAgreementRecordSerializer(serializers.Serializer): """ Serializer for UserAgreementRecord model """ diff --git a/openedx/core/djangoapps/agreements/tests/test_api.py b/openedx/core/djangoapps/agreements/tests/test_api.py index eb1e02956dc5..52400fa82dcb 100644 --- a/openedx/core/djangoapps/agreements/tests/test_api.py +++ b/openedx/core/djangoapps/agreements/tests/test_api.py @@ -22,7 +22,7 @@ get_lti_pii_signature, get_pii_receiving_lti_tools, get_latest_user_agreement_record, - get_user_agreements + get_user_agreement_records ) from ..models import LTIPIITool @@ -201,11 +201,11 @@ def setUp(self): self.user = UserFactory() def test_get_user_agreements(self, ): - result = list(get_user_agreements(self.user)) + result = list(get_user_agreement_records(self.user)) assert len(result) == 0 record = create_user_agreement_record(self.user, 'test_type') - result = list(get_user_agreements(self.user)) + result = list(get_user_agreement_records(self.user)) assert len(result) == 1 assert result[0].agreement_type == 'test_type' diff --git a/openedx/core/djangoapps/agreements/tests/test_models.py b/openedx/core/djangoapps/agreements/tests/test_models.py new file mode 100644 index 000000000000..80b18cb37384 --- /dev/null +++ b/openedx/core/djangoapps/agreements/tests/test_models.py @@ -0,0 +1,82 @@ +""" +Tests for Agreements models +""" + +from django.db import IntegrityError +from django.test import TestCase +from openedx.core.djangoapps.agreements.models import UserAgreement + + +class UserAgreementModelTest(TestCase): + """ + Tests for the UserAgreement model. + """ + def test_agreement_must_have_text_or_url(self): + """ + Verify that a UserAgreement must have at least a url or text. + """ + # Case 1: Both text and url are provided (Success) + agreement = UserAgreement.objects.create( + type='type1', + name='Name 1', + summary='Summary 1', + text='Some text', + url='https://example.com' + ) + self.assertIsNotNone(agreement.pk) + + # Case 2: Only text is provided (Success) + agreement = UserAgreement.objects.create( + type='type2', + name='Name 2', + summary='Summary 2', + text='Some text', + url=None + ) + self.assertIsNotNone(agreement.pk) + + # Case 3: Only url is provided (Success) + agreement = UserAgreement.objects.create( + type='type3', + name='Name 3', + summary='Summary 3', + text=None, + url='https://example.com' + ) + self.assertIsNotNone(agreement.pk) + + # Case 4: Neither text nor url is provided (Failure) + with self.assertRaises(IntegrityError): + UserAgreement.objects.create( + type='type4', + name='Name 4', + summary='Summary 4', + text=None, + url=None + ) + + def test_agreement_with_empty_strings(self): + """ + Verify behavior with empty strings. + Since the constraint is `isnull=False`, empty strings should pass + if the DB allows them as NOT NULL. + """ + # Case 5: text is empty string, url is None (Success - because text is NOT NULL) + agreement = UserAgreement.objects.create( + type='type5', + name='Name 5', + summary='Summary 5', + text='', + url=None + ) + self.assertIsNotNone(agreement.pk) + + # Case 6: text is None, url is empty string (Success - because url is NOT NULL) + agreement = UserAgreement.objects.create( + type='type6', + name='Name 6', + summary='Summary 6', + text=None, + url='' + ) + self.assertIsNotNone(agreement.pk) diff --git a/openedx/core/djangoapps/agreements/urls.py b/openedx/core/djangoapps/agreements/urls.py index 902f477a7087..62cc438cef75 100644 --- a/openedx/core/djangoapps/agreements/urls.py +++ b/openedx/core/djangoapps/agreements/urls.py @@ -4,9 +4,12 @@ from django.conf import settings from django.urls import path, re_path +from rest_framework.routers import DefaultRouter -from .views import IntegritySignatureView, LTIPIISignatureView, UserAgreementsView +from .views import IntegritySignatureView, LTIPIISignatureView, UserAgreementRecordsView, UserAgreementsViewSet +router = DefaultRouter() +router.register(r'agreement', UserAgreementsViewSet, basename='user_agreements') urlpatterns = [ re_path(r'^integrity_signature/{course_id}$'.format( course_id=settings.COURSE_ID_PATTERN @@ -14,5 +17,5 @@ re_path(r'^lti_pii_signature/{course_id}$'.format( course_id=settings.COURSE_ID_PATTERN ), LTIPIISignatureView.as_view(), name='lti_pii_signature'), - path("agreement/", UserAgreementsView.as_view(), name="user_agreements"), -] + path("agreement_record/", UserAgreementRecordsView.as_view(), name="user_agreement_record"), +] + router.urls diff --git a/openedx/core/djangoapps/agreements/views.py b/openedx/core/djangoapps/agreements/views.py index daf2ce09428c..1041a4a4b613 100644 --- a/openedx/core/djangoapps/agreements/views.py +++ b/openedx/core/djangoapps/agreements/views.py @@ -7,7 +7,7 @@ from django.conf import settings from drf_yasg import openapi from opaque_keys.edx.keys import CourseKey -from rest_framework import status +from rest_framework import status, viewsets from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.views import APIView @@ -22,7 +22,9 @@ get_integrity_signature, get_latest_user_agreement_record ) -from .serializers import IntegritySignatureSerializer, LTIPIISignatureSerializer, UserAgreementsSerializer +from .models import UserAgreement +from .serializers import IntegritySignatureSerializer, LTIPIISignatureSerializer, UserAgreementRecordSerializer, \ + UserAgreementSerializer from ...lib.api.view_utils import view_auth_classes @@ -169,9 +171,9 @@ def post(self, request, course_id): @view_auth_classes(is_authenticated=True) -class UserAgreementsView(APIView): +class UserAgreementRecordsView(APIView): """ - Endpoint for the user agreements API. + Endpoint for the user agreement records API. """ class QueryFilterForm(forms.Form): @@ -197,7 +199,7 @@ class QueryFilterForm(forms.Form): ), ], responses={ - 200: UserAgreementsSerializer, + 200: UserAgreementRecordSerializer, 400: "Bad Request", 404: "Not Found", }, @@ -206,13 +208,13 @@ def get(self, request, agreement_type): """ Get a user's acknowledgement record for this agreement type. """ - params = UserAgreementsView.QueryFilterForm(request.query_params) + params = UserAgreementRecordsView.QueryFilterForm(request.query_params) if not params.is_valid(): return Response(status=status.HTTP_400_BAD_REQUEST) record = get_latest_user_agreement_record(request.user, agreement_type, params.cleaned_data.get('after')) if record is None: return Response(status=status.HTTP_404_NOT_FOUND) - serializer = UserAgreementsSerializer(record) + serializer = UserAgreementRecordSerializer(record) return Response(serializer.data) @apidocs.schema( @@ -224,7 +226,7 @@ def get(self, request, agreement_type): ), ], responses={ - 200: UserAgreementsSerializer, + 200: UserAgreementRecordSerializer, 400: "Bad Request", }, ) @@ -233,5 +235,66 @@ def post(self, request, agreement_type): Marks a user's acknowledgement of this agreement type. """ record = create_user_agreement_record(request.user, agreement_type) - serializer = UserAgreementsSerializer(record) + serializer = UserAgreementRecordSerializer(record) return Response(serializer.data, status=status.HTTP_201_CREATED) + + +@view_auth_classes(is_authenticated=True) +class UserAgreementsViewSet(viewsets.GenericViewSet): + """ + Endpoint for the user agreements API. + """ + + @apidocs.schema( + parameters=[ + apidocs.string_parameter( + 'agreement_type', + apidocs.ParameterLocation.PATH, + description="Agreement ID/Type", + ), + ], + responses={ + 200: UserAgreementSerializer, + 400: "Bad Request", + 404: "Not Found", + }, + ) + def get(self, request, agreement_type): + """ + Get all user agreements for this agreement type. + """ + agreement = UserAgreement.objects.get(type=agreement_type) + # if agreement is None: + # return Response(status=status.HTTP_404_NOT_FOUND) + serializer = UserAgreementSerializer(agreement) + return Response(serializer.data) + + @apidocs.schema( + parameters=[ + openapi.Parameter( + 'agreement_type', + apidocs.ParameterLocation.QUERY, + required=False, + type=openapi.TYPE_ARRAY, + items=openapi.Items(type=openapi.TYPE_STRING), + description="Agreement ID/Type", + ), + ], + responses={ + 200: UserAgreementSerializer, + 400: "Bad Request", + 404: "Not Found", + }, + ) + def list(self, request): + """ + Get all user agreements for this agreement type. + """ + types = request.query_params.getlist('agreement_type', None) + agreements = UserAgreement.objects.all() + if types: + agreements = agreements.filter(type__in=types) + # if agreement is None: + # return Response(status=status.HTTP_404_NOT_FOUND) + serializer = UserAgreementSerializer(agreements, many=True) + return Response(serializer.data)