From 7498ff5dccc10fe13dad2b7a794998f65da3733b Mon Sep 17 00:00:00 2001 From: Mikhail Konchits Date: Wed, 13 May 2026 16:11:13 +0200 Subject: [PATCH 1/6] test(workflows): pin TriggerType enum wire values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a parametrized test that asserts every TriggerType member keeps its exact wire-format string (e.g. TriggerType.FOR_SUBSCRIBE.value == 'for_subscribe'). Renaming any of these is a breaking change for every stored workflow trigger config in customer databases — pinning catches it at PR time instead of at upgrade time. Also asserts that every enum member is represented in the pin table, so new triggers get vetted before they ship. --- .../src/tests/unit/test_trigger_enum_pin.py | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 src/backend/src/tests/unit/test_trigger_enum_pin.py diff --git a/src/backend/src/tests/unit/test_trigger_enum_pin.py b/src/backend/src/tests/unit/test_trigger_enum_pin.py new file mode 100644 index 00000000..2f6d8589 --- /dev/null +++ b/src/backend/src/tests/unit/test_trigger_enum_pin.py @@ -0,0 +1,84 @@ +"""Enum-pin test for TriggerType. + +The wire format of `trigger.type` in stored workflow definitions is the raw +string value of each TriggerType enum member. Renaming any of these values is +a breaking change for every existing workflow row in customer databases — so +this test pins the exact strings. + +If you genuinely need to add a new trigger, append it here. If you think you +need to rename an existing one, write a migration instead. +""" + +import pytest + +from src.models.process_workflows import TriggerType + + +# (Enum-member-name, expected wire value) — one tuple per TriggerType member. +# Keep this list in sync with the enum; the test below also asserts no enum +# member is missing from the list. +_EXPECTED_TRIGGER_VALUES = [ + ("ON_CREATE", "on_create"), + ("ON_UPDATE", "on_update"), + ("ON_DELETE", "on_delete"), + ("ON_STATUS_CHANGE", "on_status_change"), + ("SCHEDULED", "scheduled"), + ("MANUAL", "manual"), + ("BEFORE_CREATE", "before_create"), + ("BEFORE_UPDATE", "before_update"), + ("BEFORE_STATUS_CHANGE", "before_status_change"), + ("ON_REQUEST_REVIEW", "on_request_review"), + ("ON_REQUEST_ACCESS", "on_request_access"), + ("ON_REQUEST_PUBLISH", "on_request_publish"), + ("ON_REQUEST_STATUS_CHANGE", "on_request_status_change"), + ("ON_JOB_SUCCESS", "on_job_success"), + ("ON_JOB_FAILURE", "on_job_failure"), + ("ON_SUBSCRIBE", "on_subscribe"), + ("ON_UNSUBSCRIBE", "on_unsubscribe"), + ("ON_REQUEST_CERTIFY", "on_request_certify"), + ("ON_CERTIFY", "on_certify"), + ("ON_DECERTIFY", "on_decertify"), + ("ON_PUBLISH", "on_publish"), + ("ON_UNPUBLISH", "on_unpublish"), + ("ON_EXPIRING", "on_expiring"), + ("ON_REVOKE", "on_revoke"), + ("FOR_APPROVAL_RESPONSE", "for_approval_response"), + ("FOR_SUBSCRIBE", "for_subscribe"), + ("FOR_REQUEST_REVIEW", "for_request_review"), + ("FOR_REQUEST_ACCESS", "for_request_access"), + ("FOR_REQUEST_PUBLISH", "for_request_publish"), + ("FOR_REQUEST_CERTIFY", "for_request_certify"), + ("FOR_REQUEST_STATUS_CHANGE", "for_request_status_change"), + ("ON_FIRST_ACCESS", "on_first_access"), +] + + +@pytest.mark.parametrize(("member_name", "wire_value"), _EXPECTED_TRIGGER_VALUES) +def test_trigger_value_is_pinned(member_name: str, wire_value: str) -> None: + """Each TriggerType member must keep its exact wire-format string.""" + member = getattr(TriggerType, member_name) + assert member.value == wire_value, ( + f"TriggerType.{member_name}.value changed from '{wire_value}' to " + f"'{member.value}' — this is a breaking change for stored workflow " + f"trigger configs. Add a migration instead of renaming the enum value." + ) + + +def test_all_enum_members_are_pinned() -> None: + """Every TriggerType member must appear in the pin table. + + Catches the case where a new trigger is added to the enum but its wire + value is not vetted here. + """ + enum_names = {m.name for m in TriggerType} + pinned_names = {name for name, _ in _EXPECTED_TRIGGER_VALUES} + missing = enum_names - pinned_names + extra = pinned_names - enum_names + assert not missing, ( + f"New TriggerType members missing from the pin table: {sorted(missing)}. " + f"Add them to _EXPECTED_TRIGGER_VALUES in this file." + ) + assert not extra, ( + f"Pin table references TriggerType members that no longer exist: " + f"{sorted(extra)}. Did you delete the enum value?" + ) From b40ef61802155f08e774bfd0fd2e8481cf7a87c0 Mon Sep 17 00:00:00 2001 From: Mikhail Konchits Date: Wed, 13 May 2026 16:11:24 +0200 Subject: [PATCH 2/6] feat(workflows): add GET /api/workflows/trigger-types catalog endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The workflow trigger dropdown today shows ~25 trigger types in a flat list, with for_* and on_* variants for the same action appearing as if they were duplicates. Authors get confused which to pick, and the entity_types scope is invisible — workflows end up with entity_types=[] ('fires for everything') by accident. This commit adds the backend half of the fix: a single read-only endpoint that returns the full UI catalog the workflow-authoring form needs. Each entry carries: - label: user-approved display string - workflow_type: 'approval' (for_*) or 'process' (everything else), so the picker can filter by the workflow being authored - group: lifecycle / request_flow / validation_gates / system_scheduled, so the picker can render s instead of a flat list - entity_types: which entity types this trigger CAN fire for, used to populate a multiselect under the picker - is_advanced: true for for_approval_response only — gated behind an explicit 'show advanced' toggle Authorization mirrors the other workflow read endpoints (settings/READ_ONLY). Tests: - Contract test: every TriggerType is represented exactly once; shape and value spaces are validated. - Round-trip test: create → get → update unchanged → get again, asserting trigger.type is byte-identical at every hop. Covers every for_* trigger plus one representative from each process category. --- src/backend/src/routes/workflows_routes.py | 173 ++++++++++++++++++ .../tests/unit/test_trigger_types_endpoint.py | 87 +++++++++ .../unit/test_workflow_trigger_roundtrip.py | 100 ++++++++++ 3 files changed, 360 insertions(+) create mode 100644 src/backend/src/tests/unit/test_trigger_types_endpoint.py create mode 100644 src/backend/src/tests/unit/test_workflow_trigger_roundtrip.py diff --git a/src/backend/src/routes/workflows_routes.py b/src/backend/src/routes/workflows_routes.py index 100739eb..d8b73758 100644 --- a/src/backend/src/routes/workflows_routes.py +++ b/src/backend/src/routes/workflows_routes.py @@ -74,6 +74,179 @@ async def get_step_types( return manager.get_step_type_schemas() +# --------------------------------------------------------------------------- +# Trigger-type catalog +# --------------------------------------------------------------------------- +# +# Returns a UI-friendly catalog of every TriggerType enum member so the +# workflow-authoring form can: +# 1. Render human-readable labels (no more raw enum values in the picker). +# 2. Group triggers (lifecycle / request_flow / validation_gates / +# system_scheduled). +# 3. Show approval (for_*) vs process (on_* / before_* / scheduled / manual) +# based on the workflow being authored. +# 4. Pre-populate the entity_types multiselect with the entity types each +# trigger is wired for in the backend. +# 5. Gate "advanced" triggers behind a toggle (today only +# for_approval_response). +# +# Wire-format contract: every TriggerType member appears exactly once. +# `value` is the raw enum string (used as the FK in stored trigger configs). +# Labels here are the canonical source of truth; the FE getTriggerLabel() +# helper mirrors them so existing labels keep working if the endpoint is +# unavailable. +# +# entity_types mapping mirrors SUPPORTED_TRIGGER_ENTITY_MAP in the FE +# (src/frontend/src/lib/workflow-labels.ts) — keep them in sync. An empty +# list means "any entity type" (manual, scheduled, on_first_access). + +_TRIGGER_LABELS: Dict[str, str] = { + "for_subscribe": "When a user subscribes (wizard)", + "on_subscribe": "After a subscription is created", + "for_request_access": "When a user requests access (wizard)", + "on_request_access": "After an access request is submitted", + "for_request_review": "When a user requests review (wizard)", + "on_request_review": "After a review request is submitted", + "for_request_publish": "When a user requests publish (wizard)", + "on_request_publish": "After a publish request is submitted", + "for_request_certify": "When a user requests certification (wizard)", + "on_request_certify": "After a certification request is submitted", + "for_request_status_change": "When a user requests status change (wizard)", + "on_request_status_change": "After a status change request is submitted", + "for_approval_response": "Approval response dialog (advanced)", + "before_create": "Before entity is created (validation)", + "before_update": "Before entity is updated (validation)", + "before_status_change": "Before status changes (validation)", + "on_create": "After entity is created", + "on_update": "After entity is updated", + "on_delete": "After entity is deleted", + "on_status_change": "After status changes", + "on_publish": "After entity is published", + "on_unpublish": "After entity is unpublished", + "on_revoke": "After access is revoked", + "on_expiring": "When access is about to expire", + "on_first_access": "First time a user accesses (consent)", + "on_unsubscribe": "After a user unsubscribes", + "on_job_success": "After a background job succeeds", + "on_job_failure": "After a background job fails", + "scheduled": "On a schedule (cron)", + # Fallbacks for enum members that are not part of the user-approved table. + # These keep the catalog 1:1 with the enum without expanding the table. + "manual": "Manually triggered", + "on_certify": "After entity is certified", + "on_decertify": "After entity is decertified", +} + +# Group assignment per the design brief. Anything not listed falls back to +# "lifecycle" for process workflows. +_TRIGGER_GROUPS_REQUEST_FLOW = { + "for_subscribe", "for_request_access", "for_request_review", + "for_request_publish", "for_request_certify", + "for_request_status_change", "for_approval_response", + "on_subscribe", "on_unsubscribe", + "on_request_access", "on_request_review", "on_request_publish", + "on_request_certify", "on_request_status_change", + "on_revoke", "on_expiring", "on_first_access", +} +_TRIGGER_GROUPS_LIFECYCLE = { + "on_create", "on_update", "on_delete", "on_status_change", + "on_publish", "on_unpublish", + # on_certify / on_decertify are lifecycle in spirit — they describe an + # entity transitioning state, not a request flow. + "on_certify", "on_decertify", +} +_TRIGGER_GROUPS_VALIDATION = { + "before_create", "before_update", "before_status_change", +} +_TRIGGER_GROUPS_SYSTEM_SCHEDULED = { + "on_job_success", "on_job_failure", "scheduled", "manual", +} + +# Entity types each trigger CAN fire for, derived from the dispatch sites in +# src/backend/src/common/workflow_triggers.py and the existing FE map +# (SUPPORTED_TRIGGER_ENTITY_MAP). Empty list = "any entity" (no constraint). +_TRIGGER_ENTITY_TYPES: Dict[str, List[str]] = { + # CRUD / lifecycle + "on_create": ["catalog", "schema", "table", "data_contract", "data_product", "domain"], + "on_update": ["data_contract", "data_product", "domain"], + "on_delete": ["data_contract", "data_product", "domain"], + "on_status_change": ["data_contract", "data_product", "data_asset_review"], + "on_publish": ["data_contract", "data_product"], + "on_unpublish": ["data_contract", "data_product"], + "on_certify": ["data_contract", "data_product"], + "on_decertify": ["data_contract", "data_product"], + # Validation gates + "before_create": ["catalog", "schema", "table"], + "before_update": ["data_contract"], + "before_status_change": ["data_contract", "data_product"], + # Request flow — process side + "on_request_review": ["data_contract", "data_product", "data_asset_review"], + "on_request_access": ["access_grant", "project", "role"], + "on_request_publish": ["data_contract", "data_product"], + "on_request_certify": ["data_contract", "data_product"], + "on_request_status_change": ["data_product"], + "on_subscribe": ["subscription", "data_product", "data_contract"], + "on_unsubscribe": ["subscription", "data_product", "data_contract"], + "on_revoke": ["access_grant"], + "on_expiring": ["access_grant"], + "on_first_access": ["user"], + # Request flow — approval (for_*) side. These mirror the matching on_* + # trigger's entity scope so the wizard targets the same kinds of objects. + "for_subscribe": ["data_product", "data_contract"], + "for_request_access": ["access_grant", "project", "role"], + "for_request_review": ["data_contract", "data_product", "data_asset_review"], + "for_request_publish": ["data_contract", "data_product"], + "for_request_certify": ["data_contract", "data_product"], + "for_request_status_change": ["data_product"], + "for_approval_response": [], # system trigger — any entity + # Background jobs + "on_job_success": ["job"], + "on_job_failure": ["job"], + # Scheduled / manual — no entity binding + "scheduled": [], + "manual": [], +} + + +def _trigger_group(value: str) -> str: + if value in _TRIGGER_GROUPS_REQUEST_FLOW: + return "request_flow" + if value in _TRIGGER_GROUPS_VALIDATION: + return "validation_gates" + if value in _TRIGGER_GROUPS_SYSTEM_SCHEDULED: + return "system_scheduled" + if value in _TRIGGER_GROUPS_LIFECYCLE: + return "lifecycle" + # Safe fallback — should never hit if mappings stay in sync with the enum. + return "lifecycle" + + +@router.get("/trigger-types", response_model=List[Dict[str, Any]]) +async def get_trigger_types( + request: Request, + _: bool = Depends(PermissionChecker('settings', FeatureAccessLevel.READ_ONLY)), +) -> List[Dict[str, Any]]: + """Get the UI catalog of all trigger types. + + Returns one entry per TriggerType enum member with the metadata the + workflow-authoring picker needs (label, group, workflow_type, + entity_types, is_advanced). See module-level comments for the contract. + """ + out: List[Dict[str, Any]] = [] + for tt in TriggerType: + value = tt.value + workflow_type = "approval" if value.startswith("for_") else "process" + out.append({ + "value": value, + "label": _TRIGGER_LABELS.get(value, value.replace("_", " ").title()), + "workflow_type": workflow_type, + "entity_types": list(_TRIGGER_ENTITY_TYPES.get(value, [])), + "is_advanced": value == "for_approval_response", + "group": _trigger_group(value), + }) + return out + + @router.get("/executions", response_model=WorkflowExecutionListResponse) async def list_executions( request: Request, diff --git a/src/backend/src/tests/unit/test_trigger_types_endpoint.py b/src/backend/src/tests/unit/test_trigger_types_endpoint.py new file mode 100644 index 00000000..04bd4248 --- /dev/null +++ b/src/backend/src/tests/unit/test_trigger_types_endpoint.py @@ -0,0 +1,87 @@ +"""Contract test for GET /api/workflows/trigger-types. + +Asserts the response shape and that every TriggerType enum member is +represented exactly once. The endpoint is the canonical catalog consumed by +the frontend workflow-authoring picker — drift here breaks the UX without +breaking type checks. +""" + +import asyncio +from typing import Any, Dict, List +from unittest.mock import MagicMock + +import pytest + +from src.models.process_workflows import TriggerType +from src.routes.workflows_routes import get_trigger_types + + +def _call_endpoint() -> List[Dict[str, Any]]: + """Invoke the async route handler directly, bypassing FastAPI auth.""" + # PermissionChecker is a Depends() — bypassed by calling the underlying + # coroutine directly with positional args. request is unused by the + # handler body, so a MagicMock is sufficient. + return asyncio.get_event_loop().run_until_complete( + get_trigger_types(request=MagicMock(), _=True) + ) + + +def test_every_trigger_type_represented_exactly_once() -> None: + payload = _call_endpoint() + values = [entry["value"] for entry in payload] + expected = {tt.value for tt in TriggerType} + assert set(values) == expected, ( + f"Trigger-types catalog drift: missing={expected - set(values)}, " + f"extra={set(values) - expected}" + ) + # No duplicates + assert len(values) == len(set(values)), ( + f"Duplicate entries in trigger-types catalog: {values}" + ) + assert len(values) == len(expected) + + +def test_response_shape() -> None: + payload = _call_endpoint() + required_keys = {"value", "label", "workflow_type", "entity_types", "is_advanced", "group"} + for entry in payload: + assert required_keys.issubset(entry.keys()), ( + f"Trigger-types entry missing keys: {required_keys - entry.keys()} " + f"(entry={entry})" + ) + assert isinstance(entry["value"], str) + assert isinstance(entry["label"], str) and entry["label"] + assert entry["workflow_type"] in {"process", "approval"} + assert isinstance(entry["entity_types"], list) + for et in entry["entity_types"]: + assert isinstance(et, str) + assert isinstance(entry["is_advanced"], bool) + assert entry["group"] in { + "lifecycle", "request_flow", "validation_gates", "system_scheduled", + } + + +def test_for_triggers_are_approval_workflow_type() -> None: + """Every for_* trigger must be classified as approval, everything else as process.""" + payload = _call_endpoint() + for entry in payload: + if entry["value"].startswith("for_"): + assert entry["workflow_type"] == "approval", entry + else: + assert entry["workflow_type"] == "process", entry + + +def test_only_for_approval_response_is_advanced() -> None: + payload = _call_endpoint() + advanced = [e["value"] for e in payload if e["is_advanced"]] + assert advanced == ["for_approval_response"], ( + f"Only for_approval_response should be advanced today, got: {advanced}" + ) + + +def test_approval_triggers_are_in_request_flow_group() -> None: + """All for_* approval triggers should live under request_flow in the UI.""" + payload = _call_endpoint() + for entry in payload: + if entry["workflow_type"] == "approval": + assert entry["group"] == "request_flow", entry diff --git a/src/backend/src/tests/unit/test_workflow_trigger_roundtrip.py b/src/backend/src/tests/unit/test_workflow_trigger_roundtrip.py new file mode 100644 index 00000000..e4b8c663 --- /dev/null +++ b/src/backend/src/tests/unit/test_workflow_trigger_roundtrip.py @@ -0,0 +1,100 @@ +"""Round-trip test for workflow trigger values. + +Creates a workflow with a representative trigger, fetches it, updates it +unchanged, fetches again — and asserts the raw `trigger.type` string is +byte-identical at every hop. Guards against silent enum coercion or +case-normalisation that could break stored customer workflows. +""" + +import pytest + +from src.controller.workflows_manager import WorkflowsManager +from src.models.process_workflows import ( + EntityType, + ProcessWorkflowCreate, + ProcessWorkflowUpdate, + ScopeType, + TriggerType, + WorkflowScope, + WorkflowTrigger, + WorkflowType, +) + + +# Every for_* trigger (the new approval picker uses these) plus one +# representative from each process category (on_*, before_*, scheduled). +_TRIGGER_FIXTURES = [ + # (TriggerType, entity_types, workflow_type, schedule) + (TriggerType.FOR_SUBSCRIBE, [EntityType.DATA_PRODUCT], WorkflowType.APPROVAL, None), + (TriggerType.FOR_REQUEST_ACCESS, [EntityType.ACCESS_GRANT], WorkflowType.APPROVAL, None), + (TriggerType.FOR_REQUEST_REVIEW, [EntityType.DATA_PRODUCT], WorkflowType.APPROVAL, None), + (TriggerType.FOR_REQUEST_PUBLISH, [EntityType.DATA_CONTRACT], WorkflowType.APPROVAL, None), + (TriggerType.FOR_REQUEST_CERTIFY, [EntityType.DATA_PRODUCT], WorkflowType.APPROVAL, None), + (TriggerType.FOR_REQUEST_STATUS_CHANGE, [EntityType.DATA_PRODUCT], WorkflowType.APPROVAL, None), + (TriggerType.FOR_APPROVAL_RESPONSE, [], WorkflowType.APPROVAL, None), + (TriggerType.ON_CREATE, [EntityType.TABLE], WorkflowType.PROCESS, None), + (TriggerType.BEFORE_CREATE, [EntityType.TABLE], WorkflowType.PROCESS, None), + (TriggerType.SCHEDULED, [], WorkflowType.PROCESS, "0 9 * * *"), +] + + +@pytest.mark.parametrize( + ("trigger_type", "entity_types", "workflow_type", "schedule"), + _TRIGGER_FIXTURES, + ids=lambda v: v.value if hasattr(v, "value") else str(v), +) +def test_trigger_value_survives_create_get_update_get( + db_session, + trigger_type: TriggerType, + entity_types, + workflow_type: WorkflowType, + schedule, +) -> None: + manager = WorkflowsManager(db_session) + + trigger = WorkflowTrigger( + type=trigger_type, + entity_types=entity_types, + schedule=schedule, + ) + create_payload = ProcessWorkflowCreate( + name=f"roundtrip-{trigger_type.value}", + description="roundtrip test", + trigger=trigger, + scope=WorkflowScope(type=ScopeType.ALL), + workflow_type=workflow_type, + is_active=True, + steps=[], + ) + + # Create + created = manager.create_workflow(create_payload, created_by="test_user") + wf_id = created.id + assert created.trigger.type.value == trigger_type.value, ( + f"create dropped value: expected '{trigger_type.value}', got " + f"'{created.trigger.type.value}'" + ) + + # Get + fetched = manager.get_workflow(wf_id) + assert fetched is not None + assert fetched.trigger.type.value == trigger_type.value + assert [et.value for et in fetched.trigger.entity_types] == [ + et.value for et in entity_types + ] + + # Update (unchanged trigger) + update_payload = ProcessWorkflowUpdate( + trigger=fetched.trigger, + ) + updated = manager.update_workflow(wf_id, update_payload, updated_by="test_user") + assert updated is not None + assert updated.trigger.type.value == trigger_type.value + + # Get again + refetched = manager.get_workflow(wf_id) + assert refetched is not None + assert refetched.trigger.type.value == trigger_type.value, ( + f"refetched value drifted: expected '{trigger_type.value}', got " + f"'{refetched.trigger.type.value}'" + ) From 2c6e03563b43c8f21191eb55f08a9ceec506093d Mon Sep 17 00:00:00 2001 From: Mikhail Konchits Date: Wed, 13 May 2026 16:11:32 +0200 Subject: [PATCH 3/6] feat(workflows): canonical trigger labels in workflow-labels.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the existing workflow-labels module with TRIGGER_LABELS — the canonical, user-approved display string for every TriggerType. Mirrors the backend _TRIGGER_LABELS dict in workflows_routes.py so the new picker can fall back gracefully if the trigger-types endpoint is unavailable, and so anywhere we render a trigger without an i18n context (the legacy getTriggerTypeLabel needs a TFunction) gets the same strings. getTriggerLabel(value) is the new public helper. Unknown values title-case the raw string for forward-compat with future enum members. Tests parametrize every TriggerType member against its canonical label string, plus a fallback case for unknown triggers. --- src/frontend/src/lib/workflow-labels.test.ts | 64 +++++++++++++++++++- src/frontend/src/lib/workflow-labels.ts | 58 ++++++++++++++++++ 2 files changed, 120 insertions(+), 2 deletions(-) diff --git a/src/frontend/src/lib/workflow-labels.test.ts b/src/frontend/src/lib/workflow-labels.test.ts index 02e78fdc..73f78730 100644 --- a/src/frontend/src/lib/workflow-labels.test.ts +++ b/src/frontend/src/lib/workflow-labels.test.ts @@ -1,8 +1,11 @@ /** * Unit tests for workflow-labels helpers. * - * Created alongside the for_* trigger entity-map update so the "trigger–entity - * combination is not wired" warning does not fire for valid wizard combos. + * Covers: + * - isTriggerEntitySupported + SUPPORTED_TRIGGER_ENTITY_MAP (PR #353 wizard wiring) + * - resolveRecipientDisplay (workflow recipient resolver) + * - ALL_TRIGGER_TYPES / ALL_ENTITY_TYPES sync invariants + * - getTriggerLabel + TRIGGER_LABELS (user-approved canonical labels) */ import { describe, it, expect } from 'vitest'; import { @@ -11,6 +14,8 @@ import { SUPPORTED_TRIGGER_ENTITY_MAP, ALL_TRIGGER_TYPES, ALL_ENTITY_TYPES, + getTriggerLabel, + TRIGGER_LABELS, } from './workflow-labels'; describe('isTriggerEntitySupported', () => { @@ -165,3 +170,58 @@ describe('workflow-labels', () => { }); }); }); + +describe('getTriggerLabel', () => { + // (value, expected) — one row per TriggerType, matching the backend + // _TRIGGER_LABELS dict and the user-approved table in the PR brief. + const expected: Array<[string, string]> = [ + ['for_subscribe', 'When a user subscribes (wizard)'], + ['on_subscribe', 'After a subscription is created'], + ['for_request_access', 'When a user requests access (wizard)'], + ['on_request_access', 'After an access request is submitted'], + ['for_request_review', 'When a user requests review (wizard)'], + ['on_request_review', 'After a review request is submitted'], + ['for_request_publish', 'When a user requests publish (wizard)'], + ['on_request_publish', 'After a publish request is submitted'], + ['for_request_certify', 'When a user requests certification (wizard)'], + ['on_request_certify', 'After a certification request is submitted'], + ['for_request_status_change', 'When a user requests status change (wizard)'], + ['on_request_status_change', 'After a status change request is submitted'], + ['for_approval_response', 'Approval response dialog (advanced)'], + ['before_create', 'Before entity is created (validation)'], + ['before_update', 'Before entity is updated (validation)'], + ['before_status_change', 'Before status changes (validation)'], + ['on_create', 'After entity is created'], + ['on_update', 'After entity is updated'], + ['on_delete', 'After entity is deleted'], + ['on_status_change', 'After status changes'], + ['on_publish', 'After entity is published'], + ['on_unpublish', 'After entity is unpublished'], + ['on_revoke', 'After access is revoked'], + ['on_expiring', 'When access is about to expire'], + ['on_first_access', 'First time a user accesses (consent)'], + ['on_unsubscribe', 'After a user unsubscribes'], + ['on_job_success', 'After a background job succeeds'], + ['on_job_failure', 'After a background job fails'], + ['scheduled', 'On a schedule (cron)'], + ['manual', 'Manually triggered'], + ['on_certify', 'After entity is certified'], + ['on_decertify', 'After entity is decertified'], + ]; + + it.each(expected)('returns canonical label for %s', (value, label) => { + expect(getTriggerLabel(value)).toBe(label); + }); + + it('falls back to title-cased value for unknown triggers', () => { + // Forward-compat: if a new trigger is added to the enum before the + // table is updated, we should still render something reasonable. + expect(getTriggerLabel('on_brand_new_event')).toBe('On Brand New Event'); + }); + + it('every value in ALL_TRIGGER_TYPES has a canonical label', () => { + for (const value of ALL_TRIGGER_TYPES) { + expect(TRIGGER_LABELS[value]).toBeTruthy(); + } + }); +}); diff --git a/src/frontend/src/lib/workflow-labels.ts b/src/frontend/src/lib/workflow-labels.ts index 04053edb..63681b67 100644 --- a/src/frontend/src/lib/workflow-labels.ts +++ b/src/frontend/src/lib/workflow-labels.ts @@ -110,6 +110,64 @@ export function getTriggerTypeLabel(type: TriggerType, t: TFunction): string { return t(`common:workflows.triggerTypes.${type}`, { defaultValue: formatFallback(type) }); } +/** + * Canonical, user-approved labels for every TriggerType — the same strings + * the backend GET /api/workflows/trigger-types endpoint returns. Used by: + * + * - The new workflow trigger picker, as an offline fallback if the + * endpoint is unavailable. + * - Anywhere we render a trigger label without an i18n context (the + * legacy getTriggerTypeLabel needs a TFunction). + * + * Keep in sync with _TRIGGER_LABELS in + * src/backend/src/routes/workflows_routes.py. + */ +export const TRIGGER_LABELS: Record = { + for_subscribe: 'When a user subscribes (wizard)', + on_subscribe: 'After a subscription is created', + for_request_access: 'When a user requests access (wizard)', + on_request_access: 'After an access request is submitted', + for_request_review: 'When a user requests review (wizard)', + on_request_review: 'After a review request is submitted', + for_request_publish: 'When a user requests publish (wizard)', + on_request_publish: 'After a publish request is submitted', + for_request_certify: 'When a user requests certification (wizard)', + on_request_certify: 'After a certification request is submitted', + for_request_status_change: 'When a user requests status change (wizard)', + on_request_status_change: 'After a status change request is submitted', + for_approval_response: 'Approval response dialog (advanced)', + before_create: 'Before entity is created (validation)', + before_update: 'Before entity is updated (validation)', + before_status_change: 'Before status changes (validation)', + on_create: 'After entity is created', + on_update: 'After entity is updated', + on_delete: 'After entity is deleted', + on_status_change: 'After status changes', + on_publish: 'After entity is published', + on_unpublish: 'After entity is unpublished', + on_revoke: 'After access is revoked', + on_expiring: 'When access is about to expire', + on_first_access: 'First time a user accesses (consent)', + on_unsubscribe: 'After a user unsubscribes', + on_job_success: 'After a background job succeeds', + on_job_failure: 'After a background job fails', + scheduled: 'On a schedule (cron)', + // Fallback labels for enum members not in the user-approved table. + manual: 'Manually triggered', + on_certify: 'After entity is certified', + on_decertify: 'After entity is decertified', +}; + +/** + * Get the user-facing label for a trigger value. Doesn't require i18n — + * uses the canonical TRIGGER_LABELS table that mirrors the backend + * trigger-types endpoint. Falls back to a Title-Cased version of the raw + * value if not found (handles future enum members gracefully). + */ +export function getTriggerLabel(value: string): string { + return TRIGGER_LABELS[value] ?? formatFallback(value); +} + /** * Get a human-readable label for an entity type. */ From c0df11d8ebd126df4d2b7aba0ef7347c17585146 Mon Sep 17 00:00:00 2001 From: Mikhail Konchits Date: Wed, 13 May 2026 16:11:47 +0200 Subject: [PATCH 4/6] feat(workflows): grouped trigger picker + entity-type multiselect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the flat ~25-entry trigger dropdown in the workflow designer with two cooperating components: TriggerPicker - Fetches the catalog from GET /api/workflows/trigger-types. - Filters visible entries by the workflow being authored (workflow_type === 'approval' shows only for_*; 'process' shows everything else). Today the dropdown shows both halves, which reads as duplicates ('on_subscribe' vs 'for_subscribe'). - Groups visible entries with headers — Lifecycle events / Request flow / Validation gates / System & scheduled. - Hides 'for_approval_response' behind an explicit 'Show advanced triggers' toggle (off by default). This trigger is a system hook that typical authors should never pick directly. - Hover-tooltip on every option surfaces the raw enum value (e.g. 'for_subscribe') so power users can still grep for it. - Pure filter/group logic is exported as partitionTriggers() and unit-tested directly (Radix +Badge grid for the two components above. Passes the workflow_type field through so the picker filters correctly when switching between approval and process authoring. Resets entity_types on trigger change so the multiselect re-prefills against the new supported set. - Falls back to the existing SUPPORTED_TRIGGER_ENTITY_MAP if the trigger-types endpoint hasn't returned yet, so the form renders something sensible on first paint. Tests cover the partitioning logic (filter by workflow_type, advanced toggle, group ordering, empty-group elision) and the multiselect render/toggle/auto-prefill paths. --- .../entity-type-multiselect.test.tsx | 100 +++++++++ .../workflows/entity-type-multiselect.tsx | 99 +++++++++ .../workflows/trigger-picker.test.tsx | 162 +++++++++++++++ .../components/workflows/trigger-picker.tsx | 196 ++++++++++++++++++ .../workflows/workflow-designer.tsx | 80 +++---- 5 files changed, 598 insertions(+), 39 deletions(-) create mode 100644 src/frontend/src/components/workflows/entity-type-multiselect.test.tsx create mode 100644 src/frontend/src/components/workflows/entity-type-multiselect.tsx create mode 100644 src/frontend/src/components/workflows/trigger-picker.test.tsx create mode 100644 src/frontend/src/components/workflows/trigger-picker.tsx diff --git a/src/frontend/src/components/workflows/entity-type-multiselect.test.tsx b/src/frontend/src/components/workflows/entity-type-multiselect.test.tsx new file mode 100644 index 00000000..cf7eba5d --- /dev/null +++ b/src/frontend/src/components/workflows/entity-type-multiselect.test.tsx @@ -0,0 +1,100 @@ +/** + * Tests for . + * + * Renders the component directly — Checkbox is a much simpler Radix + * primitive than Select and works reliably in jsdom. We cover: + * - Rendering each supported entity type as a row. + * - Auto-prefill when there is exactly one supported type. + * - Toggling persists the new array. + * - Empty supported set renders the muted "fires regardless of entity" + * placeholder instead of an empty box. + */ +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/react'; + +import { EntityTypeMultiselect } from './entity-type-multiselect'; + +describe('', () => { + it('renders one row per supported entity type', () => { + render( + , + ); + // All three labels visible + expect(screen.getByText('catalog')).toBeInTheDocument(); + expect(screen.getByText('schema')).toBeInTheDocument(); + expect(screen.getByText('table')).toBeInTheDocument(); + }); + + it('renders the placeholder when the trigger fires regardless of entity', () => { + render( + , + ); + expect( + screen.getByText(/fires regardless of entity type/i), + ).toBeInTheDocument(); + }); + + it('auto-prefills the single supported type', () => { + const onChange = vi.fn(); + render( + , + ); + expect(onChange).toHaveBeenCalledWith(['access_grant']); + }); + + it('does not auto-prefill when there are multiple supported types', () => { + const onChange = vi.fn(); + render( + , + ); + expect(onChange).not.toHaveBeenCalled(); + }); + + it('toggling an unchecked row adds it to value', () => { + const onChange = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByLabelText('schema')); + expect(onChange).toHaveBeenCalledWith(['catalog', 'schema']); + }); + + it('toggling a checked row removes it from value', () => { + const onChange = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByLabelText('catalog')); + expect(onChange).toHaveBeenCalledWith(['schema']); + }); +}); diff --git a/src/frontend/src/components/workflows/entity-type-multiselect.tsx b/src/frontend/src/components/workflows/entity-type-multiselect.tsx new file mode 100644 index 00000000..15090ddd --- /dev/null +++ b/src/frontend/src/components/workflows/entity-type-multiselect.tsx @@ -0,0 +1,99 @@ +/** + * EntityTypeMultiselect — companion to . + * + * Renders the entity types the chosen trigger CAN fire for (from the + * trigger-types endpoint), as a Shadcn-style checkbox-multiselect. Selected + * values get persisted into `workflow.trigger.entity_types`, which the + * backend uses to scope dispatch (see ProcessWorkflowRepository.get_for_trigger). + * + * Today the trigger.entity_types field is invisible in the UI — workflows + * end up with `[]` (= "fires for everything") by accident. Surfacing this + * picker forces the author to make the scope choice deliberately. + * + * Design choices: + * - If the trigger supports exactly one entity type, we pre-select it but + * still show the (single, checked) row so the choice is visible. + * - If the trigger has NO supported entity types (e.g. scheduled, + * for_approval_response), we render a muted placeholder explaining it + * fires regardless of entity type — no checkboxes. + * - Selecting zero options is allowed, since the backend treats `[]` as + * "any entity" (back-compat with existing rows). + */ +import { useEffect } from 'react'; +import { Checkbox } from '@/components/ui/checkbox'; +import { Label } from '@/components/ui/label'; + +export interface EntityTypeMultiselectProps { + /** The currently-selected trigger value (for context only). */ + triggerType: string; + /** Currently persisted entity_types on the workflow trigger. */ + value: string[]; + /** Called when the selection changes. Pass the full new array. */ + onChange: (next: string[]) => void; + /** + * The entity types this trigger CAN fire for, from + * GET /api/workflows/trigger-types. Empty list ⇒ "any entity". + */ + supportedEntityTypes: string[]; +} + +export function EntityTypeMultiselect({ + triggerType, + value, + onChange, + supportedEntityTypes, +}: EntityTypeMultiselectProps) { + // Auto-default: if there is exactly one supported entity type and the + // user hasn't picked anything, prefill it. Keeps the picker visible so + // the choice is auditable, but spares the user a redundant click. + useEffect(() => { + if (supportedEntityTypes.length === 1 && value.length === 0) { + onChange([supportedEntityTypes[0]]); + } + // Intentionally only react to trigger changes — re-prefilling on every + // render would steal the user's "I really mean none" choice. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [triggerType, supportedEntityTypes.join('|')]); + + if (supportedEntityTypes.length === 0) { + return ( +

+ This trigger fires regardless of entity type. +

+ ); + } + + const toggle = (et: string) => { + if (value.includes(et)) { + onChange(value.filter((v) => v !== et)); + } else { + onChange([...value, et]); + } + }; + + return ( +
+

+ Which entity types should this trigger fire on? +

+
+ {supportedEntityTypes.map((et) => { + const id = `entity-type-${et}`; + const checked = value.includes(et); + return ( +
+ toggle(et)} + /> + +
+ ); + })} +
+
+ ); +} diff --git a/src/frontend/src/components/workflows/trigger-picker.test.tsx b/src/frontend/src/components/workflows/trigger-picker.test.tsx new file mode 100644 index 00000000..20c4a202 --- /dev/null +++ b/src/frontend/src/components/workflows/trigger-picker.test.tsx @@ -0,0 +1,162 @@ +/** + * Tests for the trigger-picker filter/group logic. + * + * We test the exported pure function `partitionTriggers` rather than + * rendering the full component, because the underlying Radix )? + * - We need to fetch the trigger catalog from GET /api/workflows/trigger-types + * (so labels + entity_types + groups stay in sync with the backend enum + * in one place). + * - We filter by workflow_type (approval/process) — authors only see the + * half of the catalog that matches the workflow they are building. + * - We group entries with instead of a flat list, so the + * 25-ish triggers stop reading as duplicates ("on_subscribe" / + * "for_subscribe" now sit under the same group with disambiguated labels). + * - We hide the advanced "for_approval_response" trigger behind a toggle. + * + * The grouping/filtering logic is exported separately as + * `partitionTriggers` so it can be unit-tested without dragging Radix into + * jsdom (which has known hang issues with + + + {value ? getTriggerLabel(value) : 'Select a trigger…'} + + + + {groups.map(({ group, label, items }) => ( + + {label} + {items.map((opt) => ( + + + + + {opt.label} + + + + {opt.value} + + + + ))} + + ))} + {groups.length === 0 && ( +
+ No trigger types available. +
+ )} + + +
+ + +
+ + ); +} diff --git a/src/frontend/src/components/workflows/workflow-designer.tsx b/src/frontend/src/components/workflows/workflow-designer.tsx index b5caa500..19d455c8 100644 --- a/src/frontend/src/components/workflows/workflow-designer.tsx +++ b/src/frontend/src/components/workflows/workflow-designer.tsx @@ -112,13 +112,12 @@ import type { HttpConnectionRef, WorkflowTypeValue, } from '@/types/process-workflow'; -import { - getTriggerTypeLabel, - getEntityTypeLabel, - ALL_TRIGGER_TYPES, +import { ALL_ENTITY_TYPES, isTriggerEntitySupported, } from '@/lib/workflow-labels'; +import { TriggerPicker, type TriggerTypeOption } from './trigger-picker'; +import { EntityTypeMultiselect } from './entity-type-multiselect'; // Node types registry (default = fallback for unknown step_type) const nodeTypes = { @@ -521,6 +520,7 @@ export default function WorkflowDesigner({ workflowId }: WorkflowDesignerProps) const [compliancePolicies, setCompliancePolicies] = useState([]); const [availableRoles, setAvailableRoles] = useState<{ id: string; name: string; source: 'app' | 'business'; has_groups?: boolean; category?: string; description?: string }[]>([]); const [httpConnections, setHttpConnections] = useState([]); + const [triggerTypeOptions, setTriggerTypeOptions] = useState([]); const [showDiscardDialog, setShowDiscardDialog] = useState(false); // Form state @@ -706,6 +706,17 @@ export default function WorkflowDesigner({ workflowId }: WorkflowDesignerProps) } catch (error) { console.error('Failed to load step types:', error); } + + // Load trigger-type catalog (powers the grouped picker + entity-type + // multiselect — single source of truth, mirrors the backend enum). + try { + const triggerTypesResponse = await get('/api/workflows/trigger-types'); + if (triggerTypesResponse.data && Array.isArray(triggerTypesResponse.data)) { + setTriggerTypeOptions(triggerTypesResponse.data); + } + } catch (error) { + console.error('Failed to load trigger types:', error); + } // Load compliance policies for policy_check step selector try { @@ -1240,45 +1251,36 @@ export default function WorkflowDesigner({ workflowId }: WorkflowDesignerProps) <>
- + { + setTriggerType(v as TriggerType); + // Reset entity_types when trigger changes so the + // multiselect re-prefills against the new + // supported set instead of carrying over a stale + // (and possibly unsupported) selection. + setEntityTypes([]); + }} + workflowType={workflowType === 'approval' ? 'approval' : 'process'} + options={triggerTypeOptions.length > 0 ? triggerTypeOptions : undefined} + />
-
- {ALL_ENTITY_TYPES.map(et => ( - { - if (entityTypes.includes(et)) { - setEntityTypes(prev => prev.filter(e => e !== et)); - } else { - setEntityTypes(prev => [...prev, et]); - } - }} - > - {getEntityTypeLabel(et, t)} - - ))} -
+ setEntityTypes(next as EntityType[])} + supportedEntityTypes={ + triggerTypeOptions.find((o) => o.value === triggerType)?.entity_types + // Fallback to the static FE map when the catalog + // is still loading or unavailable. Keeps the + // multiselect non-empty so saved configs are + // visible during the first render. + ?? ALL_ENTITY_TYPES.filter((et) => isTriggerEntitySupported(triggerType, et)) + } + />
- {entityTypes.length > 0 && entityTypes.some(et => !isTriggerEntitySupported(triggerType, et)) && ( -
- Warning: This trigger–entity combination is not wired in the backend. The workflow will save but never fire automatically. -
- )} {(triggerType === 'on_status_change' || triggerType === 'before_status_change' || triggerType === 'on_request_status_change') && (
From 8984d19c2c71617e804907e84afcb91f50b281c5 Mon Sep 17 00:00:00 2001 From: Mikhail Konchits Date: Fri, 15 May 2026 07:48:19 +0200 Subject: [PATCH 5/6] feat(workflows): trigger-picker UX polish MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three user-testing follow-ups on top of PR #369: 1. Drop the "Show advanced triggers" Switch. for_approval_response was the only entry it gated, and the toggle confused users. The trigger now lists in the Request flow group like every other approval trigger. The is_advanced field is removed from the GET /api/workflows/trigger-types response and the contract test asserts it is absent. 2. Strip the redundant "(wizard)" suffix from every for_* label (and the "(advanced)" suffix from for_approval_response). After the workflow_type filter is applied the suffix added no information. Backend _TRIGGER_LABELS and frontend TRIGGER_LABELS are updated in lockstep (they must stay byte-identical); the label-table test in workflow-labels.test.ts is updated to match. Validation triggers keep their "(validation)" suffix — not flagged. 3. Pretty-print entity-type values in the multiselect (access_grant -> "Access grant", data_asset_review -> "Data asset review", etc.). The conversion is a pure helper prettyEntityTypeLabel exported alongside the component for testability. The wire format is unchanged — values passed to onChange remain snake_case, so workflow.trigger.entity_types round-trips byte-identically. Tests: - Backend unit/: 782 passing (was 754 before, label/contract drift caught) - Frontend: 583 passing (added 5 helper tests, updated 6 label assertions) - Type-check: clean for changed files (knowledge-graph.tsx / slider.tsx errors are pre-existing and unrelated) Co-authored-by: Isaac --- src/backend/src/routes/workflows_routes.py | 19 ++-- .../tests/unit/test_trigger_types_endpoint.py | 24 +++-- .../entity-type-multiselect.test.tsx | 72 +++++++++++--- .../workflows/entity-type-multiselect.tsx | 24 ++++- .../workflows/trigger-picker.test.tsx | 52 +++------- .../components/workflows/trigger-picker.tsx | 97 +++++++------------ src/frontend/src/lib/workflow-labels.test.ts | 14 +-- src/frontend/src/lib/workflow-labels.ts | 14 +-- 8 files changed, 165 insertions(+), 151 deletions(-) diff --git a/src/backend/src/routes/workflows_routes.py b/src/backend/src/routes/workflows_routes.py index d8b73758..4508db68 100644 --- a/src/backend/src/routes/workflows_routes.py +++ b/src/backend/src/routes/workflows_routes.py @@ -87,8 +87,6 @@ async def get_step_types( # based on the workflow being authored. # 4. Pre-populate the entity_types multiselect with the entity types each # trigger is wired for in the backend. -# 5. Gate "advanced" triggers behind a toggle (today only -# for_approval_response). # # Wire-format contract: every TriggerType member appears exactly once. # `value` is the raw enum string (used as the FK in stored trigger configs). @@ -101,19 +99,19 @@ async def get_step_types( # list means "any entity type" (manual, scheduled, on_first_access). _TRIGGER_LABELS: Dict[str, str] = { - "for_subscribe": "When a user subscribes (wizard)", + "for_subscribe": "When a user subscribes", "on_subscribe": "After a subscription is created", - "for_request_access": "When a user requests access (wizard)", + "for_request_access": "When a user requests access", "on_request_access": "After an access request is submitted", - "for_request_review": "When a user requests review (wizard)", + "for_request_review": "When a user requests review", "on_request_review": "After a review request is submitted", - "for_request_publish": "When a user requests publish (wizard)", + "for_request_publish": "When a user requests publish", "on_request_publish": "After a publish request is submitted", - "for_request_certify": "When a user requests certification (wizard)", + "for_request_certify": "When a user requests certification", "on_request_certify": "After a certification request is submitted", - "for_request_status_change": "When a user requests status change (wizard)", + "for_request_status_change": "When a user requests status change", "on_request_status_change": "After a status change request is submitted", - "for_approval_response": "Approval response dialog (advanced)", + "for_approval_response": "Approval response dialog", "before_create": "Before entity is created (validation)", "before_update": "Before entity is updated (validation)", "before_status_change": "Before status changes (validation)", @@ -230,7 +228,7 @@ async def get_trigger_types( Returns one entry per TriggerType enum member with the metadata the workflow-authoring picker needs (label, group, workflow_type, - entity_types, is_advanced). See module-level comments for the contract. + entity_types). See module-level comments for the contract. """ out: List[Dict[str, Any]] = [] for tt in TriggerType: @@ -241,7 +239,6 @@ async def get_trigger_types( "label": _TRIGGER_LABELS.get(value, value.replace("_", " ").title()), "workflow_type": workflow_type, "entity_types": list(_TRIGGER_ENTITY_TYPES.get(value, [])), - "is_advanced": value == "for_approval_response", "group": _trigger_group(value), }) return out diff --git a/src/backend/src/tests/unit/test_trigger_types_endpoint.py b/src/backend/src/tests/unit/test_trigger_types_endpoint.py index 04bd4248..6fc2e682 100644 --- a/src/backend/src/tests/unit/test_trigger_types_endpoint.py +++ b/src/backend/src/tests/unit/test_trigger_types_endpoint.py @@ -43,7 +43,7 @@ def test_every_trigger_type_represented_exactly_once() -> None: def test_response_shape() -> None: payload = _call_endpoint() - required_keys = {"value", "label", "workflow_type", "entity_types", "is_advanced", "group"} + required_keys = {"value", "label", "workflow_type", "entity_types", "group"} for entry in payload: assert required_keys.issubset(entry.keys()), ( f"Trigger-types entry missing keys: {required_keys - entry.keys()} " @@ -55,12 +55,24 @@ def test_response_shape() -> None: assert isinstance(entry["entity_types"], list) for et in entry["entity_types"]: assert isinstance(et, str) - assert isinstance(entry["is_advanced"], bool) assert entry["group"] in { "lifecycle", "request_flow", "validation_gates", "system_scheduled", } +def test_is_advanced_not_in_response() -> None: + """The is_advanced field was removed when the "Show advanced triggers" + toggle was dropped from the picker — for_approval_response is now shown + inline alongside the other approval triggers. Guard against it sneaking + back in. + """ + payload = _call_endpoint() + for entry in payload: + assert "is_advanced" not in entry, ( + f"is_advanced should not be returned anymore (entry={entry})" + ) + + def test_for_triggers_are_approval_workflow_type() -> None: """Every for_* trigger must be classified as approval, everything else as process.""" payload = _call_endpoint() @@ -71,14 +83,6 @@ def test_for_triggers_are_approval_workflow_type() -> None: assert entry["workflow_type"] == "process", entry -def test_only_for_approval_response_is_advanced() -> None: - payload = _call_endpoint() - advanced = [e["value"] for e in payload if e["is_advanced"]] - assert advanced == ["for_approval_response"], ( - f"Only for_approval_response should be advanced today, got: {advanced}" - ) - - def test_approval_triggers_are_in_request_flow_group() -> None: """All for_* approval triggers should live under request_flow in the UI.""" payload = _call_endpoint() diff --git a/src/frontend/src/components/workflows/entity-type-multiselect.test.tsx b/src/frontend/src/components/workflows/entity-type-multiselect.test.tsx index cf7eba5d..40b0b143 100644 --- a/src/frontend/src/components/workflows/entity-type-multiselect.test.tsx +++ b/src/frontend/src/components/workflows/entity-type-multiselect.test.tsx @@ -3,19 +3,49 @@ * * Renders the component directly — Checkbox is a much simpler Radix * primitive than Select and works reliably in jsdom. We cover: - * - Rendering each supported entity type as a row. + * - Rendering each supported entity type as a row (pretty-printed). * - Auto-prefill when there is exactly one supported type. - * - Toggling persists the new array. + * - Toggling persists the new array (wire format stays snake_case). * - Empty supported set renders the muted "fires regardless of entity" * placeholder instead of an empty box. + * - prettyEntityTypeLabel pure helper — display-only conversion. */ import { describe, it, expect, vi } from 'vitest'; import { render, screen, fireEvent } from '@testing-library/react'; -import { EntityTypeMultiselect } from './entity-type-multiselect'; +import { + EntityTypeMultiselect, + prettyEntityTypeLabel, +} from './entity-type-multiselect'; + +describe('prettyEntityTypeLabel', () => { + it('converts single-word lowercase values to Sentence case', () => { + expect(prettyEntityTypeLabel('role')).toBe('Role'); + expect(prettyEntityTypeLabel('project')).toBe('Project'); + }); + + it('converts snake_case to Sentence case (only first letter capitalized)', () => { + expect(prettyEntityTypeLabel('access_grant')).toBe('Access grant'); + expect(prettyEntityTypeLabel('data_product')).toBe('Data product'); + expect(prettyEntityTypeLabel('data_contract')).toBe('Data contract'); + }); + + it('handles multi-underscore values', () => { + expect(prettyEntityTypeLabel('data_asset_review')).toBe('Data asset review'); + }); + + it('returns an empty string unchanged', () => { + expect(prettyEntityTypeLabel('')).toBe(''); + }); + + it('leaves already-capitalized single tokens alone (idempotent for sentence-case input)', () => { + // Defensive: in case any caller hands us an already-pretty value. + expect(prettyEntityTypeLabel('Role')).toBe('Role'); + }); +}); describe('', () => { - it('renders one row per supported entity type', () => { + it('renders one row per supported entity type with Sentence-case labels', () => { render( ', () => { supportedEntityTypes={['catalog', 'schema', 'table']} />, ); - // All three labels visible - expect(screen.getByText('catalog')).toBeInTheDocument(); - expect(screen.getByText('schema')).toBeInTheDocument(); - expect(screen.getByText('table')).toBeInTheDocument(); + // Pretty-printed labels are visible + expect(screen.getByText('Catalog')).toBeInTheDocument(); + expect(screen.getByText('Schema')).toBeInTheDocument(); + expect(screen.getByText('Table')).toBeInTheDocument(); + }); + + it('renders snake_case multi-word values pretty-printed', () => { + render( + , + ); + expect(screen.getByText('Access grant')).toBeInTheDocument(); + expect(screen.getByText('Data product')).toBeInTheDocument(); }); it('renders the placeholder when the trigger fires regardless of entity', () => { @@ -44,7 +87,7 @@ describe('', () => { ).toBeInTheDocument(); }); - it('auto-prefills the single supported type', () => { + it('auto-prefills the single supported type using the raw snake_case value', () => { const onChange = vi.fn(); render( ', () => { supportedEntityTypes={['access_grant']} />, ); + // Wire format is preserved on the onChange callback — only the label + // is pretty-printed. expect(onChange).toHaveBeenCalledWith(['access_grant']); }); @@ -70,7 +115,7 @@ describe('', () => { expect(onChange).not.toHaveBeenCalled(); }); - it('toggling an unchecked row adds it to value', () => { + it('toggling an unchecked row adds the raw snake_case value to value[]', () => { const onChange = vi.fn(); render( ', () => { supportedEntityTypes={['catalog', 'schema', 'table']} />, ); - fireEvent.click(screen.getByLabelText('schema')); + fireEvent.click(screen.getByLabelText('Schema')); + // Wire format stays snake_case — the label is just display. expect(onChange).toHaveBeenCalledWith(['catalog', 'schema']); }); - it('toggling a checked row removes it from value', () => { + it('toggling a checked row removes it from value (wire format preserved)', () => { const onChange = vi.fn(); render( ', () => { supportedEntityTypes={['catalog', 'schema', 'table']} />, ); - fireEvent.click(screen.getByLabelText('catalog')); + fireEvent.click(screen.getByLabelText('Catalog')); expect(onChange).toHaveBeenCalledWith(['schema']); }); }); diff --git a/src/frontend/src/components/workflows/entity-type-multiselect.tsx b/src/frontend/src/components/workflows/entity-type-multiselect.tsx index 15090ddd..0dc1fb73 100644 --- a/src/frontend/src/components/workflows/entity-type-multiselect.tsx +++ b/src/frontend/src/components/workflows/entity-type-multiselect.tsx @@ -18,11 +18,33 @@ * fires regardless of entity type — no checkboxes. * - Selecting zero options is allowed, since the backend treats `[]` as * "any entity" (back-compat with existing rows). + * - The wire format stays snake_case (`access_grant`, `data_product`, …) + * so we don't break round-trips. `prettyEntityTypeLabel` only affects + * what the user sees in the checkbox rows. */ import { useEffect } from 'react'; import { Checkbox } from '@/components/ui/checkbox'; import { Label } from '@/components/ui/label'; +/** + * Convert a snake_case entity-type wire value to Sentence case for display. + * + * "access_grant" → "Access grant" + * "data_product" → "Data product" + * "data_asset_review" → "Data asset review" + * "role" → "Role" + * "" → "" + * + * Pure / side-effect free / exported for tests. Display only — callers + * must keep using the raw `value` for any wire-format work (FK lookups, + * persisted state, etc.). + */ +export function prettyEntityTypeLabel(value: string): string { + if (!value) return value; + const spaced = value.replace(/_/g, ' '); + return spaced.charAt(0).toUpperCase() + spaced.slice(1); +} + export interface EntityTypeMultiselectProps { /** The currently-selected trigger value (for context only). */ triggerType: string; @@ -88,7 +110,7 @@ export function EntityTypeMultiselect({ onCheckedChange={() => toggle(et)} />
); diff --git a/src/frontend/src/components/workflows/trigger-picker.test.tsx b/src/frontend/src/components/workflows/trigger-picker.test.tsx index 20c4a202..02ba432a 100644 --- a/src/frontend/src/components/workflows/trigger-picker.test.tsx +++ b/src/frontend/src/components/workflows/trigger-picker.test.tsx @@ -19,7 +19,6 @@ const CATALOG: TriggerTypeOption[] = [ label: 'After entity is created', workflow_type: 'process', entity_types: ['table'], - is_advanced: false, group: 'lifecycle', }, { @@ -27,7 +26,6 @@ const CATALOG: TriggerTypeOption[] = [ label: 'After entity is updated', workflow_type: 'process', entity_types: ['data_product'], - is_advanced: false, group: 'lifecycle', }, { @@ -35,7 +33,6 @@ const CATALOG: TriggerTypeOption[] = [ label: 'Before entity is created (validation)', workflow_type: 'process', entity_types: ['table'], - is_advanced: false, group: 'validation_gates', }, { @@ -43,7 +40,6 @@ const CATALOG: TriggerTypeOption[] = [ label: 'After an access request is submitted', workflow_type: 'process', entity_types: ['access_grant'], - is_advanced: false, group: 'request_flow', }, { @@ -51,41 +47,34 @@ const CATALOG: TriggerTypeOption[] = [ label: 'On a schedule (cron)', workflow_type: 'process', entity_types: [], - is_advanced: false, group: 'system_scheduled', }, { value: 'for_subscribe', - label: 'When a user subscribes (wizard)', + label: 'When a user subscribes', workflow_type: 'approval', entity_types: ['data_product'], - is_advanced: false, group: 'request_flow', }, { value: 'for_request_access', - label: 'When a user requests access (wizard)', + label: 'When a user requests access', workflow_type: 'approval', entity_types: ['access_grant'], - is_advanced: false, group: 'request_flow', }, { value: 'for_approval_response', - label: 'Approval response dialog (advanced)', + label: 'Approval response dialog', workflow_type: 'approval', entity_types: [], - is_advanced: true, group: 'request_flow', }, ]; describe('partitionTriggers', () => { it('hides approval entries when authoring a process workflow', () => { - const buckets = partitionTriggers(CATALOG, { - workflowType: 'process', - showAdvanced: false, - }); + const buckets = partitionTriggers(CATALOG, { workflowType: 'process' }); const allValues = buckets.flatMap((b) => b.items.map((i) => i.value)); for (const v of allValues) { const entry = CATALOG.find((c) => c.value === v)!; @@ -97,10 +86,7 @@ describe('partitionTriggers', () => { }); it('hides process entries when authoring an approval workflow', () => { - const buckets = partitionTriggers(CATALOG, { - workflowType: 'approval', - showAdvanced: false, - }); + const buckets = partitionTriggers(CATALOG, { workflowType: 'approval' }); const allValues = buckets.flatMap((b) => b.items.map((i) => i.value)); for (const v of allValues) { const entry = CATALOG.find((c) => c.value === v)!; @@ -109,29 +95,16 @@ describe('partitionTriggers', () => { expect(allValues).not.toContain('on_create'); }); - it('hides advanced triggers by default', () => { - const buckets = partitionTriggers(CATALOG, { - workflowType: 'approval', - showAdvanced: false, - }); - const allValues = buckets.flatMap((b) => b.items.map((i) => i.value)); - expect(allValues).not.toContain('for_approval_response'); - }); - - it('reveals advanced triggers when toggle is on', () => { - const buckets = partitionTriggers(CATALOG, { - workflowType: 'approval', - showAdvanced: true, - }); + it('shows for_approval_response alongside other approval triggers (no advanced gating)', () => { + const buckets = partitionTriggers(CATALOG, { workflowType: 'approval' }); const allValues = buckets.flatMap((b) => b.items.map((i) => i.value)); expect(allValues).toContain('for_approval_response'); + expect(allValues).toContain('for_subscribe'); + expect(allValues).toContain('for_request_access'); }); it('produces groups in the canonical order', () => { - const buckets = partitionTriggers(CATALOG, { - workflowType: 'process', - showAdvanced: false, - }); + const buckets = partitionTriggers(CATALOG, { workflowType: 'process' }); const groupOrder = buckets.map((b) => b.group); // Process catalog above has lifecycle + validation_gates + request_flow + system_scheduled expect(groupOrder).toEqual([ @@ -144,10 +117,7 @@ describe('partitionTriggers', () => { it('omits groups that have no visible items', () => { // Approval catalog has nothing in lifecycle / validation_gates / system_scheduled - const buckets = partitionTriggers(CATALOG, { - workflowType: 'approval', - showAdvanced: false, - }); + const buckets = partitionTriggers(CATALOG, { workflowType: 'approval' }); expect(buckets).toHaveLength(1); expect(buckets[0].group).toBe('request_flow'); expect(buckets[0].label).toBe(TRIGGER_GROUP_LABELS.request_flow); diff --git a/src/frontend/src/components/workflows/trigger-picker.tsx b/src/frontend/src/components/workflows/trigger-picker.tsx index 2644e062..be18dc21 100644 --- a/src/frontend/src/components/workflows/trigger-picker.tsx +++ b/src/frontend/src/components/workflows/trigger-picker.tsx @@ -11,7 +11,6 @@ * - We group entries with instead of a flat list, so the * 25-ish triggers stop reading as duplicates ("on_subscribe" / * "for_subscribe" now sit under the same group with disambiguated labels). - * - We hide the advanced "for_approval_response" trigger behind a toggle. * * The grouping/filtering logic is exported separately as * `partitionTriggers` so it can be unit-tested without dragging Radix into @@ -19,8 +18,6 @@ */ import { useEffect, useMemo, useState } from 'react'; import { useApi } from '@/hooks/use-api'; -import { Switch } from '@/components/ui/switch'; -import { Label } from '@/components/ui/label'; import { Select, SelectContent, @@ -47,7 +44,6 @@ export interface TriggerTypeOption { label: string; workflow_type: 'approval' | 'process'; entity_types: string[]; - is_advanced: boolean; group: 'lifecycle' | 'request_flow' | 'validation_gates' | 'system_scheduled'; } @@ -77,14 +73,9 @@ export function partitionTriggers( options: TriggerTypeOption[], args: { workflowType: 'approval' | 'process'; - showAdvanced: boolean; }, ): Array<{ group: TriggerTypeOption['group']; label: string; items: TriggerTypeOption[] }> { - const visible = options.filter((o) => { - if (o.workflow_type !== args.workflowType) return false; - if (o.is_advanced && !args.showAdvanced) return false; - return true; - }); + const visible = options.filter((o) => o.workflow_type === args.workflowType); return TRIGGER_GROUP_ORDER .map((g) => ({ group: g, @@ -113,7 +104,6 @@ export function TriggerPicker({ }: TriggerPickerProps) { const { get } = useApi(); const [options, setOptions] = useState(optionsProp ?? []); - const [showAdvanced, setShowAdvanced] = useState(false); useEffect(() => { if (optionsProp) { @@ -139,58 +129,43 @@ export function TriggerPicker({ }, [get, optionsProp]); const groups = useMemo( - () => partitionTriggers(options, { workflowType, showAdvanced }), - [options, workflowType, showAdvanced], + () => partitionTriggers(options, { workflowType }), + [options, workflowType], ); return ( -
- -
- - -
-
+ ); } diff --git a/src/frontend/src/lib/workflow-labels.test.ts b/src/frontend/src/lib/workflow-labels.test.ts index 73f78730..8949cedd 100644 --- a/src/frontend/src/lib/workflow-labels.test.ts +++ b/src/frontend/src/lib/workflow-labels.test.ts @@ -175,19 +175,19 @@ describe('getTriggerLabel', () => { // (value, expected) — one row per TriggerType, matching the backend // _TRIGGER_LABELS dict and the user-approved table in the PR brief. const expected: Array<[string, string]> = [ - ['for_subscribe', 'When a user subscribes (wizard)'], + ['for_subscribe', 'When a user subscribes'], ['on_subscribe', 'After a subscription is created'], - ['for_request_access', 'When a user requests access (wizard)'], + ['for_request_access', 'When a user requests access'], ['on_request_access', 'After an access request is submitted'], - ['for_request_review', 'When a user requests review (wizard)'], + ['for_request_review', 'When a user requests review'], ['on_request_review', 'After a review request is submitted'], - ['for_request_publish', 'When a user requests publish (wizard)'], + ['for_request_publish', 'When a user requests publish'], ['on_request_publish', 'After a publish request is submitted'], - ['for_request_certify', 'When a user requests certification (wizard)'], + ['for_request_certify', 'When a user requests certification'], ['on_request_certify', 'After a certification request is submitted'], - ['for_request_status_change', 'When a user requests status change (wizard)'], + ['for_request_status_change', 'When a user requests status change'], ['on_request_status_change', 'After a status change request is submitted'], - ['for_approval_response', 'Approval response dialog (advanced)'], + ['for_approval_response', 'Approval response dialog'], ['before_create', 'Before entity is created (validation)'], ['before_update', 'Before entity is updated (validation)'], ['before_status_change', 'Before status changes (validation)'], diff --git a/src/frontend/src/lib/workflow-labels.ts b/src/frontend/src/lib/workflow-labels.ts index 63681b67..43a078de 100644 --- a/src/frontend/src/lib/workflow-labels.ts +++ b/src/frontend/src/lib/workflow-labels.ts @@ -123,19 +123,19 @@ export function getTriggerTypeLabel(type: TriggerType, t: TFunction): string { * src/backend/src/routes/workflows_routes.py. */ export const TRIGGER_LABELS: Record = { - for_subscribe: 'When a user subscribes (wizard)', + for_subscribe: 'When a user subscribes', on_subscribe: 'After a subscription is created', - for_request_access: 'When a user requests access (wizard)', + for_request_access: 'When a user requests access', on_request_access: 'After an access request is submitted', - for_request_review: 'When a user requests review (wizard)', + for_request_review: 'When a user requests review', on_request_review: 'After a review request is submitted', - for_request_publish: 'When a user requests publish (wizard)', + for_request_publish: 'When a user requests publish', on_request_publish: 'After a publish request is submitted', - for_request_certify: 'When a user requests certification (wizard)', + for_request_certify: 'When a user requests certification', on_request_certify: 'After a certification request is submitted', - for_request_status_change: 'When a user requests status change (wizard)', + for_request_status_change: 'When a user requests status change', on_request_status_change: 'After a status change request is submitted', - for_approval_response: 'Approval response dialog (advanced)', + for_approval_response: 'Approval response dialog', before_create: 'Before entity is created (validation)', before_update: 'Before entity is updated (validation)', before_status_change: 'Before status changes (validation)', From 9cc1c45ad9c157f29f13f350d85e6bbe7f708ffe Mon Sep 17 00:00:00 2001 From: Mikhail Konchits Date: Fri, 15 May 2026 07:56:02 +0200 Subject: [PATCH 6/6] feat(workflows): rename picker labels (Fires on / Applies to) + access_grant display override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UX feedback on PR #369 final round: 1. Trigger-picker field label "Type" → "Fires on" with helper "What action makes this workflow run". Label now lives inside so the parent form doesn't need a duplicate. 2. Entity-type multiselect field label "Category" → "Applies to" with helper "Which kinds of objects this fires on". Label now lives inside for the same reason. 3. prettyEntityTypeLabel: add a small OVERRIDES map and remap access_grant → "Data object". All other entity types still flow through the snake_case → Sentence case rule. Wire format is unchanged — only the visible label moves. End result reads naturally in the picker: Fires on: When a user requests access Applies to: Data object / Project / App role Tests: - entity-type-multiselect.test.tsx: assert override case and new "Applies to" field label. - trigger-picker.test.tsx: assert exported TRIGGER_PICKER_LABEL constant (Radix hangs — see + // file header), so we assert on the exported label constant the + // component renders verbatim above the Select. + it('exposes the "Fires on" field label so the parent form does not need a duplicate', () => { + expect(TRIGGER_PICKER_LABEL).toBe('Fires on'); + }); +}); diff --git a/src/frontend/src/components/workflows/trigger-picker.tsx b/src/frontend/src/components/workflows/trigger-picker.tsx index be18dc21..dbdbda6e 100644 --- a/src/frontend/src/components/workflows/trigger-picker.tsx +++ b/src/frontend/src/components/workflows/trigger-picker.tsx @@ -18,6 +18,7 @@ */ import { useEffect, useMemo, useState } from 'react'; import { useApi } from '@/hooks/use-api'; +import { Label } from '@/components/ui/label'; import { Select, SelectContent, @@ -35,6 +36,11 @@ import { } from '@/components/ui/tooltip'; import { getTriggerLabel } from '@/lib/workflow-labels'; +/** Field-label constants for the trigger picker. Exported so the parent + * form and tests can reference the same source of truth. */ +export const TRIGGER_PICKER_LABEL = 'Fires on'; +export const TRIGGER_PICKER_HELPER = 'What action makes this workflow run'; + /** * Wire-format entry from GET /api/workflows/trigger-types. * One per TriggerType enum member. @@ -134,38 +140,42 @@ export function TriggerPicker({ ); return ( - +
+ +

{TRIGGER_PICKER_HELPER}

+ +
); } diff --git a/src/frontend/src/components/workflows/workflow-designer.tsx b/src/frontend/src/components/workflows/workflow-designer.tsx index 19d455c8..d0005355 100644 --- a/src/frontend/src/components/workflows/workflow-designer.tsx +++ b/src/frontend/src/components/workflows/workflow-designer.tsx @@ -1,6 +1,5 @@ import { useState, useCallback, useMemo, useEffect } from 'react'; import { useNavigate, useParams, useSearchParams } from 'react-router-dom'; -import { useTranslation } from 'react-i18next'; import ReactFlow, { Node, Edge, @@ -507,7 +506,6 @@ export default function WorkflowDesigner({ workflowId }: WorkflowDesignerProps) const { get, post, put } = useApi(); const { toast } = useToast(); - const { t } = useTranslation(['common']); const setStaticSegments = useBreadcrumbStore((state) => state.setStaticSegments); const setDynamicTitle = useBreadcrumbStore((state) => state.setDynamicTitle); @@ -1250,7 +1248,6 @@ export default function WorkflowDesigner({ workflowId }: WorkflowDesignerProps) // Trigger configuration <>
- { @@ -1266,7 +1263,6 @@ export default function WorkflowDesigner({ workflowId }: WorkflowDesignerProps) />
-