Feature/finish live kitchen - #83
Conversation
✅ Deploy Preview for revive-front-end ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR adds role-aware dashboard routing, period-based analytics, unified kitchen ticket actions, staff and chef management, profile picture and account deletion flows, normalized authentication and health data, expanded profile validation, and broader order/status mappings. ChangesDashboard and kitchen operations
Profile and account management
Authentication and order contracts
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Login
participant AuthStore
participant ProfileService
participant DashboardRoute
User->>Login: submit credentials
Login->>AuthStore: authenticate
AuthStore-->>Login: authenticated user and role
Login->>ProfileService: validate client profile for regular users
Login->>DashboardRoute: navigate by role or show profile error
sequenceDiagram
participant ProfileUser
participant ProfilePage
participant ProfileStore
participant ClientProfileAPI
participant AuthUserAPI
ProfileUser->>ProfilePage: confirm account deletion
ProfilePage->>ProfileStore: deleteAccount
ProfileStore->>ClientProfileAPI: delete client profile
ProfileStore->>AuthUserAPI: delete auth user
ProfileStore-->>ProfilePage: clear state and log out
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/store/authStore.js (1)
163-174: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEmail validation in
restoreSessionis less strict than inlogin.In
login(lines 82–84), the guard checkstypeof rawUser.email === "string" && rawUser.email.trim().length > 0, butrestoreSession(line 174) only checksrawUser.emailfor truthiness. A whitespace-only string like" "would fail inloginbut pass inrestoreSession, allowing an invalid email into the persisted user.🛡️ Proposed fix: align restoreSession validation with login
set({ token: data.token, - user: rawUser.id != null && rawUser.email ? rawUser : get().user, + user: + rawUser.id != null && + typeof rawUser.email === "string" && + rawUser.email.trim().length > 0 + ? rawUser + : get().user, isAuthenticated: true, expiresAt: data.expiresAt, loading: false, });🤖 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/authStore.js` around lines 163 - 174, Update the user assignment guard in restoreSession to validate rawUser.email exactly as login does: require it to be a string with nonzero trimmed length, while retaining the existing rawUser.id check and fallback to get().user.
🧹 Nitpick comments (6)
src/store/authStore.js (1)
77-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
fullNameandnameare always identical — consider consolidating.Both fields are derived from the same expression
`${data.firstName || ""} ${data.lastName || ""}`.trim()in bothloginandrestoreSession. If downstream consumers only need one, keeping both adds maintenance overhead and confusion about which to use.Also applies to: 168-169
🤖 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/authStore.js` around lines 77 - 78, Consolidate the duplicate `fullName` and `name` fields in both `login` and `restoreSession` by retaining a single canonical field derived from the shared first-name/last-name expression. Update downstream references in `authStore` to use that field, and remove the redundant property consistently in both locations.src/pages/auth/Login.jsx (1)
37-43: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider adding a timeout or error boundary for the
getProfilecall.The
getProfile(user.id)call on line 39 has no timeout. If the backend is slow or unresponsive, the user is stuck on the login page with no feedback after authentication has already succeeded. Consider adding a timeout or surfacing a loading state during the profile check.🤖 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/auth/Login.jsx` around lines 37 - 43, Update the getProfile call within the Login component’s authentication flow to enforce a bounded wait or otherwise surface an explicit loading state while the profile check is pending. Ensure slow or unresponsive requests cannot leave the user indefinitely stuck after authentication, while preserving the existing missing-profile error handling.src/components/Dashboard/DashboardView.jsx (1)
27-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
useRevenueDatadefault period doesn't match the app's period vocabulary.
revenuePeriodhere is initialized to"This Month"and only ever set to one of"This Day"/"This Week"/"This Month"/"This Year"(viaTimeFilter), butuseRevenueData's own default parameter is"6m". Currently harmless since this call always passes an explicit period, but it's a latent trap for any future caller relying on the hook's default.♻️ Suggested alignment (in useDashboard.js)
-export function useRevenueData(period = "6m") { +export function useRevenueData(period = "This Month") {🤖 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/components/Dashboard/DashboardView.jsx` around lines 27 - 31, Update the default period parameter in useRevenueData to use the app’s established period vocabulary, specifically the same "This Month" value used by revenuePeriod and TimeFilter, instead of "6m". Preserve the hook’s existing behavior when callers provide an explicit period.src/components/auth/StepTwo.jsx (1)
104-117: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
GOAL_OPTIONSfromsrc/constants.jshere. This keeps the goal list in one place and matches the existing pattern used by other form 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/components/auth/StepTwo.jsx` around lines 104 - 117, Replace the inline goal options array in the StepTwo goal radio-group mapping with the shared GOAL_OPTIONS constant imported from src/constants.js. Keep the existing mapping, checked state, and onChange behavior unchanged.src/pages/Profile/components/HealthForm.jsx (1)
234-262: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNew "NONE" health-condition option has no mutual-exclusivity handling.
constants.jsnow includes{ value: "NONE", label: "None" }alongside real conditions, but the toggle logic here just adds/removes the clicked value independently — a user can select "NONE" together with "DIABETES", etc., producing a contradictory health profile.💡 Suggested fix
onClick={(e) => { e.preventDefault(); const current = form.healthConditions || []; const exists = current.includes(val); - const next = exists - ? current.filter((c) => c !== val) - : [...current, val]; + let next; + if (val === "NONE") { + next = exists ? [] : ["NONE"]; + } else { + next = exists + ? current.filter((c) => c !== val) + : [...current.filter((c) => c !== "NONE"), val]; + } setForm((s) => ({ ...s, healthConditions: next })); }}🤖 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 234 - 262, Update the health-condition toggle handler in the form component so selecting the "NONE" value clears all other health conditions, while selecting any real condition removes "NONE" before adding it. Preserve the existing toggle behavior for deselecting values and keep the state update within the current setForm flow.src/pages/Profile/Profile.test.jsx (1)
18-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNew photo upload/removal and account-deletion flows have no test coverage.
The mocked store still only exposes
user,fetchProfile,updateHealth;uploadProfilePicture,deleteProfilePicture, anddeleteAccountaren't mocked or exercised at all, so this test suite doesn't cover any of the new functionality added toProfile.jsx(upload, remove picture, delete account, error/toast paths).🤖 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/Profile.test.jsx` around lines 18 - 29, Extend the Profile test suite’s mocked store to include uploadProfilePicture, deleteProfilePicture, and deleteAccount, then add tests exercising photo upload, photo removal, and account deletion through the Profile UI. Cover the associated success and error/toast paths, using the existing MemoryRouter and test utilities.
🤖 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/components/Dashboard/DashboardSidebar.jsx`:
- Line 13: Remove the unused isAdminUser named import from the roleUtils import
in DashboardSidebar.jsx, while retaining isKitchenOnlyUser and isSuperAdmin.
In `@src/components/Dashboard/shared/OrderDetailsModal.jsx`:
- Around line 7-19: The orderItems mapping in the dishes-list parsing logic
currently prefixes quantities, causing downstream parsing to treat them as part
of the dish name. Update the order.orderItems branch to emit each item with the
quantity after the name, such as “Chicken Delight x2” or “Chicken Delight (x2)”,
while preserving the existing default quantity and item-name fallbacks.
In `@src/pages/Profile/components/HealthForm.jsx`:
- Around line 28-33: Align the phone field identifier used by the input’s
onChange with the existing phone error key: update the call to update in the
phone input to use “phone” instead of “phoneNumber,” while preserving the
current form-state update behavior and rendered error handling.
- Around line 52-66: Update the height and weight validation in the form submit
logic to account for the selected units, converting values to a common unit or
applying unit-specific bounds before checking ranges. Preserve the existing
error behavior while ensuring valid m, ft, in, and lb values are not rejected by
the cm/kg limits.
In `@src/pages/Profile/Profile.jsx`:
- Around line 33-44: The rawJoinDate fallback should use an explicit empty-state
value such as "Unknown" instead of new Date(), and formattedJoinDate must
preserve that value without attempting to format it as a date. Update the
toLocaleDateString options in the join-date formatting flow to include timeZone:
"UTC", keeping the existing date-field precedence and display format.
- Around line 86-98: Update deleteAccount and the handleDeleteAccount flow so
failures from either deleteClientProfile(id) or deleteAuthUser(id) are
propagated instead of swallowed, and the operation does not return true after a
partial failure. Preserve the existing successful cleanup, success toast, modal
closing, and navigation only when both backend deletions complete successfully.
In `@src/pages/Profile/ProfileLayout.jsx`:
- Around line 44-64: Update the ProfileLayout component’s !user handling to
distinguish fetch errors from a genuinely missing profile: preserve a dedicated
error state UI with a retry action that re-invokes fetchProfile, and show
“Profile Not Found” only when no error occurred. Keep the existing logout
behavior for the missing-profile state.
- Around line 73-80: Update the avatar fallback chain in ProfileLayout so
freshly fetched user picture fields from user take precedence over authUser
fields. Preserve the existing field order within each object and the placeholder
fallback, ensuring uploads or removals reflected by clientProfileService are
displayed even when authUser is stale.
In `@src/services/clientProfileService.js`:
- Around line 35-43: Remove the manually specified Content-Type header from
uploadProfilePicture’s api.patch request. Leave the FormData request headers
unset so Axios/the browser automatically adds the multipart boundary.
In `@src/services/dashboardService.js`:
- Around line 411-423: Clean up the best-effort synchronization block in
updateKitchenStatus by removing the redundant outer try/catch, since
getActiveKitchenTickets and updateTicketStatus already swallow their expected
failures. Preserve the existing ticket lookup and conditional status update
behavior without introducing an unused catch parameter or empty catch block.
In `@src/store/profileStore.js`:
- Around line 147-174: Update deleteAccount so failures from deleteClientProfile
and deleteAuthUser are not swallowed: remove their inline catches or rethrow
after logging, allowing the outer catch to handle them. Make the outer catch set
loading/error appropriately, avoid clearing local user state or logging out as
if deletion succeeded, and return or propagate failure instead of always
returning true; retain the success cleanup and true result only after both
deletions complete.
---
Outside diff comments:
In `@src/store/authStore.js`:
- Around line 163-174: Update the user assignment guard in restoreSession to
validate rawUser.email exactly as login does: require it to be a string with
nonzero trimmed length, while retaining the existing rawUser.id check and
fallback to get().user.
---
Nitpick comments:
In `@src/components/auth/StepTwo.jsx`:
- Around line 104-117: Replace the inline goal options array in the StepTwo goal
radio-group mapping with the shared GOAL_OPTIONS constant imported from
src/constants.js. Keep the existing mapping, checked state, and onChange
behavior unchanged.
In `@src/components/Dashboard/DashboardView.jsx`:
- Around line 27-31: Update the default period parameter in useRevenueData to
use the app’s established period vocabulary, specifically the same "This Month"
value used by revenuePeriod and TimeFilter, instead of "6m". Preserve the hook’s
existing behavior when callers provide an explicit period.
In `@src/pages/auth/Login.jsx`:
- Around line 37-43: Update the getProfile call within the Login component’s
authentication flow to enforce a bounded wait or otherwise surface an explicit
loading state while the profile check is pending. Ensure slow or unresponsive
requests cannot leave the user indefinitely stuck after authentication, while
preserving the existing missing-profile error handling.
In `@src/pages/Profile/components/HealthForm.jsx`:
- Around line 234-262: Update the health-condition toggle handler in the form
component so selecting the "NONE" value clears all other health conditions,
while selecting any real condition removes "NONE" before adding it. Preserve the
existing toggle behavior for deselecting values and keep the state update within
the current setForm flow.
In `@src/pages/Profile/Profile.test.jsx`:
- Around line 18-29: Extend the Profile test suite’s mocked store to include
uploadProfilePicture, deleteProfilePicture, and deleteAccount, then add tests
exercising photo upload, photo removal, and account deletion through the Profile
UI. Cover the associated success and error/toast paths, using the existing
MemoryRouter and test utilities.
In `@src/store/authStore.js`:
- Around line 77-78: Consolidate the duplicate `fullName` and `name` fields in
both `login` and `restoreSession` by retaining a single canonical field derived
from the shared first-name/last-name expression. Update downstream references in
`authStore` to use that field, and remove the redundant property consistently in
both locations.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fd8f25c7-f2a1-4cdb-9345-e53f6c6b2c83
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.json
📒 Files selected for processing (34)
src/Layout/AppLayout.jsxsrc/components/Dashboard/DashboardSidebar.jsxsrc/components/Dashboard/DashboardView.jsxsrc/components/Dashboard/LiveKitchen/KitchenTicketsTable.jsxsrc/components/Dashboard/LiveKitchenView.jsxsrc/components/Dashboard/MetricCards.jsxsrc/components/Dashboard/OrdersView.jsxsrc/components/Dashboard/RevenueChart.jsxsrc/components/Dashboard/StaffManagementView.jsxsrc/components/Dashboard/shared/OrderDetailsModal.jsxsrc/components/Dashboard/shared/OrdersOverviewChart.jsxsrc/components/auth/StepTwo.jsxsrc/components/ui/AllergiesDropdown.jsxsrc/constants.jssrc/hooks/dashboard/useDashboard.jssrc/hooks/dashboard/useKitchenOrders.jssrc/pages/Profile/Profile.jsxsrc/pages/Profile/Profile.test.jsxsrc/pages/Profile/ProfileLayout.jsxsrc/pages/Profile/ProfileOrders.jsxsrc/pages/Profile/components/HealthForm.jsxsrc/pages/Profile/components/InfoGrid.jsxsrc/pages/Profile/components/Sidebar.jsxsrc/pages/auth/Login.jsxsrc/pages/auth/Signup.jsxsrc/services/auth.service.jssrc/services/clientProfileService.jssrc/services/dashboardService.jssrc/services/mappers/dashboardMappers.jssrc/services/order.service.jssrc/store/__tests__/profileStore.test.jssrc/store/authStore.jssrc/store/orderStore.jssrc/store/profileStore.js
| import { FiShoppingBag, FiLogOut, FiUsers } from "react-icons/fi"; | ||
| import useAuthStore from "../../store/authStore"; | ||
| import { isKitchenOnlyUser, isAdminUser } from "../../utils/roleUtils"; | ||
| import { isKitchenOnlyUser, isAdminUser, isSuperAdmin } from "../../utils/roleUtils"; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the unused isAdminUser import.
isAdminUser is no longer referenced in this file (replaced by isSuperAdmin), and ESLint flags it as an error, which can fail the lint gate.
🧹 Proposed fix
-import { isKitchenOnlyUser, isAdminUser, isSuperAdmin } from "../../utils/roleUtils";
+import { isKitchenOnlyUser, isSuperAdmin } from "../../utils/roleUtils";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { isKitchenOnlyUser, isAdminUser, isSuperAdmin } from "../../utils/roleUtils"; | |
| import { isKitchenOnlyUser, isSuperAdmin } from "../../utils/roleUtils"; |
🧰 Tools
🪛 ESLint
[error] 13-13: 'isAdminUser' is defined but never used. Allowed unused vars must match /^[A-Z_]/u.
(no-unused-vars)
🤖 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/components/Dashboard/DashboardSidebar.jsx` at line 13, Remove the unused
isAdminUser named import from the roleUtils import in DashboardSidebar.jsx,
while retaining isKitchenOnlyUser and isSuperAdmin.
Source: Linters/SAST tools
| // Handle parsing items: it might be an array of objects (orderItems), array of strings (Live Kitchen), or a comma-separated string. | ||
| let dishesList = []; | ||
| if (Array.isArray(order.items)) { | ||
| if (Array.isArray(order.orderItems) && order.orderItems.length > 0) { | ||
| dishesList = order.orderItems.map(item => `${item.quantity || 1}x ${item.name || "Item"}`); | ||
| } else if (Array.isArray(order.items)) { | ||
| dishesList = order.items; | ||
| } else if (typeof order.name === 'string') { | ||
| } else if (typeof order.name === 'string' && order.name.includes(',')) { | ||
| dishesList = order.name.split(',').map(d => d.trim()); | ||
| } else if (typeof order.items === 'string') { | ||
| dishesList = order.items.split(',').map(d => d.trim()); | ||
| } else { | ||
| dishesList = ["Custom Order"]; | ||
| dishesList = [order.name || "Custom Order"]; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
[skip_cloning]
python3 - <<'EOF'
import re
pattern = re.compile(r'(.+?)(?:\s*\(?x(\d+)\)?)?$', re.IGNORECASE)
for s in ["2x Chicken Delight", "Chicken Delight x2", "Chicken Delight (x2)"]:
m = pattern.match(s)
print(repr(s), "->", "name=", m.group(1), "qty=", m.group(2))
EOFRepository: Revive-Graduation-Project/Revive-Front-End
Length of output: 395
Use trailing quantity format for order.orderItems
orderItems strings need to be emitted as Chicken Delight x2 / Chicken Delight (x2). The current 2x Chicken Delight format is parsed as part of the dish name, so the UI falls back to 1x and duplicates the quantity in the label.
Proposed fix
- dishesList = order.orderItems.map(item => `${item.quantity || 1}x ${item.name || "Item"}`);
+ dishesList = order.orderItems.map(item => `${item.name || "Item"} x${item.quantity || 1}`);📝 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.
| // Handle parsing items: it might be an array of objects (orderItems), array of strings (Live Kitchen), or a comma-separated string. | |
| let dishesList = []; | |
| if (Array.isArray(order.items)) { | |
| if (Array.isArray(order.orderItems) && order.orderItems.length > 0) { | |
| dishesList = order.orderItems.map(item => `${item.quantity || 1}x ${item.name || "Item"}`); | |
| } else if (Array.isArray(order.items)) { | |
| dishesList = order.items; | |
| } else if (typeof order.name === 'string') { | |
| } else if (typeof order.name === 'string' && order.name.includes(',')) { | |
| dishesList = order.name.split(',').map(d => d.trim()); | |
| } else if (typeof order.items === 'string') { | |
| dishesList = order.items.split(',').map(d => d.trim()); | |
| } else { | |
| dishesList = ["Custom Order"]; | |
| dishesList = [order.name || "Custom Order"]; | |
| } | |
| // Handle parsing items: it might be an array of objects (orderItems), array of strings (Live Kitchen), or a comma-separated string. | |
| let dishesList = []; | |
| if (Array.isArray(order.orderItems) && order.orderItems.length > 0) { | |
| dishesList = order.orderItems.map(item => `${item.name || "Item"} x${item.quantity || 1}`); | |
| } else if (Array.isArray(order.items)) { | |
| dishesList = order.items; | |
| } else if (typeof order.name === 'string' && order.name.includes(',')) { | |
| dishesList = order.name.split(',').map(d => d.trim()); | |
| } else if (typeof order.items === 'string') { | |
| dishesList = order.items.split(',').map(d => d.trim()); | |
| } else { | |
| dishesList = [order.name || "Custom 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/components/Dashboard/shared/OrderDetailsModal.jsx` around lines 7 - 19,
The orderItems mapping in the dishes-list parsing logic currently prefixes
quantities, causing downstream parsing to treat them as part of the dish name.
Update the order.orderItems branch to emit each item with the quantity after the
name, such as “Chicken Delight x2” or “Chicken Delight (x2)”, while preserving
the existing default quantity and item-name fallbacks.
| const update = (field) => (e) => { | ||
| const value = | ||
| e.target.type === "checkbox" ? e.target.checked : e.target.value; | ||
| setForm((s) => ({ ...s, [field]: value })); | ||
| setErrors((prev) => ({ ...prev, [field]: "" })); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clearing the phone error on change doesn't work — field-name/error-key mismatch.
errors uses the key phone (Line 26), but the phone input's onChange calls update("phoneNumber") (Line 136), and inside update, setErrors((prev) => ({ ...prev, [field]: "" })) sets errors.phoneNumber = "" — a different key than errors.phone, which is what's actually rendered (Line 143). So after a phone validation error appears, editing the phone field does not clear the displayed error; it persists until the next submit().
🐛 Suggested fix
+ const errorKeyMap = { phoneNumber: "phone" };
+
const update = (field) => (e) => {
const value =
e.target.type === "checkbox" ? e.target.checked : e.target.value;
setForm((s) => ({ ...s, [field]: value }));
- setErrors((prev) => ({ ...prev, [field]: "" }));
+ setErrors((prev) => ({ ...prev, [errorKeyMap[field] || field]: "" }));
};Also applies to: 136-142
🤖 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 28 - 33, Align the
phone field identifier used by the input’s onChange with the existing phone
error key: update the call to update in the phone input to use “phone” instead
of “phoneNumber,” while preserving the current form-state update behavior and
rendered error handling.
| if (form.height !== "" && form.height !== null) { | ||
| const heightNum = Number(form.height); | ||
| if (isNaN(heightNum) || heightNum < 30 || heightNum > 300) { | ||
| newErrors.height = "Height must be between 30 and 300"; | ||
| hasError = true; | ||
| } | ||
| } | ||
|
|
||
| if (form.weight !== "" && form.weight !== null) { | ||
| const weightNum = Number(form.weight); | ||
| if (isNaN(weightNum) || weightNum < 10 || weightNum > 400) { | ||
| newErrors.weight = "Weight must be between 10 and 400"; | ||
| hasError = true; | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -B1 -A3 'HEIGHT_UNITS|WEIGHT_UNITS' src/constants.jsRepository: Revive-Graduation-Project/Revive-Front-End
Length of output: 380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the validation logic and unit handling in the form.
ast-grep outline src/pages/Profile/components/HealthForm.jsx --view expanded
printf '\n--- file excerpt ---\n'
cat -n src/pages/Profile/components/HealthForm.jsx | sed -n '1,220p'
printf '\n--- constants ---\n'
cat -n src/constants.js | sed -n '1,120p'Repository: Revive-Graduation-Project/Revive-Front-End
Length of output: 13290
Validate height/weight against the selected unit at src/pages/Profile/components/HealthForm.jsx:52-66. The fixed 30–300 / 10–400 bounds only fit cm/kg, so m/ft/in/lb entries will be rejected incorrectly; convert to a common unit or apply unit-specific ranges before submit.
🤖 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 52 - 66, Update the
height and weight validation in the form submit logic to account for the
selected units, converting values to a common unit or applying unit-specific
bounds before checking ranges. Preserve the existing error behavior while
ensuring valid m, ft, in, and lb values are not rejected by the cm/kg limits.
| const rawJoinDate = | ||
| user?.createdAt || | ||
| user?.createdDate || | ||
| user?.joinDate || | ||
| user?.registrationDate || | ||
| new Date(); | ||
|
|
||
| const formattedJoinDate = new Date(rawJoinDate).toLocaleDateString("en-GB", { | ||
| day: "2-digit", | ||
| month: "short", | ||
| year: "numeric", | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant implementation and test files with line numbers.
git ls-files 'src/pages/Profile/Profile.jsx' 'src/pages/Profile/Profile.test.jsx' | xargs -r -I{} sh -c 'echo "==== {} ===="; wc -l "{}"; cat -n "{}" | sed -n "1,260p"'Repository: Revive-Graduation-Project/Revive-Front-End
Length of output: 12983
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where the profile user date fields are defined/used and inspect nearby code.
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' '\b(createdAt|createdDate|joinDate|registrationDate)\b' src
# Show the profile store slice if present.
fd -a 'store' srcRepository: Revive-Graduation-Project/Revive-Front-End
Length of output: 5509
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the profile store and its tests for the user shape and date fields.
wc -l src/store/profileStore.js src/store/__tests__/profileStore.test.js
cat -n src/store/profileStore.js | sed -n '1,260p'
echo "==== tests ===="
cat -n src/store/__tests__/profileStore.test.js | sed -n '1,260p'Repository: Revive-Graduation-Project/Revive-Front-End
Length of output: 9495
Join date should not default to today; format the test date in UTC
- When none of the profile date fields exist,
new Date()makes the profile look like it has a join date of today. Use an empty state such as"Unknown"instead. - The test expectation for
"15 Jan 2024"depends on the browser timezone. Format this date withtimeZone: "UTC"or assert against a timezone-safe value.
🤖 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/Profile.jsx` around lines 33 - 44, The rawJoinDate fallback
should use an explicit empty-state value such as "Unknown" instead of new
Date(), and formattedJoinDate must preserve that value without attempting to
format it as a date. Update the toLocaleDateString options in the join-date
formatting flow to include timeZone: "UTC", keeping the existing date-field
precedence and display format.
| if (!user) { | ||
| return ( | ||
| <div className="min-h-screen flex items-center justify-center p-8 text-gray-500"> | ||
| No profile data available. | ||
| <div className="min-h-screen flex flex-col items-center justify-center p-8 text-center"> | ||
| <div className="bg-white p-8 rounded-2xl shadow-md max-w-md border border-gray-100"> | ||
| <h3 className="text-xl font-semibold text-gray-800 mb-2"> | ||
| Profile Not Found | ||
| </h3> | ||
| <p className="text-sm text-gray-600 mb-6"> | ||
| Your profile appears to have been deleted or does not exist. | ||
| </p> | ||
| <button | ||
| onClick={() => useAuthStore.getState().logout()} | ||
| className="bg-(--color-orange) hover:bg-orange-500 text-white px-6 py-2 rounded-full text-sm font-semibold transition cursor-pointer" | ||
| > | ||
| Log Out | ||
| </button> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Removing the distinct error UI conflates transient fetch failures with a genuinely missing profile.
Previously error had its own UI; now any !user state (including a transient network/server error during fetchProfile) shows "Profile Not Found" with only a logout action — no retry option. A user hitting a temporary error is forced to log out rather than retry, which is a worse experience than before.
🤖 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 44 - 64, Update the
ProfileLayout component’s !user handling to distinguish fetch errors from a
genuinely missing profile: preserve a dedicated error state UI with a retry
action that re-invokes fetchProfile, and show “Profile Not Found” only when no
error occurred. Keep the existing logout behavior for the missing-profile state.
| const avatar = | ||
| user?.avatar || user?.photo || "/images/avatar-placeholder.jpeg"; | ||
| authUser?.profilePictureUrl || | ||
| authUser?.avatar || | ||
| authUser?.photo || | ||
| user?.profilePictureUrl || | ||
| user?.avatar || | ||
| user?.photo || | ||
| "/images/avatar-placeholder.jpeg"; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Avatar precedence favors authUser over the freshly-fetched profile user, which can show a stale picture.
Profile pictures are managed via clientProfileService (/api/clients/profile/:id/picture), not the auth-user record. Since avatar checks authUser?.profilePictureUrl || authUser?.avatar || authUser?.photo before user?.profilePictureUrl, the Sidebar avatar can keep showing an old/stale value from authUser even after a successful upload or removal via Profile.jsx, if authStore's user object isn't also kept in sync.
💡 Suggested fix
const avatar =
- authUser?.profilePictureUrl ||
- authUser?.avatar ||
- authUser?.photo ||
user?.profilePictureUrl ||
user?.avatar ||
user?.photo ||
+ authUser?.profilePictureUrl ||
+ authUser?.avatar ||
+ authUser?.photo ||
"/images/avatar-placeholder.jpeg";📝 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 avatar = | |
| user?.avatar || user?.photo || "/images/avatar-placeholder.jpeg"; | |
| authUser?.profilePictureUrl || | |
| authUser?.avatar || | |
| authUser?.photo || | |
| user?.profilePictureUrl || | |
| user?.avatar || | |
| user?.photo || | |
| "/images/avatar-placeholder.jpeg"; | |
| const avatar = | |
| user?.profilePictureUrl || | |
| user?.avatar || | |
| user?.photo || | |
| authUser?.profilePictureUrl || | |
| authUser?.avatar || | |
| authUser?.photo || | |
| "/images/avatar-placeholder.jpeg"; |
🤖 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 73 - 80, Update the avatar
fallback chain in ProfileLayout so freshly fetched user picture fields from user
take precedence over authUser fields. Preserve the existing field order within
each object and the placeholder fallback, ensuring uploads or removals reflected
by clientProfileService are displayed even when authUser is stale.
| export const uploadProfilePicture = (id, file) => { | ||
| const formData = new FormData(); | ||
| formData.append("file", file); | ||
| return api.patch(`/api/clients/profile/${id}/picture`, formData, { | ||
| headers: { | ||
| "Content-Type": "multipart/form-data", | ||
| }, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## files\n'
git ls-files | rg '^(src/services/clientProfileService\.js|src/|package\.json)$'
printf '\n## clientProfileService.js\n'
cat -n src/services/clientProfileService.js
printf '\n## api wrapper candidates\n'
rg -n "axios|create\\(|interceptors|Content-Type|multipart/form-data|FormData" src package.json
printf '\n## package.json\n'
cat -n package.jsonRepository: Revive-Graduation-Project/Revive-Front-End
Length of output: 8196
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## src/services/api.js\n'
cat -n src/services/api.js
printf '\n## multipart upload call sites\n'
sed -n '480,570p' src/services/dashboardService.js
printf '\n---\n'
sed -n '1,80p' src/services/user.service.js
printf '\n## focused search for request config mutation\n'
rg -n "transformRequest|headers\\.|Content-Type|FormData" src/services/api.js src/services/*.jsRepository: Revive-Graduation-Project/Revive-Front-End
Length of output: 9485
🌐 Web query:
axios multipart/form-data boundary automatic FormData browser docs
💡 Result:
When working with FormData in the browser, you should not manually set the Content-Type header [1][2][3]. If you set the Content-Type header manually, you risk overriding the browser's ability to automatically generate and append the necessary multipart boundary string [2][3]. The browser, web worker, or React Native environment must generate this unique boundary itself to correctly structure the multipart/form-data payload [1][2]. Key points for browser usage: 1. Automatic Detection: When you pass a FormData instance as the payload to an Axios request, Axios automatically detects the type and ensures the request is sent with the correct configuration [1][4]. 2. Leave Content-Type Unset: You should leave the Content-Type header unset in your request configuration [1][5]. The browser will automatically set the header to multipart/form-data and include the required boundary parameter [4][2]. 3. Automatic Serialization: Starting from version 0.27.0, if you need to send an object as FormData, you can set the Content-Type header to multipart/form-data in your request config, and Axios will perform the serialization automatically [1][6][7]. This is distinct from passing an already-constructed FormData object, where you should avoid setting the header [1][2]. 4. Shortcut Methods: Axios provides convenience methods such as postForm, putForm, and patchForm, which automatically preset the Content-Type header to multipart/form-data [6][5][7]. If you are using Node.js, the behavior may differ because the environment does not have a native browser-like FormData with automatic boundary management in the same way, often requiring the form-data library or native FormData in newer Node.js versions [1][2][8]. However, for browser-based development, strictly avoiding manual Content-Type header intervention is the standard best practice to ensure the boundary is handled correctly [2][3].
Citations:
- 1: https://github.com/axios/axios
- 2: https://fossies.org/linux/www/axios-1.18.0.tar.gz/axios-1.18.0/docs/pages/advanced/multipart-form-data-format.md
- 3: https://axios.rest/pages/advanced/multipart-form-data-format
- 4: https://www.mintlify.com/axios/axios/advanced/form-data
- 5: https://github.com/axios/axios?tab=readme-ov-file
- 6: https://github.com/axios/axios-docs/blob/master/posts/en/multipart.md
- 7: https://github.com/axios/axios/blob/v1.15.0/docs/pages/advanced/multipart-form-data-format.md
- 8: https://github.com/axios/axios/blob/v1.15.0/docs/pages/advanced/file-posting.md
Remove the manual multipart header here
FormData uploads should leave Content-Type unset so Axios/the browser can add the boundary parameter. Sending "multipart/form-data" explicitly can break parsing on the server.
🤖 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/services/clientProfileService.js` around lines 35 - 43, Remove the
manually specified Content-Type header from uploadProfilePicture’s api.patch
request. Leave the FormData request headers unset so Axios/the browser
automatically adds the multipart boundary.
| export const updateKitchenStatus = async (orderId, status) => { | ||
| const numericId = String(orderId).replace('#', ''); | ||
| const result = await updateOrderStatus(numericId, status); | ||
| // Synchronize with active kitchen tickets if present | ||
| try { | ||
| const tickets = await getActiveKitchenTickets().catch(() => []); | ||
| const matchingTicket = tickets.find(t => String(t.orderId) === numericId || String(t.id) === numericId); | ||
| if (matchingTicket && matchingTicket.id) { | ||
| await updateTicketStatus(matchingTicket.id, status).catch(() => {}); | ||
| } | ||
| } catch (_e) {} | ||
| return result; | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Clean up the best-effort ticket-sync block.
_e is flagged as unused and the empty catch {} triggers no-empty. Since getActiveKitchenTickets() and updateTicketStatus() already have inner .catch(...), the outer try/catch is largely redundant; either drop it or annotate the intentional swallow.
🧹 Proposed cleanup
const result = await updateOrderStatus(numericId, status);
// Synchronize with active kitchen tickets if present
- try {
- const tickets = await getActiveKitchenTickets().catch(() => []);
- const matchingTicket = tickets.find(t => String(t.orderId) === numericId || String(t.id) === numericId);
- if (matchingTicket && matchingTicket.id) {
- await updateTicketStatus(matchingTicket.id, status).catch(() => {});
- }
- } catch (_e) {}
+ const tickets = await getActiveKitchenTickets().catch(() => []);
+ const matchingTicket = tickets.find(t => String(t.orderId) === numericId || String(t.id) === numericId);
+ if (matchingTicket && matchingTicket.id) {
+ await updateTicketStatus(matchingTicket.id, status).catch(() => {});
+ }
return result;📝 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 updateKitchenStatus = async (orderId, status) => { | |
| const numericId = String(orderId).replace('#', ''); | |
| const result = await updateOrderStatus(numericId, status); | |
| // Synchronize with active kitchen tickets if present | |
| try { | |
| const tickets = await getActiveKitchenTickets().catch(() => []); | |
| const matchingTicket = tickets.find(t => String(t.orderId) === numericId || String(t.id) === numericId); | |
| if (matchingTicket && matchingTicket.id) { | |
| await updateTicketStatus(matchingTicket.id, status).catch(() => {}); | |
| } | |
| } catch (_e) {} | |
| return result; | |
| }; | |
| export const updateKitchenStatus = async (orderId, status) => { | |
| const numericId = String(orderId).replace('#', ''); | |
| const result = await updateOrderStatus(numericId, status); | |
| // Synchronize with active kitchen tickets if present | |
| const tickets = await getActiveKitchenTickets().catch(() => []); | |
| const matchingTicket = tickets.find(t => String(t.orderId) === numericId || String(t.id) === numericId); | |
| if (matchingTicket && matchingTicket.id) { | |
| await updateTicketStatus(matchingTicket.id, status).catch(() => {}); | |
| } | |
| return result; | |
| }; |
🧰 Tools
🪛 ESLint
[error] 421-421: '_e' is defined but never used.
(no-unused-vars)
[error] 421-421: Empty block statement.
(no-empty)
🤖 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/services/dashboardService.js` around lines 411 - 423, Clean up the
best-effort synchronization block in updateKitchenStatus by removing the
redundant outer try/catch, since getActiveKitchenTickets and updateTicketStatus
already swallow their expected failures. Preserve the existing ticket lookup and
conditional status update behavior without introducing an unused catch parameter
or empty catch block.
Source: Linters/SAST tools
| deleteAccount: async () => { | ||
| set({ loading: true, error: null }); | ||
| try { | ||
| const id = useAuthStore.getState().user?.id; | ||
| if (id) { | ||
| await deleteClientProfile(id).catch((err) => { | ||
| console.warn("Delete profile API response:", err); | ||
| }); | ||
| await deleteAuthUser(id).catch((err) => { | ||
| console.warn("Delete auth user API response:", err); | ||
| }); | ||
| } | ||
| set({ user: null, loading: false, error: null }); | ||
| try { | ||
| localStorage.removeItem("revive-profile-store"); | ||
| } catch (e) {} | ||
| await useAuthStore.getState().logout(false); | ||
| return true; | ||
| } catch (error) { | ||
| set({ user: null, loading: false, error: null }); | ||
| try { | ||
| localStorage.removeItem("revive-profile-store"); | ||
| } catch (e) {} | ||
| await useAuthStore.getState().logout(false); | ||
| return true; | ||
| } | ||
| }, | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
deleteAccount always returns true, even when both delete calls fail.
Both deleteClientProfile and deleteAuthUser errors are swallowed via .catch((err) => console.warn(...)), and the outer catch block duplicates the exact same "success" side effects (clear user, wipe localStorage, logout, return true). This means a total backend failure (network error, 403, 500) still reports success to the caller — the UI will tell the user their account was deleted and log them out, while the account and its data remain intact server-side. For an account-deletion flow this is a real correctness and compliance concern (e.g. "right to be forgotten" flows should not report success on failure).
This also matches the static-analysis hints: unused e/error and empty catch blocks at Lines 162, 165, and 169 are symptomatic of the swallowed errors.
🛠️ Proposed fix: propagate actual failure instead of always succeeding
deleteAccount: async () => {
set({ loading: true, error: null });
- try {
- const id = useAuthStore.getState().user?.id;
- if (id) {
- await deleteClientProfile(id).catch((err) => {
- console.warn("Delete profile API response:", err);
- });
- await deleteAuthUser(id).catch((err) => {
- console.warn("Delete auth user API response:", err);
- });
- }
- set({ user: null, loading: false, error: null });
- try {
- localStorage.removeItem("revive-profile-store");
- } catch (e) {}
- await useAuthStore.getState().logout(false);
- return true;
- } catch (error) {
- set({ user: null, loading: false, error: null });
- try {
- localStorage.removeItem("revive-profile-store");
- } catch (e) {}
- await useAuthStore.getState().logout(false);
- return true;
- }
+ const id = useAuthStore.getState().user?.id;
+ if (!id) {
+ set({ loading: false, error: "User ID not found" });
+ return false;
+ }
+ try {
+ await deleteClientProfile(id);
+ await deleteAuthUser(id);
+ } catch (error) {
+ set({ loading: false, error: error.message || "Failed to delete account" });
+ return false;
+ }
+ set({ user: null, loading: false, error: null });
+ try {
+ localStorage.removeItem("revive-profile-store");
+ } catch {}
+ await useAuthStore.getState().logout(false);
+ return true;
},📝 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.
| deleteAccount: async () => { | |
| set({ loading: true, error: null }); | |
| try { | |
| const id = useAuthStore.getState().user?.id; | |
| if (id) { | |
| await deleteClientProfile(id).catch((err) => { | |
| console.warn("Delete profile API response:", err); | |
| }); | |
| await deleteAuthUser(id).catch((err) => { | |
| console.warn("Delete auth user API response:", err); | |
| }); | |
| } | |
| set({ user: null, loading: false, error: null }); | |
| try { | |
| localStorage.removeItem("revive-profile-store"); | |
| } catch (e) {} | |
| await useAuthStore.getState().logout(false); | |
| return true; | |
| } catch (error) { | |
| set({ user: null, loading: false, error: null }); | |
| try { | |
| localStorage.removeItem("revive-profile-store"); | |
| } catch (e) {} | |
| await useAuthStore.getState().logout(false); | |
| return true; | |
| } | |
| }, | |
| deleteAccount: async () => { | |
| set({ loading: true, error: null }); | |
| const id = useAuthStore.getState().user?.id; | |
| if (!id) { | |
| set({ loading: false, error: "User ID not found" }); | |
| return false; | |
| } | |
| try { | |
| await deleteClientProfile(id); | |
| await deleteAuthUser(id); | |
| } catch (error) { | |
| set({ loading: false, error: error.message || "Failed to delete account" }); | |
| return false; | |
| } | |
| set({ user: null, loading: false, error: null }); | |
| try { | |
| localStorage.removeItem("revive-profile-store"); | |
| } catch {} | |
| await useAuthStore.getState().logout(false); | |
| return true; | |
| }, |
🧰 Tools
🪛 ESLint
[error] 162-162: 'e' is defined but never used.
(no-unused-vars)
[error] 162-162: Empty block statement.
(no-empty)
[error] 165-165: 'error' is defined but never used.
(no-unused-vars)
[error] 169-169: 'e' is defined but never used.
(no-unused-vars)
[error] 169-169: Empty block statement.
(no-empty)
🤖 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 147 - 174, Update deleteAccount so
failures from deleteClientProfile and deleteAuthUser are not swallowed: remove
their inline catches or rethrow after logging, allowing the outer catch to
handle them. Make the outer catch set loading/error appropriately, avoid
clearing local user state or logging out as if deletion succeeded, and return or
propagate failure instead of always returning true; retain the success cleanup
and true result only after both deletions complete.
Source: Linters/SAST tools
1 participant
Summary by CodeRabbit