SANC-108: Made agency name dynamic - #78
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR refactors how the Navbar component obtains the agency name by moving data fetching from prop-based injection to internal TRPC query execution. It introduces a new protected server procedure Changes
Sequence Diagram(s)sequenceDiagram
participant Navbar as Navbar Component
participant TRPC as TRPC Client
participant Server as API Server
participant DB as Database
Note over Navbar: view === "agency"
Navbar->>TRPC: useQuery("getUserOrgName", enabled: true)
TRPC->>Server: protectedProcedure.query()
Server->>Server: Verify activeOrganizationId exists
Server->>DB: findFirst(id: activeOrganizationId)
DB-->>Server: organization record
Server-->>TRPC: org.name
TRPC-->>Navbar: agencyName data
Navbar->>Navbar: Render "[agencyName] Home"
sequenceDiagram
participant User as User
participant Profile as Profile Component
participant Router as Next Router
participant QC as React Query Client
User->>Profile: Click logout
Profile->>Router: router.push("/logout")
Router-->>Profile: Navigation complete
Profile->>QC: queryClient.clear()
QC->>QC: Purge all cached queries
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/_components/common/navbar.tsx (1)
41-58:⚠️ Potential issue | 🟠 MajorGate org-name query to agency view and separate loading vs error UI states.
At line 41, the query runs unconditionally for all navbar variants (admin, agency, driver), but
agencyNameis only used whenview === "agency". This triggers unnecessary API requests in admin and driver pages. Additionally, line 58 shows"Loading..."for both loading and error states, making network failures indistinguishable from genuine loading.The suggested fix uses the
enabledoption (supported by@tanstack/react-query5.69.0 in this project) to gate the query and extractsisErrorto differentiate failure states:Suggested fix
- const agencyName = api.organization.getUserOrgName.useQuery().data; + const { + data: agencyName, + isLoading: isAgencyNameLoading, + isError: isAgencyNameError, + } = api.organization.getUserOrgName.useQuery(undefined, { + enabled: view === "agency", + }); @@ - <Text>{view === "agency" ? (agencyName ? `${agencyName} Home` : "Loading...") : ""}</Text> + <Text> + {view === "agency" + ? isAgencyNameLoading + ? "Loading..." + : isAgencyNameError + ? "Agency Home" + : `${agencyName} Home` + : ""} + </Text>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/_components/common/navbar.tsx` around lines 41 - 58, The navbar always runs api.organization.getUserOrgName.useQuery() even when view !== "agency" and conflates loading/error UI; update the hook call to const { data: agencyName, isLoading, isError } = api.organization.getUserOrgName.useQuery(undefined, { enabled: view === "agency" }) so the query only runs for the agency view, then change the Text rendering logic (where view === "agency") to show a distinct error state when isError (e.g., "Error loading name") vs a loading state when isLoading ("Loading...") and otherwise display `${agencyName} Home`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/app/_components/common/navbar.tsx`:
- Around line 41-58: The navbar always runs
api.organization.getUserOrgName.useQuery() even when view !== "agency" and
conflates loading/error UI; update the hook call to const { data: agencyName,
isLoading, isError } = api.organization.getUserOrgName.useQuery(undefined, {
enabled: view === "agency" }) so the query only runs for the agency view, then
change the Text rendering logic (where view === "agency") to show a distinct
error state when isError (e.g., "Error loading name") vs a loading state when
isLoading ("Loading...") and otherwise display `${agencyName} Home`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 52fe48bd-d27b-4128-b98a-7ea95eba0e52
📒 Files selected for processing (2)
src/app/_components/common/navbar.tsxsrc/server/api/routers/organizations.ts
- Added a new endpoint, getUserOrgName, to return a user's organization name from the DB - Used the new endpoint to return the user's organization name to the client, and had that info displayed in the nav bar
b6c61e6 to
9dd4c7b
Compare
promatty
left a comment
promatty
left a comment
There was a problem hiding this comment.
i found this bug:
- log in as agency one
- log out. log in as agency three
shows agency one as the name still
- Added client cache clearing when the user logs out to ensure that data between accounts aren't saved and reused upon visiting the same page
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/app/_components/common/profile/profile.tsx (1)
22-23: Userouter.replace("/logout")in the logout flow.
pushadds a history entry, allowing users to back-navigate to authenticated pages, which is a security and UX concern.replaceremoves the current entry from history, preventing back-button access to protected routes after logout.Suggested change
- router.push("/logout"); + router.replace("/logout"); queryClient.clear();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/_components/common/profile/profile.tsx` around lines 22 - 23, Replace the history-pushing navigation call with a history-replacing one in the logout flow: change router.push("/logout") to router.replace("/logout") (leave queryClient.clear() as-is) so the current entry is replaced and users cannot back-navigate to authenticated pages; update any logout handler in this component (the code invoking router.push and queryClient.clear, e.g., the logout function in profile.tsx) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/app/_components/common/profile/profile.tsx`:
- Around line 22-23: Replace the history-pushing navigation call with a
history-replacing one in the logout flow: change router.push("/logout") to
router.replace("/logout") (leave queryClient.clear() as-is) so the current entry
is replaced and users cannot back-navigate to authenticated pages; update any
logout handler in this component (the code invoking router.push and
queryClient.clear, e.g., the logout function in profile.tsx) accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c53062ec-9568-4594-8016-417f62dcc238
📒 Files selected for processing (3)
src/app/_components/common/navbar.tsxsrc/app/_components/common/profile/profile.tsxsrc/server/api/routers/organizations.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/server/api/routers/organizations.ts
- src/app/_components/common/navbar.tsx
Thanks for catching it! It turns out that the client's browser was caching data and reusing it briefly upon reentering the same page. Both the schedule data and organization name were stale and reused from a previous session Going forward, upon logging out, the client's browser clears its cache so that reentering the same page as a different user will trigger a refetch of data. Before, this occurred only when the window was focused |
2 participants
Added a new endpoint, getUserOrgName, to return a user's organization name from the DB. Used the new endpoint to return the user's organization name to the client, and had that info displayed in the nav bar
video.mp4
Summary by CodeRabbit