Skip to content

FIX: stop pushing a duplicate history entry on first page finding saves - #3667

Merged
hannahwestra25 merged 5 commits into
microsoft:mainfrom
ManoharPaturi:fix/finding-save-duplicate-history
Nov 6, 2026
Merged

hannahwestra25 merged 5 commits into
microsoft:mainfrom
ManoharPaturi:fix/finding-save-duplicate-history

Conversation

@ManoharPaturi

Copy link
Copy Markdown
Contributor

Description

Fixes #3666

Saving or deleting a finding always rewrote the findings page offset in the URL. On the first page there is no offset to change, so the router received the identical URL and still pushed a new history entry for it. Each save or delete on the first page added one dead Back press, and leaving the page afterwards took as many presses as there had been saves.

The offset navigation is now skipped when the URL already carries the requested offset. The refresh after a save or delete keeps working through the existing revision bump, and navigating back from a later page to the first still updates the URL exactly as before.

Tests and Documentation

Added a regression that records every router location during a save on the first page and asserts the history holds a single entry. It fails on main with a second entry for the identical URL and passes with the change. The full operation detail suite passes (30 tests) and tsc is clean.

No documentation change needed, so I did not run JupyText.

Saving or deleting a finding on the first findings page rewrote an identical URL, and the router pushed a new history entry for it, so Back needed one extra press per save. The offset navigation is skipped when the requested offset is already in the URL.

Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
@hannahwestra25 hannahwestra25 self-assigned this Nov 2, 2026
The location key assertion pinned the implementation and would fail a replace based fix that works; the router driven cases now start from the operations list, save or delete a finding, press Back, and assert the list is restored, covering both paths through navigatePage(0).

Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 5, 2026 08:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@ManoharPaturi

Copy link
Copy Markdown
Contributor Author

You were right that the test pinned the implementation: it is replaced in 4b0e5a9 with the behavior block you suggested, which drives a real memory router from the operations list, saves or deletes a finding on the first page, goes Back, and asserts the list is restored. It covers both paths through navigatePage(0), which the old test never did for delete.

Verified in both directions before pushing: with the offset bail disabled the two Back cases fail, and with the component swapped to main they fail as you found, while a replace based fix would pass. The suite passes at 31 tests with tsc clean.

Build the next search params once and skip navigation only when the
serialized query is unchanged, so the guard and the write cannot drift.
Remove stray blank lines left by the test rewrite.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

@hannahwestra25 hannahwestra25 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for contributing! added a few small changes jfyi

@hannahwestra25
hannahwestra25 added this pull request to the merge queue Nov 5, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 5, 2026
@hannahwestra25
hannahwestra25 added this pull request to the merge queue Nov 6, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 6, 2026
@hannahwestra25
hannahwestra25 added this pull request to the merge queue Nov 6, 2026
Merged via the queue into microsoft:main with commit 8a1e07e Nov 6, 2026
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Saving or deleting a finding on the first page pushes a duplicate browser history entry

4 participants