Skip to content

Conversation

@KacpiDrewniak
Copy link
Collaborator

No description provided.

@vercel
Copy link

vercel bot commented Oct 21, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
b2c-marketplace-storefront-4m3w Canceled Canceled Oct 21, 2025 8:20am
b2c-plugin-test Ready Ready Preview Comment Oct 21, 2025 8:20am
b2c-sandbox Canceled Canceled Oct 21, 2025 8:20am
mercurb2c-testing Canceled Canceled Oct 21, 2025 8:20am

💡 Enable Vercel Agent with $100 free credit for automated AI reviews

@KacpiDrewniak KacpiDrewniak requested a review from pfulara October 21, 2025 08:10
# Conflicts:
#	src/lib/data/products.ts
Comment on lines +18 to +22
const sortOptions = [
{ label: "Newest", value: "created_at" },
{ label: "Price: Low to High", value: "price_asc" },
{ label: "Price: High to Low", value: "price_desc" },
]
Copy link
Collaborator

Choose a reason for hiding this comment

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

this already exists in code, how about moving this to consts?

]

export default function Wishlist() {
const [user, setUser] = useState<any>(null)
Copy link
Collaborator

Choose a reason for hiding this comment

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

please type this without any

Comment on lines +52 to +64
const sortProducts = (products: any[]) => {
if (!products) return []

return [...products].sort((a, b) => {
if (sortBy === "created_at") {
return new Date(b.created_at).getTime() - new Date(a.created_at).getTime()
} else if (sortBy === "price_asc") {
return a.calculated_amount - b.calculated_amount
} else if (sortBy === "price_desc") {
return b.calculated_amount - a.calculated_amount
}
return 0
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. couldn't we sort on BE side?
  2. if we don't, there's helper function for FE sorting in lib/helpers

Copy link
Contributor

Choose a reason for hiding this comment

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

Of course, the only right way is to do it on BE side.

const [wishlist, setWishlist] = useState<WishlistType[]>([])
const [sortBy, setSortBy] = useState("created_at")
const [loading, setLoading] = useState(true)
const [totalCount, setTotalCount] = useState(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the point of tocalCount, if it's not used anywhere anyway?

@AlanJanicki
Copy link
Contributor

AlanJanicki commented Oct 22, 2025

IMO approach to handle pagination and sorting on FE side, using endpoint in current state - is fundamentally wrong.

I strongly believe that BE needs to provide an endpoint that will allow to fetch user’s wishlist's products paginated and sorted data (relevance, createdAt_desc, updatedAt_desc). Currently there is only endpoint /store/wisthlist which returns array of wishlists so front for now iterates over wishlists?.[0].items with no pagination and sorting. FE imo needs to iterate over wishlist's (one wishlist - there cannot be multiple) items and wishlists itself.

I strongly advice to not merge current approach.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants