Skip to content

Commit 6f8fd6d

Browse files
committed
refactor: make draft history models consistent with publishing
1 parent 2891ee2 commit 6f8fd6d

8 files changed

Lines changed: 127 additions & 101 deletions

File tree

openedx_learning/apps/authoring/publishing/admin.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010
from openedx_learning.lib.admin_utils import ReadOnlyModelAdmin, one_to_one_related_model_html
1111

1212
from .models import (
13-
DraftChange,
14-
DraftChangeSet,
13+
DraftChangeLogRecord,
14+
DraftChangeLog,
1515
LearningPackage,
1616
PublishableEntity,
1717
PublishLog,
@@ -179,7 +179,7 @@ def message(self, published_obj):
179179

180180

181181
class DraftChangeTabularInline(admin.TabularInline):
182-
model = DraftChange
182+
model = DraftChangeLogRecord
183183

184184
fields = (
185185
"entity",
@@ -194,17 +194,17 @@ def get_queryset(self, request):
194194
return queryset.select_related("entity", "old_version", "new_version") \
195195
.order_by("entity__key")
196196

197-
def old_version_num(self, draft_change: DraftChange):
197+
def old_version_num(self, draft_change: DraftChangeLogRecord):
198198
if draft_change.old_version is None:
199199
return "-"
200200
return draft_change.old_version.version_num
201201

202-
def new_version_num(self, draft_change: DraftChange):
202+
def new_version_num(self, draft_change: DraftChangeLogRecord):
203203
if draft_change.new_version is None:
204204
return "-"
205205
return draft_change.new_version.version_num
206206

207-
def title(self, draft_change: DraftChange):
207+
def title(self, draft_change: DraftChangeLogRecord):
208208
"""
209209
Get the title to display for the DraftChange
210210
"""
@@ -215,7 +215,7 @@ def title(self, draft_change: DraftChange):
215215
return ""
216216

217217

218-
@admin.register(DraftChangeSet)
218+
@admin.register(DraftChangeLog)
219219
class DraftChangeSetAdmin(ReadOnlyModelAdmin):
220220
"""
221221
Read-only admin to view Draft changes (via inline tables)

openedx_learning/apps/authoring/publishing/api.py

Lines changed: 46 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,13 @@
1616

1717
from .models.publish_log import Published
1818

19-
from .contextmanagers import DraftChangeSetContext
19+
from .contextmanagers import DraftChangeLogContext
2020
from .models import (
2121
Container,
2222
ContainerVersion,
2323
Draft,
24-
DraftChange,
25-
DraftChangeSet,
24+
DraftChangeLogRecord,
25+
DraftChangeLog,
2626
EntityList,
2727
EntityListRow,
2828
LearningPackage,
@@ -454,6 +454,10 @@ def set_draft_version(
454454
edited some content). Setting a Draft's version to None is like deleting it
455455
from Studio's editing point of view (see ``soft_delete_draft`` for more
456456
details).
457+
458+
Calling this function attaches a new DraftChange and attaches it to a
459+
DraftChangeLog.
460+
457461
"""
458462
if set_at is None:
459463
set_at = datetime.now(tz=timezone.utc)
@@ -467,54 +471,56 @@ def set_draft_version(
467471
learning_package_id = draft.entity.learning_package_id
468472

469473
# Check to see if we're inside a context manager for an active
470-
# DraftChangeSet (i.e. what happens if the caller is using the public
471-
# draft_changes_for() API call), or if we have to make our own.
472-
active_change_set = DraftChangeSetContext.get_active_draft_change_set(learning_package_id)
474+
# DraftChangeLog (i.e. what happens if the caller is using the public
475+
# bulk_draft_changes_for() API call), or if we have to make our own.
476+
active_change_log = DraftChangeLogContext.get_active_draft_change_log(learning_package_id)
473477

474-
if active_change_set:
475-
# If we get here, it means there is an active DraftChangeSet that may
476-
# have many DraftChanges associated with it. A DraftChangeSet can only
478+
if active_change_log:
479+
# If we get here, it means there is an active DraftChangeLog that may
480+
# have many DraftChanges associated with it. A DraftChangeLog can only
477481
# have one DraftChange per PublishableEntity, e.g. the same Component
478-
# can't go from v1 to v3 and v1 to v4 in the same DraftChangeSet.
482+
# can't go from v1 to v3 and v1 to v4 in the same DraftChangeLog.
479483
try:
480484
# If there was already a DraftChange for this PublishableEntity,
481485
# we update the new_version_id. We keep the old_version_id value
482486
# though, because that represents what it was before this
483-
# DraftChangeSet, and we don't want to lose that information.
484-
change = DraftChange.objects.get(
485-
change_set=active_change_set,
487+
# DraftChangeLog, and we don't want to lose that information.
488+
change = DraftChangeLogRecord.objects.get(
489+
draft_change_log=active_change_log,
486490
entity_id=publishable_entity_id,
487491
)
488492
change.new_version_id = publishable_entity_version_pk
489493
change.save()
490-
except DraftChange.DoesNotExist:
494+
except DraftChangeLogRecord.DoesNotExist:
491495
# If we're here, this is the first DraftChange we're making for
492-
# this PublishableEntity in the active DraftChangeSet.
493-
change = DraftChange.objects.create(
494-
change_set=active_change_set,
496+
# this PublishableEntity in the active DraftChangeLog.
497+
change = DraftChangeLogRecord.objects.create(
498+
draft_change_log=active_change_log,
495499
entity_id=publishable_entity_id,
496500
old_version_id=old_version_id,
497501
new_version_id=publishable_entity_version_pk,
498502
)
499503
else:
500-
# This means there is no active DraftChangeSet, so we create our own
504+
# This means there is no active DraftChangeLog, so we create our own
501505
# add add our DraftChange to it. This has the minor optimization
502506
# that we don't have to check for an existing DraftChange, because
503-
# we're creating the whole DraftChangeSet right here.
504-
new_change_set = DraftChangeSet.objects.create(
507+
# we're creating the whole DraftChangeLog right here.
508+
new_change_log = DraftChangeLog.objects.create(
505509
learning_package_id=learning_package_id,
506510
changed_at=set_at,
507511
changed_by_id=set_by,
508512
)
509-
change = DraftChange.objects.create(
510-
change_set=new_change_set,
513+
change = DraftChangeLogRecord.objects.create(
514+
draft_change_log=new_change_log,
511515
entity_id=publishable_entity_id,
512516
old_version_id=old_version_id,
513517
new_version_id=publishable_entity_version_pk,
514518
)
515519

516520
# One way or another, we've created our DraftChange at this point. Now
517-
# to see if we need to add any parent Draft containers...
521+
# to see if we need to add any parent Draft containers that this change
522+
# may have affected (i.e. all unpinned references from Draft containers
523+
# to the entity that we just changed the Draft entry for)...
518524
containers = get_containers_with_entity(publishable_entity_id, ignore_pinned=True)
519525

520526

@@ -585,7 +591,7 @@ def reset_drafts_to_published(
585591
)
586592
# If there's nothing to reset because there are no changes from the
587593
# published version, just return early rather than making an empty
588-
# DraftChangeSet.
594+
# DraftChangeLog.
589595
if not draft_qset:
590596
return
591597

@@ -594,8 +600,8 @@ def reset_drafts_to_published(
594600
# rework this into a bulk update or custom SQL if it becomes a performance
595601
# issue.
596602
with atomic():
597-
active_change_set = DraftChangeSetContext.get_active_draft_change_set(learning_package_id)
598-
change_set = active_change_set or DraftChangeSet.objects.create(
603+
active_change_log = DraftChangeLogContext.get_active_draft_change_log(learning_package_id)
604+
change_log = active_change_log or DraftChangeLog.objects.create(
599605
learning_package_id=learning_package_id,
600606
changed_at=reset_at,
601607
changed_by_id=reset_by,
@@ -607,32 +613,32 @@ def reset_drafts_to_published(
607613
else:
608614
published_version_id = None
609615

610-
if active_change_set:
616+
if active_change_log:
611617
# We're inside a draft_changes_for() context call and we need to
612-
# attach our DraftChanges to an existing DraftChangeSet.
618+
# attach our DraftChanges to an existing DraftChangeLog.
613619
try:
614620
# If there was already a DraftChange for this publishable
615621
# entity, we update the new_version_id. We keep the
616622
# old_version_id value though, because that represents what
617-
# it was before this DraftChangeSet.
618-
change = DraftChange.objects.get(
619-
change_set=active_change_set,
623+
# it was before this DraftChangeLog.
624+
change = DraftChangeLogRecord.objects.get(
625+
draft_change_log=active_change_log,
620626
entity_id=draft.entity_id,
621627
)
622628
change.new_version_id = published_version_id
623629
change.save()
624-
except DraftChange.DoesNotExist:
630+
except DraftChangeLogRecord.DoesNotExist:
625631
# If we're here, this is the first DraftChange we're making for this
626-
# PublishableEntity in this DraftChangeSet.
627-
DraftChange.objects.create(
628-
change_set=active_change_set,
632+
# PublishableEntity in this DraftChangeLog.
633+
DraftChangeLogRecord.objects.create(
634+
draft_change_log=active_change_log,
629635
entity_id=draft.entity_id,
630636
old_version_id=draft.version_id,
631637
new_version_id=published_version_id,
632638
)
633639
else:
634-
DraftChange.objects.create(
635-
change_set=change_set,
640+
DraftChangeLogRecord.objects.create(
641+
draft_change_log=change_log,
636642
entity_id=draft.entity_id,
637643
old_version_id=draft.version_id,
638644
new_version_id=published_version_id,
@@ -1082,6 +1088,7 @@ def get_containers_with_entity(
10821088
ignore_pinned: if true, ignore any pinned references to the entity.
10831089
"""
10841090
if ignore_pinned:
1091+
# TODO: Do we need to run distinct() on this?
10851092
qs = Container.objects.filter(
10861093
# Note: these two conditions must be in the same filter() call, or the query won't be correct.
10871094
publishable_entity__draft__version__containerversion__entity_list__entitylistrow__entity_id=publishable_entity_pk, # pylint: disable=line-too-long # noqa: E501
@@ -1100,10 +1107,10 @@ def get_containers_with_entity(
11001107
return qs
11011108

11021109

1103-
def bulk_draft_changes_for(learning_package_id: int) -> DraftChangeSet:
1110+
def bulk_draft_changes_for(learning_package_id: int) -> DraftChangeLog:
11041111
"""
11051112
Context manager to do a single batch of Draft changes in.
11061113
11071114
TODO: better description here with example usage.
11081115
"""
1109-
return DraftChangeSetContext(learning_package_id)
1116+
return DraftChangeLogContext(learning_package_id)

openedx_learning/apps/authoring/publishing/contextmanagers.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33

44
from django.db.transaction import Atomic
55

6-
from .models import DraftChangeSet
6+
from .models import DraftChangeLog
77

88

9-
class DraftChangeSetContext(Atomic):
9+
class DraftChangeLogContext(Atomic):
1010
"""
1111
Context manager for batching draft changes into DraftChangeSets.
1212
@@ -50,7 +50,7 @@ def __init__(self, learning_package_id, changed_by=None, changed_at=None):
5050
self.changed_at = changed_at or datetime.now(tz=timezone.utc)
5151

5252
@classmethod
53-
def get_active_draft_change_set(cls, learning_package_id: int) -> DraftChangeSet | None:
53+
def get_active_draft_change_log(cls, learning_package_id: int) -> DraftChangeLog | None:
5454
"""
5555
Get the DraftChangeSet that new DraftChanges should be attached to.
5656
@@ -88,7 +88,7 @@ def get_active_draft_change_set(cls, learning_package_id: int) -> DraftChangeSet
8888
def __enter__(self):
8989
super().__enter__()
9090

91-
self.draft_change_set = DraftChangeSet.objects.create(
91+
self.draft_change_set = DraftChangeLog.objects.create(
9292
learning_package_id=self.learning_package_id,
9393
changed_by=self.changed_by,
9494
changed_at=self.changed_at,

openedx_learning/apps/authoring/publishing/migrations/0004_draftchangeset_and_more.py renamed to openedx_learning/apps/authoring/publishing/migrations/0004_draftchangelog.py

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Generated by Django 4.2.18 on 2025-03-18 01:15
1+
# Generated by Django 4.2.18 on 2025-03-27 20:01
22

33
from django.conf import settings
44
from django.db import migrations, models
@@ -16,7 +16,7 @@ class Migration(migrations.Migration):
1616

1717
operations = [
1818
migrations.CreateModel(
19-
name='DraftChangeSet',
19+
name='DraftChangeLog',
2020
fields=[
2121
('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
2222
('uuid', models.UUIDField(default=uuid.uuid4, editable=False, unique=True, verbose_name='UUID')),
@@ -30,22 +30,33 @@ class Migration(migrations.Migration):
3030
},
3131
),
3232
migrations.CreateModel(
33-
name='DraftChange',
33+
name='DraftChangeLogRecord',
3434
fields=[
3535
('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
36-
('change_set', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='changes', to='oel_publishing.draftchangeset')),
36+
('draft_change_log', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='changes', to='oel_publishing.draftchangelog')),
3737
('entity', models.ForeignKey(on_delete=django.db.models.deletion.RESTRICT, to='oel_publishing.publishableentity')),
3838
('new_version', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.RESTRICT, to='oel_publishing.publishableentityversion')),
3939
('old_version', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.RESTRICT, related_name='+', to='oel_publishing.publishableentityversion')),
4040
],
4141
options={
42-
'verbose_name': 'Draft Change',
43-
'verbose_name_plural': 'Draft Changes',
44-
'indexes': [models.Index(fields=['entity', '-change_set'], name='oel_dc_idx_entity_rchangeset')],
42+
'verbose_name': 'Draft Log',
43+
'verbose_name_plural': 'Draft Log',
4544
},
4645
),
46+
migrations.CreateModel(
47+
name='DraftSideEffect',
48+
fields=[
49+
('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
50+
('cause', models.ForeignKey(on_delete=django.db.models.deletion.RESTRICT, related_name='+', to='oel_publishing.draftchangelogrecord')),
51+
('effect', models.ForeignKey(on_delete=django.db.models.deletion.RESTRICT, related_name='+', to='oel_publishing.draftchangelogrecord')),
52+
],
53+
),
54+
migrations.AddIndex(
55+
model_name='draftchangelogrecord',
56+
index=models.Index(fields=['entity', '-draft_change_log'], name='oel_dlr_idx_entity_rdcl'),
57+
),
4758
migrations.AddConstraint(
48-
model_name='draftchange',
49-
constraint=models.UniqueConstraint(fields=('change_set', 'entity'), name='oel_dc_uniq_changeset_entity'),
59+
model_name='draftchangelogrecord',
60+
constraint=models.UniqueConstraint(fields=('draft_change_log', 'entity'), name='oel_dlr_uniq_dcl'),
5061
),
5162
]

0 commit comments

Comments
 (0)