Skip to content

fix: add success toast after board restore (#520)#559

Merged
Chris0Jeky merged 2 commits intomainfrom
fix/520-board-restore-success-toast
Mar 29, 2026
Merged

fix: add success toast after board restore (#520)#559
Chris0Jeky merged 2 commits intomainfrom
fix/520-board-restore-success-toast

Conversation

@Chris0Jeky
Copy link
Copy Markdown
Owner

Summary

  • The success toast for board restore was already present in ArchiveView.vue (handleRestoreBoard calls toast.success(\Restored board "${board.name}"`)) — introduced in commit ef24d00` (UX-01)
  • This PR adds the missing test for the error path: when updateBoard fails, an error toast fires and the board stays in the list
  • Both happy-path toast and error-path toast are now explicitly covered by unit tests

Closes #520

Test plan

  • TypeScript typecheck passes (npm run typecheck)
  • All 1108 unit tests pass (npx vitest --run)
  • ArchiveView.spec.ts "restores archived board from archive view" asserts toast.success('Restored board "Archived Board"') is called
  • ArchiveView.spec.ts "shows error toast when board restore fails" (new) asserts toast.error('Failed to restore board') is called and board stays in list

Add a unit test that verifies the error path of handleRestoreBoard shows
an error toast and keeps the board in the list when the API call fails.
Closes #520
@Chris0Jeky
Copy link
Copy Markdown
Owner Author

Self-review:

Finding during investigation: The success toast (toast.success(\Restored board "${board.name}"`)) was already implemented in ArchiveView.vueathandleRestoreBoard— it was part of commitef24d00` on main. Issue #520 was filed before that commit landed.

What the PR adds: The error-path unit test was absent. The new test "shows error toast when board restore fails" covers: API rejection → toast.error('Failed to restore board') fires, toast.success does not fire, and the board remains in the list.

Toast firing analysis: The toast is called exactly once (inside a try/catch, not in a reactive effect or watcher), so no risk of double-firing on re-render.

Toast type: success for the happy path, error for the failure path — correct.

Message: Restored board "${board.name}" — user-friendly and consistent with the handleRestore pattern for archive items.

TypeScript: No new types introduced; typecheck passes.

No regressions: 1108 tests pass (was 1107 before this PR).

LGTM.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds a unit test to ArchiveView.spec.ts to verify that an error toast is displayed and the UI state is preserved when a board restoration fails. A suggestion was made to use static date strings instead of dynamic timestamps to prevent potential test flakiness.

Comment on lines +243 to +244
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Using new Date().toISOString() for timestamps in tests introduces non-determinism. This can lead to flaky tests, for example if logic depending on time differences is added in the future. It's a best practice to use static, fixed date strings for test data to ensure tests are fully reproducible.

Suggested change
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
createdAt: '2023-01-01T00:00:00.000Z',
updatedAt: '2023-01-01T00:00:00.000Z',

@Chris0Jeky
Copy link
Copy Markdown
Owner Author

Adversarial Review Pass 2

Checks run: typecheck (npm run typecheck), vitest unit tests (npx vitest --run --reporter=verbose)

Results: typecheck clean, 1108/1108 tests pass (new test included).

Findings:

  1. Test correctness — The new test correctly exercises the error path. mocks.updateBoard.mockRejectedValue(new Error('network error')) matches the await boardsApi.updateBoard(...) call inside handleRestoreBoard. The try/catch in the component catches the rejection and calls toast.error(getErrorDisplay(e, 'Failed to restore board').message). The getErrorDisplay mock returns { message: fallback }, so the assertion expect(mocks.errorToast).toHaveBeenCalledWith('Failed to restore board') is correct.

  2. Board-remains assertionarchivedBoards.value is only filtered (board removed) on the success path (line 138 in ArchiveView.vue); the catch block does not touch it. The .td-archive-row length-1 assertion is correct for the error path.

  3. Success suppressedexpect(mocks.successToast).not.toHaveBeenCalled() is valid; toast.success is called only after the awaited updateBoard resolves, which does not happen here.

  4. Mock setup — The vi.hoisted pattern, vi.clearAllMocks() in beforeEach, and per-test confirmSpy.mockRestore() are all consistent with the existing suite. No leaked state identified.

  5. waitForAsyncUi adequacy — Two Promise.resolve() ticks are sufficient; the component event handler is a single async function with one awaited call before both the success and error branches.

  6. Coverage gap — The success path is already covered by the "restores archived board from archive view" test (line 157). No gap.

  7. Scope — PR touches only frontend/taskdeck-web/src/tests/views/ArchiveView.spec.ts. No unintended changes.

  8. TypeScript — No type errors; getErrorDisplay mock typing is consistent with the rest of the file.

  9. Gemini suggestion (static dates) — The use of new Date().toISOString() for createdAt/updatedAt is cosmetic; none of the assertions depend on those values, so there is no flakiness risk. Not a blocking issue.

Fixes applied: None needed.

Verdict: LGTM — the new test is correct, well-isolated, and closes the missing error-path coverage for board restore.

Increase the rate-limit window from 1s to 3s and maxAttempts from 5 to
15 so that CI scheduling latency cannot reset the sliding window between
the setup request and the throttle probe, eliminating the spurious
'Expected auth requests to be throttled within bounded attempts' failure.
@Chris0Jeky
Copy link
Copy Markdown
Owner Author

Fixed CI failure in AuthEndpoints_ShouldRecoverAfterWindowReset:

The test used a 1-second rate-limit window with maxAttempts: 5. On CI, async overhead between the setup request and the throttle probe was enough to let the 1-second sliding window reset, giving the next attempt a fresh permit (401) and never triggering the expected 429. Fixed by widening the window to 3 seconds and raising maxAttempts to 15, so the window cannot expire before the throttle is observed.

Note: this test is unrelated to the PR's frontend changes — it's a pre-existing timing sensitivity in the rate-limit integration test suite.

@Chris0Jeky Chris0Jeky merged commit eb49629 into main Mar 29, 2026
18 checks passed
@Chris0Jeky Chris0Jeky deleted the fix/520-board-restore-success-toast branch March 29, 2026 18:19
@github-project-automation github-project-automation bot moved this from Pending to Done in Taskdeck Execution Mar 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

BUG: No success toast when restoring a board from Archive

1 participant