-
Notifications
You must be signed in to change notification settings - Fork 76
Fix navigation and routing issues across the application by correcting broken links #203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Keep API-based role fetching with isLoading state and localStorage fallback. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add loading state and improved redirect logic for role-based access control. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update paths from /dashboard/guest and /dashboard/host to /dashboard/tenant-dashboard and /dashboard/host-dashboard. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add LIST_PROPERTY, INVITATIONS, FAQ, CRYPTO_GUIDE, CONTACT, PRIVACY, TERMS, and COOKIES routes with proper auth requirements. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Wrap page with ProtectedRoute to require login before accessing. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR consolidates navigation routing, adds UI breadcrumbs to dashboards, replaces role-based guards with protected route wrappers, removes the RoleGuard component, and refactors useUserRole to fetch user profile data via API with localStorage fallback for role computation. Changes
Sequence Diagram(s)sequenceDiagram
participant Component as Component/<br/>useUserRole Hook
participant API as profileAPI
participant Storage as localStorage
participant Logic as Role<br/>Computation
Component->>Storage: Check for cached<br/>hostStatus/hasProperties
Component->>API: Fetch user profile
Note over API: Get hostStatus,<br/>hasProperties
alt API Success
API-->>Component: Return profile data
Component->>Storage: Cache hostStatus<br/>& hasProperties
Component->>Logic: Derive role<br/>(guest/host/dual)
else API Failure
API-->>Component: Error
Component->>Storage: Retrieve fallback values
Component->>Logic: Derive role from<br/>cached/fallback data
end
Logic-->>Component: Return role + isLoading
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
94a1ec9 to
6876f80
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/components/guards/RoleGuard.tsx (1)
14-47: The default fallback path is safe, but the component has incomplete role checking.The
/dashboardroute is a safe fallback—it intelligently redirects users based on their role rather than guarding content. However, the newRoleGuardcomponent only validatesrequiredRole === 'host'and ignores'guest'and'dual'roles. Compare with the originalRoleGuardinhooks/auth/, which properly validates all role types. Before this component is used, add role checks for 'guest' and 'dual' to match the original implementation, or clarify if it's intentionally host-only.apps/web/src/lib/navigation.ts (1)
59-63: Redirect URLs use incorrect path — will cause 404 after login.The redirect parameter in both
getBookingsUrlandgetDashboardUrluses/tenant-dashboard, but the actual route defined above is/dashboard/tenant-dashboard. After login, users will be redirected to a non-existent route.Proposed fix
export const navigationHelpers = { getBookingsUrl: (isAuthenticated: boolean) => - isAuthenticated ? '/dashboard/tenant-dashboard' : '/login?redirect=/tenant-dashboard', + isAuthenticated ? '/dashboard/tenant-dashboard' : '/login?redirect=/dashboard/tenant-dashboard', getDashboardUrl: (isAuthenticated: boolean) => - isAuthenticated ? '/dashboard/tenant-dashboard' : '/login?redirect=/tenant-dashboard', + isAuthenticated ? '/dashboard/tenant-dashboard' : '/login?redirect=/dashboard/tenant-dashboard',
🤖 Fix all issues with AI agents
In `@apps/web/src/app/privacy/page.tsx`:
- Around line 9-65: The Privacy Policy in apps/web/src/app/privacy/page.tsx
(sections titled "1. Information We Collect", "3. Blockchain Data", and "5.
Contact Us") is inaccurate and incomplete: update the "Information We Collect"
section to clearly state wallet address is optional and only collected when
users opt into wallet authentication (vs. email/password registration), and
explicitly list optional profile fields collected (phone, address, preferences,
social links); expand the "Blockchain Data" and "3. Blockchain Data"/transaction
paragraphs to specify exactly which transaction details are stored (e.g.,
payment hashes, escrow addresses, booking details), clarify retention periods
and a data deletion/ERASURE procedure (retention timeframe and how users request
deletion), and add an explicit disclosure that escrow addresses are created and
stored on-chain; finally, ensure the "Last updated" date is updated when the
revised policy is approved.
respp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, I’m not finding any references to /dashboard/bookings, which is mentioned in the issue. Since you have a better overview of the issue, what’s your take on it?
Delete duplicate file in components/guards, keep the one in hooks/auth which is already being used by the dashboards. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix redirect paths from /tenant-dashboard to /dashboard/tenant-dashboard - Remove route definitions for pages that don't exist (FAQ, contact, etc.) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
@respp Requested changes corrected, please check again |
|
Pull Request | StellarRent
📝 Summary
Fix navigation and routing issues across the application by correcting broken links, resolving merge conflicts, adding authentication protection to protected routes, and implementing breadcrumb navigation.
🔗 Related Issues
Closes #202
🔄 Changes Made
Bug Fixes:
useUserRole.tsxandRoleGuard.tsxmenu-items.ts(/dashboard/guest→/dashboard/tenant-dashboard,/dashboard/host→/dashboard/host-dashboard)/add-property→/listin dashboard page/list/create→/listin HeroSection and Footer componentsNew Features:
/invitationsroute usingProtectedRoutenavigation.tswith missing routes and properrequiresAuthflags🖼️ Current Output
/dashboard/tenant-dashboard/dashboard/host-dashboard/invitations🧪 Testing
Manual testing performed on all routes:
/invitationsredirects unauthenticated users to login✅ Testing Checklist
🚀 Next Steps & Improvements
/messagesand/applicationsroutes (currently referenced in menu but not implemented)💬 Comments
RoleGuardcomponent now includes a loading state to prevent flash of unauthorized contentuseUserRolehook fetches role from API with localStorage fallback for better reliabilitySummary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.