Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds per-variant membership support to the merch product page: introduces a Variant type, computes activeMemberLists from the selected variant or uniform lists across variants, and adds membership checks with an in-memory membershipCache, activeMembershipKey guarding async responses, and caching invalidation on login/logout. Integrates membership-aware pricing and UI (shouldShowMembershipStatus, renderMembershipStatus), per-variant max-quantity calculation (getMaxQuantity), Turnstile verification and token handling, unified API error payload handling with 404 behavior, and an error/verification modal. Also makes ApiVariant.memberLists optional and defaults LegacyItem.memberLists to an empty array. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Deploying with Cloudflare Pages
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/(membership)/merch/page.tsx (1)
159-172:⚠️ Potential issue | 🟠 MajorNon‑404 product fetch failures are silently ignored.
If the API returns anything other than 404, the UI remains blank and no modal opens. Route those errors through the shared handler (and mark the fetch as failed so the close handler can redirect).
🔧 Suggested fix
.catch((error) => { if (error.response && error.response.status === 404) { setTimeout(() => { setErrorMessage({ title: "Error retrieving product", code: 404, message: error.response.data.message, }); setMerchList({ failed: true }); setIsLoading(false); modalErrorMessage.onOpen(); }, 1000); + } else { + setMerchList({ failed: true }); + handleApiError(error); } });
🤖 Fix all issues with AI agents
In `@src/app/`(membership)/merch/page.tsx:
- Around line 324-329: validateQuantity currently allows positive quantities
even when getMaxQuantity(size) returns 0; update the function so that after
computing const maxQty = getMaxQuantity(size) it returns false when maxQty === 0
(i.e., treat zero availability as invalid for a selected size). Keep the
existing NaN and >max checks, but add the explicit maxQty === 0 check
(referencing validateQuantity, getMaxQuantity, size, and maxQty) so any attempt
to pick a quantity when a size has zero stock is rejected.
- Around line 457-517: The purchase flow must be blocked while membership status
is unresolved: update the main purchase Button (the component using
onPress={purchaseHandler} and props isDisabled and isLoading) to also disable
when isPaidMember === null (e.g., include "|| isPaidMember === null" in its
isDisabled expression) and optionally change the button label to indicate
membership check (e.g., show "Checking membership..." when isPaidMember ===
null); ensure any client-side validation that gates purchase (isFormValidated,
totalCapacity(), isLoading) remains intact.
- Around line 539-552: Selected size display names are being stored/used as
variantIds (causing failures in getMaxQuantity and checkout); map the selected
display name back to its actual variantId from merchList.variants and use that
variantId for validation and payloads: update changeSize (or introduce a helper)
to look up merchList.variants for the id given the name, store the variantId in
size (or keep both name/id), and ensure getMaxQuantity reads
merchList.total_avail[variantId] and checkout payloads send variantId (not the
display name). Reference: Select/SelectItem, changeSize, getMaxQuantity,
merchList.total_avail and merchList.variants.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/(membership)/merch-store/transform.ts (1)
34-47: 🧹 Nitpick | 🔵 TrivialType
variantsto includememberListsfor stronger guarantees.UI code now consumes
variant.memberLists, butLegacyItemdoesn’t declare avariantsshape, so you lose type checking. Consider typing it explicitly.🔧 Suggested type update
interface LegacyItem { member_price: string; nonmember_price: string; item_image: string; sizes: string[]; item_price: { paid: number; others: number }; eventDetails: string; item_id: string; total_sold: Record<string, number>; total_avail: Record<string, number>; limit_per_person: number; item_sales_active_utc: number; item_name: string; + variants?: { id: string; name: string; memberLists: string[] }[]; }Also applies to: 96-100
🤖 Fix all issues with AI agents
In `@src/app/`(membership)/merch/page.tsx:
- Around line 113-117: Guard against stale async membership checks by
introducing an "active cache key" ref and resetting member state when a new
check begins: create a ref like activeMembershipKeyRef = useRef<string |
null>(null), set activeMembershipKeyRef.current to the cache key whenever you
start a membership lookup (before calling setIsCheckingMembership(true) and
before early-return using membershipCache), clear/set setIsPaidMember(false)
when a new check starts, and when the async call resolves only apply
setIsPaidMember(result) and membershipCache.set(key, result) if
activeMembershipKeyRef.current === key (ignore/stale otherwise); also clear
activeMembershipKeyRef.current and setIsCheckingMembership(false) after handling
the response. Ensure this pattern replaces direct uses of membershipCache,
isCheckingMembership, setIsCheckingMembership and setIsPaidMember in the
membership lookup logic so out-of-order responses for the previous variant are
ignored.
- Around line 226-233: getMaxQuantity currently assumes merchList.total_avail
has per-variant keys and returns 0 when merchList.total_avail[variantId] is
missing; change it to treat PER_PRODUCT inventory by falling back to
merchList.total_avail.total when the variantId entry is absent — compute
available as Math.min((merchList.total_avail[variantId] ||
merchList.total_avail.total || 0), 10) and then apply merchList.limit_per_person
cap (using Math.min) before returning; update the logic inside getMaxQuantity to
use that fallback.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.