Skip to content
Merged
Changes from all 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
108 changes: 108 additions & 0 deletions apps/api/plane/db/mixins.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# Type imports
from typing import Any

# Django imports
from django.db import models
from django.utils import timezone
Expand Down Expand Up @@ -80,3 +83,108 @@ class AuditModel(TimeAuditModel, UserAuditModel, SoftDeleteModel):

class Meta:
abstract = True


class ChangeTrackerMixin:

Copilot AI Nov 20, 2025

Copy link

Choose a reason for hiding this comment

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

The ChangeTrackerMixin should inherit from models.Model to ensure proper Method Resolution Order (MRO) when used with other Django model mixins. Django model mixins should typically inherit from models.Model with abstract = True in the Meta class.

Suggested fix:

class ChangeTrackerMixin(models.Model):
    """
    A mixin to track changes in model fields between initialization and save.
    ...
    """
    
    _original_values: dict[str, Any]
    
    class Meta:
        abstract = True
    
    def __init__(self, *args: Any, **kwargs: Any) -> None:
        ...

This ensures compatibility with Django's model system and prevents potential MRO issues when combined with other mixins like AuditModel.

Copilot uses AI. Check for mistakes.
"""
A mixin to track changes in model fields between initialization and save.

This mixin captures the initial state of model fields when the instance is
created and provides utilities to detect which fields have changed.

Usage:
To track specific fields, define a TRACKED_FIELDS list on your model:

class MyModel(ChangeTrackerMixin, models.Model):
TRACKED_FIELDS = ['field1', 'field2', 'field3']
field1 = models.CharField(max_length=100)
field2 = models.IntegerField()
field3 = models.BooleanField()

If TRACKED_FIELDS is not defined, all non-deferred fields will be tracked.

Properties:
changed_fields: A list of field names that have changed since initialization.
old_values: A dictionary mapping field names to their original values.

Methods:
has_changed(field_name): Check if a specific field has changed.

Notes:
- Deferred fields (from .defer() or .only()) are automatically excluded
from tracking to avoid triggering database queries.
- Field values are captured in __init__, so changes are tracked relative
to the initial state when the instance was loaded from the database.
"""

_original_values: dict[str, Any]

def __init__(self, *args: Any, **kwargs: Any) -> None:
super().__init__(*args, **kwargs)
self._original_values = {}
self._track_fields()

def _track_fields(self) -> None:
"""
Capture the initial values of fields to track.

This method stores the current values of fields that should be tracked.
If TRACKED_FIELDS is defined on the model, only those fields are tracked.
Otherwise, all non-deferred fields are tracked. Deferred fields are
automatically excluded to prevent unnecessary database queries.
"""
deferred_fields = self.get_deferred_fields()
tracked_fields = getattr(self, "TRACKED_FIELDS", None)
if tracked_fields:
for field in tracked_fields:
if field not in deferred_fields:
self._original_values[field] = getattr(self, field)
else:
for field in self._meta.fields:
if field.attname not in deferred_fields:
self._original_values[field.attname] = getattr(self, field.attname)
Comment on lines +127 to +145

Copilot AI Nov 20, 2025

Copy link

Choose a reason for hiding this comment

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

Storing field values directly with getattr(self, field) creates a shallow reference to mutable objects (like lists from ArrayField or dicts from JSONField). If the original object is mutated in place (e.g., issue.metadata['key'] = 'value'), the tracked value will also change, preventing accurate change detection.

Consider deep copying mutable field values during tracking:

import copy

def _track_fields(self) -> None:
    """
    Capture the initial values of fields to track.
    ...
    """
    deferred_fields = self.get_deferred_fields()
    tracked_fields = getattr(self, "TRACKED_FIELDS", None)
    if tracked_fields:
        for field in tracked_fields:
            if field not in deferred_fields:
                value = getattr(self, field)
                # Deep copy mutable types to prevent reference issues
                self._original_values[field] = copy.deepcopy(value) if isinstance(value, (dict, list)) else value
    else:
        for field in self._meta.fields:
            if field.attname not in deferred_fields:
                value = getattr(self, field.attname)
                self._original_values[field.attname] = copy.deepcopy(value) if isinstance(value, (dict, list)) else value

This ensures that in-place mutations don't affect the original tracked values.

Copilot uses AI. Check for mistakes.

def has_changed(self, field_name: str) -> bool:
"""
Check if a specific field has changed since initialization.

Args:
field_name (str): The name of the field to check.

Returns:
bool: True if the field has changed, False otherwise. Returns False
if the field was not being tracked or is deferred.
"""
if field_name not in self._original_values:
return False
return self._original_values[field_name] != getattr(self, field_name)

Copilot AI Nov 20, 2025

Copy link

Choose a reason for hiding this comment

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

The equality comparison in has_changed may not work correctly for mutable field types like JSONField, ArrayField, or related objects, which are commonly used in this codebase. This is the same issue as in the changed_fields property.

The comparison should be consistent with the changed_fields property to ensure has_changed(field) and field in changed_fields always return the same result.

Copilot uses AI. Check for mistakes.

@property
def changed_fields(self) -> list[str]:
"""
Get a list of all fields that have changed since initialization.

Returns:
list[str]: A list of field names that have different values than
when the instance was initialized. Returns an empty list
if no fields have changed.
"""
changed = []
for field, old_val in self._original_values.items():
new_val = getattr(self, field)
if old_val != new_val:
changed.append(field)
return changed
Comment on lines +172 to +177

Copilot AI Nov 20, 2025

Copy link

Choose a reason for hiding this comment

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

The equality comparison old_val != new_val may not work correctly for mutable field types like JSONField, ArrayField, or related objects. Django field values for these types might be different object instances with the same content, leading to false positives for changes.

Consider using Django's field comparison or deep equality checks for complex field types. For example:

@property
def changed_fields(self) -> list[str]:
    """
    Get a list of all fields that have changed since initialization.
    
    Returns:
        list[str]: A list of field names that have different values than
                   when the instance was initialized. Returns an empty list
                   if no fields have changed.
    """
    changed = []
    for field_name, old_val in self._original_values.items():
        new_val = getattr(self, field_name)
        # Use field-specific comparison for complex types
        field = self._meta.get_field(field_name)
        if hasattr(field, 'to_python'):
            # Compare using field's internal comparison
            if old_val != new_val:
                changed.append(field_name)
        else:
            if old_val != new_val:
                changed.append(field_name)
    return changed

This ensures accurate change detection for all Django field types, including JSON fields which are commonly used in this codebase (as seen in models like Integration and WorkspaceIntegration).

Copilot uses AI. Check for mistakes.

@property
def old_values(self) -> dict[str, Any]:
"""
Get a dictionary of the original field values from initialization.

Returns:
dict: A dictionary mapping field names to their original values
as they were when the instance was initialized. Only includes
fields that are being tracked (either via TRACKED_FIELDS or
all non-deferred fields).
"""
return self._original_values

Copilot AI Nov 20, 2025

Copy link

Choose a reason for hiding this comment

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

Returning the internal _original_values dictionary directly exposes it to external modification. Consider returning a copy to prevent unintended mutations:

@property
def old_values(self) -> dict[str, Any]:
    """
    Get a dictionary of the original field values from initialization.
    
    Returns:
        dict: A dictionary mapping field names to their original values
              as they were when the instance was initialized. Only includes
              fields that are being tracked (either via TRACKED_FIELDS or
              all non-deferred fields).
    """
    return self._original_values.copy()

This prevents callers from accidentally modifying the tracked values, which could lead to incorrect change detection.

Suggested change
return self._original_values
return self._original_values.copy()

Copilot uses AI. Check for mistakes.
Loading