Update Hooks#147
Conversation
WalkthroughReplaces mock flows in dashboard hooks with real API integrations, adds axios-based error handling including 404 fallbacks, consolidates fetch logic, introduces parallel refresh orchestration with Promise.allSettled, updates public types (DashboardStats import, expanded UserProfile), and adjusts data shaping for dashboard bookings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Dashboard UI
participant Hook as useDashboard
participant API as services/api
Note over UI,Hook: User triggers refreshAll
UI->>Hook: refreshAll()
par Parallel fetches
Hook->>API: fetchProfile()
API-->>Hook: Profile | 404 -> null
Hook->>API: fetchBookings()
API-->>Hook: Booking[] | 404 -> []
Hook->>API: fetchTransactions()
API-->>Hook: Transaction[] | 404 -> []
Hook->>API: fetchStats()
API-->>Hook: DashboardStats | 404 -> null
end
Note right of Hook: Promise.allSettled<br/>Collect successes/errors
Hook-->>UI: Update state (partial OK), aggregate error message
sequenceDiagram
autonumber
participant UI as Booking Details View
participant Hook as useBookingDetails
participant API as bookingAPI
Note over UI,Hook: On mount or id change
UI->>Hook: useEffect -> fetchBookingData(id)
alt valid bookingId
Hook->>API: getBookingById(id)
API-->>Hook: Booking | 404
alt 200 OK
Hook-->>UI: set bookingData, loading=false
else 404
Hook-->>UI: bookingData=null, loading=false
end
else invalid bookingId
Hook-->>UI: set error, clear data, loading=false
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 7
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/services/api.ts (1)
18-149: Remove duplicate API utility functionsThe file contains duplicate implementations of API utility functions (
_apiCallandapiUtils.request). Both functions have identical logic for making API requests.Remove the duplicate
_apiCallfunction (lines 118-149) and update any references to useapiUtils.requestinstead:-// Generic API call function -const _apiCall = async <T>( - endpoint: string, - options: RequestInit = {} -): Promise<T> => { - const url = `${API_BASE_URL}${endpoint}`; - const token = localStorage.getItem("authToken"); - - const config: RequestInit = { - headers: { - "Content-Type": "application/json", - ...(token && { Authorization: `Bearer ${token}` }), - ...options.headers, - }, - ...options, - }; - - try { - const response = await fetch(url, config); - - if (!response.ok) { - const errorData = await response.json().catch(() => ({})); - throw new Error( - errorData.message || `HTTP error! status: ${response.status}` - ); - } - - return await response.json(); - } catch (error) { - console.error("API request failed:", error); - throw error; - } -};Also applies to: 177-206
apps/web/src/hooks/useBookingDetails.ts (1)
63-161: Clean up large commented-out code blockThere's a massive block of commented-out code (99 lines) that should be removed rather than kept in the codebase.
Remove lines 63-161 entirely. If this code needs to be preserved for reference, it should be in version control history, not as comments in the active codebase.
🧹 Nitpick comments (1)
apps/web/src/hooks/useDashboard.ts (1)
82-82: Inconsistent type assertions across fetch functionsThe type assertions are inconsistent and potentially unsafe across the different fetch functions.
Create a type guard or validation utility to handle API responses consistently:
function validateApiResponse<T>(response: unknown): T | null { if (response && typeof response === 'object' && 'data' in response) { return (response as { data: T }).data; } return null; } // Then use it consistently: setProfile(validateApiResponse<UserProfile>(response)); setTransactions(validateApiResponse<Transaction[]>(response) || []); setStats(validateApiResponse<DashboardStats>(response));Also applies to: 103-103, 124-124
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
apps/web/src/hooks/useBookingDetails.ts(2 hunks)apps/web/src/hooks/useDashboard.ts(5 hunks)apps/web/src/services/api.ts(14 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
apps/web/src/hooks/useBookingDetails.ts (2)
apps/web/src/types/booking.ts (2)
UseBookingDetailsReturn(32-37)BookingData(3-22)apps/web/src/services/api.ts (2)
bookingAPI(247-288)handleAPIError(446-468)
apps/web/src/hooks/useDashboard.ts (3)
apps/web/src/services/api.ts (5)
bookingAPI(247-288)handleAPIError(446-468)profileAPI(214-245)walletAPI(354-384)dashboardAPI(417-444)apps/web/src/types/index.ts (3)
DashboardBooking(5-19)UserProfile(37-56)Transaction(75-83)apps/web/src/types/shared.ts (3)
UserProfile(4-20)Transaction(66-75)DashboardStats(88-97)
apps/web/src/services/api.ts (1)
apps/web/src/types/shared.ts (7)
UserProfile(4-20)BookingFilters(163-171)APIResponse(109-113)Booking(22-40)BookingFormData(126-133)PropertyFilters(173-181)Property(42-64)
🔇 Additional comments (3)
apps/web/src/services/api.ts (2)
445-468: LGTM! Well-structured error handlingThe error handling function correctly maps HTTP status codes to user-friendly messages and handles authentication errors appropriately by clearing auth tokens and redirecting to login.
151-170: Confirm backend support for new profile fields and align preferences mapping
- Backend docs list preferences JSONB with keys
notifications,newsletter,language, but code defaults tonotifications,emailUpdates,pushNotifications. Update the transform to match or verify with backend team.total_bookings,total_spent, andcreated_ataren’t documented in PROFILE_SYSTEM_README.md—confirm these fields are returned by the profile API or adjust fallbacks.apps/web/src/hooks/useDashboard.ts (1)
146-154: Good use of Promise.allSettled for parallel fetchingThe implementation correctly uses
Promise.allSettledto ensure partial failures don't block successful API calls, which aligns well with the PR objectives of handling missing endpoints gracefully.
| import { useCallback, useEffect, useState } from 'react'; | ||
| import { bookingAPI, handleAPIError } from '@/services/api'; | ||
| import type { BookingData, UseBookingDetailsReturn } from '@/types/booking'; | ||
| import axios from 'axios'; |
There was a problem hiding this comment.
Inconsistent error handling libraries
The file imports axios for error checking but uses bookingAPI which is based on the native fetch API. This creates an inconsistency in error handling.
Since bookingAPI.getBookingById uses fetch internally (as seen in api.ts), the axios error check at line 37 will never match. Either:
- Use native fetch error handling, or
- Migrate the API layer to use axios consistently
Apply this diff to fix the immediate issue:
-import axios from 'axios';And update the error handling:
- if (axios.isAxiosError(err) && err.response?.status === 404) {
+ if (msg.includes('404') || msg === 'The requested resource was not found.') {
// Endpoint doesn't exist → fallback gracefully
setBookingData(null);
}📝 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.
| import { useCallback, useEffect, useState } from 'react'; | |
| import { bookingAPI, handleAPIError } from '@/services/api'; | |
| import type { BookingData, UseBookingDetailsReturn } from '@/types/booking'; | |
| import axios from 'axios'; | |
| // apps/web/src/hooks/useBookingDetails.ts | |
| import { useCallback, useEffect, useState } from 'react'; | |
| import { bookingAPI, handleAPIError } from '@/services/api'; | |
| import type { BookingData, UseBookingDetailsReturn } from '@/types/booking'; | |
| // import axios from 'axios'; <-- removed | |
| export function useBookingDetails(id: string): UseBookingDetailsReturn { | |
| const [bookingData, setBookingData] = useState<BookingData | null>(null); | |
| const [error, setError] = useState<Error | null>(null); | |
| const fetchBooking = useCallback(async () => { | |
| try { | |
| const data = await bookingAPI.getBookingById(id); | |
| setBookingData(data); | |
| } catch (err: any) { | |
| handleAPIError(err); | |
| const msg = err.message; | |
| // Endpoint doesn't exist → fallback gracefully | |
| if (msg.includes('404') || msg === 'The requested resource was not found.') { | |
| setBookingData(null); | |
| } else { | |
| setError(err); | |
| } | |
| } | |
| }, [id]); | |
| useEffect(() => { | |
| fetchBooking(); | |
| }, [fetchBooking]); | |
| return { bookingData, error, reload: fetchBooking }; | |
| } |
🤖 Prompt for AI Agents
In apps/web/src/hooks/useBookingDetails.ts around lines 3 to 6, the file imports
axios but bookingAPI.getBookingById uses fetch, so axios-specific error checks
will never match; remove the unused axios import and update the catch block to
use the existing handleAPIError helper (or check for native Error/fetch error
shapes) — i.e., on failure call handleAPIError(error) and set local error state
from its result or from error.message, and if bookingAPI returns a non-ok
response ensure getBookingById throws a descriptive Error so the hook can catch
it consistently.
| @@ -1,21 +1,14 @@ | |||
| // src/hooks/useDashboard.ts | |||
| import { useCallback, useEffect, useState } from 'react'; | |||
| import axios from 'axios'; | |||
There was a problem hiding this comment.
Remove axios dependency - inconsistent with fetch-based API
Similar to useBookingDetails.ts, this file imports axios but the underlying API services use fetch. The axios error checks will never match because the errors thrown are not axios errors.
Remove the axios import and update all error handling:
-import axios from 'axios';Update all axios error checks:
- if (axios.isAxiosError(err) && err.response?.status === 404) {
+ if (msg.includes('404') || msg === 'The requested resource was not found.') {This pattern appears in lines 65-68, 87-90, 108-111, and 129-132.
Also applies to: 52-71, 87-91, 108-112, 129-133
| refreshAll: () => Promise<void>; | ||
| } | ||
|
|
||
| /* ... DashboardStats, UseDashboardReturn definitions same as yours ... */ |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove extensive commented-out code
Similar to the other file, there's a huge block of commented code (166 lines) that should be removed.
Remove lines 32 and 183-348. The comment on line 32 is also unnecessary.
Also applies to: 183-348
🤖 Prompt for AI Agents
In apps/web/src/hooks/useDashboard.ts at line 32 and lines 183-348, there is a
large block of commented-out code and an unnecessary comment on line 32; remove
the comment on line 32 and delete the entire commented block spanning 183-348,
then run project linter/formatter and ensure there are no remaining references
or TODOs that relied on that commented code (if anything referenced it, convert
to a small helper or proper tests instead).
| const bookingsData: DashboardBooking[] = (response.data || []).map((booking: any) => ({ | ||
| ...booking, | ||
| propertyLocation: booking.propertyLocation ?? booking.property?.location ?? '', | ||
| hostName: booking.hostName ?? booking.host?.name ?? '', | ||
| })); |
There was a problem hiding this comment.
Improve type safety for booking data transformation
Using any type and relying on fallback chains with ?? operators indicates uncertainty about the data structure.
Add proper typing and validation:
- const bookingsData: DashboardBooking[] = (response.data || []).map((booking: any) => ({
+ import type { Booking } from '../types/shared';
+ const bookingsData: DashboardBooking[] = ((response.data || []) as Booking[]).map((booking) => ({
...booking,
- propertyLocation: booking.propertyLocation ?? booking.property?.location ?? '',
- hostName: booking.hostName ?? booking.host?.name ?? '',
+ propertyLocation: booking.location || '',
+ hostName: '', // This field doesn't exist in the Booking type
}));Note: The hostName field doesn't exist in the Booking type from shared.ts. This transformation seems to be adding fields that don't exist in the source data.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/src/hooks/useDashboard.ts around lines 54-58, the mapping currently
uses "any" and adds derived fields without proper types; replace "any" by
importing and using the shared Booking type (e.g., response.data as Booking[]),
map from that typed Booking to DashboardBooking, derive propertyLocation from
booking.property?.location ?? '' and hostName from booking.host?.name ?? '' (or
extend DashboardBooking/type to include these derived fields), and add a simple
runtime guard (e.g., ensure response.data is an array) or validator if the shape
may vary instead of using unchecked "any".
| setError(prev => (prev ? `${prev}; ${msg}` : msg)); | ||
| console.error('Failed to fetch bookings:', err); |
There was a problem hiding this comment.
Accumulating error messages creates confusing UX
The error accumulation pattern prev ? ${prev}; ${msg} : msg will create very long, hard-to-read error messages if multiple API calls fail.
Consider showing only the most recent error or using a more structured approach:
- setError(prev => (prev ? `${prev}; ${msg}` : msg));
+ setError(msg); // Show only the most recent error
+ // Or use an array: setErrors(prev => [...prev, msg]);📝 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.
| setError(prev => (prev ? `${prev}; ${msg}` : msg)); | |
| console.error('Failed to fetch bookings:', err); | |
| setError(msg); // Show only the most recent error | |
| // Or use an array: setErrors(prev => [...prev, msg]); | |
| console.error('Failed to fetch bookings:', err); |
🤖 Prompt for AI Agents
In apps/web/src/hooks/useDashboard.ts around lines 62-63, the current
setError(prev => (prev ? `${prev}; ${msg}` : msg)) accumulates messages and
makes the UX noisy; change it to store only the most recent error (e.g.,
setError(msg)) or switch to a structured error object (e.g., setError({ message:
msg, timestamp: Date.now() })) and update the error state type and any UI
consumers to render just error.message or the latest string — do not concatenate
prior errors.
| guests: number; | ||
| totalAmount: number; | ||
| status: 'pending' | 'confirmed' | 'completed' | 'cancelled'; | ||
| status: "pending" | "confirmed" | "completed" | "cancelled"; |
There was a problem hiding this comment.
Fix inconsistent quote style for status union type
The status type has been changed from single quotes to double quotes, which is inconsistent with the rest of the codebase and the type definitions in apps/web/src/types/shared.ts that use single quotes.
Apply this diff to maintain consistency:
- status: "pending" | "confirmed" | "completed" | "cancelled";
+ status: 'pending' | 'confirmed' | 'completed' | 'cancelled';📝 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.
| status: "pending" | "confirmed" | "completed" | "cancelled"; | |
| status: 'pending' | 'confirmed' | 'completed' | 'cancelled'; |
🤖 Prompt for AI Agents
In apps/web/src/services/api.ts around line 106 the union type for status uses
double quotes ("pending" | "confirmed" | "completed" | "cancelled") which is
inconsistent with the codebase and apps/web/src/types/shared.ts that use single
quotes; change the union to use single quotes ('pending' | 'confirmed' |
'completed' | 'cancelled') so the file matches the project's quote style and
type definitions.
respp
left a comment
respp
left a comment
There was a problem hiding this comment.
please use bun as package manager and fix the issues
| @@ -1,21 +1,14 @@ | |||
| // src/hooks/useDashboard.ts | |||
| import { useCallback, useEffect, useState } from 'react'; | |||
| import axios from 'axios'; | |||
There was a problem hiding this comment.
I don’t want you to use Axios for this, but rather the native fetch, so that we can take advantage of Next’s ISR.
| import { useCallback, useEffect, useState } from 'react'; | ||
| import { bookingAPI, handleAPIError } from '@/services/api'; | ||
| import type { BookingData, UseBookingDetailsReturn } from '@/types/booking'; | ||
| import axios from 'axios'; |
|
any updates? @Emmzyemms |
|
@respp On it now sorry |
2 participants
Pull Request | StellarRent
📝 Summary
Provide a brief description of what this PR accomplishes.
🔗 Related Issues
Closes: #141 (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
Improvements