diff --git a/app/modules/services/default_task_sync.py b/app/modules/services/default_task_sync.py index 75bb93a..2b19d40 100644 --- a/app/modules/services/default_task_sync.py +++ b/app/modules/services/default_task_sync.py @@ -176,6 +176,36 @@ def sync_firm_tasks_from_system_defaults( firm_task_ids=[int(row.id) for row in firm_rows if getattr(row, "id", None)], ) + # ------------------------------------------------------------------ + # IMPORTANT: two-phase sequence re-numbering + # + # The database enforces a UNIQUE constraint on: + # (tenant_id, service_catalogue_id, sequence_no) + # + # Existing duplicate imports may contain several generations of the same + # task at different sequence numbers. During a sync, moving a canonical + # row directly onto the latest system-default sequence can therefore + # collide with another historical row that still owns that sequence. + # + # Marking that historical row inactive does NOT release the unique key. + # Park every existing firm template on a guaranteed-unique temporary + # sequence first, flush, and only then apply final/default sequences. + # ------------------------------------------------------------------ + max_existing_sequence = max( + [int(getattr(row, "sequence_no", 0) or 0) for row in firm_rows] + [0] + ) + max_default_sequence = max( + [int(getattr(row, "sequence_no", 0) or 0) for row in defaults] + [0] + ) + parking_base = max(max_existing_sequence, max_default_sequence, 0) + 100000 + + for offset, row in enumerate(sorted(firm_rows, key=lambda item: int(item.id)), start=1): + row.sequence_no = parking_base + offset + + # This flush is intentional and must happen before any canonical row is + # assigned a system-default sequence number. + db.flush() + # First clean pre-existing duplicate groups, regardless of whether the task # remains in the current system defaults. A unique firm-only task is untouched. for candidates in by_name.values(): @@ -188,6 +218,10 @@ def sync_firm_tasks_from_system_defaults( for duplicate in candidates: if duplicate.id == canonical.id: continue + + # Duplicate rows are retained for historical FK safety but remain + # on their unique parked sequence number. The UNIQUE sequence + # constraint therefore remains satisfied even after deactivation. if getattr(duplicate, "is_active", True): duplicate.is_active = False if ( @@ -221,28 +255,13 @@ def sync_firm_tasks_from_system_defaults( # No name match. A unique sequence match is a conservative fallback for a # system-default rename while still avoiding arbitrary replacement. - sequence_no = getattr(default, "sequence_no", None) - sequence_candidates = [ - row - for row in firm_rows - if getattr(row, "sequence_no", None) == sequence_no - and getattr(row, "is_active", True) - ] - if len(sequence_candidates) == 1: - target = sequence_candidates[0] - changed = _copy_default_columns(default, target) - if ( - updated_by_user_id is not None - and hasattr(target, "updated_by_user_id") - ): - target.updated_by_user_id = updated_by_user_id - if changed: - result.updated += 1 - else: - result.unchanged += 1 - # Make subsequent defaults see the new name. - by_name[_normalise_name(getattr(target, "task_name", None))].append(target) - continue + # Do NOT use the pre-sync sequence number as an identity fallback here. + # All existing rows have deliberately been parked on temporary sequence + # numbers to satisfy the DB unique constraint. More importantly, + # sequence number is ordering metadata, not a stable task identity. + # A renamed system-default task without a name match is therefore + # treated as a new task rather than risking replacement of the wrong + # firm-specific task. # Missing task: construct from the intersection of mapped columns. source_cols = _column_names(ServiceDefaultTaskTemplate)