View as selector and tests

[ericokuma]

This PR addresses the follow-up feedback for the "View As" feature (APP-765) by improving code structure, efficiency, and test coverage.

Key Changes:

• Shared View As Project Permissions Selector: Extracted the GetDeploymentCredentials → GetProjectWithBearerToken → effective permissions query chain into a new shared selector (useViewAsProjectPermissions). This deduplicates logic previously duplicated in ProjectLayout and TopNavigationBar, making the code more maintainable and reducing component complexity.
• E2E Tests for View As: Added comprehensive Playwright tests to cover critical scenarios for the "View As" feature, including UI visibility, state persistence across refreshes and navigation, and state clearing. This significantly improves test coverage for a security-adjacent feature. (Note: Effective permissions assertions are commented out pending viewerPage fixture setup).
• Optimized initForProject: Implemented an early return in initForProject to prevent redundant sessionStorage reads when the project scope has not changed, improving efficiency.

Checklist:

• Covered by tests
• Ran it and it works as intended
• Reviewed the diff before requesting a review
• Checked for unhandled edge cases
• Linked the issues it closes
• Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
• Intend to cherry-pick into the release branch
• I'm proud of this work!

---

Linear Issue: APP-765

  

[cursor]

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
_{Learn more about Cursor Agents}

[ericpgreen2]

Issues

1. E2E tests don't validate the core behavior of View As (High)

The test suite tests UI mechanics (popover opens, chip renders, sessionStorage persists) but not the actual purpose of View As — that an admin sees restricted permissions when impersonating a less-privileged user.

The e2e environment only has one user (the admin), so every test selects the admin to "view as" themselves. The comment on line 72 makes this explicit: // Select the first user in the list (the admin user itself). Since the effective permissions are identical to the admin's real permissions, the query chain (GetDeploymentCredentials → GetProjectWithBearerToken → effective permissions) is never exercised in a way that produces observable differences.

The skipped test ("Effective permissions update correctly when viewing as a viewer") is the one that would cover this, but it requires a second user with viewer-level permissions to be provisioned in setup.ts. Without it, the tests give confidence that the UI plumbing works but not that security policy enforcement actually functions through View As.

Consider either:

• Provisioning a viewer user in the e2e setup and enabling the skipped test, or
• Adding a unit/integration test that asserts the effective permissions reflect the mocked user's restrictions (not the admin's) when View As is active.

2. The shared selectors are thin wrappers — consider a compound hook instead (Medium)

view-as-project-permissions.ts extracts three functions, but both consumers (+layout.svelte:138-158, TopNavigationBar.svelte:80-99) still repeat the same multi-step chain: read viewAsUserStore → create credentials query → pipe access token into project query → derive effective permissions. The wrappers move individual steps behind functions without actually reducing the duplication.

Worse, the two consumers compute effective permissions differently — the layout uses computeEffectivePermissions (switches on the whole object), while the navbar uses inline ?? fallback per field (falls through to the admin's permission if a mocked field is undefined).

A single compound hook that encapsulates the entire chain would eliminate both problems:

// Returns the mocked user's project permissions (or undefined when View As is inactive)
export function createViewAsPermissions(org: string, project: string) {
  const mockedUserId = get(viewAsUserStore)?.id; // or accept as param
  const credentialsQuery = createAdminServiceGetDeploymentCredentials(...);
  const projectQuery = createAdminServiceGetProjectWithBearerToken(...);
  // return derived store with projectPermissions
}

Both consumers would call one thing and get back the mocked permissions, then merge with their actual permissions in a single consistent place.

3. Silent test skip inside "Admin can change the viewed user" (Low)

view-as.spec.ts:247-259: The if (userCount > 1) block means the core assertion (selecting a different user) is silently skipped when only one user exists — which is always the case in the current e2e environment. The test passes without testing its stated purpose. Consider asserting userCount > 1 so CI fails loudly if the fixture is misconfigured, or skip the test with a message.

Minor

• test.describe.configure({ mode: "serial" }) may not be needed — each test navigates fresh and doesn't depend on prior test state. Parallel mode would be faster in CI.

---

Developed in collaboration with Claude Code