Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions apps/api/plane/bgtasks/dummy_data_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ def create_pages(workspace, project, user_id, pages_count):
Faker.seed(0)

pages = []
for _ in range(0, pages_count):
for index in range(0, pages_count):
text = fake.text(max_nb_chars=60000)
pages.append(
Page(
Expand All @@ -231,13 +231,14 @@ def create_pages(workspace, project, user_id, pages_count):
description_html=f"<p>{text}</p>",
archived_at=None,
is_locked=False,
sort_order=Page.DEFAULT_SORT_ORDER * index + 10000,
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sort_order calculation Page.DEFAULT_SORT_ORDER * index + 10000 produces values like 10000, 75535, 141070, etc. This differs significantly from the model's _get_sort_order() method which increments by 10000 from the largest existing value (e.g., 10000, 20000, 30000).

For consistency with the model logic, the formula should be index * 10000 (without the multiplication by DEFAULT_SORT_ORDER), which would produce 0, 10000, 20000, etc., matching the incremental pattern used in production code.

Suggested change
sort_order=Page.DEFAULT_SORT_ORDER * index + 10000,
sort_order=index * 10000,

Copilot uses AI. Check for mistakes.
)
)
# Bulk create pages
pages = Page.objects.bulk_create(pages, ignore_conflicts=True)
# Add Page to project
ProjectPage.objects.bulk_create(
[ProjectPage(page=page, project=project, workspace=workspace) for page in pages],
[ProjectPage(page=page, project=project, workspace=workspace, sort_order=page.sort_order) for page in pages],
batch_size=1000,
)

Expand Down
32 changes: 32 additions & 0 deletions apps/api/plane/db/migrations/0114_projectpage_sort_order.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Generated by Django 4.2.22 on 2026-01-05 13:28

from django.db import migrations, models

def update_projectpage_sort_order(apps, schema_editor):
ProjectPage = apps.get_model("db", "ProjectPage")
Project = apps.get_model("db", "Project")

for project in Project.objects.all():
pages = list(
ProjectPage.objects.filter(project=project).order_by("created_at")
)

for index, page in enumerate(pages):
page.sort_order = index * 10000

ProjectPage.objects.bulk_update(pages, ["sort_order"], batch_size=3000)

class Migration(migrations.Migration):

dependencies = [
('db', '0113_webhook_version'),
]

operations = [
migrations.AddField(
model_name='projectpage',
name='sort_order',
field=models.FloatField(default=65535),
),
migrations.RunPython(update_projectpage_sort_order, reverse_code=migrations.RunPython.noop),
]
78 changes: 77 additions & 1 deletion apps/api/plane/db/models/page.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@
from django.utils import timezone

# Django imports
from django.db import models
from django.db import connection, models, transaction

# Module imports
from plane.utils.html_processor import strip_tags
from plane.utils.uuid import convert_uuid_to_integer

from .base import BaseModel

Expand Down Expand Up @@ -63,13 +64,49 @@ def __str__(self):
"""Return owner email and page name"""
return f"{self.owned_by.email} <{self.name}>"

def _get_sort_order(self, project):
"""Get the next sort order for the page within a specific project."""
if self.access == Page.PRIVATE_ACCESS:
largest = ProjectPage.objects.filter(
page__access=Page.PRIVATE_ACCESS,
page__owned_by=self.owned_by,
project=project,
).aggregate(largest=models.Max("sort_order"))["largest"]
else:
largest = ProjectPage.objects.filter(
page__access=Page.PUBLIC_ACCESS,
project=project,
).aggregate(largest=models.Max("sort_order"))["largest"]

return (largest or self.DEFAULT_SORT_ORDER) + 10000

def save(self, *args, **kwargs):
# Strip the html tags using html parser
self.description_stripped = (
None
if (self.description_html == "" or self.description_html is None)
else strip_tags(self.description_html)
)

if not self._state.adding:
original = Page.objects.get(pk=self.pk)
if original.access != self.access:
with transaction.atomic():
# Get the project pages for the page and update the sort order
project_pages = list(ProjectPage.objects.filter(page=self).select_related("project"))

# Acquire advisory locks for all projects to prevent race conditions
for project_page in project_pages:
lock_key = convert_uuid_to_integer(project_page.project_id)
with connection.cursor() as cursor:
cursor.execute("SELECT pg_advisory_xact_lock(%s)", [lock_key])
Comment on lines +99 to +102
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a page belongs to multiple projects, advisory locks are acquired in the order the ProjectPages are returned by the database query (line 96). This non-deterministic ordering could lead to deadlocks if two concurrent transactions try to lock the same set of projects in different orders.

For example:

  • Transaction A: Updates page X (projects [P1, P2]) - locks P1, then P2
  • Transaction B: Updates page Y (projects [P2, P1]) - locks P2, then P1
  • Result: Potential deadlock

To prevent this, sort the project_pages list by project_id before acquiring locks to ensure a consistent lock acquisition order across all transactions.

Copilot uses AI. Check for mistakes.

for project_page in project_pages:
project_page.sort_order = self._get_sort_order(project_page.project)
# Bulk update all project pages in a single query
if project_pages:
ProjectPage.objects.bulk_update(project_pages, ["sort_order"])

super(Page, self).save(*args, **kwargs)
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The super().save() is called after the transaction commits (line 110 is outside the atomic block that ends at line 108). This creates a consistency issue:

  1. ProjectPages are updated with new sort_order values within the transaction
  2. Transaction commits
  3. Page is saved with new access value

Between steps 2 and 3, the database shows ProjectPages with sort_order values calculated for the new access level, but the Page still has the old access value. This inconsistent state is visible to other transactions.

Additionally, if the Page.save() fails after the transaction commits, the ProjectPages will have been updated but the Page's access won't have changed, leaving the database in an inconsistent state.

The super().save() should be moved inside the transaction.atomic() block, before the ProjectPage updates (after line 94), to ensure atomicity and consistency.

Copilot uses AI. Check for mistakes.


Expand Down Expand Up @@ -129,9 +166,12 @@ def __str__(self):


class ProjectPage(BaseModel):
DEFAULT_SORT_ORDER = 65535

project = models.ForeignKey("db.Project", on_delete=models.CASCADE, related_name="project_pages")
page = models.ForeignKey("db.Page", on_delete=models.CASCADE, related_name="project_pages")
workspace = models.ForeignKey("db.Workspace", on_delete=models.CASCADE, related_name="project_pages")
sort_order = models.FloatField(default=DEFAULT_SORT_ORDER)

class Meta:
unique_together = ["project", "page", "deleted_at"]
Expand All @@ -150,6 +190,42 @@ class Meta:
def __str__(self):
return f"{self.project.name} {self.page.name}"

def _get_sort_order(self):
"""Get the next sort order for the project page based on page access type."""
if self.page.access == Page.PRIVATE_ACCESS:
# For private pages, get max sort_order among pages owned by same user in same project
largest = ProjectPage.objects.filter(
page__access=Page.PRIVATE_ACCESS,
page__owned_by=self.page.owned_by,
project=self.project,
).aggregate(largest=models.Max("sort_order"))["largest"]
else:
# For public pages, get max sort_order among all public pages in same project
largest = ProjectPage.objects.filter(
page__access=Page.PUBLIC_ACCESS,
project=self.project,
).aggregate(largest=models.Max("sort_order"))["largest"]

Comment on lines +197 to +208
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The _get_sort_order() query doesn't exclude the current ProjectPage being saved when calculating the maximum sort_order. When updating an existing ProjectPage (though currently the save method only sets sort_order on creation), this could include the current object's sort_order in the Max() calculation, potentially returning the same value instead of getting a new one.

While this is currently not a problem since sort_order is only set when _state.adding is True (line 213), it makes the method less reusable. Consider adding an exclusion filter like .exclude(pk=self.pk) to make the method safer for potential future updates.

Suggested change
largest = ProjectPage.objects.filter(
page__access=Page.PRIVATE_ACCESS,
page__owned_by=self.page.owned_by,
project=self.project,
).aggregate(largest=models.Max("sort_order"))["largest"]
else:
# For public pages, get max sort_order among all public pages in same project
largest = ProjectPage.objects.filter(
page__access=Page.PUBLIC_ACCESS,
project=self.project,
).aggregate(largest=models.Max("sort_order"))["largest"]
qs = ProjectPage.objects.filter(
page__access=Page.PRIVATE_ACCESS,
page__owned_by=self.page.owned_by,
project=self.project,
)
else:
# For public pages, get max sort_order among all public pages in same project
qs = ProjectPage.objects.filter(
page__access=Page.PUBLIC_ACCESS,
project=self.project,
)
# Exclude the current instance (when it already exists) from the aggregation
if self.pk:
qs = qs.exclude(pk=self.pk)
largest = qs.aggregate(largest=models.Max("sort_order"))["largest"]

Copilot uses AI. Check for mistakes.
return (largest or self.DEFAULT_SORT_ORDER) + 10000

def save(self, *args, **kwargs):
# Set sort_order for new project pages
if self._state.adding:
with transaction.atomic():
# Create a lock for this specific project using a transaction-level advisory lock
# This ensures only one transaction per project can execute this code at a time
# The lock is automatically released when the transaction ends
lock_key = convert_uuid_to_integer(self.project_id)

with connection.cursor() as cursor:
# Get an exclusive transaction-level lock using the project ID as the lock key
cursor.execute("SELECT pg_advisory_xact_lock(%s)", [lock_key])

self.sort_order = self._get_sort_order()
super(ProjectPage, self).save(*args, **kwargs)
else:
super(ProjectPage, self).save(*args, **kwargs)


class PageVersion(BaseModel):
workspace = models.ForeignKey("db.Workspace", on_delete=models.CASCADE, related_name="page_versions")
Expand Down
Loading