feat: enhance meal recommendation system and update profile handling - #62
Closed
NorhanElyann wants to merge 3 commits into
Closed
feat: enhance meal recommendation system and update profile handling#62NorhanElyann wants to merge 3 commits into
NorhanElyann wants to merge 3 commits into
Conversation
❌ Deploy Preview for revive-front-end failed.
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThe PR updates environment-based API selection, rewires suggested meals around an authenticated AI recommendation flow with a guest teaser, migrates profile APIs and store access to id-scoped client endpoints, and changes Axios request and 401 refresh handling. ChangesRecommendations, profile, and auth refactor
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant HomeOrMenu
participant RecommendationStore
participant RecommendationService
participant MenuAPI
participant AIAPI
HomeOrMenu->>RecommendationStore: fetchRecommendations()
RecommendationStore->>RecommendationService: getSuggestedMeals()
RecommendationService->>MenuAPI: getMenu() and profile lookup
RecommendationService->>AIAPI: POST user, meals, top_n
AIAPI-->>RecommendationService: recommendations
RecommendationService-->>RecommendationStore: enriched meals
sequenceDiagram
participant Client
participant ApiClient
participant AuthStore
participant RefreshService
Client->>ApiClient: request
ApiClient-->>Client: 401 response
ApiClient->>AuthStore: check refresh state and tokens
ApiClient->>RefreshService: restoreSessionService()
RefreshService-->>ApiClient: new token
ApiClient->>AuthStore: setAccessToken(..., expiresAt)
ApiClient-->>Client: retry queued request
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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
🧹 Nitpick comments (2)
src/store/recommendationStore.js (1)
27-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale JSDoc and vestigial
roleargument.The docstring still describes the removed
param/userId-or-object contract, andgetSuggestedMeals(role)passes an argument the service function (no-arg signature perrecommendation.service.js) silently ignores. Update the doc and drop the unused argument for clarity.📝 Suggested fix
/** * Fetch AI recommendations / suggested meals from backend. - * Can be passed a userId directly, an object containing userId/id, - * or will fallback to the currently authenticated user in authStore. - * - * `@param` {string|number|Object} [param] - userId or context object + * Derives the current user's role from authStore; requires an + * eligible authenticated user before fetching. */ fetchRecommendations: async () => { ... - const response = await getSuggestedMeals(role); + const response = await getSuggestedMeals();Also applies to: 45-45
🤖 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/recommendationStore.js` around lines 27 - 33, The JSDoc above the recommendation store method is stale and still describes the removed param/userId-or-object behavior, and the `getSuggestedMeals` call in `recommendationStore` is passing a vestigial `role` argument to a no-arg service function. Update the documentation to match the current API, and remove the unused argument from the `getSuggestedMeals` call so the store aligns with `recommendation.service.js` and the method names stay clear.src/services/recommendation.service.js (1)
6-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded external AI endpoint URL.
The AI endpoint URL is hardcoded rather than sourced from an environment variable, inconsistent with the env-driven config approach used elsewhere in this PR stack (e.g.,
api.js'sBASE_URLSkeyed byVITE_ENV). This makes it hard to point at a staging/mock AI backend without a code change.♻️ Suggested fix
-const AI_API_URL = - "https://youssef-ashraf-healthy-meal-ai-api.hf.space/recommend"; +const AI_API_URL = + import.meta.env.VITE_AI_RECOMMENDATION_URL ?? + "https://youssef-ashraf-healthy-meal-ai-api.hf.space/recommend";🤖 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/recommendation.service.js` around lines 6 - 7, The AI endpoint in recommendation.service.js is hardcoded, so update the AI_API_URL constant to read from an environment-driven config instead of a fixed URL. Follow the same pattern used in api.js with BASE_URLS and VITE_ENV so the recommendation service can target staging or mock backends without code changes; keep the fallback behavior explicit if the env var is missing.
🤖 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 @.env:
- Line 6: The committed .env default is pointing local/dev runs at the
production backend because VITE_ENV is set to prod and is consumed by
src/services/api.js for the Axios baseURL. Change the default environment value
in .env to a local-safe option like local or dev, and ensure prod is only
supplied through deployment-specific environment configuration. Keep the fix
scoped to the VITE_ENV setting so the API client behavior remains driven by the
environment value.
In `@src/pages/Home/Sections/SuggestedMeals.jsx`:
- Around line 24-26: The useEffect in SuggestedMeals returns early when
user?.role is missing, which leaves loading stuck at true and the spinner
visible forever. Update the effect so the no-role path still resolves the
loading state by calling setLoading(false) before returning, and make sure any
other early-exit paths in this effect also clear loading. Keep the fix within
the SuggestedMeals useEffect logic that controls loading.
In `@src/pages/Profile/Profile.jsx`:
- Around line 25-29: The Profile page is passing only user into InfoGrid, but
InfoGrid.jsx reads all display fields from its profile prop and defaults to an
empty object, so the health/profile values render as placeholders. Update the
Profile component to pass the same data into InfoGrid via profile={user} (or
equivalent) while keeping user as needed, matching how HealthForm uses
initial={user} and reads the fields directly from user.
- Line 21: ProfileHeader is still rendering joinDate but Profile.jsx only passes
onEdit, so the join date field will be blank. Update the ProfileHeader usage in
Profile and/or the ProfileHeader component so joinDate is passed and rendered
again, keeping the prop name consistent with the destructuring in ProfileHeader.
In `@src/services/api.js`:
- Around line 65-66: Skip the 401 refresh flow for auth endpoints in the api
interceptor so invalid login/signup responses don’t trigger
restoreSessionService(). Update the 401 handling in api.js to check the original
request URL/path before retrying, and bypass the token refresh logic for
/auth/login and /auth/signup. Use the existing interceptor block around
originalRequest._retry and the 401 branch to keep the original auth error
visible to callers.
- Around line 37-43: The request interceptor in api.js is setting X-User-Role
from useAuthStore/user.role, which is mutable client state and should not be
trusted for authorization or scoping. Remove this header assignment from the API
client and ensure any role-based decision comes from the server-side session or
access token instead, keeping only the authenticated bearer token logic in the
interceptor.
In `@src/services/recommendation.service.js`:
- Around line 19-29: In recommendation.service.js, the Promise.allSettled flow
in the profile/meals fetch needs two fixes: fail fast when
useAuthStore.getState().user?.id is missing instead of calling api.get with an
undefined id, and preserve the original rejection details when a request fails.
Update the fetch logic around the api.get call and the Promise.allSettled checks
so the profile request is only made with a valid user id, and when profileRes or
mealsRes is rejected, surface profileRes.reason or mealsRes.reason in the thrown
error from this function rather than replacing it with a generic message.
- Around line 34-40: Add a timeout to the AI request in recommendation service
and access the response defensively. In `recommendation.service.js`, update the
`axios.post` call used for `AI_API_URL` so a slow external Space cannot hang the
flow, then validate `aiResponse.data` before reading `recommendations` in the
same `recommendations` assignment path. If the payload is null or malformed,
fall back to an empty list rather than assuming `data.recommendations` exists.
In `@src/services/user.service.js`:
- Around line 23-30: The uploadProfilePicture helper is setting a manual
multipart Content-Type header on a FormData request, which can prevent the
browser/Axios from adding the required boundary. Update uploadProfilePicture to
send the FormData without an explicit Content-Type header so the multipart
request is constructed correctly.
In `@src/store/profileStore.js`:
- Around line 12-16: Bind the profile cache to the current auth user in
fetchProfile. The early return in profileStore should not reuse get().user
unless its id matches useAuthStore.getState().user?.id, otherwise a persisted
profile from a previous session can leak across logins. Update the fetchProfile
flow in profileStore to compare the cached profile against the active auth user
before short-circuiting, or clear the profile store from authStore.logout so
stale data cannot be reused.
In `@src/store/recommendationStore.js`:
- Around line 35-45: The fetch gate in fetchRecommendations currently only
checks that a user role exists, so non-client users can still reach the
client-only profile endpoint through getSuggestedMeals(). Update the role check
in recommendationStore’s fetchRecommendations flow to allow only client accounts
to proceed, and for staff/admin (or any non-client role) return early with
recommendations cleared and loading stopped before calling getSuggestedMeals().
Use the existing fetchRecommendations and getSuggestedMeals symbols to place the
guard in the right spot.
---
Nitpick comments:
In `@src/services/recommendation.service.js`:
- Around line 6-7: The AI endpoint in recommendation.service.js is hardcoded, so
update the AI_API_URL constant to read from an environment-driven config instead
of a fixed URL. Follow the same pattern used in api.js with BASE_URLS and
VITE_ENV so the recommendation service can target staging or mock backends
without code changes; keep the fallback behavior explicit if the env var is
missing.
In `@src/store/recommendationStore.js`:
- Around line 27-33: The JSDoc above the recommendation store method is stale
and still describes the removed param/userId-or-object behavior, and the
`getSuggestedMeals` call in `recommendationStore` is passing a vestigial `role`
argument to a no-arg service function. Update the documentation to match the
current API, and remove the unused argument from the `getSuggestedMeals` call so
the store aligns with `recommendation.service.js` and the method names stay
clear.
🪄 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: 099ababf-7955-4a06-a261-fa71bc27c66d
📒 Files selected for processing (8)
.envsrc/pages/Home/Sections/SuggestedMeals.jsxsrc/pages/Profile/Profile.jsxsrc/services/api.jssrc/services/recommendation.service.jssrc/services/user.service.jssrc/store/profileStore.jssrc/store/recommendationStore.js
|
|
||
| # Set environment to local, dev, or prod | ||
| VITE_ENV=local | ||
| VITE_ENV=prod |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify which env files define VITE_ENV and API URLs.
fd -H '^\.env' . -x sh -c 'echo "### $1"; sed -n "1,80p" "$1"' sh {}
rg -n 'VITE_ENV|VITE_API_URL_' .Repository: Revive-Graduation-Project/Revive-Front-End
Length of output: 32560
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '## package.json\n'
sed -n '1,220p' package.json
printf '\n## .gitignore\n'
sed -n '1,220p' .gitignore
printf '\n## README.md (env references)\n'
rg -n 'VITE_ENV|\.env|VITE_API_URL|local|prod|dev' README.md docs . 2>/dev/null | head -n 200
printf '\n## src/services/api.js\n'
sed -n '1,140p' src/services/api.jsRepository: Revive-Graduation-Project/Revive-Front-End
Length of output: 50400
Keep the committed .env default local-safe.
.env:6 sets VITE_ENV=prod, and src/services/api.js uses that to pick Axios baseURL, so local/dev runs will point at the production backend. Set the committed default to local/dev and move prod to deployment-specific env config.
🤖 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 @.env at line 6, The committed .env default is pointing local/dev runs at the
production backend because VITE_ENV is set to prod and is consumed by
src/services/api.js for the Axios baseURL. Change the default environment value
in .env to a local-safe option like local or dev, and ensure prod is only
supplied through deployment-specific environment configuration. Keep the fix
scoped to the VITE_ENV setting so the API client behavior remains driven by the
environment value.
Comment on lines
+19
to
+29
| const [profileRes, mealsRes] = await Promise.allSettled([ | ||
| api.get(`/api/clients/profile/${useAuthStore.getState().user?.id}`), | ||
| getMenu(), | ||
| ]); | ||
|
|
||
| if (profileRes.status !== "fulfilled") { | ||
| throw new Error("Failed to fetch user profile"); | ||
| } | ||
| if (mealsRes.status !== "fulfilled") { | ||
| throw new Error("Failed to fetch meals"); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
No guard for missing user id; original fetch errors are discarded.
If useAuthStore.getState().user?.id is undefined, the request is fired against /api/clients/profile/undefined instead of failing fast. Also, the thrown errors on lines 25/28 discard profileRes.reason/mealsRes.reason, hiding the actual cause (network error vs 404 vs auth failure) from callers/logs.
🛡️ Suggested fix
+ const userId = useAuthStore.getState().user?.id;
+ if (!userId) {
+ throw new Error("User is not authenticated");
+ }
+
const [profileRes, mealsRes] = await Promise.allSettled([
- api.get(`/api/clients/profile/${useAuthStore.getState().user?.id}`),
+ api.get(`/api/clients/profile/${userId}`),
getMenu(),
]);
if (profileRes.status !== "fulfilled") {
- throw new Error("Failed to fetch user profile");
+ throw new Error(profileRes.reason?.message || "Failed to fetch user profile");
}
if (mealsRes.status !== "fulfilled") {
- throw new Error("Failed to fetch meals");
+ throw new Error(mealsRes.reason?.message || "Failed to fetch meals");
}📝 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.
Suggested change
| const [profileRes, mealsRes] = await Promise.allSettled([ | |
| api.get(`/api/clients/profile/${useAuthStore.getState().user?.id}`), | |
| getMenu(), | |
| ]); | |
| if (profileRes.status !== "fulfilled") { | |
| throw new Error("Failed to fetch user profile"); | |
| } | |
| if (mealsRes.status !== "fulfilled") { | |
| throw new Error("Failed to fetch meals"); | |
| } | |
| const userId = useAuthStore.getState().user?.id; | |
| if (!userId) { | |
| throw new Error("User is not authenticated"); | |
| } | |
| const [profileRes, mealsRes] = await Promise.allSettled([ | |
| api.get(`/api/clients/profile/${userId}`), | |
| getMenu(), | |
| ]); | |
| if (profileRes.status !== "fulfilled") { | |
| throw new Error(profileRes.reason?.message || "Failed to fetch user profile"); | |
| } | |
| if (mealsRes.status !== "fulfilled") { | |
| throw new Error(mealsRes.reason?.message || "Failed to fetch meals"); | |
| } |
🤖 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/recommendation.service.js` around lines 19 - 29, In
recommendation.service.js, the Promise.allSettled flow in the profile/meals
fetch needs two fixes: fail fast when useAuthStore.getState().user?.id is
missing instead of calling api.get with an undefined id, and preserve the
original rejection details when a request fails. Update the fetch logic around
the api.get call and the Promise.allSettled checks so the profile request is
only made with a valid user id, and when profileRes or mealsRes is rejected,
surface profileRes.reason or mealsRes.reason in the thrown error from this
function rather than replacing it with a generic message.
…enhance meal recommendation handling
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit