Implement handling for new iCard swiper #384
Conversation
WalkthroughAdds URL state management for item filtering using Changes
Sequence DiagramsequenceDiagram
participant Component as ScanTickets Component
participant URL as URL Params
participant State as Local State
participant API as API/Data
rect rgb(200, 220, 255)
Note over Component,API: Initialization
Component->>URL: Read searchParams (itemId)
Component->>API: Load organizations & ticket items
API-->>Component: Items loaded
Component->>State: Validate itemId against items
alt itemId valid
Component->>State: Initialize selectedItemFilter
Component->>URL: Keep itemId param
else itemId invalid
Component->>State: Clear selectedItemFilter
Component->>URL: Clear itemId param
end
end
rect rgb(220, 240, 220)
Note over Component,State: User Interaction
Component->>Component: User selects item
Component->>Component: handleItemFilterChange triggered
Component->>State: Update selectedItemFilter
Component->>URL: Sync itemId to URL params
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
💰 Infracost reportMonthly estimate generatedThis comment will be updated when code changes. |
devksingh4
force-pushed
the
dsingh14/new
branch
from
62dd94d to
9fa360c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
src/ui/pages/tickets/ScanTickets.page.tsx(8 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
src/ui/pages/tickets/ScanTickets.page.tsx
[error] 171-171: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 178-178: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 185-185: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 197-197: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🪛 GitHub Actions: Required Reviews
src/ui/pages/tickets/ScanTickets.page.tsx
[error] 1-1: Requirement "Base Requirement" is not satisfied by the existing reviews.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build Application
- GitHub Check: Run Unit Tests
🔇 Additional comments (6)
src/ui/pages/tickets/ScanTickets.page.tsx (6)
20-23: LGTM! Clean import additions.The addition of
useCallbackanduseSearchParamsaligns with the new URL state management and callback stabilization features.
118-119: LGTM! Proper URL state initialization.The
useSearchParamshook is correctly initialized and the initialitemIdis properly read from the URL to initialize the filter state.Also applies to: 151-154
364-375: LGTM! Robust URL validation.The validation logic ensures the
itemIdfrom the URL exists in the loaded items, and properly clears invalid values. Usingreplace: trueprevents polluting browser history.
832-843: LGTM! Well-implemented memoized handler.The memoized
handleItemFilterChangeproperly synchronizes both local state and URL parameters, withsetSearchParamsas a stable dependency.
864-864: LGTM! Proper usage of memoized handler.The Select component now uses the memoized handler, improving performance by preventing unnecessary re-renders.
394-394: No action required.The component is used without any props in
src/ui/Router.tsx(line 189), so the concern about prop function stability is not applicable. The dependencies on line 394 are correct for the current usage pattern. The component works as intended with the optional props mechanism.
| const getOrganizations = | ||
| getOrganizationsProp || | ||
| (async () => { | ||
| useCallback(async () => { | ||
| const response = await api.get("/api/v1/organizations"); | ||
| return response.data; | ||
| }); | ||
| }, [api]); | ||
|
|
||
| const getTicketItems = | ||
| getTicketItemsProp || | ||
| (async () => { | ||
| useCallback(async () => { | ||
| const response = await api.get("/api/v1/tickets"); | ||
| return response.data; | ||
| }); | ||
| }, [api]); | ||
|
|
||
| const getPurchasesByEmail = | ||
| getPurchasesByEmailProp || | ||
| (async (email: string) => { | ||
| const response = await api.get<PurchasesByEmailResponse>( | ||
| `/api/v1/tickets/purchases/${encodeURIComponent(email)}`, | ||
| ); | ||
| return response.data; | ||
| }); | ||
| useCallback( | ||
| async (email: string) => { | ||
| const response = await api.get<PurchasesByEmailResponse>( | ||
| `/api/v1/tickets/purchases/${encodeURIComponent(email)}`, | ||
| ); | ||
| return response.data; | ||
| }, | ||
| [api], | ||
| ); | ||
|
|
||
| const checkInTicket = | ||
| checkInTicketProp || | ||
| (async (data: any) => { | ||
| const response = await api.post( | ||
| `/api/v1/tickets/checkIn`, | ||
| recursiveToCamel(data), | ||
| ); | ||
| return response.data as APIResponseSchema; | ||
| }); | ||
| useCallback( | ||
| async (data: any) => { | ||
| const response = await api.post( | ||
| `/api/v1/tickets/checkIn`, | ||
| recursiveToCamel(data), | ||
| ); | ||
| return response.data as APIResponseSchema; | ||
| }, | ||
| [api], | ||
| ); | ||
|
|
||
| const getEmailFromUINDefault = async (uin: string): Promise<string> => { | ||
| try { | ||
| const response = await api.post(`/api/v1/users/findUserByUin`, { uin }); | ||
| return response.data.email; | ||
| } catch (error: any) { | ||
| const samp = new ValidationError({ | ||
| message: "Failed to convert UIN to email.", | ||
| }); | ||
| if ( | ||
| error.response?.status === samp.httpStatusCode && | ||
| error.response?.data.id === samp.id | ||
| ) { | ||
| const validationData = error.response.data; | ||
| throw new ValidationError(validationData.message || samp.message); | ||
| const getEmailFromUINDefault = useCallback( | ||
| async (uin: string): Promise<string> => { | ||
| try { | ||
| const response = await api.post(`/api/v1/users/findUserByUin`, { uin }); | ||
| return response.data.email; | ||
| } catch (error: any) { | ||
| const samp = new ValidationError({ | ||
| message: "Failed to convert UIN to email.", | ||
| }); | ||
| if ( | ||
| error.response?.status === samp.httpStatusCode && | ||
| error.response?.data.id === samp.id | ||
| ) { | ||
| const validationData = error.response.data; | ||
| throw new ValidationError(validationData.message || samp.message); | ||
| } | ||
| throw error; | ||
| } | ||
| throw error; | ||
| } | ||
| }; | ||
| }, | ||
| [api], | ||
| ); |
There was a problem hiding this comment.
Fix conditional hook calls - violates Rules of Hooks.
The pattern const fn = propFn || useCallback(...) violates the Rules of Hooks because the OR operator short-circuits. When propFn is truthy, useCallback is never called, changing hook call order between renders.
Apply this pattern to fix all instances:
- const getOrganizations =
- getOrganizationsProp ||
- useCallback(async () => {
+ const getOrganizationsDefault = useCallback(async () => {
const response = await api.get("/api/v1/organizations");
return response.data;
}, [api]);
+ const getOrganizations = getOrganizationsProp || getOrganizationsDefault;
- const getTicketItems =
- getTicketItemsProp ||
- useCallback(async () => {
+ const getTicketItemsDefault = useCallback(async () => {
const response = await api.get("/api/v1/tickets");
return response.data;
}, [api]);
+ const getTicketItems = getTicketItemsProp || getTicketItemsDefault;
- const getPurchasesByEmail =
- getPurchasesByEmailProp ||
- useCallback(
- async (email: string) => {
+ const getPurchasesByEmailDefault = useCallback(
+ async (email: string) => {
const response = await api.get<PurchasesByEmailResponse>(
`/api/v1/tickets/purchases/${encodeURIComponent(email)}`,
);
return response.data;
},
[api],
);
+ const getPurchasesByEmail = getPurchasesByEmailProp || getPurchasesByEmailDefault;
- const checkInTicket =
- checkInTicketProp ||
- useCallback(
- async (data: any) => {
+ const checkInTicketDefault = useCallback(
+ async (data: any) => {
const response = await api.post(
`/api/v1/tickets/checkIn`,
recursiveToCamel(data),
);
return response.data as APIResponseSchema;
},
[api],
);
+ const checkInTicket = checkInTicketProp || checkInTicketDefault;Note: getEmailFromUINDefault at lines 208-228 is already correct since it's always called unconditionally.
📝 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.
| const getOrganizations = | |
| getOrganizationsProp || | |
| (async () => { | |
| useCallback(async () => { | |
| const response = await api.get("/api/v1/organizations"); | |
| return response.data; | |
| }); | |
| }, [api]); | |
| const getTicketItems = | |
| getTicketItemsProp || | |
| (async () => { | |
| useCallback(async () => { | |
| const response = await api.get("/api/v1/tickets"); | |
| return response.data; | |
| }); | |
| }, [api]); | |
| const getPurchasesByEmail = | |
| getPurchasesByEmailProp || | |
| (async (email: string) => { | |
| const response = await api.get<PurchasesByEmailResponse>( | |
| `/api/v1/tickets/purchases/${encodeURIComponent(email)}`, | |
| ); | |
| return response.data; | |
| }); | |
| useCallback( | |
| async (email: string) => { | |
| const response = await api.get<PurchasesByEmailResponse>( | |
| `/api/v1/tickets/purchases/${encodeURIComponent(email)}`, | |
| ); | |
| return response.data; | |
| }, | |
| [api], | |
| ); | |
| const checkInTicket = | |
| checkInTicketProp || | |
| (async (data: any) => { | |
| const response = await api.post( | |
| `/api/v1/tickets/checkIn`, | |
| recursiveToCamel(data), | |
| ); | |
| return response.data as APIResponseSchema; | |
| }); | |
| useCallback( | |
| async (data: any) => { | |
| const response = await api.post( | |
| `/api/v1/tickets/checkIn`, | |
| recursiveToCamel(data), | |
| ); | |
| return response.data as APIResponseSchema; | |
| }, | |
| [api], | |
| ); | |
| const getEmailFromUINDefault = async (uin: string): Promise<string> => { | |
| try { | |
| const response = await api.post(`/api/v1/users/findUserByUin`, { uin }); | |
| return response.data.email; | |
| } catch (error: any) { | |
| const samp = new ValidationError({ | |
| message: "Failed to convert UIN to email.", | |
| }); | |
| if ( | |
| error.response?.status === samp.httpStatusCode && | |
| error.response?.data.id === samp.id | |
| ) { | |
| const validationData = error.response.data; | |
| throw new ValidationError(validationData.message || samp.message); | |
| const getEmailFromUINDefault = useCallback( | |
| async (uin: string): Promise<string> => { | |
| try { | |
| const response = await api.post(`/api/v1/users/findUserByUin`, { uin }); | |
| return response.data.email; | |
| } catch (error: any) { | |
| const samp = new ValidationError({ | |
| message: "Failed to convert UIN to email.", | |
| }); | |
| if ( | |
| error.response?.status === samp.httpStatusCode && | |
| error.response?.data.id === samp.id | |
| ) { | |
| const validationData = error.response.data; | |
| throw new ValidationError(validationData.message || samp.message); | |
| } | |
| throw error; | |
| } | |
| throw error; | |
| } | |
| }; | |
| }, | |
| [api], | |
| ); | |
| const getOrganizationsDefault = useCallback(async () => { | |
| const response = await api.get("/api/v1/organizations"); | |
| return response.data; | |
| }, [api]); | |
| const getOrganizations = getOrganizationsProp || getOrganizationsDefault; | |
| const getTicketItemsDefault = useCallback(async () => { | |
| const response = await api.get("/api/v1/tickets"); | |
| return response.data; | |
| }, [api]); | |
| const getTicketItems = getTicketItemsProp || getTicketItemsDefault; | |
| const getPurchasesByEmailDefault = useCallback( | |
| async (email: string) => { | |
| const response = await api.get<PurchasesByEmailResponse>( | |
| `/api/v1/tickets/purchases/${encodeURIComponent(email)}`, | |
| ); | |
| return response.data; | |
| }, | |
| [api], | |
| ); | |
| const getPurchasesByEmail = getPurchasesByEmailProp || getPurchasesByEmailDefault; | |
| const checkInTicketDefault = useCallback( | |
| async (data: any) => { | |
| const response = await api.post( | |
| `/api/v1/tickets/checkIn`, | |
| recursiveToCamel(data), | |
| ); | |
| return response.data as APIResponseSchema; | |
| }, | |
| [api], | |
| ); | |
| const checkInTicket = checkInTicketProp || checkInTicketDefault; | |
| const getEmailFromUINDefault = useCallback( | |
| async (uin: string): Promise<string> => { | |
| try { | |
| const response = await api.post(`/api/v1/users/findUserByUin`, { uin }); | |
| return response.data.email; | |
| } catch (error: any) { | |
| const samp = new ValidationError({ | |
| message: "Failed to convert UIN to email.", | |
| }); | |
| if ( | |
| error.response?.status === samp.httpStatusCode && | |
| error.response?.data.id === samp.id | |
| ) { | |
| const validationData = error.response.data; | |
| throw new ValidationError(validationData.message || samp.message); | |
| } | |
| throw error; | |
| } | |
| }, | |
| [api], | |
| ); |
🧰 Tools
🪛 Biome (2.1.2)
[error] 171-171: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 178-178: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 185-185: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 197-197: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
Summary by CodeRabbit
New Features