feat(profile): implement comprehensive profile and order tracking fea… - #42
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughIntroduces a Profile feature with routing, pages (Profile, Orders, Rewards), a ProfileLayout/Sidebar, order tracking/history components, a new profileStore, order-store extensions for order history/cancellation, order helper utilities, updated mock data/enums, plus navbar dropdown, toast notifications, theme CSS, and authStore restoreSession fixes. ChangesProfile feature implementation
Navbar, layout, and store fixes
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant OrdersPage as Orders
participant OrderStore as useOrderStore
participant OrderHelpers
participant API as OrderService
User->>OrdersPage: view /profile/orders
OrdersPage->>OrderStore: fetchMyOrders()
OrderStore->>API: getMyOrders()
API-->>OrderStore: orders list
OrderStore-->>OrdersPage: myOrders updated
OrdersPage->>OrderHelpers: mergeOrdersWithLastOrder / groupOrdersByDate / pickTrackingOrder
OrderHelpers-->>OrdersPage: mergedOrdersList, groupedOrders, trackingOrder
OrdersPage-->>User: render OrderCard list / OrderTracking
User->>OrdersPage: cancel order
OrdersPage->>OrderStore: cancelMyOrder(orderId)
OrderStore->>OrderHelpers: isOrderCancellable(order)
OrderStore->>API: cancelOrder(orderId)
API-->>OrderStore: result
OrderStore->>OrderStore: fetchMyOrders()
OrderStore-->>OrdersPage: {ok, message}
OrdersPage-->>User: toast success/error
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![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 (1)
src/index.css (1)
3-13: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winUnclosed
@layer baseblock breaks CSS — confirmed by Stylelint.
@layer base { button { cursor: pointer; }never gets a closing}before@theme {starts, so@theme(and everything that follows) ends up nested inside@layer base. Stylelint reports this asCssSyntaxError: Unclosed blockat line 4. Nesting@themeinside a layer also breaks Tailwind's token-registration for--color-green/--color-orange, which are consumed across the app (e.g. Navbar'stext-greenclasses).🐛 Proposed fix: close `@layer base` before `@theme`
`@layer` base { button { cursor: pointer; } +} `@theme` { --color-green: ...; --color-orange: ...; }Since Tailwind's handling of
@themeblocks nested in@layercan vary by version, please double-check against your Tailwind v4 setup.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/index.css` around lines 3 - 13, Close the unclosed `@layer` base block in src/index.css so `@theme` is no longer nested inside it; the issue is in the top-level stylesheet around `@layer` base, button, and `@theme`. Add the missing closing brace after the button rule, then keep `@theme` at the root level so Tailwind can register --color-green and --color-orange correctly. Verify the resulting CSS parses cleanly with Stylelint and that the theme tokens remain available to components like Navbar.Source: Linters/SAST tools
♻️ Duplicate comments (1)
src/pages/Profile/Orders.jsx (1)
43-50: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNo in-flight guard around cancellation.
handleCancelOrdercan be invoked multiple times concurrently since nothing tracks a pending state here or disables the triggering control inOrderTracking.jsx(see companion comment there on the unguarded Cancel button). Consider adding a localisCancellingstate here and passing it down to disable the button while a cancellation is in flight.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Profile/Orders.jsx` around lines 43 - 50, The cancellation flow in handleCancelOrder is missing an in-flight guard, so repeated clicks can start multiple concurrent requests. Add a local isCancelling state in Orders.jsx around handleCancelOrder to set true before cancelMyOrder and reset it in finally, then pass that state down to OrderTracking.jsx so the Cancel control can be disabled while a cancellation is pending.
🧹 Nitpick comments (11)
src/store/profileStore.js (1)
28-31: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider storing
error.messageinstead of the rawErrorobject.Storing the raw caught
errorobject in state risks a React "Objects are not valid as a React child" crash if any consumer renderserrordirectly (e.g.,{error}in JSX), rather thanerror.messageorerror.toString().Also applies to: 42-44, 55-57
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/store/profileStore.js` around lines 28 - 31, The catch blocks in profileStore are storing the raw Error object in state, which can break consumers that render the value directly. Update the error handling in the affected store methods to save a string value like error.message (or a safe fallback) instead of the full object, while keeping loading false and the null return behavior unchanged. Use the existing catch(error) paths in profileStore to make the change consistently across the noted sections.src/pages/Profile/Orders.jsx (1)
12-22: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSelecting the entire store causes unnecessary re-renders.
useOrderStore((state) => state)subscribes to every field in the shared order store (cart items, customer details, payment method, etc.), so this page re-renders whenever any part oforderStorechanges, not just order-history state. Consider selecting only the needed fields (optionally withzustand/shallow) to scope re-renders to what this page actually uses.♻️ Suggested change
- const { - myOrders, - lastOrder, - myOrdersLoading, - myOrdersError, - fetchMyOrders, - cancelMyOrder, - getMergedOrders, - getGroupedOrders, - getTrackingOrder, - } = useOrderStore((state) => state); + const { + myOrders, + lastOrder, + myOrdersLoading, + myOrdersError, + fetchMyOrders, + cancelMyOrder, + getMergedOrders, + getGroupedOrders, + getTrackingOrder, + } = useOrderStore( + (state) => ({ + myOrders: state.myOrders, + lastOrder: state.lastOrder, + myOrdersLoading: state.myOrdersLoading, + myOrdersError: state.myOrdersError, + fetchMyOrders: state.fetchMyOrders, + cancelMyOrder: state.cancelMyOrder, + getMergedOrders: state.getMergedOrders, + getGroupedOrders: state.getGroupedOrders, + getTrackingOrder: state.getTrackingOrder, + }), + shallow + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Profile/Orders.jsx` around lines 12 - 22, The Orders page is subscribing to the entire order store via useOrderStore((state) => state), which causes unnecessary re-renders on unrelated store updates. Update the selector in Orders.jsx to pick only the fields actually used by this page (such as myOrders, lastOrder, myOrdersLoading, myOrdersError, fetchMyOrders, cancelMyOrder, getMergedOrders, getGroupedOrders, and getTrackingOrder), and use zustand/shallow if needed to keep the subscription scoped.src/pages/Profile/components/OrderCard.jsx (3)
47-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated time-formatting logic.
getFriendlyTimehere duplicates the createdAt→locale-time logic inOrderTracking.jsx'sfriendlyTimeuseMemo. Consider extracting a shared helper (e.g. intosrc/utils/orderHelpers.js, which already centralizes order-related utilities) to avoid drift between the two implementations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Profile/components/OrderCard.jsx` around lines 47 - 58, The time formatting logic in OrderCard’s getFriendlyTime duplicates the createdAt-to-locale-time behavior already implemented in OrderTracking’s friendlyTime useMemo. Extract that shared conversion into a common helper in the existing order utilities (for example, the centralized order helper module) and update both OrderCard and OrderTracking to call it so the formatting stays consistent and avoids drift.
45-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove dead/unused variables flagged by ESLint.
🧹 Proposed cleanup
-const date = new Date(order.createdAt); - const getFriendlyTime = () => { if (order?.time) return order.time; if (order?.createdAt) { try { const d = new Date(order.createdAt); return d.toLocaleTimeString([], { hour: "2-digit", minute: "2-digit" }); - } catch (e) { + } catch { return ""; } } return ""; };Also applies to: 53-53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Profile/components/OrderCard.jsx` at line 45, Remove the dead/unused date variable in OrderCard, since it is flagged by ESLint and not used anywhere in the component. Update the relevant logic in OrderCard.jsx, especially the createdAt handling around the order date formatting/rendering, and also clean up the other unused variable referenced by the review comment so the component has no remaining unused declarations.Source: Linters/SAST tools
75-80: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCurrency formatting inconsistency.
order.totalPriceis rendered without.toFixed(2)here (e.g.12.5$), whileOrderDetailsModal.jsxformats the same kind of value withNumber(totalPrice).toFixed(2)(12.50$). Consider aligning the format across both components.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Profile/components/OrderCard.jsx` around lines 75 - 80, The total price display in OrderCard is inconsistent with OrderDetailsModal because it renders order.totalPrice without fixed decimal formatting. Update the price rendering in OrderCard.jsx to match the same formatting used elsewhere (via Number(...).toFixed(2)) so values like 12.5 display consistently as 12.50. Use the OrderCard component and the totalPrice render block as the place to make the change.src/pages/Profile/components/OrderDetailsModal.jsx (1)
10-23: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider adding Escape-key support to close the modal.
Currently the modal only closes via backdrop click or the "✕" button; there's no keyboard (Escape) affordance, which is a common expectation for dialogs.
♿ Suggested addition
+import React, { useEffect } from "react"; import { DELIVERY_FEE } from "../../../constants"; ... const OrderDetailsModal = ({ order, onClose }) => { if (!order) return null; + useEffect(() => { + const handleKeyDown = (e) => { + if (e.key === "Escape") onClose?.(); + }; + document.addEventListener("keydown", handleKeyDown); + return () => document.removeEventListener("keydown", handleKeyDown); + }, [onClose]); + const { items = [], totalPrice = 0} = order;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Profile/components/OrderDetailsModal.jsx` around lines 10 - 23, Add Escape-key dismissal to OrderDetailsModal so the dialog can be closed from the keyboard as well as the backdrop and close button. Update the OrderDetailsModal component to register a keydown listener when order is present, call onClose when the pressed key is Escape, and clean up the listener on unmount or when the modal closes. Keep the behavior scoped to the modal lifecycle so it doesn’t affect the rest of the Profile page.src/pages/Profile/components/HealthForm.jsx (2)
183-216: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer option value over index as key.
HEALTH_CONDITIONSitems are unique strings; usingoptionas the key is more idiomatic than the array index.♻️ Proposed fix
{HEALTH_CONDITIONS.map((option, index) => { const checked = form.healthConditions.includes(option); return ( <label - key={index} + key={option}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Profile/components/HealthForm.jsx` around lines 183 - 216, Using the array index as the key in the HEALTH_CONDITIONS render loop is unnecessary because each option is already a unique string. Update the map in HealthForm.jsx to use option as the key for the label element instead of index, keeping the existing toggle logic in place.Source: Linters/SAST tools
64-69: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd
min="0"to numeric health inputs.Age, height, and weight inputs accept negative values with no client-side constraint before submission.
Apply similarly to the height and weight `` elements.♻️ Proposed fix
<input type="number" + min="0" value={form.age} onChange={update("age")}Also applies to: 115-120, 138-143
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Profile/components/HealthForm.jsx` around lines 64 - 69, The numeric health inputs in HealthForm currently allow negative values, so add a minimum constraint to the age, height, and weight fields. Update the relevant <input type="number"> elements in HealthForm.jsx to include a min of 0, keeping the existing update handler and styling unchanged. Make sure the same fix is applied consistently across the age, height, and weight inputs so they all reject negative values client-side.src/pages/Profile/components/InfoGrid.jsx (1)
89-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer condition string over array index as key.
Since
profile.healthConditionsentries are unique strings, use the value itself as the key instead of the index for more stable reconciliation.♻️ Proposed fix
{conditions.map((condition, index) => ( <span - key={index} + key={condition} className="px-3 py-1.5 rounded-full text-sm bg-white border border-orange-200 text-orange-700 shadow-sm" > {condition} </span> ))}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Profile/components/InfoGrid.jsx` around lines 89 - 96, The key used in the conditions list rendering is based on the array index, which is unstable for React reconciliation. Update the map in InfoGrid.jsx to use the condition string itself as the key for each span, since profile.healthConditions entries are unique. Keep the rest of the rendering logic unchanged and make the change in the conditions.map callback.Source: Linters/SAST tools
src/pages/Profile/ProfileLayout.jsx (1)
8-26: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winInitial
loadingstate causes a flash of placeholder content; fetch errors are silently swallowed.
loadingstarts asfalse, so before the effect runs, the layout renders once with "Your Name"/placeholder avatar, then flips to the spinner — a visible flicker on every cold load. Also,fetchProfile()rejections/errors are discarded (.catch(() => {})) even though the store already tracks anerrorstate; users get no feedback when the profile fails to load.🔧 Suggested fix
export default function ProfileLayout() { const user = useProfileStore((s) => s.user); - const [loading, setLoading] = useState(false); + const [loading, setLoading] = useState(!user); const fetchProfile = useProfileStore((s) => s.fetchProfile); + const error = useProfileStore((s) => s.error);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Profile/ProfileLayout.jsx` around lines 8 - 26, ProfileLayout is initializing with a false loading state and swallowing fetchProfile failures, which causes a placeholder flash and hides load errors. Update the ProfileLayout component to derive the initial loading state from whether user is already present (or set it before the first render path), and let fetchProfile surface its failure into the existing store error state instead of catching and discarding it. Use the ProfileLayout function, fetchProfile, and the loading/error handling around useEffect to keep the spinner visible until the profile resolves and to show failure feedback when the request fails.src/pages/Profile/Rewards.jsx (1)
135-471: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider memoizing the static confetti SVG.
ConfettiBgtakes no props and is purely decorative but gets re-created on everyRewardsrender (e.g., wheneverpointschanges). Wrapping it inReact.memoavoids needless re-creation of ~24 SVG elements.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Profile/Rewards.jsx` around lines 135 - 471, The static decorative SVG in ConfettiBg is recreated on every Rewards render even though it takes no props. Wrap ConfettiBg in React.memo (or otherwise memoize it) so updates like points changes in Rewards do not re-render the confetti markup unnecessarily, and keep the existing SVG structure unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/App.jsx`:
- Line 15: There is a duplicate Orders binding in App that breaks compilation
because src/App.jsx imports Orders from both ./pages and ./pages/Profile. Update
the import from ./pages/Profile to use a distinct alias such as ProfileOrders,
then use that aliased symbol in the /profile/orders route so the module has
unique bindings throughout App.
In `@src/constants.js`:
- Around line 58-61: Update NON_CANCELLABLE_ORDER_STATUSES in constants.js to
include CANCELED so isOrderCancellable in src/utils/orderHelpers.js correctly
treats already-canceled orders as not cancellable. Keep the normalization flow
unchanged and ensure the status list used by
includes(normalizeStatus(order.status)) covers every terminal non-cancellable
state.
In `@src/pages/Profile/components/OrderTracking.jsx`:
- Around line 16-23: getActiveStepIndex in OrderTracking.jsx only maps
pending/confirmed/preparing/ready, so terminal orders fall back to the pending
stepper. Update the OrderTracking component to detect terminal statuses like
canceled and delivered before rendering the stepper, and branch to a separate
summary view instead of returning 0. Also review the order selection flow in
pickTrackingOrder from orderHelpers.js so a non-active order passed into this
component is rendered with the correct terminal-state UI.
- Around line 85-118: The fallback values in OrderTracking are using hardcoded
mock-like data instead of a safe empty/placeholder state, and the order.time
branch in the estimatedDeliveryTime logic formats hours inconsistently. Update
the friendlyTime and estimatedDeliveryTime useMemo blocks in OrderTracking.jsx
so missing order.time/order.createdAt/order.id return a generic placeholder or
blank state instead of fabricated times/ids, and make the order.time path
zero-pad the computed hour to match the createdAt formatting. Also adjust the
order id display logic in the same component to avoid showing a specific default
number when order.id is absent.
- Around line 335-341: The Cancel Order button in OrderTracking.jsx can trigger
duplicate concurrent cancellation requests because onCancelOrder calls
cancelMyOrder/cancelOrder without any in-flight guard. Add a pending state (for
example, a local isCancelling flag or a loading prop passed down from
Orders.jsx) around the Cancel Order button in OrderTracking so it is disabled
while the request is running, and clear that state when the cancellation
completes or fails. Use the existing isOrderCancellable and onCancelOrder flow
to wire the guard in the same component that renders the button.
In `@src/store/orderStore.js`:
- Around line 361-372: The cancelMyOrder flow in orderStore is using
isOrderCancellable(orderToCancel) for both missing and non-cancellable orders,
so the same message is returned in both cases. Update cancelMyOrder to first
check whether getMergedOrders().find(...) returned an order; if not, return a
distinct “order not found” response, and only call isOrderCancellable for
existing orders. Keep the existing cancellation message for the non-cancellable
case and use the cancelMyOrder identifier to locate the branch.
In `@src/store/profileStore.js`:
- Around line 48-59: The updateHealth action can leave loading stuck true when
updateHealthProfile returns no user data. In profileStore.js, update the
updateHealth function so it always clears loading after the API call, mirroring
updateUser’s behavior by adding a fallback branch when res?.data is falsy. Keep
the existing success path that sets user, loading, and error, and ensure the
error path still resets loading as well.
---
Outside diff comments:
In `@src/index.css`:
- Around line 3-13: Close the unclosed `@layer` base block in src/index.css so
`@theme` is no longer nested inside it; the issue is in the top-level stylesheet
around `@layer` base, button, and `@theme`. Add the missing closing brace after the
button rule, then keep `@theme` at the root level so Tailwind can register
--color-green and --color-orange correctly. Verify the resulting CSS parses
cleanly with Stylelint and that the theme tokens remain available to components
like Navbar.
---
Duplicate comments:
In `@src/pages/Profile/Orders.jsx`:
- Around line 43-50: The cancellation flow in handleCancelOrder is missing an
in-flight guard, so repeated clicks can start multiple concurrent requests. Add
a local isCancelling state in Orders.jsx around handleCancelOrder to set true
before cancelMyOrder and reset it in finally, then pass that state down to
OrderTracking.jsx so the Cancel control can be disabled while a cancellation is
pending.
---
Nitpick comments:
In `@src/pages/Profile/components/HealthForm.jsx`:
- Around line 183-216: Using the array index as the key in the HEALTH_CONDITIONS
render loop is unnecessary because each option is already a unique string.
Update the map in HealthForm.jsx to use option as the key for the label element
instead of index, keeping the existing toggle logic in place.
- Around line 64-69: The numeric health inputs in HealthForm currently allow
negative values, so add a minimum constraint to the age, height, and weight
fields. Update the relevant <input type="number"> elements in HealthForm.jsx to
include a min of 0, keeping the existing update handler and styling unchanged.
Make sure the same fix is applied consistently across the age, height, and
weight inputs so they all reject negative values client-side.
In `@src/pages/Profile/components/InfoGrid.jsx`:
- Around line 89-96: The key used in the conditions list rendering is based on
the array index, which is unstable for React reconciliation. Update the map in
InfoGrid.jsx to use the condition string itself as the key for each span, since
profile.healthConditions entries are unique. Keep the rest of the rendering
logic unchanged and make the change in the conditions.map callback.
In `@src/pages/Profile/components/OrderCard.jsx`:
- Around line 47-58: The time formatting logic in OrderCard’s getFriendlyTime
duplicates the createdAt-to-locale-time behavior already implemented in
OrderTracking’s friendlyTime useMemo. Extract that shared conversion into a
common helper in the existing order utilities (for example, the centralized
order helper module) and update both OrderCard and OrderTracking to call it so
the formatting stays consistent and avoids drift.
- Line 45: Remove the dead/unused date variable in OrderCard, since it is
flagged by ESLint and not used anywhere in the component. Update the relevant
logic in OrderCard.jsx, especially the createdAt handling around the order date
formatting/rendering, and also clean up the other unused variable referenced by
the review comment so the component has no remaining unused declarations.
- Around line 75-80: The total price display in OrderCard is inconsistent with
OrderDetailsModal because it renders order.totalPrice without fixed decimal
formatting. Update the price rendering in OrderCard.jsx to match the same
formatting used elsewhere (via Number(...).toFixed(2)) so values like 12.5
display consistently as 12.50. Use the OrderCard component and the totalPrice
render block as the place to make the change.
In `@src/pages/Profile/components/OrderDetailsModal.jsx`:
- Around line 10-23: Add Escape-key dismissal to OrderDetailsModal so the dialog
can be closed from the keyboard as well as the backdrop and close button. Update
the OrderDetailsModal component to register a keydown listener when order is
present, call onClose when the pressed key is Escape, and clean up the listener
on unmount or when the modal closes. Keep the behavior scoped to the modal
lifecycle so it doesn’t affect the rest of the Profile page.
In `@src/pages/Profile/Orders.jsx`:
- Around line 12-22: The Orders page is subscribing to the entire order store
via useOrderStore((state) => state), which causes unnecessary re-renders on
unrelated store updates. Update the selector in Orders.jsx to pick only the
fields actually used by this page (such as myOrders, lastOrder, myOrdersLoading,
myOrdersError, fetchMyOrders, cancelMyOrder, getMergedOrders, getGroupedOrders,
and getTrackingOrder), and use zustand/shallow if needed to keep the
subscription scoped.
In `@src/pages/Profile/ProfileLayout.jsx`:
- Around line 8-26: ProfileLayout is initializing with a false loading state and
swallowing fetchProfile failures, which causes a placeholder flash and hides
load errors. Update the ProfileLayout component to derive the initial loading
state from whether user is already present (or set it before the first render
path), and let fetchProfile surface its failure into the existing store error
state instead of catching and discarding it. Use the ProfileLayout function,
fetchProfile, and the loading/error handling around useEffect to keep the
spinner visible until the profile resolves and to show failure feedback when the
request fails.
In `@src/pages/Profile/Rewards.jsx`:
- Around line 135-471: The static decorative SVG in ConfettiBg is recreated on
every Rewards render even though it takes no props. Wrap ConfettiBg in
React.memo (or otherwise memoize it) so updates like points changes in Rewards
do not re-render the confetti markup unnecessarily, and keep the existing SVG
structure unchanged.
In `@src/store/profileStore.js`:
- Around line 28-31: The catch blocks in profileStore are storing the raw Error
object in state, which can break consumers that render the value directly.
Update the error handling in the affected store methods to save a string value
like error.message (or a safe fallback) instead of the full object, while
keeping loading false and the null return behavior unchanged. Use the existing
catch(error) paths in profileStore to make the change consistently across the
noted sections.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4c19f65b-1b4e-4a29-b518-1f20869f847f
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/images/avatar-placeholder.jpegis excluded by!**/*.jpeg
📒 Files selected for processing (30)
index.htmlpackage.jsonsrc/App.jsxsrc/Layout/AppLayout.jsxsrc/components/layout/Navbar.jsxsrc/components/ui/AllergiesDropdown.jsxsrc/constants.jssrc/index.csssrc/mocks/enums.jssrc/mocks/orders.jssrc/pages/Profile/Orders.jsxsrc/pages/Profile/Profile.jsxsrc/pages/Profile/ProfileLayout.jsxsrc/pages/Profile/Rewards.jsxsrc/pages/Profile/components/HealthForm.jsxsrc/pages/Profile/components/InfoGrid.jsxsrc/pages/Profile/components/OrderCard.jsxsrc/pages/Profile/components/OrderDetailsModal.jsxsrc/pages/Profile/components/OrderTracking.jsxsrc/pages/Profile/components/ProfileHeader.jsxsrc/pages/Profile/components/Sidebar.jsxsrc/pages/Profile/index.jssrc/pages/index.jssrc/store/authStore.jssrc/store/healthStore.jssrc/store/index.jssrc/store/orderStore.jssrc/store/profileStore.jssrc/tests/stores/stores.test.jssrc/utils/orderHelpers.js
💤 Files with no reviewable changes (1)
- src/store/healthStore.js
| Thanks, | ||
| Favorites, | ||
| StoreDebug, | ||
| Profile, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Confirm the full destructured import from "./pages" in App.jsx includes `Orders`,
# and confirm both `Orders` bindings collide in the same module scope.
rg -n -B2 -A2 '^\s*Orders\b|from "\./pages"|from "\./pages/Profile"' src/App.jsxRepository: Revive-Graduation-Project/Revive-Front-End
Length of output: 485
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' src/App.jsx | nl -ba | sed -n '1,120p'Repository: Revive-Graduation-Project/Revive-Front-End
Length of output: 220
🏁 Script executed:
#!/bin/bash
set -euo pipefail
awk '{printf "%4d %s\n", NR, $0}' src/App.jsx | sed -n '1,140p'Repository: Revive-Graduation-Project/Revive-Front-End
Length of output: 4321
Alias the profile Orders import src/App.jsx:24,68,98 imports Orders twice in the same module (./pages and ./pages/Profile), which creates a duplicate binding and breaks compilation. Rename one import, e.g. Orders as ProfileOrders, and use that in the /profile/orders route.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/App.jsx` at line 15, There is a duplicate Orders binding in App that
breaks compilation because src/App.jsx imports Orders from both ./pages and
./pages/Profile. Update the import from ./pages/Profile to use a distinct alias
such as ProfileOrders, then use that aliased symbol in the /profile/orders route
so the module has unique bindings throughout App.
| export const NON_CANCELLABLE_ORDER_STATUSES = [ | ||
| "PREPARING", | ||
| "READY", | ||
| ]; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
NON_CANCELLABLE_ORDER_STATUSES omits CANCELED, allowing already-canceled orders to appear cancellable.
This array is the source powering isOrderCancellable in src/utils/orderHelpers.js (Line 50: return !NON_CANCELLABLE_ORDER_STATUSES.includes(normalizeStatus(order.status));). Since CANCELED isn't in this list, an order already in CANCELED status would incorrectly evaluate as cancellable, exposing a "Cancel" action for orders that can no longer be canceled.
🐛 Proposed fix
export const NON_CANCELLABLE_ORDER_STATUSES = [
"PREPARING",
"READY",
+ "CANCELED",
];📝 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.
| export const NON_CANCELLABLE_ORDER_STATUSES = [ | |
| "PREPARING", | |
| "READY", | |
| ]; | |
| export const NON_CANCELLABLE_ORDER_STATUSES = [ | |
| "PREPARING", | |
| "READY", | |
| "CANCELED", | |
| ]; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/constants.js` around lines 58 - 61, Update NON_CANCELLABLE_ORDER_STATUSES
in constants.js to include CANCELED so isOrderCancellable in
src/utils/orderHelpers.js correctly treats already-canceled orders as not
cancellable. Keep the normalization flow unchanged and ensure the status list
used by includes(normalizeStatus(order.status)) covers every terminal
non-cancellable state.
| const getActiveStepIndex = () => { | ||
| const s = order?.status?.toLowerCase() || ""; | ||
| if (s === "ready") return 3; | ||
| if (s === "preparing") return 2; | ||
| if (s === "confirmed") return 1; | ||
| if (s === "pending") return 0; | ||
| return 0; | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Terminal statuses (canceled/delivered) aren't handled by the tracking stepper.
getActiveStepIndex only recognizes pending/confirmed/preparing/ready; any other status (e.g. a canceled order) falls through to return 0, so the UI would render it as "Pending" with the full active-tracking stepper and cancellation policy. Since pickTrackingOrder (src/utils/orderHelpers.js) can fall back to orders[0] irrespective of status when no order is actively "in progress", a canceled or delivered order can reach this component and be shown misleadingly as newly placed.
const activeOrder = orders.find((order) => ACTIVE_TRACKING_ORDER_STATUSES.includes(normalizeStatus(order.status)) ); return activeOrder || lastOrder || orders[0] || null;
Consider special-casing terminal statuses (e.g. render a "Canceled"/"Delivered" summary card instead of the progress stepper) rather than defaulting to Pending.
Also applies to: 120-146
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Profile/components/OrderTracking.jsx` around lines 16 - 23,
getActiveStepIndex in OrderTracking.jsx only maps
pending/confirmed/preparing/ready, so terminal orders fall back to the pending
stepper. Update the OrderTracking component to detect terminal statuses like
canceled and delivered before rendering the stepper, and branch to a separate
summary view instead of returning 0. Also review the order selection flow in
pickTrackingOrder from orderHelpers.js so a non-active order passed into this
component is rendered with the correct terminal-state UI.
| const friendlyTime = useMemo(() => { | ||
| if (order?.time) return order.time; | ||
| if (!order?.createdAt) return "11:45"; | ||
| try { | ||
| const date = new Date(order.createdAt); | ||
| return date.toLocaleTimeString([], { | ||
| hour: "2-digit", | ||
| minute: "2-digit", | ||
| hour12: true, | ||
| }); | ||
| } catch { | ||
| return "11:45"; | ||
| } | ||
| }, [order?.time, order?.createdAt]); | ||
|
|
||
| const estimatedDeliveryTime = useMemo(() => { | ||
| if (order?.time) { | ||
| const [hours, minutes] = order.time.split(":").map(Number); | ||
| const newHours = (hours + 1) % 24; | ||
| return `${newHours}:${minutes.toString().padStart(2, "0")}`; | ||
| } | ||
| if (!order?.createdAt) return "12:45"; | ||
| try { | ||
| const date = new Date(order.createdAt); | ||
| date.setHours(date.getHours() + 1); | ||
| return date.toLocaleTimeString([], { | ||
| hour: "2-digit", | ||
| minute: "2-digit", | ||
| hour12: true, | ||
| }); | ||
| } catch { | ||
| return "12:45"; | ||
| } | ||
| }, [order?.time, order?.createdAt]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Hardcoded placeholder fallbacks look like leftover mock data.
The fallback values "11:45", "12:45", and "1054" (order id) are specific, plausible-looking values rather than a generic empty/placeholder state. If order.time/order.createdAt/order.id are ever missing, users would see fabricated-looking data. Also, on line 104 the order.time branch doesn't zero-pad newHours, producing inconsistent formats (e.g. 9:05 vs 09:05 from the createdAt branch).
Also applies to: 130-130
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Profile/components/OrderTracking.jsx` around lines 85 - 118, The
fallback values in OrderTracking are using hardcoded mock-like data instead of a
safe empty/placeholder state, and the order.time branch in the
estimatedDeliveryTime logic formats hours inconsistently. Update the
friendlyTime and estimatedDeliveryTime useMemo blocks in OrderTracking.jsx so
missing order.time/order.createdAt/order.id return a generic placeholder or
blank state instead of fabricated times/ids, and make the order.time path
zero-pad the computed hour to match the createdAt formatting. Also adjust the
order id display logic in the same component to avoid showing a specific default
number when order.id is absent.
| {isOrderCancellable(order) ? ( | ||
| <button | ||
| onClick={() => onCancelOrder?.(order.id)} | ||
| className="w-full bg-red-600 hover:bg-red-700 text-white font-semibold py-2.5 px-4 rounded-xl transition-colors cursor-pointer text-sm shadow-md hover:shadow-lg" | ||
| > | ||
| Cancel Order | ||
| </button> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Cancel button has no guard against duplicate/concurrent clicks.
onCancelOrder triggers cancelMyOrder, which calls the external cancelOrder API — a non-idempotent write. Nothing here disables the button or tracks an in-flight state, so rapid clicks can fire multiple concurrent cancellation requests for the same order. Consider a local isCancelling state (or a loading prop from Orders.jsx) to disable the button while the request is pending.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Profile/components/OrderTracking.jsx` around lines 335 - 341, The
Cancel Order button in OrderTracking.jsx can trigger duplicate concurrent
cancellation requests because onCancelOrder calls cancelMyOrder/cancelOrder
without any in-flight guard. Add a pending state (for example, a local
isCancelling flag or a loading prop passed down from Orders.jsx) around the
Cancel Order button in OrderTracking so it is disabled while the request is
running, and clear that state when the cancellation completes or fails. Use the
existing isOrderCancellable and onCancelOrder flow to wire the guard in the same
component that renders the button.
| cancelMyOrder: async (orderId) => { | ||
| const orderToCancel = get() | ||
| .getMergedOrders() | ||
| .find((order) => String(order.id) === String(orderId)); | ||
|
|
||
| if (!isOrderCancellable(orderToCancel)) { | ||
| return { | ||
| ok: false, | ||
| message: | ||
| "This order can no longer be cancelled because preparation has already started.", | ||
| }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Misleading message when order isn't found.
isOrderCancellable(orderToCancel) returns false both when the order is genuinely non-cancellable and when orderToCancel is undefined (order not found in merged list, e.g. stale reference or race). Either way the same "preparation has already started" message is shown, which is inaccurate for the not-found case.
🐛 Proposed fix to disambiguate not-found vs non-cancellable
const orderToCancel = get()
.getMergedOrders()
.find((order) => String(order.id) === String(orderId));
+ if (!orderToCancel) {
+ return { ok: false, message: "Order not found." };
+ }
+
if (!isOrderCancellable(orderToCancel)) {📝 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.
| cancelMyOrder: async (orderId) => { | |
| const orderToCancel = get() | |
| .getMergedOrders() | |
| .find((order) => String(order.id) === String(orderId)); | |
| if (!isOrderCancellable(orderToCancel)) { | |
| return { | |
| ok: false, | |
| message: | |
| "This order can no longer be cancelled because preparation has already started.", | |
| }; | |
| } | |
| cancelMyOrder: async (orderId) => { | |
| const orderToCancel = get() | |
| .getMergedOrders() | |
| .find((order) => String(order.id) === String(orderId)); | |
| if (!orderToCancel) { | |
| return { ok: false, message: "Order not found." }; | |
| } | |
| if (!isOrderCancellable(orderToCancel)) { | |
| return { | |
| ok: false, | |
| message: | |
| "This order can no longer be cancelled because preparation has already started.", | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/store/orderStore.js` around lines 361 - 372, The cancelMyOrder flow in
orderStore is using isOrderCancellable(orderToCancel) for both missing and
non-cancellable orders, so the same message is returned in both cases. Update
cancelMyOrder to first check whether getMergedOrders().find(...) returned an
order; if not, return a distinct “order not found” response, and only call
isOrderCancellable for existing orders. Keep the existing cancellation message
for the non-cancellable case and use the cancelMyOrder identifier to locate the
branch.
| updateHealth: async (data) => { | ||
| set({ loading: true, error: null }); | ||
| try { | ||
| const res = await updateHealthProfile(data); | ||
| const user = res?.data || null; | ||
| if (user) set({ user, loading: false, error: null }); | ||
| return user; | ||
| } catch (error) { | ||
| set({ error, loading: false }); | ||
| return null; | ||
| } | ||
| }, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
updateHealth can leave loading stuck at true.
Unlike updateUser (Lines 34-46), which has an else set({ loading: false }) branch when user is falsy, updateHealth only resets loading inside the if (user) block. If res?.data is null/undefined, the function returns without ever clearing loading, leaving the UI stuck showing a loading state.
🐛 Proposed fix
updateHealth: async (data) => {
set({ loading: true, error: null });
try {
const res = await updateHealthProfile(data);
const user = res?.data || null;
- if (user) set({ user, loading: false, error: null });
+ if (user) set({ user, loading: false, error: null });
+ else set({ loading: false });
return user;
} catch (error) {
set({ error, loading: false });
return 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.
| updateHealth: async (data) => { | |
| set({ loading: true, error: null }); | |
| try { | |
| const res = await updateHealthProfile(data); | |
| const user = res?.data || null; | |
| if (user) set({ user, loading: false, error: null }); | |
| return user; | |
| } catch (error) { | |
| set({ error, loading: false }); | |
| return null; | |
| } | |
| }, | |
| updateHealth: async (data) => { | |
| set({ loading: true, error: null }); | |
| try { | |
| const res = await updateHealthProfile(data); | |
| const user = res?.data || null; | |
| if (user) set({ user, loading: false, error: null }); | |
| else set({ loading: false }); | |
| return user; | |
| } catch (error) { | |
| set({ error, loading: false }); | |
| return null; | |
| } | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/store/profileStore.js` around lines 48 - 59, The updateHealth action can
leave loading stuck true when updateHealthProfile returns no user data. In
profileStore.js, update the updateHealth function so it always clears loading
after the API call, mirroring updateUser’s behavior by adding a fallback branch
when res?.data is falsy. Keep the existing success path that sets user, loading,
and error, and ensure the error path still resets loading as well.
1 participant
يا رب الستر
Summary by CodeRabbit
New Features
Bug Fixes