-
Notifications
You must be signed in to change notification settings - Fork 313
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
handled all the changes in the enatega admin #1003
Conversation
WalkthroughThis pull request makes extensive updates across multiple modules. The active orders GraphQL query now accepts pagination, action, and search parameters while its return type is restructured. The restaurant layout context and several UI forms have been enhanced to manage add-on options with new state variables. Dispatch components and associated table functionalities now support improved search, pagination, and rider assignment logic. Additionally, upload validation has been simplified and several interfaces have been extended to include new properties. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Table as Dispatch Table
participant Dispatch as Dispatch Main Component
participant API as GraphQL API
User->>Table: Change search input / page selection
Table->>Dispatch: Trigger onPageChange/fetch action
Dispatch->>API: Execute GetActiveOrders(query params)
API-->>Dispatch: Return orders data (totalCount & orders)
Dispatch->>Table: Update and render updated order list
sequenceDiagram
participant User as User
participant Form as Add-on Form
participant Context as Restaurant Layout Context
User->>Form: Click to open options interface
Form->>Context: call setIsAddOptionsVisible(true)
Context-->>Form: Options visibility updated (true)
Form->>User: Display OptionsAddForm for managing add-on options
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for polite-fairy-234917 canceled.
|
✅ Deploy Preview for cheery-zabaione-34f12e ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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: 4
🔭 Outside diff range comments (3)
enatega-multivendor-admin/lib/ui/screen-components/protected/super-admin/dispatch/view/header/table-header/index.tsx (1)
303-355
: 🛠️ Refactor suggestionImprove error handling for rider assignment.
The rider dropdown component lacks error handling for edge cases:
- No error state for failed assignments
- No loading state feedback for users
Add error handling:
<Dropdown options={riderOptions} loading={ isRiderLoading._id === selectedRider._id && isRiderLoading.bool === true && isRiderLoading.orderId === rowData._id } value={selectedRider} placeholder={t('Select Rider')} onChange={(e: DropdownChangeEvent) => handleAssignRider(e.value, rowData) } className="outline outline-1 min-w-[120px] outline-gray-300" + error={rowData.assignmentError} + onFocus={() => clearAssignmentError(rowData._id)} />enatega-multivendor-admin/lib/ui/screen-components/protected/restaurant/add-on/add-form/index.tsx (1)
189-214
:⚠️ Potential issueAdd null checks in mapOptionIds function.
The function could potentially include undefined values in the matched_options array if a matching option is not found.
Apply this diff to add proper null checks:
const matched_options = optionIds.map((id) => { const matchedOption = optionsData.find((op) => op.code === id); - return { label: matchedOption?.label, code: matchedOption?.code }; + return matchedOption + ? { label: matchedOption.label, code: matchedOption.code } + : null; }); const updated_addon = addon ? JSON.parse(JSON.stringify(addon)) : ({} as IAddonForm); delete updated_addon.options; setInitialValues({ addons: [ { ...initialFormValuesTemplate, ...updated_addon, - options: matched_options, + options: matched_options.filter(Boolean), }, ], });enatega-multivendor-admin/lib/ui/useable-components/table/columns/dispatch-columns.tsx (1)
212-237
: 🛠️ Refactor suggestionEnhance error handling in rider assignment.
The
handleAssignRider
function should reset loading state even if the assignment fails.const handleAssignRider = async ( item: IDropdownSelectItem, rowData: IActiveOrders ) => { + try { if (item._id) { setIsRiderLoading({ _id: item._id, bool: true, orderId: rowData._id, }); const { data } = await assignRider({ variables: { id: rowData._id, riderId: item._id, }, }); if (data) { showToast({ type: 'success', title: t('Assign Rider'), message: `${t('The order')} ${rowData.orderId} ${t('has been successfully assigned to rider')} ${item.label}`, }); } } + } catch (error) { + showToast({ + type: 'error', + title: t('Assign Rider'), + message: t('Failed to assign rider'), + }); + } finally { + setIsRiderLoading({ + _id: '', + orderId: '', + bool: false, + }); + } };
🧹 Nitpick comments (17)
enatega-multivendor-admin/lib/api/graphql/queries/orders/index.ts (1)
18-19
: Ensure the component or data layer accommodates the newtotalCount
field
AddingtotalCount
offers straightforward pagination. Confirm that whichever component or context consumesgetActiveOrders
properly retrieves and displays thetotalCount
value.enatega-multivendor-admin/lib/ui/useable-components/upload/upload-image.tsx (3)
43-50
: Refine allowed file types if not handling videos.
Althoughvideo/webm
andvideo/mp4
remain infileTypes
, video validation is effectively commented out. If you no longer plan to accept videos, consider removing these entries to reduce confusion and potential user errors.
81-86
: Clarify or remove deprecated validation logic.
isValid
is trivially set to true, and the video/image validation code is commented out. Either implement a working validation or remove these stale comments to keep the codebase clear and concise.
224-239
: Avoid brittleincludes('video/')
checks.
Usingincludes('video/')
in the URL is a bit risky—some Cloudinary transformations or parameters might not containvideo/
. Consider storing file type metadata during upload or verifying via a known extension or MIME type.enatega-multivendor-admin/lib/utils/constants/url.ts (1)
3-4
: Verify accessibility of the updated LOCAL URLs.
The IP addresses have changed. If this IP is tied to a local environment, confirm it’s reachable by your local developer machines or staging environment. For broader maintainability, consider using environment variables or a DNS name.enatega-multivendor-admin/lib/utils/interfaces/table.interface.ts (1)
31-38
: LGTM! Consider type improvements for better type safety.The new properties enhance the table component with pagination, sorting, and scrolling capabilities. The implementation aligns well with PrimeReact's DataTable component.
Consider these type improvements:
- totalRecords?: number; + totalRecords: number; - currentPage?: number; + currentPage: number; - sortField?:string; + sortField: string; - sortOrder?:SortOrder + sortOrder: SortOrder;This ensures required pagination properties are always provided when implementing server-side pagination.
enatega-multivendor-admin/lib/ui/screen-components/protected/super-admin/dispatch/view/main/index.tsx (1)
27-30
: Consider adding error handling for the pagination state.The pagination states could benefit from validation to ensure
page
androwsPerPage
remain within valid ranges.- const [page, setPage] = useState(1); - const [rowsPerPage, setRowsPerPage] = useState(10); + const [page, setPage] = useState(1); + const [rowsPerPage, setRowsPerPage] = useState(10); + + const setValidPage = (newPage: number) => { + setPage(Math.max(1, newPage)); + }; + + const setValidRowsPerPage = (newRowsPerPage: number) => { + setRowsPerPage(Math.max(1, Math.min(50, newRowsPerPage))); + };enatega-multivendor-admin/lib/ui/useable-components/table/index.tsx (1)
52-58
: Consider adding type safety for page calculation.The page calculation in
handlePageChange
could benefit from additional type safety and validation.const handlePageChange = (event: DataTablePageEvent) => { if (onPageChange) { - const page = Math.floor(event.first / event.rows) + 1; + const page = Math.max(1, Math.floor(event.first / event.rows) + 1); + const validRows = Math.max(1, event.rows); - onPageChange(page, event.rows); + onPageChange(page, validRows); } };enatega-multivendor-admin/lib/ui/screen-components/protected/super-admin/dispatch/view/header/table-header/index.tsx (3)
16-16
: Consider using a dedicated debounce hook.The
useCallback
import suggests debounce functionality. Consider extracting the debounce logic into a reusable custom hook for better code organization and reusability.Create a custom hook:
// hooks/useDebounce.ts import { useCallback } from 'react'; export function useDebounce<T>(callback: (value: T) => void, delay: number) { return useCallback((value: T) => { let timer: ReturnType<typeof setTimeout>; callback(value); return () => { clearTimeout(timer); timer = setTimeout(() => callback(value), delay); }; }, [callback, delay]); }
84-88
: Consider adding aria-label for accessibility.The search input should have an aria-label for better accessibility.
<CustomTextField type="text" name="vendorFilter" maxLength={35} showLabel={false} value={search} onChange={(e: ChangeEvent<HTMLInputElement>) => debounceSearch(300, e.target.value) } placeholder={t('Keyword Search')} + aria-label={t('Search vendors')} />
399-414
: Consider using an enum for order statuses.The status codes are used as magic strings. Consider using an enum for better type safety and maintainability.
enum OrderStatus { PENDING = 'PENDING', ACCEPTED = 'ACCEPTED', DELIVERED = 'DELIVERED', CANCELLED = 'CANCELLED' } // Then use it in the filter: const availableStatuses = rowData.isPickedUp ? actionStatusOptions.filter((status) => [ OrderStatus.PENDING, OrderStatus.ACCEPTED, OrderStatus.DELIVERED, OrderStatus.CANCELLED ].includes(status.code as OrderStatus) ) : actionStatusOptions;enatega-multivendor-admin/lib/ui/screen-components/protected/restaurant/category/add-form/index.tsx (1)
140-143
: Consider adding error handling for refetch.The refetch call should handle potential failures.
-if (typeof refetchCategories === 'function') { - refetchCategories(); -} +if (typeof refetchCategories === 'function') { + try { + await refetchCategories(); + } catch (error) { + showToast({ + type: 'error', + title: t('Refresh Categories'), + message: t('Failed to refresh categories'), + }); + } +}enatega-multivendor-admin/lib/ui/screen-components/protected/restaurant/add-on/add-form/index.tsx (1)
312-367
: Consider adding validation for quantity relationship.The minimum quantity should not exceed the maximum quantity. Currently, there's no validation to ensure this relationship.
Consider adding cross-field validation in the Formik form:
<CustomNumberField name={`addons[${index}].quantityMinimum`} min={1} max={99999} + max={value.quantityMaximum} minFractionDigits={0} maxFractionDigits={0} placeholder={t('Minimum Quantity')} showLabel={true} value={value.quantityMinimum} onChangeFieldValue={setFieldValue} style={{ borderColor: onErrorMessageMatcher( 'quantityMinimum', _errors[index]?.quantityMinimum, AddonsErrors ) ? 'red' : '', }} /> // ... <CustomNumberField name={`addons[${index}].quantityMaximum`} min={1} + min={value.quantityMinimum} max={99999} minFractionDigits={0} maxFractionDigits={0} placeholder={t('Maximum Quantity')} showLabel={true} value={value.quantityMaximum} onChangeFieldValue={setFieldValue} style={{ borderColor: onErrorMessageMatcher( 'quantityMaximum', _errors[index]?.quantityMaximum, AddonsErrors ) ? 'red' : '', }} />enatega-multivendor-admin/lib/ui/screen-components/protected/restaurant/food/form/add-form/variations.tsx (2)
476-490
: Consider improving modal z-index management.While the z-index of 999 works, it's better to use a more maintainable approach for managing z-indices.
Consider creating a z-index management system:
// constants/z-index.ts export const Z_INDEX = { MODAL: 100, MODAL_OVERLAY: 90, DROPDOWN: 80, // ... other z-indices } as const;Then update the className:
-className='z-[999]' +className={`z-[${Z_INDEX.MODAL}]`}
85-87
: Consider using object destructuring for context values.The current destructuring pattern could be more concise.
Apply this diff to improve readability:
-const { - restaurantLayoutContextData: { restaurantId }, - option, - setOption -} = useContext(RestaurantLayoutContext); +const { restaurantLayoutContextData: { restaurantId }, option, setOption } = useContext(RestaurantLayoutContext);enatega-multivendor-admin/lib/ui/useable-components/table/columns/dispatch-columns.tsx (2)
81-112
: Consider memoizing the status options array.The
actionStatusOptions
array is recreated on every render. Since it depends only on translations, it can be memoized for better performance.+ const actionStatusOptions = useMemo(() => [ - const actionStatusOptions = [ { label: t('Pending'), code: 'PENDING', body: () => <Tag value={t('Pending')} severity="secondary" rounded />, }, // ... other options - ]; + ], [t]);
400-414
: Extract status codes as constants.Status codes are used in multiple places and should be centralized as constants to prevent typos and improve maintainability.
+ const ORDER_STATUS = { + PENDING: 'PENDING', + ACCEPTED: 'ACCEPTED', + DELIVERED: 'DELIVERED', + CANCELLED: 'CANCELLED', + } as const; const availableStatuses = rowData.isPickedUp ? actionStatusOptions.filter((status) => - ['PENDING', 'ACCEPTED', 'DELIVERED', 'CANCELLED'].includes( - status.code - ) + [ + ORDER_STATUS.PENDING, + ORDER_STATUS.ACCEPTED, + ORDER_STATUS.DELIVERED, + ORDER_STATUS.CANCELLED + ].includes(status.code) ) : actionStatusOptions; - const isDelivered = rowData.orderStatus === 'DELIVERED'; + const isDelivered = rowData.orderStatus === ORDER_STATUS.DELIVERED;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
enatega-multivendor-admin/lib/api/graphql/queries/orders/index.ts
(1 hunks)enatega-multivendor-admin/lib/context/restaurant/layout-restaurant.context.tsx
(3 hunks)enatega-multivendor-admin/lib/ui/screen-components/protected/restaurant/add-on/add-form/index.tsx
(6 hunks)enatega-multivendor-admin/lib/ui/screen-components/protected/restaurant/category/add-form/index.tsx
(2 hunks)enatega-multivendor-admin/lib/ui/screen-components/protected/restaurant/food/form/add-form/food.index.tsx
(3 hunks)enatega-multivendor-admin/lib/ui/screen-components/protected/restaurant/food/form/add-form/variations.tsx
(2 hunks)enatega-multivendor-admin/lib/ui/screen-components/protected/super-admin/dispatch/view/header/table-header/index.tsx
(8 hunks)enatega-multivendor-admin/lib/ui/screen-components/protected/super-admin/dispatch/view/main/index.tsx
(2 hunks)enatega-multivendor-admin/lib/ui/screens/admin/restaurant/food-management/category/index.tsx
(1 hunks)enatega-multivendor-admin/lib/ui/useable-components/table/columns/dispatch-columns.tsx
(9 hunks)enatega-multivendor-admin/lib/ui/useable-components/table/index.tsx
(5 hunks)enatega-multivendor-admin/lib/ui/useable-components/upload/upload-image.tsx
(9 hunks)enatega-multivendor-admin/lib/utils/constants/url.ts
(1 hunks)enatega-multivendor-admin/lib/utils/interfaces/add-on.interface.ts
(2 hunks)enatega-multivendor-admin/lib/utils/interfaces/category.interface.ts
(1 hunks)enatega-multivendor-admin/lib/utils/interfaces/dispatch.interface.ts
(3 hunks)enatega-multivendor-admin/lib/utils/interfaces/layout.interface.ts
(2 hunks)enatega-multivendor-admin/lib/utils/interfaces/table.interface.ts
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- enatega-multivendor-admin/lib/ui/screens/admin/restaurant/food-management/category/index.tsx
🔇 Additional comments (19)
enatega-multivendor-admin/lib/api/graphql/queries/orders/index.ts (2)
4-10
: Extend validation or type-checking for new parameters.
The newly introduced parameters$page
,$rowsPerPage
,$actions
, and$search
expand the query's capabilities. Consider ensuring all downstream usage (e.g., UI form bindings, server-side resolvers) properly handles edge cases like negative page numbers, unusual search strings, and an empty list of actions.
21-49
: Verify usage of new fields (zone
,deliveryAddress
,isPickedUp
, etc.).
These nested fields provide valuable order details. Double-check that the UI or the business logic are updated accordingly to handle cases likeundefined
addresses or riders. Also ensure that newly introduced booleans such asisPickedUp
align with the real-world scenarios (e.g., it may affect how the UI displays "Pickup" vs. "Delivery").enatega-multivendor-admin/lib/utils/interfaces/add-on.interface.ts (1)
19-22
: LGTM! Well-structured state management for add-on options.The new properties follow React's state management patterns and provide clear type definitions for managing add-on options visibility and state.
enatega-multivendor-admin/lib/utils/interfaces/dispatch.interface.ts (3)
5-5
: LGTM! Enhanced order tracking capability.The addition of
isPickedUp
improves order status tracking.
42-42
: LGTM! Improved pagination support.The restructured
getActiveOrders
response now includestotalCount
, enabling proper pagination implementation.
79-80
: LGTM! Added search functionality.Search capabilities added to dispatch table header with proper TypeScript types.
enatega-multivendor-admin/lib/utils/interfaces/category.interface.ts (1)
20-20
: LGTM! Added refresh capability for categories.The optional
refetchCategories
callback enables refreshing the category list after mutations, following good React patterns.enatega-multivendor-admin/lib/context/restaurant/layout-restaurant.context.tsx (3)
8-10
: LGTM! Clean interface import organization.The addition of
IOptions
import is properly placed with related interfaces.Also applies to: 14-14
27-30
: LGTM! Well-structured state management.The new states for options management follow React's state management best practices.
66-69
: LGTM! Context value properly updated.The new states are correctly exposed through the context value.
enatega-multivendor-admin/lib/ui/useable-components/table/index.tsx (1)
77-86
: LGTM! Well-structured pagination props.The pagination props are properly organized and conditionally applied based on server-side pagination status.
enatega-multivendor-admin/lib/utils/interfaces/layout.interface.ts (1)
64-68
: LGTM! Well-typed interface extensions.The new properties for options management are properly typed with React's Dispatch and follow TypeScript best practices.
enatega-multivendor-admin/lib/ui/screen-components/protected/super-admin/dispatch/view/header/table-header/index.tsx (1)
42-63
: LGTM! Enhanced status options.The menu items array has been expanded to include "Picked" and "Delivered" statuses, improving the order tracking workflow.
enatega-multivendor-admin/lib/ui/screen-components/protected/restaurant/category/add-form/index.tsx (1)
58-59
: LGTM! Added refetch capability.The
refetchCategories
prop enables immediate UI updates after category changes.enatega-multivendor-admin/lib/ui/screen-components/protected/restaurant/food/form/add-form/food.index.tsx (3)
110-110
: Verify the impact of no-cache policy.Using 'no-cache' might impact performance by causing unnecessary network requests.
Consider if 'cache-and-network' with proper cache invalidation would be more efficient:
-fetchPolicy: 'no-cache', +fetchPolicy: 'cache-and-network',
139-139
: LGTM! Improved dependency tracking.The dependency array now correctly includes the categories data, ensuring proper reactivity.
428-428
: LGTM! Added category refresh capability.The
refetchCategories
prop is properly passed to enable immediate UI updates.enatega-multivendor-admin/lib/ui/screen-components/protected/restaurant/add-on/add-form/index.tsx (1)
11-17
: LGTM! Good choice of reusable components.The use of custom components like
CustomButton
,CustomMultiSelectComponent
, etc., promotes consistency and reusability across the application.enatega-multivendor-admin/lib/ui/useable-components/table/columns/dispatch-columns.tsx (1)
2-5
: LGTM! Import changes improve type safety.The addition of
IAssignRider
interface and proper type imports enhance type checking and maintainability.
console.log(':rocket: ~ .then ~ url:', url); | ||
isValid = false; | ||
console.log(':rocket: ~ Valid url idk about the response'); | ||
onSetImageUrl(name, url); | ||
if (!url) { | ||
showToast({ | ||
type: 'error', | ||
title: title, | ||
message: `${fileTypes.includes('video/webm') || fileTypes.includes('video/mp4') ? t('File') : t('Image')} ${t('Upload Failed')}`, | ||
duration: 2500, | ||
}); | ||
setImageValidationErr({ | ||
bool: true, | ||
msg: 'Cloudinary Url Invalid', | ||
}); | ||
setImageFile(''); | ||
return; | ||
} |
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.
🛠️ Refactor suggestion
Revisit the isValid = false
assignment after a successful upload.
Setting isValid
to false
immediately after logging a successful URL is confusing and may lead to misinterpretations. If this flag is for toggling a subsequent check, consider renaming it and documenting its usage for clarity.
- isValid = false;
+ // If the upload is successful, consider setting a distinct variable (e.g., `uploadComplete`)
+ // or removing this line entirely if it's unnecessary.
📝 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.
console.log(':rocket: ~ .then ~ url:', url); | |
isValid = false; | |
console.log(':rocket: ~ Valid url idk about the response'); | |
onSetImageUrl(name, url); | |
if (!url) { | |
showToast({ | |
type: 'error', | |
title: title, | |
message: `${fileTypes.includes('video/webm') || fileTypes.includes('video/mp4') ? t('File') : t('Image')} ${t('Upload Failed')}`, | |
duration: 2500, | |
}); | |
setImageValidationErr({ | |
bool: true, | |
msg: 'Cloudinary Url Invalid', | |
}); | |
setImageFile(''); | |
return; | |
} | |
console.log(':rocket: ~ .then ~ url:', url); | |
// If the upload is successful, consider setting a distinct variable (e.g., `uploadComplete`) | |
// or removing this line entirely if it's unnecessary. | |
console.log(':rocket: ~ Valid url idk about the response'); | |
onSetImageUrl(name, url); | |
if (!url) { | |
showToast({ | |
type: 'error', | |
title: title, | |
message: `${fileTypes.includes('video/webm') || fileTypes.includes('video/mp4') ? t('File') : t('Image')} ${t('Upload Failed')}`, | |
duration: 2500, | |
}); | |
setImageValidationErr({ | |
bool: true, | |
msg: 'Cloudinary Url Invalid', | |
}); | |
setImageFile(''); | |
return; | |
} |
useEffect(() => { | ||
fetchActiveOrders(); | ||
fetchActiveOrders({ | ||
variables: { | ||
page, | ||
rowsPerPage, | ||
search, | ||
actions: selectedActions, | ||
restaurantId: '', | ||
}, | ||
}); | ||
setIsLoading(true); | ||
}, []); | ||
}, [rowsPerPage, page, selectedActions, search]); |
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.
🛠️ Refactor suggestion
Consider implementing debouncing for search and error handling for the query.
The search functionality could benefit from debouncing to prevent excessive API calls, and error handling should be added for the query execution.
+ const debouncedSearch = useDebounce(search, 300);
+
useEffect(() => {
fetchActiveOrders({
variables: {
page,
rowsPerPage,
- search,
+ search: debouncedSearch,
actions: selectedActions,
restaurantId: '',
},
+ }).catch((error) => {
+ console.error('Error fetching orders:', error);
+ setIsLoading(false);
});
setIsLoading(true);
- }, [rowsPerPage, page, selectedActions, search]);
+ }, [rowsPerPage, page, selectedActions, debouncedSearch]);
📝 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.
useEffect(() => { | |
fetchActiveOrders(); | |
fetchActiveOrders({ | |
variables: { | |
page, | |
rowsPerPage, | |
search, | |
actions: selectedActions, | |
restaurantId: '', | |
}, | |
}); | |
setIsLoading(true); | |
}, []); | |
}, [rowsPerPage, page, selectedActions, search]); | |
const debouncedSearch = useDebounce(search, 300); | |
useEffect(() => { | |
fetchActiveOrders({ | |
variables: { | |
page, | |
rowsPerPage, | |
search: debouncedSearch, | |
actions: selectedActions, | |
restaurantId: '', | |
}, | |
}).catch((error) => { | |
console.error('Error fetching orders:', error); | |
setIsLoading(false); | |
}); | |
setIsLoading(true); | |
}, [rowsPerPage, page, selectedActions, debouncedSearch]); |
const debounceSearch = useCallback((delay: number, val: string) => { | ||
let timer: ReturnType<typeof setTimeout>; | ||
setSearch(val); | ||
return () => { | ||
clearTimeout(timer); | ||
timer = setTimeout(() => setSearch(val), delay); | ||
}; | ||
}, []); |
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.
Fix potential memory leak in debounce implementation.
The current implementation has two issues:
- The timer cleanup function is never called
- The timer variable is recreated on every call
Apply this fix:
- const debounceSearch = useCallback((delay: number, val: string) => {
- let timer: ReturnType<typeof setTimeout>;
- setSearch(val);
- return () => {
- clearTimeout(timer);
- timer = setTimeout(() => setSearch(val), delay);
- };
- }, []);
+ const debounceSearch = useCallback((delay: number, val: string) => {
+ setSearch(val);
+ const timer = setTimeout(() => setSearch(val), delay);
+ return () => clearTimeout(timer);
+ }, [setSearch]);
📝 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 debounceSearch = useCallback((delay: number, val: string) => { | |
let timer: ReturnType<typeof setTimeout>; | |
setSearch(val); | |
return () => { | |
clearTimeout(timer); | |
timer = setTimeout(() => setSearch(val), delay); | |
}; | |
}, []); | |
const debounceSearch = useCallback((delay: number, val: string) => { | |
setSearch(val); | |
const timer = setTimeout(() => setSearch(val), delay); | |
return () => clearTimeout(timer); | |
}, [setSearch]); |
// { | ||
// propertyName: 'orderStatus', | ||
// headerName: t('Status'), | ||
|
||
// body: (rowData: IActiveOrders) => { | ||
// const currentStatus = actionStatusOptions.find( | ||
// (status: IDropdownSelectItem) => status.code === rowData?.orderStatus | ||
// ); | ||
|
||
// return ( | ||
// <> | ||
// <Dropdown | ||
// value={currentStatus} | ||
// onChange={(e) => handleStatusDropDownChange(e, rowData)} | ||
// options={actionStatusOptions} | ||
// itemTemplate={itemTemplate} | ||
// valueTemplate={valueTemplate} | ||
// loading={ | ||
// isStatusUpdating.bool && isStatusUpdating._id === rowData._id | ||
// } | ||
// className="outline outline-1 outline-gray-300" | ||
// /> | ||
// </> | ||
// ); | ||
// }, | ||
// }, |
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.
🛠️ Refactor suggestion
Remove commented code.
Large blocks of commented code should be removed as they're already tracked in version control. This improves code readability and maintainability.
- // {
- // propertyName: 'orderStatus',
- // headerName: t('Status'),
- // body: (rowData: IActiveOrders) => {
- // const currentStatus = actionStatusOptions.find(
- // (status: IDropdownSelectItem) => status.code === rowData?.orderStatus
- // );
- // return (
- // <>
- // <Dropdown
- // value={currentStatus}
- // onChange={(e) => handleStatusDropDownChange(e, rowData)}
- // options={actionStatusOptions}
- // itemTemplate={itemTemplate}
- // valueTemplate={valueTemplate}
- // loading={
- // isStatusUpdating.bool && isStatusUpdating._id === rowData._id
- // }
- // className="outline outline-1 outline-gray-300"
- // />
- // </>
- // );
- // },
- // },
📝 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.
// { | |
// propertyName: 'orderStatus', | |
// headerName: t('Status'), | |
// body: (rowData: IActiveOrders) => { | |
// const currentStatus = actionStatusOptions.find( | |
// (status: IDropdownSelectItem) => status.code === rowData?.orderStatus | |
// ); | |
// return ( | |
// <> | |
// <Dropdown | |
// value={currentStatus} | |
// onChange={(e) => handleStatusDropDownChange(e, rowData)} | |
// options={actionStatusOptions} | |
// itemTemplate={itemTemplate} | |
// valueTemplate={valueTemplate} | |
// loading={ | |
// isStatusUpdating.bool && isStatusUpdating._id === rowData._id | |
// } | |
// className="outline outline-1 outline-gray-300" | |
// /> | |
// </> | |
// ); | |
// }, | |
// }, |
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: 0
🧹 Nitpick comments (1)
enatega-multivendor-admin/lib/utils/interfaces/add-on.interface.ts (1)
19-22
: Add JSDoc comments and fix spacing.The new properties are well-typed and correctly implemented. Consider adding JSDoc comments to document their purpose and maintaining consistent spacing after colons.
- isAddOptionsVisible?:boolean; - setIsAddOptionsVisible?:Dispatch<SetStateAction<boolean>>; - option?:IOptions | null; - setOption?:Dispatch<SetStateAction<IOptions | null>>; + /** Controls the visibility of add-on options */ + isAddOptionsVisible?: boolean; + /** Updates the visibility state of add-on options */ + setIsAddOptionsVisible?: Dispatch<SetStateAction<boolean>>; + /** Currently selected option */ + option?: IOptions | null; + /** Updates the currently selected option */ + setOption?: Dispatch<SetStateAction<IOptions | null>>;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
enatega-multivendor-admin/lib/utils/interfaces/add-on.interface.ts
(2 hunks)
🔇 Additional comments (1)
enatega-multivendor-admin/lib/utils/interfaces/add-on.interface.ts (1)
7-7
: LGTM!The import of
IOptions
is correctly added and necessary for the new interface properties.
No description provided.