refactor: migrate account fetch to TanStack Start server function#999
Closed
igboyes wants to merge 5 commits into
Closed
refactor: migrate account fetch to TanStack Start server function#999igboyes wants to merge 5 commits into
igboyes wants to merge 5 commits into
Conversation
Add src/server/account/ as a TanStack Start server feature backing the account fetch directly with Postgres, ahead of repointing the client query off the Python GET /api/account endpoint. - data.ts: getAccount(db, userId) — concurrent user lookup + group join, permission union across groups, primary-group resolution, settings coerced over defaults, AccountNotFoundError on a missing user row. - functions.ts: createServerFn shell reading userId from context.session (authenticated by default via the global middleware), mapping AccountNotFoundError to a 404. Settings now read from Postgres; docs/database.md marks Settings as Postgres-primary to match.
Switch fetchAccount() from the superagent REST client to the getAccount() server function. Update _authenticated route to distinguish auth failures (UnauthorizedError) from transient errors — only genuine 401s redirect to /login. Update tests to match the new server-function-based flow.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates user settings to Postgres and introduces a new server function getAccount to fetch account data, permissions, and settings using Drizzle. It also updates the authenticated route to handle transient fetch errors gracefully and adds corresponding tests. The review feedback highlights three key issues: first, the rethrowAsHttp call in the server function is not awaited, which will lead to unhandled promise rejections; second, swallowing transient errors in beforeLoad when no cached data exists can result in an infinite loading spinner; and third, casting the server response to Promise<Account> bypasses TypeScript nullability checks for administrator_role and primary_group, introducing a type-safety risk.
- await rethrowAsHttp(err) in functions.ts so auth/not-found errors propagate correctly instead of silently becoming unhandled rejections - re-throw transient beforeLoad errors when no account is cached, replacing an infinite spinner with TanStack Router's error boundary - fix Account type: administrator_role and primary_group are nullable; update hasSufficientAdminRole, Nav, and Sidebar to accept null; remove the as Promise<Account> cast from fetchAccount() - wire getAccount server fn mock via vi.hoisted() in setup.tsx to avoid a TDZ crash (setup → routeTree → _authenticated → account/api eagerly imports @server/account/functions before ./api/account loads) - move mockApiGetAccount to tests/api/account.ts and update 18 test files to import it from there instead of tests/fake/account
…ate-account-fetch-to-tanstack-start-server-function # Conflicts: # apps/web/src/account/api.ts # apps/web/src/administration/components/__tests__/AdministratorCreate.test.tsx # apps/web/src/administration/components/__tests__/AdministratorList.test.tsx # apps/web/src/routes/_authenticated.tsx # apps/web/src/users/components/__tests__/ManageUsers.test.tsx # apps/web/src/users/components/__tests__/UserDetail.test.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
superagentREST call for/accountwith a TanStack Start server function (getAccount) backed by Drizzle/Postgres, following thedata.ts→functions.tslayering convention_authenticated.tsxso only genuineUnauthorizedError(401) bounces the user to/login; transient failures (network blips, 5xx) are swallowed so cached account data keeps the layout mountedgetAccountdata layer and rewrites the_authenticatedroute tests to drivebeforeLoaddirectly instead of relying on nock HTTP interceptionTest plan
/loginwith the correctredirectsearch parampnpm --filter @virtool/web exec vitest run src/server/account/__tests__/data.test.tspnpm --filter @virtool/web exec vitest run src/routes/__tests__/-_authenticated.test.tsx