From e8cd0035b05e6c2b53d6789e5f3e9d0e11895381 Mon Sep 17 00:00:00 2001 From: Rommy <255708385+cosmic-fire-eng@users.noreply.github.com> Date: Sun, 21 Jun 2026 16:21:23 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20fix(pages):=20use=20page.descrip?= =?UTF-8?q?tion=5Fjson=20in=20track=5Fpage=5Fversion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit track_page_version read page.description when populating a PageVersion. The description field was removed from the Page model, so every invocation raised AttributeError: 'Page' object has no attribute 'description'. Because the task body is wrapped in except Exception: log_exception(e); return, the failure was swallowed and no version was ever written — page version-history silently stopped working with no user-visible error. Use page.description_json (the current field) in both the update-existing and create-new version paths, and add unit tests covering the create path, in-place update, the no-op case, post-timeout new version, and a sentinel asserting Page has no bare description attribute. Co-Authored-By: Claude --- apps/api/plane/bgtasks/page_version_task.py | 4 +- .../unit/bg_tasks/test_page_version_task.py | 165 ++++++++++++++++++ 2 files changed, 167 insertions(+), 2 deletions(-) create mode 100644 apps/api/plane/tests/unit/bg_tasks/test_page_version_task.py diff --git a/apps/api/plane/bgtasks/page_version_task.py b/apps/api/plane/bgtasks/page_version_task.py index 7b41e3c445c..a09b1b39460 100644 --- a/apps/api/plane/bgtasks/page_version_task.py +++ b/apps/api/plane/bgtasks/page_version_task.py @@ -42,7 +42,7 @@ def track_page_version(page_id, existing_instance, user_id): ): page_version.description_html = page.description_html page_version.description_binary = page.description_binary - page_version.description_json = page.description + page_version.description_json = page.description_json page_version.description_stripped = page.description_stripped page_version.sub_pages_data = sub_pages page_version.save( @@ -60,7 +60,7 @@ def track_page_version(page_id, existing_instance, user_id): PageVersion.objects.create( page_id=page_id, workspace_id=page.workspace_id, - description_json=page.description, + description_json=page.description_json, description_html=page.description_html, description_binary=page.description_binary, description_stripped=page.description_stripped, diff --git a/apps/api/plane/tests/unit/bg_tasks/test_page_version_task.py b/apps/api/plane/tests/unit/bg_tasks/test_page_version_task.py new file mode 100644 index 00000000000..09ee48ce963 --- /dev/null +++ b/apps/api/plane/tests/unit/bg_tasks/test_page_version_task.py @@ -0,0 +1,165 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +""" +Unit tests for the page-version tracking task. + +``track_page_version`` populates a ``PageVersion`` snapshot from a ``Page``. +The task body is wrapped in a broad ``except Exception: log_exception(e)``, +so any attribute error inside it is swallowed and simply results in *no* +version being written -- silently breaking page version-history with no +user-visible error. + +These tests pin the regression: the task must read the page's +``description_json`` field (not the long-removed ``description`` field), +must not raise, and must persist that JSON onto the created/updated +``PageVersion`` row. +""" + +import json + +import pytest +from django.utils import timezone + +from plane.bgtasks.page_version_task import ( + PAGE_VERSION_TASK_TIMEOUT, + track_page_version, +) +from plane.db.models import Page, PageVersion +from plane.tests.factories import UserFactory, WorkspaceFactory + + +def _make_page(workspace, user, description_json, description_html): + """Create a Page with explicit description fields set.""" + return Page.objects.create( + workspace=workspace, + owned_by=user, + name="Test Page", + description_json=description_json, + description_html=description_html, + ) + + +@pytest.mark.unit +@pytest.mark.django_db +class TestTrackPageVersion: + def test_create_path_persists_description_json(self): + """First edit of a page must create a PageVersion whose + ``description_json`` is copied from ``page.description_json``. + + Regression guard: before the fix the task read ``page.description`` + -- a field that no longer exists -- raising AttributeError that the + broad except swallowed, so zero versions were ever written. + """ + user = UserFactory() + workspace = WorkspaceFactory(owner=user) + description_json = {"type": "doc", "content": [{"type": "paragraph"}]} + page = _make_page( + workspace, + user, + description_json=description_json, + description_html="

hello

", + ) + + # existing_instance=None => current_instance is {}, so the + # description_html comparison differs and the version branch runs. + track_page_version(page.id, None, user.id) + + versions = PageVersion.objects.filter(page_id=page.id) + assert versions.count() == 1, "exactly one version should be written" + version = versions.first() + assert version.description_json == description_json + assert version.description_html == "

hello

" + assert version.owned_by_id == user.id + + def test_update_path_updates_existing_version_in_place(self): + """A subsequent edit by the same user within the timeout window must + update the existing version in place (not create a second one), and + the updated ``description_json`` must reflect the new page state. + """ + user = UserFactory() + workspace = WorkspaceFactory(owner=user) + page = _make_page( + workspace, + user, + description_json={"v": 1}, + description_html="

v1

", + ) + + # First edit -> creates the initial version. + track_page_version(page.id, None, user.id) + assert PageVersion.objects.filter(page_id=page.id).count() == 1 + + # Mutate the page, then re-run as the same user within the timeout. + page.description_json = {"v": 2} + page.description_html = "

v2

" + page.save() + + previous_state = json.dumps({"description_html": "

v1

"}) + track_page_version(page.id, previous_state, user.id) + + versions = PageVersion.objects.filter(page_id=page.id) + assert versions.count() == 1, "existing version should be updated in place" + version = versions.first() + assert version.description_json == {"v": 2} + assert version.description_html == "

v2

" + + def test_noop_when_description_html_unchanged(self): + """If ``description_html`` is unchanged, no version is written.""" + user = UserFactory() + workspace = WorkspaceFactory(owner=user) + page = _make_page( + workspace, + user, + description_json={"v": 1}, + description_html="

same

", + ) + + unchanged_state = json.dumps({"description_html": "

same

"}) + track_page_version(page.id, unchanged_state, user.id) + + assert PageVersion.objects.filter(page_id=page.id).count() == 0 + + def test_new_version_created_after_timeout(self): + """An edit past PAGE_VERSION_TASK_TIMEOUT creates a fresh version + rather than overwriting the previous one -- exercising the create + branch a second time and confirming description_json is carried over. + """ + user = UserFactory() + workspace = WorkspaceFactory(owner=user) + page = _make_page( + workspace, + user, + description_json={"v": 1}, + description_html="

v1

", + ) + + track_page_version(page.id, None, user.id) + first = PageVersion.objects.get(page_id=page.id) + # Backdate the first version beyond the timeout window. + stale = timezone.now() - timezone.timedelta(seconds=PAGE_VERSION_TASK_TIMEOUT + 1) + PageVersion.objects.filter(pk=first.pk).update(last_saved_at=stale) + + page.description_json = {"v": 2} + page.description_html = "

v2

" + page.save() + track_page_version( + page.id, json.dumps({"description_html": "

v1

"}), user.id + ) + + versions = PageVersion.objects.filter(page_id=page.id).order_by("last_saved_at") + assert versions.count() == 2 + assert versions.last().description_json == {"v": 2} + + +@pytest.mark.unit +class TestPageFieldName: + def test_page_has_no_description_attribute(self): + """Sentinel for the root cause: the Page model exposes + ``description_json`` (and ``description_html``/``_binary``/``_stripped``) + but no bare ``description`` field. Reading ``page.description`` -- as + the buggy task did -- raises AttributeError. + """ + assert not hasattr(Page, "description") + assert hasattr(Page, "description_json")