Skip to content

Skip importing triggers whose flow type isn't allowed for the trigger type - #7131

Merged
rowanseymour merged 2 commits into
mainfrom
rowan/ticket-trigger-import-circumvent-75654d
Oct 21, 2026
Merged

rowanseymour merged 2 commits into
mainfrom
rowan/ticket-trigger-import-circumvent-75654d

Conversation

@rowanseymour

@rowanseymour rowanseymour commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Summary

Each trigger type's allowed flow types were only enforced by the create/update form, so importing a definition could create a trigger the UI wouldn't allow, e.g. a closed ticket trigger on a messaging flow. Imports now skip such triggers.

Changes

  • Trigger.import_def returns None (skips the trigger) when the resolved flow's type isn't in allowed_flow_types for the trigger type, the same way it skips triggers over the org limit.
    • The check runs after the flow is resolved rather than in clean_import_def, because the flow may be created by the same import.
    • It runs before groups are resolved, so a skipped trigger doesn't create or reactivate groups that nothing uses.
  • Tests in test_import_invalid: a closed ticket trigger on a messaging flow (skipped, its group not created), a keyword trigger on a background flow (skipped) and a closed ticket trigger on a background flow (imported).

Behavior examples

Import of Before After
Closed ticket trigger → messaging flow Trigger created Trigger skipped
Keyword trigger → background flow Trigger created Trigger skipped
Closed ticket trigger → background flow Trigger created Trigger created

Test plan

  • temba.triggers, temba.flows, temba.campaigns, temba.orgs tests pass
  • code_check.py passes

Comment thread temba/triggers/models.py Outdated
@claude

claude Bot commented Oct 21, 2026

Copy link
Copy Markdown

The approach makes sense: the check has to happen after flow resolution, since the flow may come from the same import, and returning None matches how triggers over the org limit are already skipped.

Things to consider:

  • Side effects before the skip (inline): groups are created or reactivated before the flow type check, so a skipped trigger can leave groups that nothing uses. Moving the check before group resolution avoids this.
  • Silent skip: the import gives no feedback when a trigger is dropped. That's the same as the org-limit case, but now more existing exports could lose triggers without anyone noticing. Old exports containing combinations that used to be allowed would be affected, e.g. a missed call trigger on a voice flow, or keyword or catch-all triggers on background flows. You could at least log skipped triggers, or note this in the changelog.
  • Tests: only closed ticket triggers are covered. That's probably fine since the check is generic, but a keyword trigger on a background flow would also exercise the more common case.

🤖 Generated with Claude Code

@rowanseymour
rowanseymour merged commit 20cae58 into main Oct 21, 2026
4 checks passed
@rowanseymour
rowanseymour deleted the rowan/ticket-trigger-import-circumvent-75654d branch October 21, 2026 18:57
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 21, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant