Feat/create message placeholder page#215
Feat/create message placeholder page#215FrancoEspinosa08 wants to merge 3 commits intoStellar-Rent:mainfrom
Conversation
📝 WalkthroughWalkthroughThis PR restructures the app layout with Navbar and RightSidebar, adds a Messages page, consolidates menu item constants, reworks user role resolution to use profileAPI with localStorage fallback, and validates USDC/Stellar configuration while updating transaction handling to use XDR. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant useUserRole
participant profileAPI
participant localStorage
Client->>useUserRole: call hook (on mount)
useUserRole->>profileAPI: fetchProfile(userId)
alt API success
profileAPI-->>useUserRole: profile { hostStatus, hasProperties }
useUserRole->>localStorage: set hostStatus / hasProperties
useUserRole-->>Client: return { role, canAccessHostDashboard, hasProperties, isLoading:false }
else API failure
profileAPI-->>useUserRole: error
useUserRole->>localStorage: read hostStatus / hasProperties fallback
useUserRole-->>Client: return { role (from fallback), isLoading:false }
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@apps/web/src/app/layout.tsx`:
- Around line 5-18: RootLayout is missing the Providers wrapper so
context-dependent pieces (useAuth/useUserRole in RightSidebar, ThemeToggle in
Navbar, toasts) break; update RootLayout to wrap the app tree (Navbar, main
children, RightSidebar) with the existing Providers component (and include any
Toast/Theme providers that Providers normally supplies), remove the hardcoded
className="dark" from the <html> element so theme comes from the provider, and
ensure Providers is imported and used around the children and components
referenced (RootLayout, Navbar, RightSidebar, ThemeToggle).
In `@apps/web/src/app/messages/page.tsx`:
- Line 10: The container's height uses calc(100vh-64px) which mismatches the
Navbar's h-14 (56px) and the layout's pt-14, causing visual gap; update the div
in page.tsx (the element with className "flex h-[calc(100vh-64px)] ...") to
either use h-[calc(100vh-56px)] to match Navbar h-14 or replace the calc with
h-full and rely on the parent <main> sizing (and keep pt-14 on the layout),
ensuring the Navbar component (Navbar, h-14) and page padding remain consistent.
In `@apps/web/src/app/search/page.tsx`:
- Around line 78-79: PropertyGrid currently ignores passed props because it has
no props type and renders mockProperties; update PropertyGrid to accept a props
interface (e.g., PropertyGridProps with properties: Property[] and optional
onLoadMore: () => void), use those props inside the component to map properties
to PropertyCard and call/render onLoadMore for pagination, then remove the "as
any" alias in page.tsx so the real typing error surfaces and the caller passes
its properties and onLoadMore correctly to PropertyGrid.
In `@apps/web/src/lib/config/config.ts`:
- Around line 26-28: The thrown Error referencing the USDC issuer uses the short
name "USDC_ISSUER_MAINNET" but should reference the actual env var
NEXT_PUBLIC_USDC_ISSUER_MAINNET; update the error thrown where rawIssuer is
validated (the block that throws new Error("CRITICAL: USDC_ISSUER_MAINNET is not
defined...")) to mention NEXT_PUBLIC_USDC_ISSUER_MAINNET so the message
accurately reflects the expected environment variable.
- Line 14: Replace the incorrect hardcoded constant by updating
TESTNET_FALLBACK_ISSUER to the correct Stellar testnet USDC issuer; locate the
constant named TESTNET_FALLBACK_ISSUER and set its value to
"GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5".
🧹 Nitpick comments (6)
apps/web/src/app/messages/page.tsx (1)
7-7: Unused state:searchQueryis captured but never used.The search input updates
searchQuerystate but it's not used for any filtering or API calls. If this is intentional for the placeholder, consider adding a brief comment. Otherwise, this is dead code.apps/web/src/hooks/useUserRole.tsx (3)
78-78: API errors are silently discarded; consider logging for debugging.The API error is caught but not logged, making it difficult to diagnose why the fallback path is being used.
Suggested fix
- } catch (_apiError) { + } catch (apiError) { + console.warn('Failed to fetch user profile, falling back to localStorage:', apiError); // Fallback to localStorage if API fails
45-109: Nested try blocks add complexity; the outer try has no catch.The outer
try(Line 45) has no correspondingcatch—only afinally. The innertry(Line 48) handles all errors. This structure is confusing and the outertryserves no purpose.Suggested simplification
- try { - setIsLoading(true); - - try { + setIsLoading(true); + try { const response = await profileAPI.getUserProfile(userId); // ... success handling ... - } catch (_apiError) { + } catch (apiError) { + console.warn('Failed to fetch user profile:', apiError); // Fallback to localStorage if API fails // ... fallback handling ... - } } finally { setIsLoading(false); }
59-65: Clarify: verified hosts without properties cannot access host dashboard.Line 64 sets
canAccessHostDashboard = falsefor the'host'role (verified but no properties). If this is intentional business logic (hosts need at least one property to access the dashboard), a brief comment would help future maintainers.apps/web/src/lib/stellar.ts (1)
87-93: Consider passingnetworkPassphraseto Freighter'ssignTransactionfor explicit network validation.While the Freighter API's
signTransactionacceptsnetworkPassphraseas an optional parameter, explicitly passing it ensures the wallet signs for the intended network rather than relying on implicit default behavior.Suggested improvement
// `@ts-ignore`: Freighter API global access - const signedTransaction = await window.freighterApi.signTransaction(transactionXDR); + const signedTransaction = await window.freighterApi.signTransaction(transactionXDR, { + networkPassphrase: NETWORK_PASSPHRASE, + });apps/web/src/constants/menu-items.ts (1)
28-30: Consider isolating role menus from shared object mutation.
Line 28–30 creates new arrays but reuses the same item objects; if any downstream code mutates a menu item (e.g., adds anactiveflag), the change will bleed across roles. If mutation is possible, clone items per role or freeze the base list.♻️ Possible defensive clone
-export const TENANT_MENU_ITEMS: MenuItem[] = [...GUEST_MENU_ITEMS]; -export const HOST_MENU_ITEMS: MenuItem[] = [...GUEST_MENU_ITEMS]; -export const DUAL_MENU_ITEMS: MenuItem[] = [...GUEST_MENU_ITEMS]; +export const TENANT_MENU_ITEMS: MenuItem[] = GUEST_MENU_ITEMS.map((item) => ({ ...item })); +export const HOST_MENU_ITEMS: MenuItem[] = GUEST_MENU_ITEMS.map((item) => ({ ...item })); +export const DUAL_MENU_ITEMS: MenuItem[] = GUEST_MENU_ITEMS.map((item) => ({ ...item }));
| // Alias para evitar el error de IntrinsicAttributes | ||
| const Grid = PropertyGrid as any; |
There was a problem hiding this comment.
The as any cast masks a real bug: PropertyGrid ignores the passed props.
Per the relevant snippet from PropertyGrid.tsx (lines 128-136), the component accepts no props and renders mockProperties directly. The passed properties and onLoadMore props on Line 102 are silently ignored, meaning:
- Filtering/sorting logic (Lines 35-58) has no effect on rendered output
- Pagination via
loadNextPagedoesn't work - Users always see hardcoded mock data regardless of filters
The as any cast hides the TypeScript error that would have caught this.
Recommended approach
Update PropertyGrid to accept and use props:
// In PropertyGrid.tsx
interface PropertyGridProps {
properties: Property[];
onLoadMore?: () => void;
}
export const PropertyGrid = ({ properties, onLoadMore }: PropertyGridProps) => {
return (
<div className="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 xl:grid-cols-4 gap-8">
{properties.map((property) => (
<PropertyCard key={property.id} property={property} />
))}
{onLoadMore && (
<button onClick={onLoadMore}>Load More</button>
)}
</div>
);
};Then remove the as any cast in this file:
- // Alias para evitar el error de IntrinsicAttributes
- const Grid = PropertyGrid as any;
...
- <Grid properties={visibleProperties} onLoadMore={loadNextPage} />
+ <PropertyGrid properties={visibleProperties} onLoadMore={loadNextPage} />Also applies to: 102-102
🤖 Prompt for AI Agents
In `@apps/web/src/app/search/page.tsx` around lines 78 - 79, PropertyGrid
currently ignores passed props because it has no props type and renders
mockProperties; update PropertyGrid to accept a props interface (e.g.,
PropertyGridProps with properties: Property[] and optional onLoadMore: () =>
void), use those props inside the component to map properties to PropertyCard
and call/render onLoadMore for pagination, then remove the "as any" alias in
page.tsx so the real typing error surfaces and the caller passes its properties
and onLoadMore correctly to PropertyGrid.
apps/web/src/lib/config/config.ts
Outdated
| if (!rawIssuer) { | ||
| throw new Error("CRITICAL: USDC_ISSUER_MAINNET is not defined in environment variables."); | ||
| } |
There was a problem hiding this comment.
Minor: Error message should reference the full env var name.
The error mentions "USDC_ISSUER_MAINNET" but the actual env var is NEXT_PUBLIC_USDC_ISSUER_MAINNET.
Suggested fix
- throw new Error("CRITICAL: USDC_ISSUER_MAINNET is not defined in environment variables.");
+ throw new Error("CRITICAL: NEXT_PUBLIC_USDC_ISSUER_MAINNET is not defined in environment variables.");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!rawIssuer) { | |
| throw new Error("CRITICAL: USDC_ISSUER_MAINNET is not defined in environment variables."); | |
| } | |
| if (!rawIssuer) { | |
| throw new Error("CRITICAL: NEXT_PUBLIC_USDC_ISSUER_MAINNET is not defined in environment variables."); | |
| } |
🤖 Prompt for AI Agents
In `@apps/web/src/lib/config/config.ts` around lines 26 - 28, The thrown Error
referencing the USDC issuer uses the short name "USDC_ISSUER_MAINNET" but should
reference the actual env var NEXT_PUBLIC_USDC_ISSUER_MAINNET; update the error
thrown where rawIssuer is validated (the block that throws new Error("CRITICAL:
USDC_ISSUER_MAINNET is not defined...")) to mention
NEXT_PUBLIC_USDC_ISSUER_MAINNET so the message accurately reflects the expected
environment variable.
1 participant
Pull Request | StellarRent
📝 Summary
Provide a brief description of what this PR accomplishes.
🔗 Related Issues
Closes #(issue number) (Replace with the actual issue number).
🔄 Changes Made
Provide a general description of the changes. Include any relevant background information or context to help reviewers understand the purpose of this PR.
🖼️ Current Output
Provide visual evidence of the changes:
🧪 Testing
If applicable, describe the tests performed. Include screenshots, test outputs, or any resources that help reviewers understand how the changes were tested.
✅ Testing Checklist
List any possible issues that might arise with this change.
🚀 Next Steps & Improvements
This change lays a solid foundation for further optimizations. Some areas that could benefit from future improvements include:
💬 Comments
Any additional context, questions, or considerations for reviewers.
Summary by CodeRabbit
New Features
Refactor
Bug Fixes / UI
✏️ Tip: You can customize this high-level summary in your review settings.