fix(designer): warn before enabling trigger concurrency - #9560
Draft
Elaina Lee (Elaina-Lee) wants to merge 2 commits into
Draft
fix(designer): warn before enabling trigger concurrency#9560Elaina Lee (Elaina-Lee) wants to merge 2 commits into
Elaina Lee (Elaina-Lee) wants to merge 2 commits into
Conversation
Enabling trigger concurrency control is irreversible for a trigger,
so require explicit user confirmation before the toggle takes effect.
Adds a reusable confirm button label to the shared Confirm dialog and
wires a warning confirmation into the v1 and v2 designer General
settings sections when a trigger's concurrency control is switched on.
Fixes serverless-paas-balam/logic-apps-central#610
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
🤖 AI PR Validation ReportPR Review ResultsThank you for your submission! Here's detailed feedback on your PR title and body compliance:✅ PR Title
✅ Commit Type
✅ Risk Level
✅ What & Why
✅ Impact of Change
✅ Test Plan
|
| Section | Status | Recommendation |
|---|---|---|
| Title | ✅ | No change needed |
| Commit Type | ✅ | No change needed |
| Risk Level | ✅ | risk:low is correct |
| What & Why | ✅ | No change needed |
| Impact of Change | ✅ | No change needed |
| Test Plan | ✅ | Unit tests confirmed |
| Contributors | Credit PM/designer/reviewers | |
| Screenshots/Videos | ❌ | Add dialog screenshots (v1 save + v2 publish) |
Blocked: everything checks out except Screenshots/Videos. Please attach screenshots of the updated confirmation dialog (both the v1 "save" and v2 "publish" wording) and this PR will pass. Risk level is correctly declared as low.
Powered by: Copilot CLI (claude-opus-4.8) | Last updated: Sat, 22 Aug 2026 00:41:15 GMT
Contributor
|
📊 Coverage check completed. See workflow run for details. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds confirmation before irreversibly enabling trigger concurrency in both designer versions.
Changes:
- Gates trigger concurrency behind an Enable/Cancel dialog.
- Adds customizable confirmation-button text.
- Adds localized strings and unit coverage.
Readiness: Missing required screenshot; validate-pr, build, coverage, and several E2E checks are not green. Two formatting findings remain.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
Localize/lang/strings.json |
Adds dialog strings. |
libs/designer/src/lib/ui/settings/sections/general.tsx |
Adds v1 confirmation flow. |
libs/designer/src/lib/ui/settings/__tests__/general.spec.tsx |
Tests v1 behavior. |
libs/designer-v2/src/lib/ui/settings/sections/general.tsx |
Adds v2 confirmation flow. |
libs/designer-v2/src/lib/ui/settings/__tests__/general.spec.tsx |
Tests v2 behavior. |
libs/designer-ui/src/lib/dialogs/confirm.tsx |
Adds configurable confirm text. |
libs/designer-ui/src/lib/dialogs/__test__/confirm.spec.tsx |
Tests dialog labels and callbacks. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| description: 'Title for the confirmation dialog before enabling trigger concurrency control', | ||
| }); | ||
| const enableConcurrencyConfirmationMessage = intl.formatMessage({ | ||
| defaultMessage: `After you turn on concurrency control, this setting can't be changed.`, |
| description: 'Title for the confirmation dialog before enabling trigger concurrency control', | ||
| }); | ||
| const enableConcurrencyConfirmationMessage = intl.formatMessage({ | ||
| defaultMessage: `After you turn on concurrency control, this setting can't be changed.`, |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Elaina Lee (Elaina-Lee)
marked this pull request as draft
August 22, 2026 00:40
This was referenced Aug 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Commit Type
Risk Level
What & Why
Enabling trigger concurrency control is irreversible for a trigger once turned on � the setting cannot be switched off again after being saved/published. Previously, the designer's General settings toggle enabled this option immediately on click, with no warning that the choice is permanent.
This change adds an explicit user consent step: clicking the toggle to enable trigger concurrency control (from off to on, trigger operations only) now opens a confirmation dialog that states the setting can't be turned off once it takes effect. The toggle only takes effect after the user explicitly confirms via the dialog's "Enable" button; canceling leaves the setting untouched. Enabling/disabling concurrency on non-trigger (action) operations, and turning concurrency control off, are unaffected and continue to apply immediately, since only the trigger-enable path is irreversible.
This follow-up makes the lock itself save/publish-aware and reversible until then:
Implements the fix requested in serverless-paas-balam/logic-apps-central#610.
Impact of Change
Confirmdialog component (libs/designer-ui/src/lib/dialogs/confirm.tsx) has an optionalconfirmTextprop to override the default "OK" primary button label. The v1 (libs/designer/...) and v2 (libs/designer-v2/...) General settings sections each add: a persisted-definition read (isTriggerConcurrencyEnabledInDefinitionincore/actions/bjsworkflow/settings.ts) that checks the last-loaded operation definition rather than in-session edits; a workflow selector (getPersistedNodeOperation/getOriginalNodeIdincore/state/workflow/workflowSelectors.ts) that resolves a node's persisted operation by direct id first and falls back to the pre-rename id viaidReplacements; and alockTriggerConcurrencyprop threaded fromsettings/index.tsxintosettings/sections/general.tsxthat marks the trigger-concurrency toggle read-only once persisted-enabled. Warning copy for the confirmation dialog is now save/publish-specific per surface.Test Plan
triggerConcurrencyLock.spec.tscoveringisTriggerConcurrencyEnabledInDefinition,getOriginalNodeId, andgetPersistedNodeOperationresolution (direct-id-first with reverse-id fallback, before and after a same-session save/reinitialize) for both v1 (libs/designer) and v2 (libs/designer-v2); updatedgeneral.spec.tsxfor both v1 and v2 covering the locked/reversible toggle states and confirmation copy. Targeted Biome/ESLint formatting and lint checks passed on all changed files.Contributors
Screenshots/Videos
Pending � the confirmation dialog copy changed in this follow-up (save-specific wording for v1, publish-specific wording for v2), so the previously attached screenshot no longer reflects the current dialog text. Requesting a fresh screenshot of the updated confirmation dialog (both v1 "save" and v2 "publish" variants if feasible) before this leaves draft. No screenshot is being fabricated here.