Skip to content

Add wishlist feature and update FitMart#600

Open
Akshatsrii wants to merge 4 commits into
parthbuilds-community:mainfrom
Akshatsrii:main
Open

Add wishlist feature and update FitMart#600
Akshatsrii wants to merge 4 commits into
parthbuilds-community:mainfrom
Akshatsrii:main

Conversation

@Akshatsrii

@Akshatsrii Akshatsrii commented Jun 12, 2026

Copy link
Copy Markdown

📋 What does this PR do?

Adds a full-stack Wishlist feature that allows authenticated users to save products for later and manage them from a dedicated wishlist page.

📂 File Structure Changes

Backend

  • Added server/models/Wishlist.js – Wishlist schema
  • Added server/routes/wishlist.js – Wishlist API routes
  • Updated server/index.js – Registered /api/wishlist route

Frontend

  • Added client/src/hooks/useWishlist.js – Custom hook for wishlist operations
  • Added client/src/components/WishlistButton.jsx – Reusable wishlist toggle button
  • Added client/src/pages/WishlistPage.jsx – Wishlist page UI
  • Updated client/src/components/Navbar.jsx – Wishlist icon with count badge
  • Updated client/src/App.jsx – Added /wishlist route
  • Updated client/src/pages/HomePage.jsx – Integrated wishlist functionality

🔗 Related Issue

N/A

🧪 How was this tested?

  • Signed in with a test account
  • Added products to the wishlist using the heart icon
  • Verified the wishlist count in the navbar
  • Opened the Wishlist page and confirmed products were displayed
  • Tested Add to Cart and Remove actions
  • Signed out and signed back in to verify wishlist persistence

📸 Screenshots (if UI changes)

N/A

✅ Checklist

  • I've read the CONTRIBUTING guide
  • My code follows the project's style guidelines
  • I've tested my changes locally
  • I've linked the related issue
  • I haven't introduced any new secrets or API keys

@github-actions github-actions Bot added enhancement New feature or request refactor Improve code without changing functionality UI UI-related improvements including layouts, responsiveness, styling, animations, and UX enhancements. frontend backend labels Jun 12, 2026
@Akshatsrii Akshatsrii closed this Jun 12, 2026
@Akshatsrii Akshatsrii reopened this Jun 12, 2026
AnshulSharma-11

This comment was marked as duplicate.

@AnshulSharma-11 AnshulSharma-11 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice implementation of the wishlist feature with a clean separation between UI (WishlistButton), state management (useWishlist) and backend APIs
The optimistic update pattern improves ux
but useWishlist could suffer from stale state issues since wishlistIds is included in the useCallback dependency array
Consider adding loading/error feedback for wishlist actions and validating that users can only access their own wishlist on the backend

@github-actions github-actions Bot added the changes-requested PR requires changes before it can be merged. label Jun 13, 2026
@Akshatsrii

Copy link
Copy Markdown
Author

Thanks for the detailed review! Here's what I've addressed:
Stale state fix — Replaced wishlistIds in the useCallback dependency array with a useRef that stays in sync via a useEffect. Now toggle only depends on user, eliminating stale closure issues entirely.
Loading + error feedback — Added a loadingIds Set to track per-item loading state. The heart button now shows a spinner (Loader2) while the request is in flight and disables itself to prevent duplicate calls. Error state is also returned from the hook so the UI can surface failures and the optimistic update reverts cleanly on error.
Backend ownership validation — Added a verifyOwnership middleware that compares req.user.uid (set by verifyFirebaseToken) against req.params.userId and returns a 403 Forbidden if they don't match. Applied it to all four wishlist routes — GET, ADD, REMOVE, and DELETE.
Let me know if anything else needs attention!

@Tanusree-samanta

Copy link
Copy Markdown

Thanks for the thorough implementation and for addressing AnshulSharma-11's review points so promptly!

A few observations from looking over the changes:

What's well done:

  • Clean separation of concerns — WishlistButton, useWishlist, and the backend routes are nicely decoupled.
  • The useRef fix for stale closure in useCallback is the right approach.
  • Per-item loading state with loadingIds is a good UX touch — prevents duplicate requests cleanly.
  • Backend ownership validation via verifyOwnership middleware is a solid security addition.

A few things worth considering:

  • It would be helpful to see screenshots or a short screen recording of the wishlist flow in action, especially the loading spinner and error revert behavior — just to confirm the UX feels smooth end-to-end.
  • Has the verifyOwnership middleware been tested for edge cases, like a missing or malformed userId param? A 400 response for invalid input alongside the 403 for unauthorized access might be worth adding.
  • Minor: the checklist notes "I've linked the related issue" but the PR lists N/A — just worth keeping consistent for future PRs.

Overall the implementation looks solid. The author has been responsive to feedback which is great to see!

@github-actions github-actions Bot added the needs-review Pull request is awaiting review from maintainers. label Jun 15, 2026
@Akshatsrii

Copy link
Copy Markdown
Author

Thank you for the feedback!
I've addressed the verifyOwnership suggestion by adding the necessary validation. I believe all of the originally requested review points have now been resolved.
Please let me know if there's anything else that needs attention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend changes-requested PR requires changes before it can be merged. enhancement New feature or request frontend needs-review Pull request is awaiting review from maintainers. refactor Improve code without changing functionality UI UI-related improvements including layouts, responsiveness, styling, animations, and UX enhancements.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants