Skip to content
Open
Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ To be released
- Add support for `Python 3.13` (GH-#628)
- Add formal support for `Django 5.2` (GH-#641)
- Drop support for older versions than `Django 4.2`
- Fixes MonitorField not updating on creation with `when` and `null=True`
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The changelog entry should reference the GitHub issue numbers. According to the PR description, this fixes issues GH-401 and GH-352. The entry should follow the format used in other entries, such as "Fixes MonitorField not updating on creation with when and null=True (GH-#401, GH-#352)".

Suggested change
- Fixes MonitorField not updating on creation with `when` and `null=True`
- Fixes MonitorField not updating on creation with `when` and `null=True` (GH-#401, GH-#352)

Copilot uses AI. Check for mistakes.

5.0.0 (2024-09-01)
------------------
Expand Down
12 changes: 8 additions & 4 deletions model_utils/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,14 @@ def pre_save(self, model_instance: models.Model, add: bool) -> Any:
value = now()
previous = getattr(model_instance, self.monitor_attname, None)
current = self.get_monitored_value(model_instance)
if previous != current:
if self.when is None or current in self.when:
setattr(model_instance, self.attname, value)
self._save_initial(model_instance.__class__, model_instance)

should_update = previous != current
if self.when is not None:
should_update = (should_update or add) and current in self.when

if should_update:
setattr(model_instance, self.attname, value)
self._save_initial(model_instance.__class__, model_instance)
return super().pre_save(model_instance, add)

def deconstruct(self) -> tuple[str, str, Sequence[Any], dict[str, Any]]:
Expand Down
5 changes: 5 additions & 0 deletions tests/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@ class MonitorWhenEmpty(models.Model):
name_changed = MonitorField(monitor="name", when=[])


class MonitorWhenNullable(models.Model):
name = models.CharField(max_length=25)
name_changed = MonitorField(monitor="name", when=["Jose"], null=True)


class DoubleMonitored(models.Model):
name = models.CharField(max_length=25)
name_changed = MonitorField(monitor="name")
Expand Down
25 changes: 24 additions & 1 deletion tests/test_fields/test_monitor_field.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@
from django.test import TestCase

from model_utils.fields import MonitorField
from tests.models import DoubleMonitored, Monitored, MonitorWhen, MonitorWhenEmpty
from tests.models import (
DoubleMonitored,
Monitored,
MonitorWhen,
MonitorWhenEmpty,
MonitorWhenNullable,
)


class MonitorFieldTests(TestCase):
Expand Down Expand Up @@ -85,6 +91,23 @@ def test_double_save(self) -> None:
self.assertEqual(self.instance.name_changed, changed)


class MonitorWhenNullableFieldTests(TestCase):
"""
Will record changes only when name is 'Jose'
"""
def test_created_Jose(self) -> None:
with time_machine.travel(datetime(2016, 1, 1, 12, 0, 0, tzinfo=timezone.utc)):
instance = MonitorWhenNullable(name='Jose')
instance.save()
self.assertEqual(instance.name_changed, datetime(2016, 1, 1, 12, 0, 0, tzinfo=timezone.utc))

def test_created_Charlie(self) -> None:
with time_machine.travel(datetime(2016, 1, 1, 12, 0, 0, tzinfo=timezone.utc)):
instance = MonitorWhenNullable(name='Charlie')
instance.save()
self.assertIsNone(instance.name_changed)

Comment on lines +94 to +109
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The test class lacks coverage for update scenarios. Consider adding tests for:

  1. Updating from a non-matching value (e.g., 'Charlie') to a matching value ('Jose') to verify the monitor field updates correctly
  2. Updating from a matching value ('Jose') to a non-matching value to verify the monitor field remains unchanged
  3. Updating from one matching value to another matching value (if multiple values in 'when')
  4. Double save behavior similar to the MonitorWhenFieldTests class

These tests would ensure the fix doesn't break existing update functionality and only affects creation behavior.

Copilot uses AI. Check for mistakes.

class MonitorWhenEmptyFieldTests(TestCase):
"""
Monitor should never be updated id when is an empty list.
Expand Down