unbridled-41 opened a new pull request, #4892:
URL: https://github.com/apache/rocketmq-dashboard/pull/4892

   ## Problem
   
   Restoring a query-history entry whose data source needs credentials applies 
**half of the entry before anything is confirmed**: the drawer closes, the 
range, the profile (and the persisted profile) and — for a custom entry — the 
query text are all replaced, while only the data-source switch waits for 
`handleAuthSubmit`. Cancelling the credentials prompt therefore leaves the 
explorer in a state it was explicitly designed not to reach: the profile 
selector shows the restored profile while the panel grid still holds the 
previous profile's data, so the restored profile's cards are empty with no 
query in flight to fill them, and the profile written to `localStorage` is the 
one the operator declined.
   
   ## Evidence
   
   `web/src/components/MetricsExplorer.tsx` (base `1ef5d860`):
   
   - `handleRestoreHistory` writes the selection before the prompt: 
`setRangeId(nextRange.value)`; `setCustomPromql(entry.promql)` for a custom 
entry; `localStorage.setItem(PROFILE_STORAGE_KEY, nextProfile.id)` + 
`setProfileId(nextProfile.id)` for a profile entry — and only then calls 
`restoreProtectedDataSource(...)`.
   - `restoreProtectedDataSource` stores the replay and opens the modal; it 
issues no query.
   - `handleAuthCancel` dropped only the pending source 
(`pendingAuthReplayRef.current = null; setPendingDataSource(null)`), so the 
replaced selection stayed.
   - The intent is documented in the same function: *"the current source stays 
active while credentials are being asked for, so cancelling the dialog leaves 
the explorer exactly where it was instead of stranded on an unauthenticated 
source"* — that contract was implemented for the source alone (#4643, for 
#4644) and not for the selection the same restore had already replaced.
   
   Reproduction (deterministic Vitest, `MetricsExplorer.test.tsx` › `leaves the 
picked profile and range alone when a protected history restore is cancelled`): 
the explorer starts on `RocketMQ 5.x Native` over the default `1h` window; a 
history entry restoring `RocketMQ 4.x Exporter` over `6h` with a `Basic Auth` 
data source is restored and the prompt cancelled. On the unfixed source the 
assertion fails with `Unable to find an element with the text: RocketMQ 5.x 
Native` — the selector had switched to the declined entry's profile, the 
segmented control to `6h`, and `rocketmq-studio.metric-profile` had been 
written.
   
   ## Root cause
   
   The restore mutates live selection state eagerly and defers only the source, 
so "cancel" had no way back to the previous selection.
   
   ## Fix
   
   The deferred restore now carries the selection it replaced 
(`RestoreCheckpoint`: profile id, range id, custom expression, persisted 
profile id), captured before the entry is applied. `handleAuthCancel` puts all 
four back before dropping the pending source, so declining the prompt is a 
no-op again. A confirmed prompt is untouched: `handleAuthSubmit` still replays 
the entry against the newly authenticated source, and an entry whose source 
needs no credentials is still applied immediately without a prompt.
   
   ## Tests
   
   - `npx vitest run src/components/__tests__/MetricsExplorer.test.tsx` → **30 
passed** (29 pre-existing + the new regression). On the unfixed source the new 
test fails as quoted above (test kept, source reverted).
   - `npx vitest run src/components` → **88 passed** (8 files); the 
neighbouring cancel/restore coverage still passes: `keeps the current data 
source when cancelling a protected history restore`, `requires credentials 
again when restoring a protected data source history item`, `restores profile, 
range, and source from query history`.
   - `npx tsc --noEmit` → clean; `npx eslint src/components/MetricsExplorer.tsx 
src/components/__tests__/MetricsExplorer.test.tsx` → clean.
   
   ## Scoring
   
   PRIORITY 58 (impact 20: the explorer is left showing a profile it never 
queried, with a declined profile persisted; reach 12: every history restore of 
a protected source; reproducibility 16: deterministic test; maintenance value 
10: completes the cancel contract of #4643 and closes the residue #4644 was 
closed for). FIX_CONFIDENCE 85.
   
   ## Risk
   
   Low and local to the cancel path: nothing changes for a submitted prompt, 
for a restore without credentials, or for switching a data source from the 
selector (where no replay is pending, so `handleAuthCancel` returns early). The 
checkpoint is read from the render scope before the entry is applied, so it 
holds the values the operator actually had.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to