fix(ui): stop a superseded Test Connection run from clobbering a newer result

[IceS2]

Hitting Test Connection again while a run is still in flight leaves the first run's
timers alive. When they fire they overwrite the newer run's state, so the dialog can show
a stale success for a connection that has since been changed and now fails — or the
reverse.

TestConnection now keeps its timers in a ref and tags each run with an id. Callbacks
from a superseded run are ignored, and every timer is cleared on unmount, which also stops
the component polling a workflow after the dialog is closed.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[IceS2]

Good catch — fixed in 9567a26.

The reachable path is the run's own expiry: the poll callback awaits getWorkflowData, the
2-minute timeout fires during that await and re-enables the button, and a run started from there
was then clobbered by the stale callback, which also cleared the new run's timers and left it
polling nothing. Re-checking runId after the await closes it.

Covered by a test that drives exactly that sequence; verified to fail with the new guard removed.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

• No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

[github-actions]

Jest test Coverage

UI tests summary

Lines	Statements	Branches	Functions
	66.22% (78174/118039)	50.22% (47205/93988)	51.44% (14227/27653)

[github-actions]

✅ Playwright Results — workflow succeeded

Validated commit 9c4148717ffe9a2898a78c70c1b5ef2c4ef83727 in Playwright run 31185335441, attempt 1.

✅ 582 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Performance

Blocking targets: ✅ met · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 55m 30s

⏱️ Max setup 3m 0s · max shard execution 18m 18s · max shard-job elapsed before upload 21m 41s · reporting 5s

🌐 201.69 requests/attempt · 2.80 app boots/UI scenario · 25.87% common-shard skew

Optimization targets still in progress:

• Common shard skew was 25.87% (convergence target: at most 15%).
• Browser traffic was 201.69 requests per attempt (convergence target: fewer than 200).
• Application boot ratio was 2.8 per UI scenario (1692 boots / 604 scenarios; convergence target: at most 1).

Shard	Passed	Failed	Flaky	Skipped	Lifecycle failed	Lifecycle flaky
✅ Shard chromium-01	146	0	0	0	0	0
✅ Shard chromium-02	127	0	0	0	0	0
✅ Shard chromium-03	137	0	0	0	0	0
✅ Shard data-asset-rules-01	61	0	0	0	0	0
✅ Shard domain-isolation-01	14	0	0	0	0	0
✅ Shard global-state-01	34	0	0	0	0	0
✅ Shard ingestion-01	22	0	0	0	0	0
✅ Shard reindex-01	2	0	0	0	0	0
✅ Shard search-01	10	0	0	0	0	0
✅ Shard search-rbac-01	29	0	0	0	0	0

📦 Download artifacts

How to debug locally

# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

⚠️ UI Checkstyle passed — lint findings in changed files

🔍 ESLint findings in this PR's files — 0 error(s), 21 warning(s)

Errors block the build. Warnings do not yet — they are rules whose backlog is still
being worked down, listed so this PR does not add to it. See docs/ui-code-quality-gate.md.

0 error(s), 21 warning(s) across 2 changed file(s).

Count	Rule
8	sonarjs/no-duplicate-string
4	i18next/no-literal-string
2	sonarjs/no-hardcoded-ip
2	react-hooks/exhaustive-deps
1	sonarjs/no-extra-arguments
1	sonarjs/cyclomatic-complexity
1	sonarjs/expression-complexity
1	jsx-a11y/control-has-associated-label
1	jsx-a11y/anchor-has-content

All findings

	Location	Rule	Message
🟡	src/components/common/TestConnection/TestConnection.test.tsx:81:52	i18next/no-literal-string	disallow literal string: Modal {connectionDisplayName} {errorMess
🟡	src/components/common/TestConnection/TestConnection.test.tsx:94:40	i18next/no-literal-string	disallow literal string: Cancel Modal
🟡	src/components/common/TestConnection/TestConnection.test.tsx:95:41	i18next/no-literal-string	disallow literal string: Confirm Modal
🟡	src/components/common/TestConnection/TestConnection.test.tsx:96:48	i18next/no-literal-string	disallow literal string: Retry Modal
🟡	src/components/common/TestConnection/TestConnection.test.tsx:160:53	sonarjs/no-duplicate-string	Define a constant instead of duplicating this literal 42 times.
🟡	src/components/common/TestConnection/TestConnection.test.tsx:236:31	sonarjs/no-duplicate-string	Define a constant instead of duplicating this literal 6 times.
🟡	src/components/common/TestConnection/TestConnection.test.tsx:258:31	sonarjs/no-duplicate-string	Define a constant instead of duplicating this literal 5 times.
🟡	src/components/common/TestConnection/TestConnection.test.tsx:500:24	sonarjs/no-duplicate-string	Define a constant instead of duplicating this literal 3 times.
🟡	src/components/common/TestConnection/TestConnection.test.tsx:537:24	sonarjs/no-duplicate-string	Define a constant instead of duplicating this literal 3 times.
🟡	src/components/common/TestConnection/TestConnection.test.tsx:622:31	sonarjs/no-duplicate-string	Define a constant instead of duplicating this literal 6 times.
🟡	src/components/common/TestConnection/TestConnection.test.tsx:1100:52	sonarjs/no-hardcoded-ip	Make sure using a hardcoded IP address 10.0.0.1 is safe here.
🟡	src/components/common/TestConnection/TestConnection.test.tsx:1113:25	sonarjs/no-hardcoded-ip	Make sure using a hardcoded IP address 10.0.0.1 is safe here.
🟡	src/components/common/TestConnection/TestConnection.test.tsx:1278:7	sonarjs/no-extra-arguments	This function expects no arguments, but 1 was provided.
🟡	src/components/common/TestConnection/TestConnection.tsx:130:4	sonarjs/cyclomatic-complexity	{"message":"Function has a complexity of 12 which is greater than 10 authorized.","cost":2,"secondaryLocations":[{"line":130,"column":3,"endLine":130,"endColumn
🟡	src/components/common/TestConnection/TestConnection.tsx:193:5	sonarjs/expression-complexity	Reduce the number of conditional operators (4) used in the expression (maximum allowed 3).
🟡	src/components/common/TestConnection/TestConnection.tsx:560:19	sonarjs/no-duplicate-string	Define a constant instead of duplicating this literal 3 times.
🟡	src/components/common/TestConnection/TestConnection.tsx:560:52	sonarjs/no-duplicate-string	Define a constant instead of duplicating this literal 3 times.
🟡	src/components/common/TestConnection/TestConnection.tsx:567:6	react-hooks/exhaustive-deps	React Hook useMemo has a missing dependency: 't'. Either include it or remove the dependency array.
🟡	src/components/common/TestConnection/TestConnection.tsx:606:13	jsx-a11y/control-has-associated-label	A control must be associated with a text label.
🟡	src/components/common/TestConnection/TestConnection.tsx:606:13	jsx-a11y/anchor-has-content	Anchors must have content and the content must be accessible by a screen reader.
🟡	src/components/common/TestConnection/TestConnection.tsx:712:6	react-hooks/exhaustive-deps	React Hook useEffect has a missing dependency: 'clearTimers'. Either include it or remove the dependency array.

---

Fix locally (fast - only checks files changed in this branch):

make ui-checkstyle-changed

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[gitar-bot]

Code Review ✅ Approved 1 resolved / 1 findings

Adds run ID tagging and timer cleanup to TestConnection to prevent superseded runs from clobbering newer results, addressing the stale-run guard finding.

✅ 1 resolved

✅ Bug: Stale-run guard not re-checked after async getWorkflowData await

📄 openmetadata-ui/src/main/resources/ui/src/components/common/TestConnection/TestConnection.tsx:348-362
The interval callback checks runId !== runIdRef.current only at the start of the tick, then does await getWorkflowData(...). If the user re-triggers Test Connection while that request is in flight, a new run bumps runIdRef and arms fresh timers; the superseded callback then resumes past its guard and calls handleCompletionStatus (writing the stale run's result over the newer one) and clearTimers() (clearing the new run's interval/timeout, so the new run stops polling and never completes). This is the same clobbering the PR intends to prevent, still reachable through the async gap. Re-check the runId immediately after the await and bail out before touching state or timers.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source}

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ui'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud