-
Notifications
You must be signed in to change notification settings - Fork 23
feat: New history log api functions [FC-0123]
#501
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
ChrisChV
wants to merge
6
commits into
openedx:main
Choose a base branch
from
open-craft:chris/FAL-4330-history-log
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+949
−0
Draft
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
6f4b1dc
feat: `get_entity_draft_history` added
ChrisChV c492314
feat: contributors, publish groups and publish entries functions added
ChrisChV 3823f5a
fix: return draft and published entries of a component with discarted…
ChrisChV 8e270a7
feat: Pre-fetch the component type in the queries
ChrisChV 723fe11
feat: get_descendant_component_entity_ids function added
ChrisChV 1fefef3
Merge branch 'main' into chris/FAL-4330-history-log
ChrisChV File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ | |
| from enum import Enum | ||
| from typing import ContextManager, Optional, TypeVar | ||
|
|
||
| from django.contrib.auth import get_user_model | ||
| from django.core.exceptions import ObjectDoesNotExist, ValidationError | ||
| from django.db.models import F, Prefetch, Q, QuerySet | ||
| from django.db.transaction import atomic | ||
|
|
@@ -71,6 +72,10 @@ | |
| "publish_from_drafts", | ||
| "get_draft_version", | ||
| "get_published_version", | ||
| "get_entity_draft_history", | ||
| "get_entity_publish_history", | ||
| "get_entity_publish_history_entries", | ||
| "get_entity_version_contributors", | ||
| "set_draft_version", | ||
| "soft_delete_draft", | ||
| "reset_drafts_to_published", | ||
|
|
@@ -584,6 +589,271 @@ def get_published_version(publishable_entity_or_id: PublishableEntity | int, /) | |
| return published.version | ||
|
|
||
|
|
||
| def get_entity_draft_history( | ||
| publishable_entity_or_id: PublishableEntity | int, / | ||
| ) -> QuerySet[DraftChangeLogRecord]: | ||
| """ | ||
| Return DraftChangeLogRecords for a PublishableEntity since its last publication, | ||
| ordered from most recent to oldest. | ||
|
|
||
| Each record pre-fetches ``entity__component__component_type`` so callers can | ||
| access ``record.entity.component.component_type`` (namespace and name) without | ||
| extra queries. Note: accessing ``.component`` on a record whose entity backs a | ||
| Container rather than a Component will raise ``RelatedObjectDoesNotExist``. | ||
|
|
||
| Edge cases: | ||
| - Never published, no versions: returns an empty queryset. | ||
| - Never published, has versions: returns all DraftChangeLogRecords. | ||
| - No changes since the last publish: returns an empty queryset. | ||
| - Last publish was a soft-delete (Published.version=None): the Published row | ||
| still exists and its published_at timestamp is used as the lower bound, so | ||
| only draft changes made after that soft-delete publish are returned. If | ||
| there are no subsequent changes, the queryset is empty. | ||
| - Unpublished soft-delete (soft-delete in draft, not yet published): the | ||
| soft-delete DraftChangeLogRecord (new_version=None) is included because | ||
| it was made after the last real publish. | ||
| """ | ||
| if isinstance(publishable_entity_or_id, int): | ||
| entity_id = publishable_entity_or_id | ||
| else: | ||
| entity_id = publishable_entity_or_id.pk | ||
|
|
||
| qs = ( | ||
| DraftChangeLogRecord.objects | ||
| .filter(entity_id=entity_id) | ||
| .select_related( | ||
| "draft_change_log__changed_by", | ||
| "entity__component__component_type", | ||
| "old_version", | ||
| "new_version", | ||
| ) | ||
| .order_by("-draft_change_log__changed_at") | ||
| ) | ||
|
|
||
| # Narrow to changes since the last publication (or last reset to published) | ||
| try: | ||
| published = Published.objects.select_related( | ||
| "publish_log_record__publish_log" | ||
| ).get(entity_id=entity_id) | ||
| published_at = published.publish_log_record.publish_log.published_at | ||
| published_version_id = published.version_id | ||
|
|
||
| # If reset_drafts_to_published() was called after the last publish, | ||
| # there will be a DraftChangeLogRecord where new_version == published | ||
| # version. Use the most recent such record's timestamp as the lower | ||
| # bound so that discarded entries no longer appear in the draft history. | ||
| last_reset_at = ( | ||
| DraftChangeLogRecord.objects | ||
| .filter( | ||
| entity_id=entity_id, | ||
| new_version_id=published_version_id, | ||
| draft_change_log__changed_at__gt=published_at, | ||
| ) | ||
| .order_by("-draft_change_log__changed_at") | ||
| .values_list("draft_change_log__changed_at", flat=True) | ||
| .first() | ||
| ) | ||
|
|
||
| lower_bound = last_reset_at if last_reset_at else published_at | ||
| qs = qs.filter(draft_change_log__changed_at__gt=lower_bound) | ||
| except Published.DoesNotExist: | ||
| pass | ||
|
|
||
| return qs | ||
|
|
||
|
|
||
| def get_entity_publish_history( | ||
| publishable_entity_or_id: PublishableEntity | int, / | ||
| ) -> QuerySet[PublishLogRecord]: | ||
| """ | ||
| Return all PublishLogRecords for a PublishableEntity, ordered most recent first. | ||
|
|
||
| Each record represents one publish event for this entity. old_version, | ||
| new_version, and ``entity__component__component_type`` are pre-fetched so | ||
| callers can access ``record.entity.component.component_type`` (namespace and | ||
| name) without extra queries. Note: accessing ``.component`` on a record whose | ||
| entity backs a Container rather than a Component will raise | ||
| ``RelatedObjectDoesNotExist``. | ||
|
|
||
| Edge cases: | ||
| - Never published: returns an empty queryset. | ||
| - Soft-delete published (new_version=None): the record is included with | ||
| old_version pointing to the last published version and new_version=None, | ||
| indicating the entity was removed from the published state. | ||
| - Multiple draft versions created between two publishes are compacted: each | ||
| PublishLogRecord captures only the version that was actually published, | ||
| not the intermediate draft versions. | ||
| """ | ||
| if isinstance(publishable_entity_or_id, int): | ||
| entity_id = publishable_entity_or_id | ||
| else: | ||
| entity_id = publishable_entity_or_id.pk | ||
|
|
||
| return ( | ||
| PublishLogRecord.objects | ||
| .filter(entity_id=entity_id) | ||
| .select_related( | ||
| "publish_log__published_by", | ||
| "entity__component__component_type", | ||
| "old_version", | ||
| "new_version", | ||
| ) | ||
| .order_by("-publish_log__published_at") | ||
| ) | ||
|
|
||
|
|
||
| def get_entity_publish_history_entries( | ||
| publishable_entity_or_id: PublishableEntity | int, | ||
| /, | ||
| publish_log_uuid: str, | ||
| ) -> QuerySet[DraftChangeLogRecord]: | ||
| """ | ||
| Return the DraftChangeLogRecords associated with a specific PublishLog. | ||
|
|
||
| Finds the PublishLogRecord for the given entity and publish_log_uuid, then | ||
| returns all DraftChangeLogRecords whose changed_at falls between the previous | ||
| publish for this entity (exclusive) and this publish (inclusive), ordered | ||
| most-recent-first. | ||
|
|
||
| Time bounds are used instead of version bounds because DraftChangeLogRecord | ||
| has no single version_num field (soft-delete records have new_version=None), | ||
| and using published_at timestamps cleanly handles all cases without extra | ||
| joins. | ||
|
|
||
| Each record pre-fetches ``entity__component__component_type`` so callers can | ||
| access ``record.entity.component.component_type`` (namespace and name) without | ||
| extra queries. Note: accessing ``.component`` on a record whose entity backs a | ||
| Container rather than a Component will raise ``RelatedObjectDoesNotExist``. | ||
|
|
||
| Edge cases: | ||
| - Each publish group is independent: only the DraftChangeLogRecords that | ||
| belong to the requested publish_log_uuid are returned; changes attributed | ||
| to other publish groups are excluded. | ||
| - Soft-delete publish (PublishLogRecord.new_version=None): the soft-delete | ||
| DraftChangeLogRecord (new_version=None) is included in the entries because | ||
| it falls within the time window of that publish group. | ||
|
|
||
| Raises PublishLogRecord.DoesNotExist if publish_log_uuid is not found for | ||
| this entity. | ||
| """ | ||
| if isinstance(publishable_entity_or_id, int): | ||
| entity_id = publishable_entity_or_id | ||
| else: | ||
| entity_id = publishable_entity_or_id.pk | ||
|
|
||
| # Fetch the PublishLogRecord for the requested PublishLog | ||
| pub_record = ( | ||
| PublishLogRecord.objects | ||
| .filter(entity_id=entity_id, publish_log__uuid=publish_log_uuid) | ||
| .select_related("publish_log") | ||
| .get() | ||
| ) | ||
| published_at = pub_record.publish_log.published_at | ||
|
|
||
| # Find the previous publish for this entity to use as the lower time bound | ||
| prev_pub_record = ( | ||
| PublishLogRecord.objects | ||
| .filter(entity_id=entity_id, publish_log__published_at__lt=published_at) | ||
| .select_related("publish_log") | ||
| .order_by("-publish_log__published_at") | ||
| .first() | ||
| ) | ||
| prev_published_at = prev_pub_record.publish_log.published_at if prev_pub_record else None | ||
|
|
||
| # All draft changes up to (and including) this publish's timestamp | ||
| draft_qs = ( | ||
| DraftChangeLogRecord.objects | ||
| .filter(entity_id=entity_id, draft_change_log__changed_at__lte=published_at) | ||
| .select_related( | ||
| "draft_change_log__changed_by", | ||
| "entity__component__component_type", | ||
| "old_version", | ||
| "new_version", | ||
| ) | ||
| .order_by("-draft_change_log__changed_at") | ||
| ) | ||
| # Exclude changes that belong to an earlier PublishLog's window | ||
| if prev_published_at: | ||
| draft_qs = draft_qs.filter(draft_change_log__changed_at__gt=prev_published_at) | ||
|
|
||
| # Find the baseline: the version that was published in the previous publish group | ||
| # (None if this is the first publish for this entity). | ||
| baseline_version_id = prev_pub_record.new_version_id if prev_pub_record else None | ||
|
|
||
| # If reset_drafts_to_published() was called within this publish window, there | ||
| # will be a DraftChangeLogRecord where new_version == baseline. Use the most | ||
| # recent such record as the new lower bound so discarded entries are excluded. | ||
| reset_filter = { | ||
| "entity_id": entity_id, | ||
| "new_version_id": baseline_version_id, | ||
| "draft_change_log__changed_at__lte": published_at, | ||
| } | ||
| if prev_published_at: | ||
| reset_filter["draft_change_log__changed_at__gt"] = prev_published_at | ||
|
|
||
| last_reset_at = ( | ||
| DraftChangeLogRecord.objects | ||
| .filter(**reset_filter) | ||
| .order_by("-draft_change_log__changed_at") | ||
| .values_list("draft_change_log__changed_at", flat=True) | ||
| .first() | ||
| ) | ||
| if last_reset_at: | ||
| draft_qs = draft_qs.filter(draft_change_log__changed_at__gt=last_reset_at) | ||
|
|
||
| return draft_qs | ||
|
|
||
|
|
||
| def get_entity_version_contributors( | ||
| publishable_entity_or_id: PublishableEntity | int, | ||
| /, | ||
| old_version_num: int, | ||
| new_version_num: int | None, | ||
| ) -> QuerySet: | ||
| """ | ||
| Return distinct User queryset of contributors (changed_by) for | ||
| DraftChangeLogRecords of a PublishableEntity after old_version_num. | ||
|
|
||
| If new_version_num is not None (normal publish), captures records where | ||
| new_version is between old_version_num (exclusive) and new_version_num (inclusive). | ||
|
|
||
| If new_version_num is None (soft delete published), captures both normal | ||
| edits after old_version_num AND the soft-delete record itself (identified | ||
| by new_version=None and old_version >= old_version_num). A soft-delete | ||
| record whose old_version falls before old_version_num is excluded. | ||
|
|
||
| Edge cases: | ||
| - If no DraftChangeLogRecords fall in the range, returns an empty queryset. | ||
| - Records with changed_by=None (system changes with no associated user) are | ||
| always excluded. | ||
| - A user who contributed multiple versions in the range appears only once | ||
| (results are deduplicated with DISTINCT). | ||
| """ | ||
| entity_id = publishable_entity_or_id if isinstance(publishable_entity_or_id, int) else publishable_entity_or_id.pk | ||
|
|
||
| if new_version_num is not None: | ||
| version_filter = Q( | ||
| new_version__version_num__gt=old_version_num, | ||
| new_version__version_num__lte=new_version_num, | ||
| ) | ||
| else: | ||
| # Soft delete: include edits after old_version_num + the soft-delete record | ||
| version_filter = ( | ||
| Q(new_version__version_num__gt=old_version_num) | | ||
| Q(new_version__isnull=True, old_version__version_num__gte=old_version_num) | ||
| ) | ||
|
|
||
| contributor_ids = ( | ||
| DraftChangeLogRecord.objects | ||
| .filter(entity_id=entity_id) | ||
| .filter(version_filter) | ||
| .exclude(draft_change_log__changed_by=None) | ||
| .values_list("draft_change_log__changed_by", flat=True) | ||
| .distinct() | ||
| ) | ||
| return get_user_model().objects.filter(pk__in=contributor_ids) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you think it is worth adding an |
||
|
|
||
|
|
||
| def set_draft_version( | ||
| draft_or_id: Draft | int, | ||
| publishable_entity_version_pk: int | None, | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know I was the one who flagged this because I wanted to make sure the code noted it, but I actually don't think we should filter out discarded changes. It's a log, and "discard changes" is a legit thing that someone in that chain of edits did. I think we should be explicit that it happened, especially if people are going here because they're thinking, "I know I was working on this, but what happened to my edits!?!" Then they could see "Dave discarded changes" and know who to
strangletalk to.