Skip to content

Commit 9400f02

Browse files
author
Francesco
committed
fix(core): simplify trigger name resolution to prevent duplicates
A new settings added `PGHISTORY_SIMPLIFY_TRIGGER_NAMES` default to False
1 parent 629ace9 commit 9400f02

2 files changed

Lines changed: 70 additions & 1 deletion

File tree

pghistory/core.py

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,14 @@
55
import copy
66
import re
77
import sys
8+
import warnings
89
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Type, Union
910

1011
import django
1112
import pgtrigger
1213
import pgtrigger.core
1314
from django.apps import apps
15+
from django.conf import settings
1416
from django.db import connections, models
1517
from django.db.models import sql
1618
from django.db.models.fields.related import RelatedField
@@ -46,6 +48,26 @@ def quote(self, obj):
4648
_registered_trackers = {}
4749

4850

51+
# avoid default trigger name duplication
52+
def _simplify_trigger_names() -> bool:
53+
"""Return True if simplified trigger names are enabled.
54+
55+
Reads PGHISTORY_SIMPLIFY_TRIGGER_NAMES from Django settings at call time
56+
so that override_settings() works correctly in tests and avoids emitting
57+
a DeprecationWarning at module import time.
58+
"""
59+
enabled = getattr(settings, "PGHISTORY_SIMPLIFY_TRIGGER_NAMES", False)
60+
if not enabled:
61+
warnings.warn(
62+
"Trigger names like 'update_update' are deprecated. "
63+
"Set PGHISTORY_SIMPLIFY_TRIGGER_NAMES=True to enable simplified names. "
64+
"This will become the default in a future release.",
65+
DeprecationWarning,
66+
stacklevel=2,
67+
)
68+
return enabled
69+
70+
4971
class Tracker:
5072
"""For tracking an event when a condition happens on a model."""
5173

@@ -107,9 +129,20 @@ def __init__(
107129
self.condition = condition if condition is not constants.UNSET else self.condition
108130
self.operation = operation or self.operation
109131
self.row = row or self.row
110-
self.trigger_name = trigger_name or self.trigger_name or f"{self.label}_{self.operation}"
111132
self.level = level if level is not None else self.level
112133

134+
simplify = _simplify_trigger_names()
135+
136+
if trigger_name:
137+
self.trigger_name = trigger_name
138+
elif self.trigger_name:
139+
self.trigger_name = self.trigger_name
140+
else:
141+
if simplify and self.label == self.operation:
142+
self.trigger_name = str(self.operation)
143+
else:
144+
self.trigger_name = f"{self.label}_{self.operation}"
145+
113146
if self.condition is constants.UNSET:
114147
self.condition = pgtrigger.AnyChange() if self.operation == pgtrigger.Update else None
115148

pghistory/tests/test_core.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import pytest
99
from django.apps import apps
1010
from django.db import DatabaseError, models
11+
from django.test.utils import override_settings
1112
from django.utils import timezone
1213

1314
import pghistory
@@ -879,3 +880,38 @@ def test_conditional_statement_trigger():
879880
first = test_models.CondStatement.objects.order_by("id").first()
880881
test_models.CondStatement.objects.filter(id=first.id).update(id=first.id + 1000, int_field1=-1)
881882
assert test_models.CondStatementEvent.objects.all().count() == 15
883+
884+
885+
@pytest.mark.django_db
886+
def test_warning_when_simplifying_trigger_names_disabled():
887+
import warnings
888+
889+
with override_settings(PGHISTORY_SIMPLIFY_TRIGGER_NAMES=False):
890+
with warnings.catch_warnings(record=True) as caught:
891+
warnings.simplefilter("always")
892+
# Instantiating UpdateEvent triggers _simplify_trigger_names() internally
893+
pghistory.core.UpdateEvent()
894+
895+
matching = [
896+
w
897+
for w in caught
898+
if issubclass(w.category, DeprecationWarning) and "Trigger names" in str(w.message)
899+
]
900+
assert matching, "Expected a DeprecationWarning about trigger names"
901+
902+
903+
@pytest.mark.django_db
904+
def test_no_warning_when_simplifying_trigger_names_enabled():
905+
import warnings
906+
907+
with override_settings(PGHISTORY_SIMPLIFY_TRIGGER_NAMES=True):
908+
with warnings.catch_warnings(record=True) as caught:
909+
warnings.simplefilter("always")
910+
pghistory.core.UpdateEvent()
911+
912+
matching = [
913+
w
914+
for w in caught
915+
if issubclass(w.category, DeprecationWarning) and "Trigger names" in str(w.message)
916+
]
917+
assert not matching, "Expected no DeprecationWarning when simplification is enabled"

0 commit comments

Comments
 (0)