fix: prevent loading state flashes on page refresh

[yannickmonney]

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Implemented role-based access control to the dashboard, ensuring proper authorization checks before displaying content.
  • Improved data loading reliability by ensuring required data is fully loaded before pages render, preventing display issues from incomplete data availability.

[greptile-apps]

Greptile Summary

Prevents loading state flashes during page refresh by gating dashboard content on authentication state and ensuring data is loaded before rendering.

• Prevents AccessDenied flash by conditionally rendering <Outlet /> only when memberContext.role exists (dashboard/$id.tsx:46,64)
• Ensures approval count data loads before navigation by switching from prefetchQuery to ensureQueryData with await
• Standardizes table padding to p-4 across automations and custom agents tables

Confidence Score: 5/5

• This PR is safe to merge with minimal risk
• Focused bug fix addressing loading state flashes with clear, straightforward changes: conditional rendering based on existing data, switching from non-blocking to blocking data fetches, and minor padding standardization. No logic complexity, security concerns, or architectural changes.
• No files require special attention

Important Files Changed

Filename	Overview
services/platform/app/routes/dashboard/$id.tsx	Gates Outlet rendering on memberContext.role to prevent flashing AccessDenied during auth initialization
services/platform/app/routes/dashboard/$id/approvals.tsx	Switches from non-blocking prefetchQuery to blocking ensureQueryData with Promise.all for count queries
services/platform/app/routes/dashboard/$id/approvals/$status.tsx	Switches from non-blocking prefetchQuery to blocking ensureQueryData for count queries

_{Last reviewed commit: 16e7c02}

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR modifies dashboard route files to change data-loading behavior and role-based rendering. In the main dashboard route, it introduces a hasRole boolean to conditionally render the routed outlet based on user role presence. In the approvals subroutes, it replaces fire-and-forget prefetchQuery calls with explicit awaited ensureQueryData calls, both individually and combined via Promise.all for concurrent execution. These changes shift from background data prefetching to synchronous data hydration before proceeding.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• fix(platform): prevent false access-denied screens during auth token refreshes #508 — Modifies role-based rendering logic in the same dashboard/$id.tsx file, touching the same control flow for handling user role presence in layout components.
• fix: preserve role cache during auth refresh to prevent admin losing … #523 — Converts fire-and-forget prefetchQuery patterns to explicit ensureQueryData calls in dashboard loaders, matching the same data-loading refactor pattern applied across approvals routes.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly reflects the main objective of the PR: preventing loading state flashes on page refresh by converting fire-and-forget prefetch calls to explicit data hydration with Promise.all and ensureQueryData.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/loading-state-flash-on-refresh

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

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]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

services/platform/app/routes/dashboard/$id/approvals/$status.tsx (1)

31-38: 🧹 Nitpick | 🔵 Trivial

The isValidStatus guard in the loader is dead code.

beforeLoad already throws notFound() for invalid statuses before loader is invoked, so isValidStatus(params.status) is always true here.

♻️ Simplify loader by removing the redundant guard

  loader: async ({ context, params }) => {
-   if (isValidStatus(params.status)) {
-     await context.queryClient.ensureQueryData(
-       convexQuery(api.approvals.queries.approxCountApprovalsByStatus, {
-         organizationId: params.id,
-         status: params.status,
-       }),
-     );
-   }
+   await context.queryClient.ensureQueryData(
+     convexQuery(api.approvals.queries.approxCountApprovalsByStatus, {
+       organizationId: params.id,
+       status: params.status as ValidStatus,
+     }),
+   );
  },

🤖 Prompt for AI Agents

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

In `@services/platform/app/routes/dashboard/`$id/approvals/$status.tsx around
lines 31 - 38, The isValidStatus guard in the loader is redundant because
beforeLoad already throws notFound() for invalid statuses; remove the if
(isValidStatus(params.status)) wrapper and always call
context.queryClient.ensureQueryData(convexQuery(api.approvals.queries.approxCountApprovalsByStatus,
{ organizationId: params.id, status: params.status })); keep the same params and
call but eliminate the dead isValidStatus check to simplify loader logic (refer
to isValidStatus, beforeLoad, loader, context.queryClient.ensureQueryData,
convexQuery, api.approvals.queries.approxCountApprovalsByStatus, params.status,
params.id).

🤖 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 `@services/platform/app/routes/dashboard/`$id/approvals/$status.tsx:
- Around line 31-38: The isValidStatus guard in the loader is redundant because
beforeLoad already throws notFound() for invalid statuses; remove the if
(isValidStatus(params.status)) wrapper and always call
context.queryClient.ensureQueryData(convexQuery(api.approvals.queries.approxCountApprovalsByStatus,
{ organizationId: params.id, status: params.status })); keep the same params and
call but eliminate the dead isValidStatus check to simplify loader logic (refer
to isValidStatus, beforeLoad, loader, context.queryClient.ensureQueryData,
convexQuery, api.approvals.queries.approxCountApprovalsByStatus, params.status,
params.id).

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f42972a and 16e7c02.

📒 Files selected for processing (3)

• services/platform/app/routes/dashboard/$id.tsx
• services/platform/app/routes/dashboard/$id/approvals.tsx
• services/platform/app/routes/dashboard/$id/approvals/$status.tsx