From 2481c0e670b58df2e74a6725c3a0a6ac7bd4ac32 Mon Sep 17 00:00:00 2001 From: "kshitij.sobti" Date: Wed, 20 Nov 2024 18:11:37 +0530 Subject: [PATCH] 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. (cherry picked from commit dc0383f6e38a3a7eb8e82ea1b18d0baaf66192c1) --- 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 2489cefb853..11ad23dddd2 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 9d843c73cb0..01d83665c00 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 00000000000..2e0985adb6d --- /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 461d7936c4b..db6e182fe2a 100644 --- a/openedx/core/djangoapps/agreements/models.py +++ b/openedx/core/djangoapps/agreements/models.py @@ -64,3 +64,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 11e9d57f405..0ecade7afd9 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 c6606578993..eb1e02956dc 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 4c52e5853f0..61cc8661fb4 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 d9d009d65ac..902f477a708 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 cc928669ffd..daf2ce09428 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)