mirror of
https://github.com/pacnpal/thrillwiki_django_no_react.git
synced 2026-02-05 06:05:18 -05:00
fix(fsm): Fix StateLog.by capture and cycle validation; add photographer field to photos
## FSM State Machine Fixes ### StateLog.by Field Capture - Modified TransitionMethodFactory to pass 'user' as 'by' kwarg to enable django-fsm-log's @fsm_log_by decorator to correctly capture the user who performed the transition - Applied fix to both escalate_transition and create_transition_method - Uses exec() to dynamically create transition functions with correct __name__ before decorators are applied, ensuring django-fsm's method registration works ### Cycle Validation Behavior - Changed validate_no_cycles() to return ValidationWarning instead of ValidationError - Cycles are now treated as warnings, not blocking errors, since cycles are often intentional in operational status FSMs (e.g., reopening after temporary closure) ### Ride Status Transitions - Added TEMPORARY_CLOSURE -> OPERATING transition (reopen after temporary closure) - Added SBNO -> OPERATING transition (revival - ride returns to operation) ## Field Parity ### Photo Models - Added 'photographer' field to RidePhoto and ParkPhoto models - Maps to frontend 'photographer_credit' field for full schema parity - Includes corresponding migrations for both apps ### Serializers - Added 'photographer' to RidePhotoSerializer and ParkPhotoSerializer read_only_fields
This commit is contained in:
@@ -113,6 +113,7 @@ class ParkPhotoOutputSerializer(serializers.ModelSerializer):
|
||||
"image_url",
|
||||
"image_variants",
|
||||
"caption",
|
||||
"photographer",
|
||||
"alt_text",
|
||||
"is_primary",
|
||||
"is_approved",
|
||||
@@ -147,6 +148,7 @@ class ParkPhotoCreateInputSerializer(serializers.ModelSerializer):
|
||||
fields = [
|
||||
"image",
|
||||
"caption",
|
||||
"photographer",
|
||||
"alt_text",
|
||||
"is_primary",
|
||||
]
|
||||
@@ -159,6 +161,7 @@ class ParkPhotoUpdateInputSerializer(serializers.ModelSerializer):
|
||||
model = ParkPhoto
|
||||
fields = [
|
||||
"caption",
|
||||
"photographer",
|
||||
"alt_text",
|
||||
"is_primary",
|
||||
]
|
||||
|
||||
@@ -117,6 +117,7 @@ class RidePhotoOutputSerializer(serializers.ModelSerializer):
|
||||
"image_url",
|
||||
"image_variants",
|
||||
"caption",
|
||||
"photographer",
|
||||
"alt_text",
|
||||
"is_primary",
|
||||
"is_approved",
|
||||
@@ -156,6 +157,7 @@ class RidePhotoCreateInputSerializer(serializers.ModelSerializer):
|
||||
fields = [
|
||||
"image",
|
||||
"caption",
|
||||
"photographer",
|
||||
"alt_text",
|
||||
"photo_type",
|
||||
"is_primary",
|
||||
@@ -169,6 +171,7 @@ class RidePhotoUpdateInputSerializer(serializers.ModelSerializer):
|
||||
model = RidePhoto
|
||||
fields = [
|
||||
"caption",
|
||||
"photographer",
|
||||
"alt_text",
|
||||
"photo_type",
|
||||
"is_primary",
|
||||
|
||||
@@ -53,6 +53,11 @@ def with_callbacks(
|
||||
def wrapper(instance, *args, **kwargs):
|
||||
# Extract user from kwargs
|
||||
user = kwargs.get("user")
|
||||
|
||||
# Pass user as 'by' for django-fsm-log's @fsm_log_by decorator
|
||||
# This must be set before calling the inner func so the decorator can capture it
|
||||
if user is not None and 'by' not in kwargs:
|
||||
kwargs['by'] = user
|
||||
|
||||
# Get source state before transition
|
||||
source_state = getattr(instance, field_name, None)
|
||||
@@ -329,6 +334,9 @@ class TransitionMethodFactory:
|
||||
)
|
||||
def approve(instance, user=None, comment: str = "", **kwargs):
|
||||
"""Approve and transition to approved state."""
|
||||
# Pass user as 'by' for django-fsm-log's @fsm_log_by decorator
|
||||
if user is not None:
|
||||
kwargs['by'] = user
|
||||
if hasattr(instance, "approved_by_id"):
|
||||
instance.approved_by = user
|
||||
if hasattr(instance, "approval_comment"):
|
||||
@@ -382,6 +390,9 @@ class TransitionMethodFactory:
|
||||
)
|
||||
def reject(instance, user=None, reason: str = "", **kwargs):
|
||||
"""Reject and transition to rejected state."""
|
||||
# Pass user as 'by' for django-fsm-log's @fsm_log_by decorator
|
||||
if user is not None:
|
||||
kwargs['by'] = user
|
||||
if hasattr(instance, "rejected_by_id"):
|
||||
instance.rejected_by = user
|
||||
if hasattr(instance, "rejection_reason"):
|
||||
@@ -435,6 +446,9 @@ class TransitionMethodFactory:
|
||||
)
|
||||
def escalate(instance, user=None, reason: str = "", **kwargs):
|
||||
"""Escalate to higher authority."""
|
||||
# Pass user as 'by' for django-fsm-log's @fsm_log_by decorator
|
||||
if user is not None:
|
||||
kwargs['by'] = user
|
||||
if hasattr(instance, "escalated_by_id"):
|
||||
instance.escalated_by = user
|
||||
if hasattr(instance, "escalation_reason"):
|
||||
@@ -483,31 +497,45 @@ class TransitionMethodFactory:
|
||||
# Get field name for callback wrapper
|
||||
field_name = field.name if hasattr(field, 'name') else 'status'
|
||||
|
||||
@fsm_log_by
|
||||
@transition(
|
||||
# Create the transition function with the correct name from the start
|
||||
# by using exec to define it dynamically. This ensures __name__ is correct
|
||||
# before decorators are applied, which is critical for django-fsm's
|
||||
# method registration.
|
||||
doc = docstring if docstring else f"Transition from {source} to {target}"
|
||||
|
||||
# Define the function dynamically with the correct name
|
||||
# IMPORTANT: We set kwargs['by'] = user so that @fsm_log_by can capture
|
||||
# who performed the transition. The decorator looks for 'by' in kwargs.
|
||||
func_code = f'''
|
||||
def {method_name}(instance, user=None, **kwargs):
|
||||
"""{doc}"""
|
||||
# Pass user as 'by' for django-fsm-log's @fsm_log_by decorator
|
||||
if user is not None:
|
||||
kwargs['by'] = user
|
||||
pass
|
||||
'''
|
||||
local_namespace: dict = {}
|
||||
exec(func_code, {}, local_namespace)
|
||||
inner_func = local_namespace[method_name]
|
||||
|
||||
# Apply decorators in correct order (innermost first)
|
||||
# @fsm_log_by -> @transition -> inner_func
|
||||
decorated = transition(
|
||||
field=field,
|
||||
source=source,
|
||||
target=target,
|
||||
permission=permission_guard,
|
||||
)
|
||||
def generic_transition(instance, user=None, **kwargs):
|
||||
"""Execute state transition."""
|
||||
pass
|
||||
|
||||
generic_transition.__name__ = method_name
|
||||
if docstring:
|
||||
generic_transition.__doc__ = docstring
|
||||
else:
|
||||
generic_transition.__doc__ = f"Transition from {source} to {target}"
|
||||
)(inner_func)
|
||||
decorated = fsm_log_by(decorated)
|
||||
|
||||
# Apply callback wrapper if enabled
|
||||
if enable_callbacks:
|
||||
generic_transition = with_callbacks(
|
||||
decorated = with_callbacks(
|
||||
field_name=field_name,
|
||||
emit_signals=emit_signals,
|
||||
)(generic_transition)
|
||||
)(decorated)
|
||||
|
||||
return generic_transition
|
||||
return decorated
|
||||
|
||||
|
||||
def with_transition_logging(transition_method: Callable) -> Callable:
|
||||
|
||||
@@ -83,7 +83,7 @@ class MetadataValidator:
|
||||
result.errors.extend(self.validate_transitions())
|
||||
result.errors.extend(self.validate_terminal_states())
|
||||
result.errors.extend(self.validate_permission_consistency())
|
||||
result.errors.extend(self.validate_no_cycles())
|
||||
result.warnings.extend(self.validate_no_cycles()) # Cycles are warnings, not errors
|
||||
result.errors.extend(self.validate_reachability())
|
||||
|
||||
# Set validity based on errors
|
||||
@@ -197,23 +197,20 @@ class MetadataValidator:
|
||||
|
||||
return errors
|
||||
|
||||
def validate_no_cycles(self) -> list[ValidationError]:
|
||||
def validate_no_cycles(self) -> list[ValidationWarning]:
|
||||
"""
|
||||
Detect invalid state cycles (excluding self-loops).
|
||||
Detect state cycles (excluding self-loops).
|
||||
|
||||
Note: Cycles are allowed in many FSMs (e.g., status transitions that allow
|
||||
reopening or revival). This method returns warnings, not errors, since
|
||||
cycles are often intentional in operational status FSMs.
|
||||
|
||||
Returns:
|
||||
List of validation errors
|
||||
List of validation warnings
|
||||
"""
|
||||
errors = []
|
||||
warnings = []
|
||||
graph = self.builder.build_transition_graph()
|
||||
|
||||
# Check for self-loops (state transitioning to itself)
|
||||
for state, targets in graph.items():
|
||||
if state in targets:
|
||||
# Self-loops are warnings, not errors
|
||||
# but we can flag them
|
||||
pass
|
||||
|
||||
# Detect cycles using DFS
|
||||
visited: set[str] = set()
|
||||
rec_stack: set[str] = set()
|
||||
@@ -240,16 +237,16 @@ class MetadataValidator:
|
||||
if state not in visited:
|
||||
cycle = has_cycle(state, [])
|
||||
if cycle:
|
||||
errors.append(
|
||||
ValidationError(
|
||||
code="STATE_CYCLE_DETECTED",
|
||||
message=(f"Cycle detected: {' -> '.join(cycle)}"),
|
||||
warnings.append(
|
||||
ValidationWarning(
|
||||
code="STATE_CYCLE_EXISTS",
|
||||
message=(f"Cycle exists (may be intentional): {' -> '.join(cycle)}"),
|
||||
state=cycle[0],
|
||||
)
|
||||
)
|
||||
break # Report first cycle only
|
||||
|
||||
return errors
|
||||
return warnings
|
||||
|
||||
def validate_reachability(self) -> list[ValidationError]:
|
||||
"""
|
||||
|
||||
@@ -0,0 +1,41 @@
|
||||
# Generated by Django 5.2.9 on 2026-01-08 18:48
|
||||
|
||||
import pgtrigger.compiler
|
||||
import pgtrigger.migrations
|
||||
from django.db import migrations, models
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('parks', '0030_company_schema_parity'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
pgtrigger.migrations.RemoveTrigger(
|
||||
model_name='parkphoto',
|
||||
name='insert_insert',
|
||||
),
|
||||
pgtrigger.migrations.RemoveTrigger(
|
||||
model_name='parkphoto',
|
||||
name='update_update',
|
||||
),
|
||||
migrations.AddField(
|
||||
model_name='parkphoto',
|
||||
name='photographer',
|
||||
field=models.CharField(blank=True, help_text='Photographer credit (maps to frontend photographer_credit)', max_length=200),
|
||||
),
|
||||
migrations.AddField(
|
||||
model_name='parkphotoevent',
|
||||
name='photographer',
|
||||
field=models.CharField(blank=True, help_text='Photographer credit (maps to frontend photographer_credit)', max_length=200),
|
||||
),
|
||||
pgtrigger.migrations.AddTrigger(
|
||||
model_name='parkphoto',
|
||||
trigger=pgtrigger.compiler.Trigger(name='insert_insert', sql=pgtrigger.compiler.UpsertTriggerSql(func='INSERT INTO "parks_parkphotoevent" ("alt_text", "caption", "created_at", "date_taken", "id", "image_id", "is_approved", "is_primary", "park_id", "pgh_context_id", "pgh_created_at", "pgh_label", "pgh_obj_id", "photographer", "updated_at", "uploaded_by_id") VALUES (NEW."alt_text", NEW."caption", NEW."created_at", NEW."date_taken", NEW."id", NEW."image_id", NEW."is_approved", NEW."is_primary", NEW."park_id", _pgh_attach_context(), NOW(), \'insert\', NEW."id", NEW."photographer", NEW."updated_at", NEW."uploaded_by_id"); RETURN NULL;', hash='151f82660bda74a8d10ddf581e509c63e4e7e6e0', operation='INSERT', pgid='pgtrigger_insert_insert_e2033', table='parks_parkphoto', when='AFTER')),
|
||||
),
|
||||
pgtrigger.migrations.AddTrigger(
|
||||
model_name='parkphoto',
|
||||
trigger=pgtrigger.compiler.Trigger(name='update_update', sql=pgtrigger.compiler.UpsertTriggerSql(condition='WHEN (OLD.* IS DISTINCT FROM NEW.*)', func='INSERT INTO "parks_parkphotoevent" ("alt_text", "caption", "created_at", "date_taken", "id", "image_id", "is_approved", "is_primary", "park_id", "pgh_context_id", "pgh_created_at", "pgh_label", "pgh_obj_id", "photographer", "updated_at", "uploaded_by_id") VALUES (NEW."alt_text", NEW."caption", NEW."created_at", NEW."date_taken", NEW."id", NEW."image_id", NEW."is_approved", NEW."is_primary", NEW."park_id", _pgh_attach_context(), NOW(), \'update\', NEW."id", NEW."photographer", NEW."updated_at", NEW."uploaded_by_id"); RETURN NULL;', hash='9a33e713d26165877f27ae3f993c9c0675f61620', operation='UPDATE', pgid='pgtrigger_update_update_42711', table='parks_parkphoto', when='AFTER')),
|
||||
),
|
||||
]
|
||||
@@ -43,6 +43,11 @@ class ParkPhoto(TrackedModel):
|
||||
)
|
||||
|
||||
caption = models.CharField(max_length=255, blank=True, help_text="Photo caption or description")
|
||||
photographer = models.CharField(
|
||||
max_length=200,
|
||||
blank=True,
|
||||
help_text="Photographer credit (maps to frontend photographer_credit)"
|
||||
)
|
||||
alt_text = models.CharField(max_length=255, blank=True, help_text="Alternative text for accessibility")
|
||||
is_primary = models.BooleanField(default=False, help_text="Whether this is the primary photo for the park")
|
||||
is_approved = models.BooleanField(default=False, help_text="Whether this photo has been approved by moderators")
|
||||
|
||||
@@ -91,6 +91,7 @@ RIDE_STATUSES = [
|
||||
"css_class": "bg-yellow-100 text-yellow-800",
|
||||
"sort_order": 2,
|
||||
"can_transition_to": [
|
||||
"OPERATING", # Reopen after temporary closure
|
||||
"SBNO",
|
||||
"CLOSING",
|
||||
],
|
||||
@@ -109,6 +110,7 @@ RIDE_STATUSES = [
|
||||
"css_class": "bg-orange-100 text-orange-800",
|
||||
"sort_order": 3,
|
||||
"can_transition_to": [
|
||||
"OPERATING", # Revival - ride returns to operation
|
||||
"CLOSED_PERM",
|
||||
"DEMOLISHED",
|
||||
"RELOCATED",
|
||||
|
||||
@@ -0,0 +1,41 @@
|
||||
# Generated by Django 5.2.9 on 2026-01-08 18:48
|
||||
|
||||
import pgtrigger.compiler
|
||||
import pgtrigger.migrations
|
||||
from django.db import migrations, models
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('rides', '0038_company_schema_parity'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
pgtrigger.migrations.RemoveTrigger(
|
||||
model_name='ridephoto',
|
||||
name='insert_insert',
|
||||
),
|
||||
pgtrigger.migrations.RemoveTrigger(
|
||||
model_name='ridephoto',
|
||||
name='update_update',
|
||||
),
|
||||
migrations.AddField(
|
||||
model_name='ridephoto',
|
||||
name='photographer',
|
||||
field=models.CharField(blank=True, help_text='Photographer credit (maps to frontend photographer_credit)', max_length=200),
|
||||
),
|
||||
migrations.AddField(
|
||||
model_name='ridephotoevent',
|
||||
name='photographer',
|
||||
field=models.CharField(blank=True, help_text='Photographer credit (maps to frontend photographer_credit)', max_length=200),
|
||||
),
|
||||
pgtrigger.migrations.AddTrigger(
|
||||
model_name='ridephoto',
|
||||
trigger=pgtrigger.compiler.Trigger(name='insert_insert', sql=pgtrigger.compiler.UpsertTriggerSql(func='INSERT INTO "rides_ridephotoevent" ("alt_text", "caption", "created_at", "date_taken", "id", "image_id", "is_approved", "is_primary", "pgh_context_id", "pgh_created_at", "pgh_label", "pgh_obj_id", "photo_type", "photographer", "ride_id", "updated_at", "uploaded_by_id") VALUES (NEW."alt_text", NEW."caption", NEW."created_at", NEW."date_taken", NEW."id", NEW."image_id", NEW."is_approved", NEW."is_primary", _pgh_attach_context(), NOW(), \'insert\', NEW."id", NEW."photo_type", NEW."photographer", NEW."ride_id", NEW."updated_at", NEW."uploaded_by_id"); RETURN NULL;', hash='b426eed3a10c63be3db15a5a9477d66388f5dd2f', operation='INSERT', pgid='pgtrigger_insert_insert_0043a', table='rides_ridephoto', when='AFTER')),
|
||||
),
|
||||
pgtrigger.migrations.AddTrigger(
|
||||
model_name='ridephoto',
|
||||
trigger=pgtrigger.compiler.Trigger(name='update_update', sql=pgtrigger.compiler.UpsertTriggerSql(condition='WHEN (OLD.* IS DISTINCT FROM NEW.*)', func='INSERT INTO "rides_ridephotoevent" ("alt_text", "caption", "created_at", "date_taken", "id", "image_id", "is_approved", "is_primary", "pgh_context_id", "pgh_created_at", "pgh_label", "pgh_obj_id", "photo_type", "photographer", "ride_id", "updated_at", "uploaded_by_id") VALUES (NEW."alt_text", NEW."caption", NEW."created_at", NEW."date_taken", NEW."id", NEW."image_id", NEW."is_approved", NEW."is_primary", _pgh_attach_context(), NOW(), \'update\', NEW."id", NEW."photo_type", NEW."photographer", NEW."ride_id", NEW."updated_at", NEW."uploaded_by_id"); RETURN NULL;', hash='9728ec4736aea41ea171c3494de909aae3f68569', operation='UPDATE', pgid='pgtrigger_update_update_93a7e', table='rides_ridephoto', when='AFTER')),
|
||||
),
|
||||
]
|
||||
@@ -44,6 +44,11 @@ class RidePhoto(TrackedModel):
|
||||
)
|
||||
|
||||
caption = models.CharField(max_length=255, blank=True)
|
||||
photographer = models.CharField(
|
||||
max_length=200,
|
||||
blank=True,
|
||||
help_text="Photographer credit (maps to frontend photographer_credit)"
|
||||
)
|
||||
alt_text = models.CharField(max_length=255, blank=True)
|
||||
is_primary = models.BooleanField(default=False)
|
||||
is_approved = models.BooleanField(default=False)
|
||||
|
||||
Reference in New Issue
Block a user