Skip to content

Commit

Permalink
Merge pull request #60 from openedx/pwnage101/ENT-8389
Browse files Browse the repository at this point in the history
feat: Add parent_content_key field to Transaction model
  • Loading branch information
pwnage101 authored Feb 16, 2024
2 parents 4856206 + 88dce61 commit 4894dc5
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 2 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ Unreleased
**********
* Nothing unreleased

[1.4.0]
*******
* feat: Add parent_content_key field to Transaction model (ENT-8389)

[1.3.3]
*******
* Upgrade requirements
Expand Down
25 changes: 25 additions & 0 deletions docs/decisions/0008-parent-content-key.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
0008 ``parent_content_key`` Field
#################################

Status
******
**Accepted** (February 2024)

Context
*******
Today, we store a ``Transaction.content_key`` field which points at the content into which a learner is enrolled.
Currently in the openedx ecosystem that would be a "course run key". However, we'd also like to know the parent
identifier for the content, most often the "course key" associated with a "course run key" within the openedx ecosystem.
In some downstream systems and frontends which consume transaction data it is essential to know the parent content key,
but dynamically fetching it from APIs can be slow when working with many transactions.

Decision
********
We'll introduce a ``parent_content_key`` field to the ``Transaction`` model. Similarly to ``content_title``, this new
field will essentially locally cache slow-changing data (content keys are arguably the slowest of slow-changing).

Consequences
************
* Creating transactions will additionally be required to pass a ``parent_content_key``, but this should be
straightforward since they already pass ``content_title``, which is already co-located with the parent content key
(``course_key``) within the system of record.
2 changes: 1 addition & 1 deletion openedx_ledger/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""
A library that records transactions against a ledger, denominated in units of value.
"""
__version__ = "1.3.3"
__version__ = "1.4.0"
5 changes: 5 additions & 0 deletions openedx_ledger/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ def create_transaction(
lms_user_id=None,
lms_user_email=None,
content_key=None,
parent_content_key=None,
content_title=None,
subsidy_access_policy_uuid=None,
state=models.TransactionStateChoices.CREATED,
Expand All @@ -65,6 +66,9 @@ def create_transaction(
content_key (str, Optional):
The identifier of the content into which the learner is enrolling. Skip if this does not represent a policy
enrolling a learner into content.
parent_content_key (str, Optional):
Identifier for the parent of the content_key. Skip if this does not represent a policy enrolling a learner
into content.
content_title (str, Optional):
The title of the content into which the learner is enrolling. Skip if this does not represent a policy
enrolling a learner into content or if the title is not readily available.
Expand Down Expand Up @@ -96,6 +100,7 @@ def create_transaction(
defaults={
"quantity": quantity,
"content_key": content_key,
"parent_content_key": parent_content_key,
"content_title": content_title,
"lms_user_id": lms_user_id,
"lms_user_email": lms_user_email,
Expand Down
23 changes: 23 additions & 0 deletions openedx_ledger/migrations/0010_transaction_parent_content_key.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Generated by Django 3.2.23 on 2024-02-15 16:22

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('openedx_ledger', '0009_add_email_and_title_to_transactions'),
]

operations = [
migrations.AddField(
model_name='historicaltransaction',
name='parent_content_key',
field=models.CharField(blank=True, db_index=True, help_text='Identifier for the parent of the content_key. Also should be joinable with ContentMetadata.content_key in enterprise-catalog. In practice, this is the course key corresponding to a course run key.', max_length=255, null=True),
),
migrations.AddField(
model_name='transaction',
name='parent_content_key',
field=models.CharField(blank=True, db_index=True, help_text='Identifier for the parent of the content_key. Also should be joinable with ContentMetadata.content_key in enterprise-catalog. In practice, this is the course key corresponding to a course run key.', max_length=255, null=True),
),
]
10 changes: 10 additions & 0 deletions openedx_ledger/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,16 @@ class Meta:
"The globally unique content identifier. Joinable with ContentMetadata.content_key in enterprise-catalog."
)
)
parent_content_key = models.CharField(
max_length=255,
blank=True,
null=True,
db_index=True,
help_text=(
"Identifier for the parent of the content_key. Also should be joinable with ContentMetadata.content_key "
"in enterprise-catalog. In practice, this is the course key corresponding to a course run key."
)
)
content_title = models.CharField(
max_length=255,
blank=True,
Expand Down
3 changes: 2 additions & 1 deletion openedx_ledger/test_utils/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ class Meta:
ledger = factory.Iterator(Ledger.objects.all())
lms_user_id = factory.Faker("random_int", min=1, max=1000)
lms_user_email = factory.Faker("email")
content_key = factory.Faker("lexify", text="???+?????101")
content_key = factory.LazyAttribute(lambda tx: f"course-v1:{tx.parent_content_key}+2023")
parent_content_key = factory.Faker("lexify", text="???+?????101")
content_title = factory.Faker("lexify", text="???: ?????? ???")


Expand Down
1 change: 1 addition & 0 deletions tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ def setUp(self):
lms_user_id=3,
lms_user_email='user@example.com',
content_key="course-v1:edX+test+course.3",
parent_content_key="edX+test",
content_title="Edx: test course 3",
quantity=-10,
state=models.TransactionStateChoices.PENDING,
Expand Down

0 comments on commit 4894dc5

Please sign in to comment.