PR 9: E2E — File Actions & Error States

[nitrobass24]

Summary

• Add file-actions.spec.ts with 11 tests covering file row selection, action button visibility/disabled states, delete confirmation double-click pattern, selection switching, and bulk action bar operations
• Add error-states.spec.ts with 5 tests covering no-enabled-pairs warning notification, notification text and alert level, restart notification on config change, and re-enabling a pair clears the warning

Test plan

• CI passes (Playwright tests require running container)
• file-actions.spec.ts tests properly skip when no files present on remote seedbox
• error-states.spec.ts tests restore original pair enabled states in finally blocks
• No flaky tests — uses Playwright built-in waiting, not setTimeout

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Added comprehensive e2e test coverage for error notifications, validating alerts for disabled path pairs, server address changes, and state recovery scenarios.
  • Added e2e tests for file actions UI, covering single and bulk file selection workflows, action button states, confirmation prompts, and selection toggling behavior.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ae6eddee-160c-4c29-abbe-aeff057f044d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• ✅ Review completed - (🔄 Check again to review again)

📝 Walkthrough

Walkthrough

Two new Playwright e2e test files are added to validate UI behavior: one tests error state alerts for disabled path pairs and server address changes, the other validates file action buttons and bulk action interactions in the dashboard interface.

Changes

Cohort / File(s)	Summary
Error State Tests src/e2e-playwright/tests/error-states.spec.ts	Validates header alert UI behavior when path pairs are disabled, handles edge cases where no pairs exist by creating temporary disabled pairs, verifies server address config changes trigger restart notifications, and confirms re-enabling pairs clears warnings.
File Action Tests src/e2e-playwright/tests/file-actions.spec.ts	Tests file action button visibility and disabled states for single files, validates selection toggling behavior, and confirms bulk action buttons appear/disappear based on file checkbox states.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

• Issue #330: These new test files directly implement the "Error States", "Path Pairs", and "File List Interactions" test expansions requested in the issue.

Possibly related PRs

• PR #309: Introduces the Playwright test framework, fixtures, and page objects that these new test specifications depend on for execution.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title 'PR 9: E2E — File Actions & Error States' accurately summarizes the two main additions: new E2E tests for file actions and error states functionality.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test/e2e-file-actions-error-states

Warning

Review ran into problems

🔥 Problems

Timed out fetching pipeline failures after 30000ms

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/e2e-playwright/tests/error-states.spec.ts`:
- Around line 141-144: The locator for the restart notification is too broad and
may match unrelated alerts; update the locator creation (the variable named
notification) to scope to the header alerts by changing the selector from
".alert" to "#header .alert" (i.e., const notification = page.locator("#header
.alert", { hasText: /restart/i });) so the expect(notification).toBeVisible(...)
only checks header-specific alerts.
- Around line 10-23: The arrange/teardown currently swallows API failures by
treating a failed GET as an empty list and not checking PUT results; update the
setup/cleanup around apiFetch calls to fail fast: after calling
apiFetch("/server/pathpairs") check/assert that res.ok is true before using
res.json() and throw or fail the test if not, and likewise assert each PUT
response (the response returned from apiFetch(`/server/pathpairs/${pair.id}`, {
method: "PUT", ... })) has .ok true and handle/throw on failure; keep using
originalStates to track prior enabled flags but ensure all apiFetch calls are
asserted so backend setup/teardown failures are surfaced immediately.
- Around line 70-118: The temporary pair cleanup (tempPairId) is done inside the
try block and can be skipped on early failures; move the deletion into the
finally block so it always runs: keep tempPairId declared where it is, remove
the in-try deletion, and in the finally block (after restoring originalStates)
check if tempPairId is set and call apiFetch(`/server/pathpairs/${tempPairId}`,
{ method: "DELETE" }) with await so the temp pair is always removed; reference
tempPairId, apiFetch, and the existing finally loop restoring originalStates
when making this change.

In `@src/e2e-playwright/tests/file-actions.spec.ts`:
- Around line 105-110: After deselecting the row (the existing await row.click()
and await expect(row).not.toHaveClass(/selected/) block), add a direct assertion
that the actions container element inside that row is hidden; locate it from the
same row variable (e.g., const actions = row.locator('.actions') or
row.locator('.file-actions')) and assert await expect(actions).not.toBeVisible()
(or the equivalent hidden-class assertion) so the test verifies UI visibility in
addition to the removed "selected" class.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 38fb15c5-c066-492f-90b6-991ff432811c

📥 Commits

Reviewing files that changed from the base of the PR and between cf856e6 and d89981c.

📒 Files selected for processing (2)

• src/e2e-playwright/tests/error-states.spec.ts
• src/e2e-playwright/tests/file-actions.spec.ts