feat: User agreements API for generic agreement records #35895

xitij2000 · 2024-11-20T12:48:40Z

Description

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.

Supporting information

Product Review: openedx/platform-roadmap#465

Testing instructions

This creates a new REST API at /api/agreements/v1/agreement/<agreement_id>
You can use GET, which will return a 404 if a user hasn't acknowledged the agreement, or the timestamp of acceptance if they have
You can do a simple empty POST to mark acknowledgement
You can use the ?after=... query param with get to filter agreements after that date

Deadline

"None"

openedx-webhooks · 2024-11-20T12:48:45Z

Thanks for the pull request, @xitij2000!

This repository is currently maintained by @openedx/wg-maintenance-openedx-platform.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

🔘 Update the status of your PR

Your PR is currently marked as a draft. After completing the steps above, update its status by clicking "Ready for Review", or removing "WIP" from the title, as appropriate.

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

openedx/core/djangoapps/agreements/api.py

openedx/core/djangoapps/agreements/serializers.py

kaustavb12 · 2024-11-29T09:47:35Z

👍

I tested this: Tested in local setup
I read through the code

xitij2000 · 2025-03-26T18:32:53Z

@mphilbrick211 We were just discussing internally about what the process for reviewing such a PR should be. It doesn't fall into the product review category, but it does introduce a new API.

feanil · 2025-06-09T18:52:20Z

@xitij2000 there is the API but I think the fact that we're introducing a model that tracks arbitrary agreements is a product feature that should get some sort of product review, your open question about whether the content of the agreement should be stored here or not is a good one. I could also imagine having the aggreements stored in their own model that this model foreign keys to for efficiency also. The other aggerements being tracked here are explicity so I think discussing why it's useful for the platform to support generic aggrements in the core is an open question.

xitij2000 · 2025-06-11T08:57:29Z

@feanil Sure, I will allocate some time to move this through product review.

Re: Storing agreements in a model. For our client's use case, this would need to be a link to an agreement page, in which case having that being in siteconfiguration was sufficient. Perhaps instead it can be an Agreement that can either have text or an external link with an update date.

In any case I'll allocate some time next sprint for this.

feanil · 2025-06-11T17:18:20Z

Another question to answer in your product review is why this needs to be a core product feature rather than a backend plugin that introduces the new agreements models you need?

xitij2000 · 2025-06-12T05:40:31Z

@feanil I don't think it particularly needs to be a core product feature other than that being ideal if such a feature is more widely desired. This would be pretty easy to extract out into a plugin.

feanil · 2025-06-20T19:07:46Z

@xitij2000 I don't think we need this as a core product feature right now, let's make it a plugin for now and if we need to make updates to the core we can do that in order to enable this plugin.

itsjeyd · 2025-06-25T08:54:54Z

@feanil Would there be an option for the plugin repo to be added to the openedx organization in the medium/long term? Speaking in terms of OEP-57, I guess another way to phrase this question would be, do you see a path forward where the plugin could become an official extension?

CC @xitij2000 @viadanna @kaustavb12 @cassiezamparini

feanil · 2025-07-17T16:53:14Z

@itsjeyd yes, I could see a path if this was something that the Product team wanted. I think to make that decision this would have to go through the product proposal process.

itsjeyd · 2025-07-24T13:56:51Z

Thanks @feanil, we'll go that route.

mphilbrick211 · 2025-08-27T14:19:26Z

Hi @itsjeyd @xitij2000! Is this still in progress?

xitij2000 · 2025-08-28T06:43:45Z

@mphilbrick211 We currently have a product proposal being worked on for this. I will post the link here once one exists.

feanil · 2025-08-28T20:44:08Z

@xitij2000 I moved this to draft until the product proposal process concludes.

itsjeyd · 2025-11-04T15:29:18Z

Product review for this PR is complete.

Product review ticket: openedx/platform-roadmap#465

@xitij2000 Could you link to the product review ticket from the PR description as well? (And remove the Open Questions section while you're at it, it's probably not needed any longer?)

xitij2000 · 2025-11-12T12:41:47Z

Product review for this PR is complete.

Product review ticket: openedx/platform-roadmap#465

@xitij2000 Could you link to the product review ticket from the PR description as well? (And remove the Open Questions section while you're at it, it's probably not needed any longer?)

I've made the updates, but I'm not clear on the next step here. Is this approved for merging? Or do we need to move it to a plugin?

itsjeyd · 2025-11-12T14:17:28Z

@xitij2000

I've made the updates,

Thanks!

but I'm not clear on the next step here. Is this approved for merging? Or do we need to move it to a plugin?

Right now, neither. Please check this update and the conversations/comments that it links to.

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.

farhaanbukhsh

I have left few comments possibly we can squash the migrations in one.

farhaanbukhsh · 2026-01-28T11:53:13Z

openedx/core/djangoapps/agreements/serializers.py

+    updated = serializers.DateTimeField(read_only=True)
+
+
+class UserAgreementRecordSerializer(serializers.Serializer):


@xitij2000 any specific reason for not using ModelSerializer? I think it might help here.

farhaanbukhsh · 2026-01-28T11:55:50Z

openedx/core/djangoapps/agreements/views.py

+from .models import UserAgreement
+from .serializers import IntegritySignatureSerializer, LTIPIISignatureSerializer, UserAgreementRecordSerializer, \
+    UserAgreementSerializer
+from ...lib.api.view_utils import view_auth_classes


Why are we changing this to relative imports?

farhaanbukhsh · 2026-01-28T11:58:30Z

openedx/core/djangoapps/agreements/views.py

+        if record is None:
+            return Response(status=status.HTTP_404_NOT_FOUND)
+        serializer = UserAgreementRecordSerializer(record)
+        return Response(serializer.data)


Suggested change

return Response(serializer.data)

return Response(serializer.data, status=...)

we should pass status here.

farhaanbukhsh · 2026-01-28T11:59:43Z

openedx/core/djangoapps/agreements/views.py

+        # if agreement is None:
+        #     return Response(status=status.HTTP_404_NOT_FOUND)
+        serializer = UserAgreementSerializer(agreements, many=True)
+        return Response(serializer.data)


Status missing here!

https://github.com/openedx/openedx-platform/pull/35895/changes#r2736307512

What do we do when there is no list?

farhaanbukhsh · 2026-01-28T12:04:30Z

openedx/core/djangoapps/agreements/serializers.py

-from openedx.core.djangoapps.agreements.models import IntegritySignature, LTIPIISignature
 from openedx.core.lib.api.serializers import CourseKeyField

+from .models import IntegritySignature, LTIPIISignature


Relative imports?

farhaanbukhsh · 2026-01-28T12:26:17Z

openedx/core/djangoapps/agreements/api.py

+    """
+    Retrieves all the agreements that the specified user has acknowledged.
+    """
+    for agreement_record in UserAgreementRecord.objects.filter(user=user).select_related("agreement"):


Suggested change

for agreement_record in UserAgreementRecord.objects.filter(user=user).select_related("agreement"):

for agreement_record in UserAgreementRecord.objects.filter(user=user).select_related("agreement", "user"):

Since we access the username in the dataclass, if we don't use this, there will be multiple queries to fetch users.

farhaanbukhsh · 2026-01-28T12:30:46Z

openedx/core/djangoapps/agreements/models.py

        app_label = 'agreements'
+
+
+class UserAgreement(models.Model):


Should we use "TimeStampedModel" here, which will give us created and updated dates

farhaanbukhsh · 2026-01-28T12:34:48Z

openedx/core/djangoapps/agreements/api.py

        return user_lti_pii_signature_hash != course_lti_pii_tools_hash
+
+
+def get_user_agreement_records(user: User) -> Iterable[UserAgreementRecordData]:


Suggested change

def get_user_agreement_records(user: User) -> Iterable[UserAgreementRecordData]:

def get_user_agreement_records(user: User) ->Iterator[UserAgreementRecordData]:

farhaanbukhsh · 2026-01-28T12:38:38Z

openedx/core/djangoapps/agreements/api.py

+    try:
+        record_query = UserAgreementRecord.objects.filter(
+            user=user,
+            agreement__type=agreement_type,
+        )
+        record = record_query.latest("timestamp")
+        return UserAgreementRecordData.from_model(record)
+    except UserAgreementRecord.DoesNotExist:
+        return None


Suggested change

try:

record_query = UserAgreementRecord.objects.filter(

user=user,

agreement__type=agreement_type,

)

record = record_query.latest("timestamp")

return UserAgreementRecordData.from_model(record)

except UserAgreementRecord.DoesNotExist:

return None

user_agreement_records = None

record_query = UserAgreementRecord.objects.filter(

user=user,

agreement__type=agreement_type,

)

record = record_query.latest("timestamp")

if record.exists():

user_agreement_records = UserAgreementRecordData.from_model(record)

return user_agreement_records

farhaanbukhsh · 2026-01-28T12:41:07Z

openedx/core/djangoapps/agreements/api.py

+    Creates a user agreement record if one doesn't already exist, or updates existing
+    record to current timestamp.


If we are creating or updating, should we use create_or_update instead? ref: https://docs.djangoproject.com/en/6.0/ref/models/querysets/#update-or-create

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Nov 20, 2024

xitij2000 force-pushed the kshitij/acknowledgements branch 4 times, most recently from 5ce0319 to da0fa90 Compare November 27, 2024 09:41

kaustavb12 reviewed Nov 28, 2024

View reviewed changes

openedx/core/djangoapps/agreements/api.py Outdated Show resolved Hide resolved

openedx/core/djangoapps/agreements/serializers.py Show resolved Hide resolved

xitij2000 force-pushed the kshitij/acknowledgements branch from da0fa90 to 8d6d690 Compare November 28, 2024 11:40

kaustavb12 approved these changes Nov 29, 2024

View reviewed changes

xitij2000 marked this pull request as draft November 29, 2024 11:58

xitij2000 force-pushed the kshitij/acknowledgements branch from 1806428 to dc0383f Compare December 20, 2024 08:58

xitij2000 marked this pull request as ready for review January 21, 2025 09:01

xitij2000 force-pushed the kshitij/acknowledgements branch from dc0383f to d410b5b Compare January 21, 2025 09:01

mphilbrick211 moved this from Waiting on Author to Ready to Merge in Contributions Mar 3, 2025

mphilbrick211 added the needs reviewer assigned PR needs to be (re-)assigned a new reviewer label Mar 3, 2025

xitij2000 force-pushed the kshitij/acknowledgements branch from d410b5b to de85bd2 Compare March 17, 2025 12:32

xitij2000 force-pushed the kshitij/acknowledgements branch from de85bd2 to e4d14d0 Compare March 26, 2025 18:30

xitij2000 force-pushed the kshitij/acknowledgements branch from e4d14d0 to 65bd7ae Compare June 2, 2025 04:07

openedx-webhooks added the core contributor PR author is a Core Contributor (who may or may not have write access to this repo). label Jun 2, 2025

xitij2000 force-pushed the kshitij/acknowledgements branch from 65bd7ae to f841b1d Compare June 11, 2025 08:52

xitij2000 force-pushed the kshitij/acknowledgements branch from f841b1d to 969a3b8 Compare June 12, 2025 05:40

feanil marked this pull request as draft August 28, 2025 20:43

mphilbrick211 added product review PR requires product review before merging and removed needs reviewer assigned PR needs to be (re-)assigned a new reviewer labels Sep 12, 2025

mphilbrick211 moved this from Ready to Merge to Product Review in Contributions Sep 12, 2025

Cup0fCoffee mentioned this pull request Oct 12, 2025

feat: prepare teak branch open-craft/openedx-platform#793

Merged

itsjeyd mentioned this pull request Oct 29, 2025

feat: plugin slots for video and file upload components and alerts openedx/frontend-app-authoring#1523

Merged

itsjeyd added product review complete PR has gone through product review and removed product review PR requires product review before merging labels Nov 4, 2025

itsjeyd moved this from Product Review to Waiting on Author in Contributions Nov 4, 2025

xitij2000 added 2 commits January 28, 2026 15:41

feat: User agreements API for generic agreement records

3b51154

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.

feat: user agreement api

1abd876

xitij2000 force-pushed the kshitij/acknowledgements branch from d6aaed2 to 1abd876 Compare January 28, 2026 10:11

farhaanbukhsh requested changes Jan 28, 2026

View reviewed changes

		updated = serializers.DateTimeField(read_only=True)


		class UserAgreementRecordSerializer(serializers.Serializer):

	return Response(serializer.data)
	return Response(serializer.data, status=...)

	for agreement_record in UserAgreementRecord.objects.filter(user=user).select_related("agreement"):
	for agreement_record in UserAgreementRecord.objects.filter(user=user).select_related("agreement", "user"):

		return user_lti_pii_signature_hash != course_lti_pii_tools_hash


		def get_user_agreement_records(user: User) -> Iterable[UserAgreementRecordData]:

		Creates a user agreement record if one doesn't already exist, or updates existing
		record to current timestamp.

feat: User agreements API for generic agreement records #35895

Are you sure you want to change the base?

feat: User agreements API for generic agreement records #35895

Uh oh!

Conversation

xitij2000 commented Nov 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Supporting information

Testing instructions

Deadline

Uh oh!

openedx-webhooks commented Nov 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kaustavb12 commented Nov 29, 2024

Uh oh!

xitij2000 commented Mar 26, 2025

Uh oh!

feanil commented Jun 9, 2025

Uh oh!

xitij2000 commented Jun 11, 2025

Uh oh!

feanil commented Jun 11, 2025

Uh oh!

xitij2000 commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

feanil commented Jun 20, 2025

Uh oh!

itsjeyd commented Jun 25, 2025

Uh oh!

feanil commented Jul 17, 2025

Uh oh!

itsjeyd commented Jul 24, 2025

Uh oh!

mphilbrick211 commented Aug 27, 2025

Uh oh!

xitij2000 commented Aug 28, 2025

Uh oh!

feanil commented Aug 28, 2025

Uh oh!

itsjeyd commented Nov 4, 2025

Uh oh!

xitij2000 commented Nov 12, 2025

Uh oh!

itsjeyd commented Nov 12, 2025

Uh oh!

farhaanbukhsh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

xitij2000 commented Nov 20, 2024 •

edited

Loading

openedx-webhooks commented Nov 20, 2024 •

edited

Loading

xitij2000 commented Jun 12, 2025 •

edited

Loading