Feature: Password Reset - #52
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: 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: 4
🧹 Nitpick comments (6)
src/pages/customization/Customization.jsx (1)
6-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid inserting declarations between import statements.
const customizeData = [];sits between twoimportlines; move it below all imports for readability and to avoid tripping import-order lint rules.Suggested reorder
import CustomizeHero from "./Sections/CustomizeHero"; import BaseSelector from "./Sections/BaseSelector"; import IngredientsSection from "./Sections/IngredientsSection"; import CommentBox from "./Sections/CommentBox"; -const customizeData = []; import SummaryBox from "./Sections/SummaryBox"; + +const customizeData = [];🤖 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/customization/Customization.jsx` around lines 6 - 8, The module-level declaration customizeData is placed between import statements, which breaks import grouping and may fail linting. Move the const customizeData = [] declaration below all imports in Customization.jsx, keeping the import section contiguous and preserving the existing CommentBox and SummaryBox imports.tests/kitchen-integration.spec.js (3)
13-14: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winHardcoded credentials in test spec.
Login/checkout credentials are hardcoded. Prefer environment variables (e.g. via
.envused elsewhere in this cohort) to avoid leaking or having to update credentials across test files.Also applies to: 32-34
🤖 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 `@tests/kitchen-integration.spec.js` around lines 13 - 14, The login and checkout flow in the kitchen integration spec uses hardcoded credentials, which should be replaced with environment-backed values. Update the page.fill calls in the test so they read the email/password from env vars (consistent with the cohort’s existing .env usage) and apply the same change in the other hardcoded credential spots referenced in this spec, keeping the test names and flow unchanged.
58-82: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy liftColumn count assertions assume an empty/clean kitchen board.
toHaveCount(1)on queue/preparing/ready/done columns (Lines 59, 66, 73, 82) assumes no pre-existing orders in the board. Against a shared or non-reset backend, this will be flaky. Consider scoping assertions to the specific created order (e.g., locate by order ID/reference) instead of raw column counts.🤖 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 `@tests/kitchen-integration.spec.js` around lines 58 - 82, The kitchen integration test is asserting raw column counts in Order Queue, Preparing, Ready, and Done, which assumes a clean board and makes the test flaky. Update the checks in kitchen-integration.spec.js to target the specific created order instead of counting all .group items, using the existing page locators around the queueColumn, prepColumn, and readyColumn flow. Keep the move-and-verify steps, but assert on the order’s unique ID/reference within each column so the test passes even when other orders already exist.
10-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAbsolute URLs bypass the configured
baseURL.
playwright.config.jssetsbaseURL: 'http://localhost:5173', but everypage.gotohere uses an absolute URL. Per Playwright's own docs, "When you use baseURL, Playwright expects you to provide a relative path in your goto calls. If you provide an absolute URL (one that starts with http:// or https://), Playwright will ignore the baseURL entirely." This defeats the purpose of configuringbaseURLand means changing it (e.g. for CI/staging) won't affect this spec.♻️ Proposed fix
- await page.goto('http://localhost:5173/auth/login'); + await page.goto('/auth/login'); ... - await page.goto('http://localhost:5173/menu'); + await page.goto('/menu'); ... - await page.goto('http://localhost:5173/checkout'); + await page.goto('/checkout'); ... - await page.goto('http://localhost:5173/dashboard/live-kitchen'); + await page.goto('/dashboard/live-kitchen');Also applies to: 24-24, 29-29, 54-54
🤖 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 `@tests/kitchen-integration.spec.js` at line 10, The kitchen integration spec is hardcoding absolute URLs in page.goto calls, so Playwright ignores the configured baseURL. Update the affected navigations in the test to use relative paths instead of full http://localhost:5173 URLs, keeping the existing route segments like /auth/login so the baseURL from playwright.config.js is actually applied.test-results/.last-run.json (1)
1-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGenerated test artifact shouldn't be committed.
test-results/.last-run.jsonis Playwright's auto-generated run-status output, regenerated on every test run. Committing it adds noise and merge conflicts; addtest-results/to.gitignoreinstead.🤖 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 `@test-results/.last-run.json` around lines 1 - 4, Remove the generated Playwright artifact from version control and ignore it going forward: delete test-results/.last-run.json from the commit and update the repo ignore rules so the entire test-results/ directory is excluded. Use the test-results/.last-run.json artifact itself as the signal that this is auto-generated output and make the change in the project’s gitignore configuration.playwright.config.js (1)
6-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winESLint
no-undefonprocess— fix via environment config, not code.
processis a Node global; this file runs under Node, so ESLint needs a Node environment configured for config files (or a.eslintrcoverride /languageOptions.globals.nodein flat config), rather than treating this as an application bug.🔧 Example ESLint fix (flat config)
+import globals from 'globals'; + export default [ + { + files: ['playwright.config.js'], + languageOptions: { globals: globals.node }, + }, // ...existing 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 `@playwright.config.js` around lines 6 - 8, ESLint is incorrectly flagging the Node global process in the Playwright config, so this should be fixed in lint configuration rather than in playwright.config.js. Update the ESLint setup to treat config files as Node محیط (for example via a .eslintrc override or flat-config languageOptions.globals.node), so the existing use of process in the Playwright configuration is accepted without changing the config code.Source: Linters/SAST tools
🤖 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/pages/customization/Customization.jsx`:
- Line 7: The customization page is hardcoded to use an empty customizeData
array, so BaseSelector never receives meal options. Update Customization.jsx to
source customizeData from the existing restaurant store/fetch flow used
elsewhere in the app instead of a static [] value, and pass that live data into
BaseSelector so the base-meal list can render correctly.
In `@src/services/api.js`:
- Around line 5-23: The getBaseURL helper in api setup returns VITE_API_URL_DEV
and VITE_API_URL_PROD without any fallback or validation, so make those branches
fail safely if the env var is missing. Update getBaseURL to verify the selected
environment URL is present before axios.create uses it, and either fall back to
a sensible default or throw a clear configuration error; keep the behavior
consistent with the local branch and ensure api always gets a valid baseURL.
- Around line 80-95: The JWT expiry parsing in the refresh flow is using atob
directly on the token payload, which breaks for base64url-encoded JWTs. Update
the decode logic in the refresh handling block around setAccessToken and
refreshQueue to convert token.split('.')[1] from base64url to standard base64
before calling atob, then parse the payload exp as before. Keep the fallback
expiresAt only for decode/parse failures, not for valid JWTs.
In `@tests/kitchen-integration.spec.js`:
- Around line 8-20: The test.beforeEach login flow in
kitchen-integration.spec.js is swallowing failures by catching the error and
only logging it, which lets later tests run with an unauthenticated page. Update
the beforeEach logic around page.goto, page.fill, page.click, and
page.waitForURL so that login errors are not suppressed; either remove the
try/catch or rethrow after logging. Keep the failure tied to the login step so
the test stops immediately when authentication does not succeed.
---
Nitpick comments:
In `@playwright.config.js`:
- Around line 6-8: ESLint is incorrectly flagging the Node global process in the
Playwright config, so this should be fixed in lint configuration rather than in
playwright.config.js. Update the ESLint setup to treat config files as Node محیط
(for example via a .eslintrc override or flat-config
languageOptions.globals.node), so the existing use of process in the Playwright
configuration is accepted without changing the config code.
In `@src/pages/customization/Customization.jsx`:
- Around line 6-8: The module-level declaration customizeData is placed between
import statements, which breaks import grouping and may fail linting. Move the
const customizeData = [] declaration below all imports in Customization.jsx,
keeping the import section contiguous and preserving the existing CommentBox and
SummaryBox imports.
In `@test-results/.last-run.json`:
- Around line 1-4: Remove the generated Playwright artifact from version control
and ignore it going forward: delete test-results/.last-run.json from the commit
and update the repo ignore rules so the entire test-results/ directory is
excluded. Use the test-results/.last-run.json artifact itself as the signal that
this is auto-generated output and make the change in the project’s gitignore
configuration.
In `@tests/kitchen-integration.spec.js`:
- Around line 13-14: The login and checkout flow in the kitchen integration spec
uses hardcoded credentials, which should be replaced with environment-backed
values. Update the page.fill calls in the test so they read the email/password
from env vars (consistent with the cohort’s existing .env usage) and apply the
same change in the other hardcoded credential spots referenced in this spec,
keeping the test names and flow unchanged.
- Around line 58-82: The kitchen integration test is asserting raw column counts
in Order Queue, Preparing, Ready, and Done, which assumes a clean board and
makes the test flaky. Update the checks in kitchen-integration.spec.js to target
the specific created order instead of counting all .group items, using the
existing page locators around the queueColumn, prepColumn, and readyColumn flow.
Keep the move-and-verify steps, but assert on the order’s unique ID/reference
within each column so the test passes even when other orders already exist.
- Line 10: The kitchen integration spec is hardcoding absolute URLs in page.goto
calls, so Playwright ignores the configured baseURL. Update the affected
navigations in the test to use relative paths instead of full
http://localhost:5173 URLs, keeping the existing route segments like /auth/login
so the baseURL from playwright.config.js is actually applied.
🪄 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: cc3fcd5b-c2d5-407c-907a-57f69c7d01eb
⛔ Files ignored due to path filters (6)
login-timeout.pngis excluded by!**/*.pngsrc/mocks/meal-images/beef-steak.jpgis excluded by!**/*.jpgsrc/mocks/meal-images/chicken-cold-cut.jpgis excluded by!**/*.jpgsrc/mocks/meal-images/green-salad.jpgis excluded by!**/*.jpgsrc/mocks/meal-images/mix-protein.jpgis excluded by!**/*.jpgsrc/mocks/meal-images/salmon.jpgis excluded by!**/*.jpg
📒 Files selected for processing (18)
.envplaywright-report/index.htmlplaywright.config.jssrc/mocks/dashboardMock.jssrc/mocks/enums.jssrc/mocks/handlers.jssrc/mocks/index.jssrc/mocks/meal-images/index.jssrc/mocks/meals.jssrc/mocks/orders.jssrc/mocks/users.jssrc/pages/OrderFlow/Cart.jsxsrc/pages/customization/Customization.jsxsrc/services/api.jssrc/store/restaurantStore.jssrc/tests/services/api.test.jstest-results/.last-run.jsontests/kitchen-integration.spec.js
💤 Files with no reviewable changes (8)
- src/mocks/enums.js
- src/mocks/index.js
- src/mocks/meals.js
- src/mocks/users.js
- src/mocks/orders.js
- src/mocks/dashboardMock.js
- src/mocks/meal-images/index.js
- src/mocks/handlers.js
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai resolve |
✅ Action performedComments resolved. Approval is disabled; enable |
1 participant
Implementation of password reset functionality.
Summary by CodeRabbit
Bug Fixes
Chores