fix profile flow - #82
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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: Path: .coderabbit.yaml 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:
✨ 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (7)
src/components/auth/StepTwo.jsx (1)
103-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
GOAL_OPTIONSinstead of hardcoding goal options inline.The goal values are duplicated from
GOAL_OPTIONSinsrc/constants.js. If the constant changes, this signup form won't reflect the update, creating a drift risk withHealthForm.jsxwhich imports and uses the same constant.♻️ Proposed refactor: import and derive labels from GOAL_OPTIONS
+ import { GOAL_OPTIONS } from "../../constants"; + // Inside the component, replace the hardcoded array: - {[ - { value: "LOSE_WEIGHT", label: "Lose Weight" }, - { value: "GAIN_WEIGHT", label: "Gain Weight" }, - { value: "BUILD_MUSCLE", label: "Build Muscle" }, - { value: "MAINTAIN_SHAPE", label: "Maintain Shape" }, - ].map((g) => ( - <label key={g.value} className="flex items-center gap-1"> - <input - type="radio" - name="goal" - value={g.value} - checked={formData.goal === g.value} - onChange={onChange} - />{" "} - {g.label} + {GOAL_OPTIONS.map((g) => { + const label = g.replace("_", " ").toLowerCase().replace(/(^|\s)\S/g, (l) => l.toUpperCase()); + return ( + <label key={g} className="flex items-center gap-1"> + <input + type="radio" + name="goal" + value={g} + checked={formData.goal === g} + onChange={onChange} + />{" "} + {label} </label> - ))} + ); + })}Alternatively, update
GOAL_OPTIONSinconstants.jsto use{value, label}objects and updateHealthForm.jsxto handle the new shape — this would unify both consumers.🤖 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 103 - 108, Update StepTwo’s goal-options rendering to import and use GOAL_OPTIONS from the constants module instead of defining the values inline, while preserving the existing displayed labels and mapping behavior. Keep the implementation consistent with HealthForm.jsx so both consumers derive their options from the shared constant.src/store/authStore.js (1)
70-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNormalize
fullName/namein both auth paths
logincopiesrawUser.fullName/namedirectly, so responses that only providefirstName/lastNamecan persistundefinedhere whilerestoreSessionalways derives those fields. Use a shared helper for both paths to keep the mapping consistent and remove the duplicated name construction.🤖 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 70 - 93, Add a shared user-name normalization helper and use it in both the login mapping and restoreSession mapping. Ensure fullName and name fall back to the trimmed combination of firstName and lastName when rawUser values are absent, while preserving provided values when present; remove the duplicated inline name construction.src/pages/Profile/Profile.jsx (1)
109-169: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo client-side file validation before enabling upload.
handleFileChangeaccepts any file matching the picker without checking size or MIME type before enabling the "Upload" button; a bad selection only fails at the API layer. Consider validatingfile.type.startsWith("image/")and a reasonable max size before settingselectedFile, to fail fast with a clear message instead of relying on the multipart PATCH round-trip to reject it.🤖 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 109 - 169, Update handleFileChange to validate the selected file’s MIME type and enforce a reasonable maximum size before calling setSelectedFile; reject invalid files with a clear user-facing message and keep the Upload button disabled by leaving selectedFile unset.src/pages/Profile/Profile.test.jsx (1)
20-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider adding coverage for picture management and account deletion.
Given account deletion is irreversible and now user-triggerable from this page, tests exercising
uploadProfilePicture,deleteProfilePicture, anddeleteAccountcall paths (success/error toasts, modal open/close, redirect on delete) would materially increase confidence before merge.🤖 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 20 - 31, Add Profile coverage for the picture-management and account-deletion flows around the Profile component, exercising uploadProfilePicture and deleteProfilePicture success/error toast behavior, delete-account modal open and close behavior, and deleteAccount success redirect. Keep the existing join-date assertion and use the component’s established UI and mocked service paths.src/store/__tests__/profileStore.test.js (1)
5-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd missing service mocks for completeness.
The mock only includes
getProfileandupdateProfile, but the store also importsuploadProfilePicture,deleteProfilePicture, anddeleteProfile. Adding these now prevents confusingTypeError: ... is not a functionerrors when future tests exercise those actions.♻️ Proposed refactor
vi.mock("../../services/clientProfileService", () => ({ getProfile: vi.fn(), updateProfile: vi.fn(), + uploadProfilePicture: vi.fn(), + deleteProfilePicture: vi.fn(), + deleteProfile: vi.fn(), }));🤖 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/__tests__/profileStore.test.js` around lines 5 - 10, Update the clientProfileService mock in the profile store tests to include vi.fn() mocks for uploadProfilePicture, deleteProfilePicture, and deleteProfile alongside the existing getProfile and updateProfile mocks, so all store actions have callable service dependencies.src/store/profileStore.js (1)
147-172: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSimplify
deleteAccountcleanup withfinallyand fix empty catch blocks.The try and catch blocks have identical cleanup code. Use
finallyto eliminate duplication. The emptycatch (e) {}blocks at lines 162 and 169 and the unusederrorat line 165 trigger ESLintno-emptyandno-unused-varswarnings.♻️ Proposed refactor
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; - } + } finally { + set({ user: null, loading: false, error: null }); + try { + localStorage.removeItem("revive-profile-store"); + } catch {} + await useAuthStore.getState().logout(false); + } + return true; },🤖 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 - 172, Refactor deleteAccount so the shared profile reset, localStorage cleanup, logout, and success return execute from a finally block instead of being duplicated in try and catch. Remove the unused caught error and replace the empty localStorage catch blocks with the project’s accepted no-op handling so no-empty and no-unused-vars warnings remain.Source: Linters/SAST tools
src/services/clientProfileService.js (1)
38-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the manual multipart header. Passing
FormDatahere lets Axios/browser set the boundary correctly; settingContent-Type: multipart/form-datacan strip that boundary and break the upload.🤖 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 38 - 42, Remove the manually specified Content-Type header from the profile picture upload request in the client profile service, allowing Axios/browser to generate the multipart boundary automatically while preserving the existing FormData patch call.
🤖 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 import from the roleUtils import in
DashboardSidebar.jsx, keeping only the role helpers referenced by the component.
In `@src/pages/auth/Login.jsx`:
- Around line 37-49: Update the catch block surrounding getProfile in the Login
component to perform logout and display the “profile deleted” message only when
the error represents an HTTP 404 response. Propagate or handle all other errors
without logging out or showing the deletion message.
In `@src/pages/Profile/components/HealthForm.jsx`:
- Around line 44-66: Update the height validation in the form validation logic
to use heightUnit when interpreting form.height. Convert meter or foot values to
the canonical unit, or apply appropriate unit-specific bounds, while preserving
the existing 30–300 centimeter constraints for cm and rejecting invalid or
out-of-range values.
In `@src/pages/Profile/Profile.jsx`:
- Around line 50-71: Update the preview URL lifecycle in handleFileChange and
handleUploadPicture by revoking the previous previewUrl before replacing it and
revoking the active URL before clearing it after a successful upload. Also
revoke the current URL on component unmount so every URL created with
URL.createObjectURL is released.
- Around line 216-268: Update both confirmation modals rendered by
showDeletePicModal and showDeleteAccountModal to use accessible dialog semantics
with role="dialog", aria-modal="true", and an accessible label or title
association. Add Escape-key dismissal for the active modal, move initial
keyboard focus into the modal when it opens, and restore focus appropriately
when it closes; ensure the implementation handles both modal flows without
duplicating conflicting listeners.
- Around line 33-44: Update the join-date handling in Profile to use "-" when
none of user?.createdAt, user?.createdDate, user?.joinDate, or
user?.registrationDate is present, and avoid constructing or formatting a
synthetic current date. When a date exists, include timeZone: "UTC" in the
toLocaleDateString options so formatted output is timezone-independent.
In `@src/pages/Profile/ProfileLayout.jsx`:
- Around line 65-80: Extract the duplicated displayName and avatar fallback
chains from ProfileLayout and InfoGrid into shared helpers, such as
getDisplayName and getAvatarUrl, accepting authUser, user, and profile where
needed. Update both components to use these helpers and preserve the complete
fallback order, including profile fields already supported by InfoGrid, so the
derivation logic remains consistent.
- Around line 44-63: Update the !user handling in ProfileLayout so the “Profile
Not Found” screen renders only when error equals “Profile not found”. For other
fetch errors, render a retry state that lets the user retry loading the profile
instead of treating the failure as deletion; preserve the existing logout action
for the explicit missing-profile case.
In `@src/store/profileStore.js`:
- Around line 77-86: Update the condition guarding the useAuthStore.setState
call to also check data.fullName, so updates containing only fullName still
synchronize the auth store. Preserve the existing user merge and
updated.name/updated.fullName assignments.
- Around line 60-93: Update the request-body construction in updateClientProfile
so the PUT payload preserves firstName, lastName, name, and fullName from
updateUser’s mergedData. Keep the existing profile fields and response handling
unchanged, ensuring these name edits are sent to the endpoint and persist after
refetch.
---
Nitpick comments:
In `@src/components/auth/StepTwo.jsx`:
- Around line 103-108: Update StepTwo’s goal-options rendering to import and use
GOAL_OPTIONS from the constants module instead of defining the values inline,
while preserving the existing displayed labels and mapping behavior. Keep the
implementation consistent with HealthForm.jsx so both consumers derive their
options from the shared constant.
In `@src/pages/Profile/Profile.jsx`:
- Around line 109-169: Update handleFileChange to validate the selected file’s
MIME type and enforce a reasonable maximum size before calling setSelectedFile;
reject invalid files with a clear user-facing message and keep the Upload button
disabled by leaving selectedFile unset.
In `@src/pages/Profile/Profile.test.jsx`:
- Around line 20-31: Add Profile coverage for the picture-management and
account-deletion flows around the Profile component, exercising
uploadProfilePicture and deleteProfilePicture success/error toast behavior,
delete-account modal open and close behavior, and deleteAccount success
redirect. Keep the existing join-date assertion and use the component’s
established UI and mocked service paths.
In `@src/services/clientProfileService.js`:
- Around line 38-42: Remove the manually specified Content-Type header from the
profile picture upload request in the client profile service, allowing
Axios/browser to generate the multipart boundary automatically while preserving
the existing FormData patch call.
In `@src/store/__tests__/profileStore.test.js`:
- Around line 5-10: Update the clientProfileService mock in the profile store
tests to include vi.fn() mocks for uploadProfilePicture, deleteProfilePicture,
and deleteProfile alongside the existing getProfile and updateProfile mocks, so
all store actions have callable service dependencies.
In `@src/store/authStore.js`:
- Around line 70-93: Add a shared user-name normalization helper and use it in
both the login mapping and restoreSession mapping. Ensure fullName and name fall
back to the trimmed combination of firstName and lastName when rawUser values
are absent, while preserving provided values when present; remove the duplicated
inline name construction.
In `@src/store/profileStore.js`:
- Around line 147-172: Refactor deleteAccount so the shared profile reset,
localStorage cleanup, logout, and success return execute from a finally block
instead of being duplicated in try and catch. Remove the unused caught error and
replace the empty localStorage catch blocks with the project’s accepted no-op
handling so no-empty and no-unused-vars warnings remain.
🪄 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: fc9281d5-2719-463b-a413-a7c17c8ecbe4
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.json
📒 Files selected for processing (21)
src/Layout/AppLayout.jsxsrc/components/Dashboard/DashboardSidebar.jsxsrc/components/Dashboard/LiveKitchenView.jsxsrc/components/Dashboard/StaffManagementView.jsxsrc/components/auth/StepTwo.jsxsrc/components/ui/AllergiesDropdown.jsxsrc/constants.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/store/__tests__/profileStore.test.jssrc/store/authStore.jssrc/store/profileStore.js
💤 Files with no reviewable changes (1)
- src/components/Dashboard/LiveKitchenView.jsx
| 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 unused isAdminUser import.
ESLint reports isAdminUser as defined but never used. This is a leftover from the previous filtering logic and should be removed to avoid CI failures if linting is enforced.
🧹 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 import from the roleUtils import in DashboardSidebar.jsx, keeping
only the role helpers referenced by the component.
Source: Linters/SAST tools
| try { | ||
| if (user?.id) { | ||
| const res = await getProfile(user.id); | ||
| if (!res?.data) { | ||
| throw new Error("Profile not found"); | ||
| } | ||
| } | ||
| navigate("/"); | ||
| } catch (err) { | ||
| await useAuthStore.getState().logout(false); | ||
| setLocalError( | ||
| "Your account profile has been deleted. Please sign up to create a new account." | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Catch block is too broad — network errors trigger misleading "profile deleted" logout.
The catch block treats all getProfile failures as "profile deleted," logging the user out and showing a deletion message. A transient network error or 500 response would incorrectly log out an authenticated user and display a misleading message. Only a 404 should trigger this flow.
🐛 Proposed fix: distinguish 404 from other errors
} catch (err) {
- await useAuthStore.getState().logout(false);
- setLocalError(
- "Your account profile has been deleted. Please sign up to create a new account."
- );
+ if (err?.response?.status === 404) {
+ await useAuthStore.getState().logout(false);
+ setLocalError(
+ "Your account profile has been deleted. Please sign up to create a new account."
+ );
+ } else {
+ setLocalError(
+ "Unable to verify your profile. Please try again."
+ );
+ }
}📝 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.
| try { | |
| if (user?.id) { | |
| const res = await getProfile(user.id); | |
| if (!res?.data) { | |
| throw new Error("Profile not found"); | |
| } | |
| } | |
| navigate("/"); | |
| } catch (err) { | |
| await useAuthStore.getState().logout(false); | |
| setLocalError( | |
| "Your account profile has been deleted. Please sign up to create a new account." | |
| ); | |
| try { | |
| if (user?.id) { | |
| const res = await getProfile(user.id); | |
| if (!res?.data) { | |
| throw new Error("Profile not found"); | |
| } | |
| } | |
| navigate("/"); | |
| } catch (err) { | |
| if (err?.response?.status === 404) { | |
| await useAuthStore.getState().logout(false); | |
| setLocalError( | |
| "Your account profile has been deleted. Please sign up to create a new account." | |
| ); | |
| } else { | |
| setLocalError( | |
| "Unable to verify your profile. Please try again." | |
| ); | |
| } | |
| } |
🧰 Tools
🪛 ESLint
[error] 45-45: 'err' is defined but never used.
(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/pages/auth/Login.jsx` around lines 37 - 49, Update the catch block
surrounding getProfile in the Login component to perform logout and display the
“profile deleted” message only when the error represents an HTTP 404 response.
Propagate or handle all other errors without logging out or showing the deletion
message.
| if (form.age !== "" && form.age !== null) { | ||
| const ageNum = Number(form.age); | ||
| if (isNaN(ageNum) || ageNum < 1 || ageNum > 120) { | ||
| newErrors.age = "Age must be between 1 and 120"; | ||
| hasError = true; | ||
| } | ||
| } | ||
|
|
||
| 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 | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "HEIGHT_UNITS|WEIGHT_UNITS" src/constants.jsRepository: Revive-Graduation-Project/Revive-Front-End
Length of output: 281
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## HealthForm.jsx outline\n'
ast-grep outline src/pages/Profile/components/HealthForm.jsx --view expanded || true
printf '\n## Relevant section of HealthForm.jsx\n'
sed -n '1,220p' src/pages/Profile/components/HealthForm.jsx
printf '\n## constants.js unit definitions\n'
sed -n '50,80p' src/constants.jsRepository: Revive-Graduation-Project/Revive-Front-End
Length of output: 9514
Height validation should account for heightUnit
HEIGHT_UNITS includes m and ft, but this check still hardcodes 30–300 as if the value were always cm. A valid 1.7 m or 5.5 ft entry will be rejected; convert to a canonical unit or validate against unit-specific bounds.
🤖 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 44 - 66, Update the
height validation in the form validation logic to use heightUnit when
interpreting form.height. Convert meter or foot values to the canonical unit, or
apply appropriate unit-specific bounds, while preserving the existing 30–300
centimeter constraints for cm and rejecting invalid or out-of-range values.
| 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 profile page date logic and related tests.
git ls-files 'src/pages/Profile/Profile.jsx' 'src/pages/Profile/Profile.test.jsx' 'src/pages/Profile/*' | sed -n '1,20p'
echo '--- Profile.jsx ---'
nl -ba src/pages/Profile/Profile.jsx | sed -n '25,55p'
echo '--- Profile.test.jsx ---'
if [ -f src/pages/Profile/Profile.test.jsx ]; then
nl -ba src/pages/Profile/Profile.test.jsx | sed -n '1,220p'
fi
# Read-only probe: show how toLocaleDateString behaves for a UTC-midnight timestamp
# with and without an explicit UTC timezone in the current environment.
node - <<'JS'
const d = new Date("2024-01-15T00:00:00.000Z");
console.log("default:", d.toLocaleDateString("en-GB", { day: "2-digit", month: "short", year: "numeric" }));
console.log("UTC :", d.toLocaleDateString("en-GB", { day: "2-digit", month: "short", year: "numeric", timeZone: "UTC" }));
console.log("tz:", Intl.DateTimeFormat().resolvedOptions().timeZone);
JSRepository: Revive-Graduation-Project/Revive-Front-End
Length of output: 749
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- Profile.jsx (lines 25-55) ---'
sed -n '25,55p' src/pages/Profile/Profile.jsx | cat -n
echo '--- Profile.test.jsx (full or first 220 lines) ---'
sed -n '1,220p' src/pages/Profile/Profile.test.jsx | cat -n
echo '--- timezone probe ---'
node - <<'JS'
const d = new Date("2024-01-15T00:00:00.000Z");
console.log("default:", d.toLocaleDateString("en-GB", { day: "2-digit", month: "short", year: "numeric" }));
console.log("UTC :", d.toLocaleDateString("en-GB", { day: "2-digit", month: "short", year: "numeric", timeZone: "UTC" }));
console.log("tz:", Intl.DateTimeFormat().resolvedOptions().timeZone);
JSRepository: Revive-Graduation-Project/Revive-Front-End
Length of output: 2468
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for tz in UTC America/Los_Angeles America/New_York Pacific/Honolulu Asia/Tokyo; do
echo "--- $tz ---"
TZ="$tz" node - <<'JS'
const d = new Date("2024-01-15T00:00:00.000Z");
console.log(d.toLocaleDateString("en-GB", { day: "2-digit", month: "short", year: "numeric" }));
JS
doneRepository: Revive-Graduation-Project/Revive-Front-End
Length of output: 349
Pin join-date formatting to UTC and avoid a synthetic fallback date
toLocaleDateString()withouttimeZone: "UTC"shifts2024-01-15T00:00:00.000Zto14 Jan 2024in negative-offset timezones, so the UI andProfile.test.jsxbecome timezone-dependent.- Falling back to
new Date()renders today when no join-date field exists; use a placeholder like"-"instead.
🤖 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, Update the join-date
handling in Profile to use "-" when none of user?.createdAt, user?.createdDate,
user?.joinDate, or user?.registrationDate is present, and avoid constructing or
formatting a synthetic current date. When a date exists, include timeZone: "UTC"
in the toLocaleDateString options so formatted output is timezone-independent.
| const handleFileChange = (e) => { | ||
| const file = e.target.files?.[0]; | ||
| if (file) { | ||
| setSelectedFile(file); | ||
| setPreviewUrl(URL.createObjectURL(file)); | ||
| } | ||
| }; | ||
|
|
||
| const handleUploadPicture = async () => { | ||
| if (!selectedFile) return; | ||
| setUploadingPic(true); | ||
| try { | ||
| await uploadProfilePicture(selectedFile); | ||
| toast.success("Profile picture updated successfully."); | ||
| setSelectedFile(null); | ||
| setPreviewUrl(null); | ||
| } catch (err) { | ||
| toast.error(err?.message || "Failed to upload profile picture."); | ||
| } finally { | ||
| setUploadingPic(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Object URLs from URL.createObjectURL are never revoked — memory leak.
handleFileChange creates a new blob URL every time a file is picked, and handleUploadPicture discards previewUrl on success without revoking it. Repeated selections accumulate un-revoked blob URLs for the session lifetime.
🔧 Proposed fix
const handleFileChange = (e) => {
const file = e.target.files?.[0];
if (file) {
+ if (previewUrl) URL.revokeObjectURL(previewUrl);
setSelectedFile(file);
setPreviewUrl(URL.createObjectURL(file));
}
};
const handleUploadPicture = async () => {
if (!selectedFile) return;
setUploadingPic(true);
try {
await uploadProfilePicture(selectedFile);
toast.success("Profile picture updated successfully.");
+ if (previewUrl) URL.revokeObjectURL(previewUrl);
setSelectedFile(null);
setPreviewUrl(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.
| const handleFileChange = (e) => { | |
| const file = e.target.files?.[0]; | |
| if (file) { | |
| setSelectedFile(file); | |
| setPreviewUrl(URL.createObjectURL(file)); | |
| } | |
| }; | |
| const handleUploadPicture = async () => { | |
| if (!selectedFile) return; | |
| setUploadingPic(true); | |
| try { | |
| await uploadProfilePicture(selectedFile); | |
| toast.success("Profile picture updated successfully."); | |
| setSelectedFile(null); | |
| setPreviewUrl(null); | |
| } catch (err) { | |
| toast.error(err?.message || "Failed to upload profile picture."); | |
| } finally { | |
| setUploadingPic(false); | |
| } | |
| }; | |
| const handleFileChange = (e) => { | |
| const file = e.target.files?.[0]; | |
| if (file) { | |
| if (previewUrl) URL.revokeObjectURL(previewUrl); | |
| setSelectedFile(file); | |
| setPreviewUrl(URL.createObjectURL(file)); | |
| } | |
| }; | |
| const handleUploadPicture = async () => { | |
| if (!selectedFile) return; | |
| setUploadingPic(true); | |
| try { | |
| await uploadProfilePicture(selectedFile); | |
| toast.success("Profile picture updated successfully."); | |
| if (previewUrl) URL.revokeObjectURL(previewUrl); | |
| setSelectedFile(null); | |
| setPreviewUrl(null); | |
| } catch (err) { | |
| toast.error(err?.message || "Failed to upload profile picture."); | |
| } finally { | |
| setUploadingPic(false); | |
| } | |
| }; |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 52-52: Avoid using the initial state variable in setState
Context: setSelectedFile(file)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🤖 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 50 - 71, Update the preview URL
lifecycle in handleFileChange and handleUploadPicture by revoking the previous
previewUrl before replacing it and revoking the active URL before clearing it
after a successful upload. Also revoke the current URL on component unmount so
every URL created with URL.createObjectURL is released.
| {/* Modal: Delete Profile Picture */} | ||
| {showDeletePicModal && ( | ||
| <div className="fixed inset-0 z-50 flex items-center justify-center bg-black/40 p-4"> | ||
| <div className="bg-white rounded-2xl p-6 max-w-sm w-full shadow-xl"> | ||
| <h3 className="text-lg font-bold text-gray-800">Remove Photo</h3> | ||
| <p className="text-sm text-gray-600 mt-2"> | ||
| Are you sure you want to remove your profile picture? It will revert to the default avatar. | ||
| </p> | ||
| <div className="flex justify-end gap-3 mt-6"> | ||
| <button | ||
| onClick={() => setShowDeletePicModal(false)} | ||
| className="px-4 py-2 text-sm bg-gray-100 rounded-xl hover:bg-gray-200 transition-colors cursor-pointer" | ||
| > | ||
| Cancel | ||
| </button> | ||
| <button | ||
| onClick={handleDeletePicture} | ||
| disabled={deletingPic} | ||
| className="px-4 py-2 text-sm bg-red-600 text-white rounded-xl hover:bg-red-700 transition-colors cursor-pointer disabled:opacity-50" | ||
| > | ||
| {deletingPic ? "Removing..." : "Remove"} | ||
| </button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Modal: Delete Account */} | ||
| {showDeleteAccountModal && ( | ||
| <div className="fixed inset-0 z-50 flex items-center justify-center bg-black/40 p-4"> | ||
| <div className="bg-white rounded-2xl p-6 max-w-md w-full shadow-xl"> | ||
| <h3 className="text-lg font-bold text-red-600">Delete Account</h3> | ||
| <p className="text-sm text-gray-700 mt-2"> | ||
| Are you sure you want to permanently delete your account? This action cannot be undone. All your profile data, order history, and rewards will be deleted. | ||
| </p> | ||
| <div className="flex justify-end gap-3 mt-6"> | ||
| <button | ||
| onClick={() => setShowDeleteAccountModal(false)} | ||
| className="px-4 py-2 text-sm bg-gray-100 rounded-xl hover:bg-gray-200 transition-colors cursor-pointer" | ||
| > | ||
| Cancel | ||
| </button> | ||
| <button | ||
| onClick={handleDeleteAccount} | ||
| disabled={deletingAccount} | ||
| className="px-4 py-2 text-sm bg-red-600 text-white rounded-xl hover:bg-red-700 transition-colors cursor-pointer disabled:opacity-50" | ||
| > | ||
| {deletingAccount ? "Deleting Account..." : "Yes, Delete Account"} | ||
| </button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Confirmation modals lack accessible dialog semantics and keyboard dismissal.
Both the "Remove Photo" and "Delete Account" modals are plain <div> overlays with no role="dialog"/aria-modal, no Escape-key handler, and no initial focus placed inside the modal (nor a backdrop click-to-close). Keyboard-only users have to tab through the entire preceding page to reach the modal buttons, and cannot dismiss with Escape.
♿ Proposed fix (apply to both modals)
{showDeletePicModal && (
- <div className="fixed inset-0 z-50 flex items-center justify-center bg-black/40 p-4">
- <div className="bg-white rounded-2xl p-6 max-w-sm w-full shadow-xl">
+ <div
+ className="fixed inset-0 z-50 flex items-center justify-center bg-black/40 p-4"
+ role="dialog"
+ aria-modal="true"
+ onClick={() => setShowDeletePicModal(false)}
+ onKeyDown={(e) => e.key === "Escape" && setShowDeletePicModal(false)}
+ >
+ <div
+ className="bg-white rounded-2xl p-6 max-w-sm w-full shadow-xl"
+ onClick={(e) => e.stopPropagation()}
+ autoFocus
+ >🤖 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 216 - 268, Update both
confirmation modals rendered by showDeletePicModal and showDeleteAccountModal to
use accessible dialog semantics with role="dialog", aria-modal="true", and an
accessible label or title association. Add Escape-key dismissal for the active
modal, move initial keyboard focus into the modal when it opens, and restore
focus appropriately when it closes; ensure the implementation handles both modal
flows without duplicating conflicting listeners.
| 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.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files ==\n'
git ls-files 'src/pages/Profile/ProfileLayout.jsx' 'src/store/profileStore.js' 'src/components/**/InfoGrid.*' 'src/pages/**/InfoGrid.*' || true
printf '\n== ProfileLayout outline ==\n'
ast-grep outline src/pages/Profile/ProfileLayout.jsx --view expanded || true
printf '\n== profileStore outline ==\n'
ast-grep outline src/store/profileStore.js --view expanded || true
printf '\n== Search for error handling / not found / failed to load ==\n'
rg -n "Profile not found|Failed to load profile|not found|error" src/pages/Profile/ProfileLayout.jsx src/store/profileStore.js src -g '!**/node_modules/**' || trueRepository: Revive-Graduation-Project/Revive-Front-End
Length of output: 37939
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== ProfileLayout (numbered) ==\n'
sed -n '1,140p' src/pages/Profile/ProfileLayout.jsx | cat -n
printf '\n== profileStore fetchProfile section (numbered) ==\n'
sed -n '1,90p' src/store/profileStore.js | cat -n
printf '\n== InfoGrid section (numbered) ==\n'
sed -n '1,140p' src/pages/Profile/components/InfoGrid.jsx | cat -nRepository: Revive-Graduation-Project/Revive-Front-End
Length of output: 14321
Differentiate not-found from load-failure states in src/pages/Profile/ProfileLayout.jsx:44-63. !user currently shows “Profile Not Found” for both the explicit missing-profile case and generic fetch failures, so a network/500 error gets treated like a deleted profile and only offers Log Out. Show this screen only when error === "Profile not found"; for other errors, surface a retry state instead.
🤖 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 - 63, Update the !user
handling in ProfileLayout so the “Profile Not Found” screen renders only when
error equals “Profile not found”. For other fetch errors, render a retry state
that lets the user retry loading the profile instead of treating the failure as
deletion; preserve the existing logout action for the explicit missing-profile
case.
| const displayName = | ||
| authUser?.fullName || | ||
| authUser?.name || | ||
| [authUser?.firstName, authUser?.lastName].filter(Boolean).join(" ") || | ||
| user?.name || | ||
| user?.fullName || | ||
| [user?.firstName, user?.lastName].filter(Boolean).join(" ") || | ||
| "Your Name"; | ||
| 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.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Duplicate displayName/avatar derivation logic vs. InfoGrid.jsx.
This fallback chain is near-identical to the one added in src/pages/Profile/components/InfoGrid.jsx (lines 45-63), and the two are already diverging (InfoGrid additionally falls back to profile?.fullName/profile?.name/profile?.firstName+lastName). Extracting a shared helper (e.g., getDisplayName(authUser, user, profile) / getAvatarUrl(...)) would prevent the two copies from drifting further.
🤖 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 65 - 80, Extract the
duplicated displayName and avatar fallback chains from ProfileLayout and
InfoGrid into shared helpers, such as getDisplayName and getAvatarUrl, accepting
authUser, user, and profile where needed. Update both components to use these
helpers and preserve the complete fallback order, including profile fields
already supported by InfoGrid, so the derivation logic remains consistent.
| updateUser: async (data) => { | ||
| set({ loading: true, error: null }); | ||
| try { | ||
| const id = useAuthStore.getState().user?.id; | ||
| const authUser = useAuthStore.getState().user || {}; | ||
| const id = authUser.id; | ||
| if (!id) throw new Error("User ID not found"); | ||
| const res = await updateProfile(id, data); | ||
| const user = res?.data || null; | ||
| if (user) set({ user, loading: false, error: null }); | ||
| else set({ loading: false }); | ||
| return user; | ||
| const existingUser = get().user || {}; | ||
| const mergedData = { ...existingUser, ...data }; | ||
| const res = await updateClientProfile(id, mergedData); | ||
| const updated = { | ||
| ...mergedData, | ||
| ...(res?.data || {}), | ||
| firstName: data.firstName || mergedData.firstName, | ||
| lastName: data.lastName || mergedData.lastName, | ||
| name: data.name || mergedData.name, | ||
| fullName: data.fullName || mergedData.fullName, | ||
| }; | ||
| if (data.firstName || data.lastName || data.name) { | ||
| useAuthStore.setState({ | ||
| user: { | ||
| ...authUser, | ||
| ...data, | ||
| name: updated.name, | ||
| fullName: updated.fullName, | ||
| }, | ||
| }); | ||
| } | ||
| set({ user: updated, loading: false, error: null }); | ||
| return updated; | ||
| } catch (error) { | ||
| set({ error: error.message, loading: false }); | ||
| return null; | ||
| set({ error: error.message || "Failed to update profile", loading: false }); | ||
| throw error; | ||
| } | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check all exported functions in auth.service.js for a name-update endpoint
rg -n 'export\s+const\s+\w+' src/services/auth.service.js
# Check if updateUser is called directly with name fields from the profile page
rg -n 'updateUser|updateProfile|updateHealth' src/pages/Profile/ --type=js -C3Repository: Revive-Graduation-Project/Revive-Front-End
Length of output: 2229
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant store/service files and inspect the precise implementations.
ast-grep outline src/store/profileStore.js --view expanded || true
ast-grep outline src/services/profile.service.js --view expanded || true
ast-grep outline src/services/auth.service.js --view expanded || true
printf '\n--- profileStore.js ---\n'
sed -n '1,220p' src/store/profileStore.js
printf '\n--- profile.service.js ---\n'
sed -n '1,220p' src/services/profile.service.js
printf '\n--- auth.service.js ---\n'
sed -n '1,220p' src/services/auth.service.js
printf '\n--- usages of updateUser in repo ---\n'
rg -n '\bupdateUser\b' src -C 2 || true
printf '\n--- usages of updateHealth in repo ---\n'
rg -n '\bupdateHealth\b' src -C 2 || trueRepository: Revive-Graduation-Project/Revive-Front-End
Length of output: 7574
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the client profile service used by profileStore.js.
fd -a 'clientProfileService.*' src || true
printf '\n--- repo files under src/services ---\n'
git ls-files 'src/services/*' || true
printf '\n--- clientProfileService.js ---\n'
sed -n '1,260p' src/services/clientProfileService.js
printf '\n--- references to updateProfile in src ---\n'
rg -n '\bupdateProfile\b' src -C 3 || true
printf '\n--- references to getProfile in src ---\n'
rg -n '\bgetProfile\b' src -C 3 || trueRepository: Revive-Graduation-Project/Revive-Front-End
Length of output: 8213
Name fields are stripped from the PUT payload. updateUser passes them through, but src/services/clientProfileService.js rebuilds the request body without firstName, lastName, name, or fullName, so those edits won’t persist after a refetch. Include them in the payload if this endpoint is the source of truth.
🤖 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 60 - 93, Update the request-body
construction in updateClientProfile so the PUT payload preserves firstName,
lastName, name, and fullName from updateUser’s mergedData. Keep the existing
profile fields and response handling unchanged, ensuring these name edits are
sent to the endpoint and persist after refetch.
| if (data.firstName || data.lastName || data.name) { | ||
| useAuthStore.setState({ | ||
| user: { | ||
| ...authUser, | ||
| ...data, | ||
| name: updated.name, | ||
| fullName: updated.fullName, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
fullName missing from auth store update condition.
The condition if (data.firstName || data.lastName || data.name) doesn't check data.fullName. If only fullName is updated, the auth store won't be synced, causing inconsistent name display across components reading from useAuthStore.
🐛 Proposed fix
- if (data.firstName || data.lastName || data.name) {
+ if (data.firstName || data.lastName || data.name || data.fullName) {📝 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.
| if (data.firstName || data.lastName || data.name) { | |
| useAuthStore.setState({ | |
| user: { | |
| ...authUser, | |
| ...data, | |
| name: updated.name, | |
| fullName: updated.fullName, | |
| }, | |
| }); | |
| } | |
| if (data.firstName || data.lastName || data.name || data.fullName) { | |
| useAuthStore.setState({ | |
| user: { | |
| ...authUser, | |
| ...data, | |
| name: updated.name, | |
| fullName: updated.fullName, | |
| }, | |
| }); | |
| } |
🤖 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 77 - 86, Update the condition
guarding the useAuthStore.setState call to also check data.fullName, so updates
containing only fullName still synchronize the auth store. Preserve the existing
user merge and updated.name/updated.fullName assignments.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes