Conversation
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 1 minutes and 51 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds subscription UI and behavior for upcoming Memes mints, introduces an ArtistPill UI component, moves per-subscription row logic into a reusable MemeSubscriptionRow, updates NowMinting components to use ArtistPill, and adds tests for LatestDropNextMintSubscribe. All changes are component additions/refactors and tests only. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant LatestDrop as LatestDropNextMintSubscribe
participant ReactQuery as React Query
participant API as Server API
participant Row as MemeSubscriptionRow
participant Auth as Auth Context
LatestDrop->>ReactQuery: query(details) / query(status)
ReactQuery->>API: GET /subscriptions/consolidation/details/{profileKey}
ReactQuery->>API: GET /subscriptions/consolidation/upcoming-memes/{tokenId}/{profileKey}
API-->>ReactQuery: details & status
ReactQuery-->>LatestDrop: data
LatestDrop->>Row: render(subscription, eligibility, minting_today, refresh)
User->>Row: toggle subscribe / change count
Row->>Auth: requestAuth()
Auth-->>Row: auth context
Row->>API: POST /subscriptions/{profileKey}/subscription
Row->>API: POST /subscriptions/{profileKey}/subscription-count
API-->>Row: success/error
Row->>ReactQuery: invalidate/refetch subscription queries
Row->>User: show toast / update UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
|
@CodeRabbit review |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
components/home/now-minting/ArtistPill.tsx (1)
21-26: Gate the identity lookup behind a real lookup key.Line 22 always calls
useIdentity, butcomponents/home/now-minting/NowMintingHeader.tsxnow renders a name-only pill with noprofileHandle, andcomponents/home/now-minting/LatestDropNextMintSection.tsxalready passespfpdirectly. That means this shared pill either asks the hook to resolve""or repeats a fetch whose result is ignored. Please skip the lookup unless a realprofileHandleis present and the avatar is actually needed.Please confirm that
useIdentityexplicitly no-ops on emptyhandleOrWalletvalues before relying on this path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/home/now-minting/ArtistPill.tsx` around lines 21 - 26, The ArtistPill currently always calls useIdentity with handleOrWallet: profileHandle ?? "" which triggers unnecessary lookups; change the component to only call useIdentity when a real profileHandle exists and the avatar is needed (e.g., when pfp is not provided), and derive resolvedPfp accordingly (pfp ?? profile?.pfp ?? null) only when the hook was invoked; additionally, verify and/or update the useIdentity implementation so it explicitly no-ops (returns null/idle state) when handleOrWallet is empty to avoid accidental fetches — adjust ArtistPill to gate calling useIdentity by a truthy profileHandle and avatar requirement and rely on pfp prop when present (affecting components like NowMintingHeader and LatestDropNextMintSection that may pass no handle or pass pfp directly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/home/now-minting/LatestDropNextMintSubscribe.tsx`:
- Around line 30-31: The component LatestDropNextMintSubscribe currently freezes
tokenId by calling useMemo(() => getCanonicalNextMintNumber(), []) which can
become stale across auth changes; remove the empty-deps memoization or include
the auth-derived dependencies (e.g., connectedProfile, activeProfileProxy) so
tokenId is recomputed when auth state changes, and update any places using
tokenId (the query key that includes tokenId) to rely on the fresh value; also
add a unit/test that simulates changing connectedProfile/activeProfileProxy and
asserts the component re-derives getCanonicalNextMintNumber() and uses the
updated tokenId for fetching/mutations.
In `@components/user/subscriptions/MemeSubscriptionRow.tsx`:
- Around line 82-91: The row can get stuck because setIsSubmitting(true) is set
before calling requestAuth() and any rejection from requestAuth() is not caught;
update both submit and handleUpdateSubscriptionCount to ensure isSubmitting is
cleared on exceptions by wrapping the await requestAuth() call in a try/catch
(or enclosing the auth + subsequent logic in a try/finally) so that on error you
call setIsSubmitting(false) before returning or rethrowing; reference the submit
and handleUpdateSubscriptionCount functions and ensure the error path always
clears isSubmitting.
- Around line 57-61: The component MemeSubscriptionRow forces selectedCount back
to 1 when props.eligibilityCount is 0, which leaves an empty <select> and an
enabled toggle; change the logic in the useEffect(s) that currently do
setSelectedCount(Math.max(1, props.eligibilityCount)) (the block referencing
selectedCount and props.eligibilityCount) to handle eligibilityCount === 0
explicitly by setting selectedCount to 0 instead. Also update any UI/toggle
enablement checks (the toggle control and any handlers that initiate
subscription changes) to disable the toggle and prevent subscription attempts
when props.eligibilityCount === 0, and ensure the submit/change handler (the
function that performs the subscription update) guards against selectedCount ===
0 before proceeding. Apply the same fix pattern to the other similar blocks
mentioned (the ranges around 197-229 and 333-368) so the component consistently
treats eligibilityCount === 0 as ineligible and disables actions.
---
Nitpick comments:
In `@components/home/now-minting/ArtistPill.tsx`:
- Around line 21-26: The ArtistPill currently always calls useIdentity with
handleOrWallet: profileHandle ?? "" which triggers unnecessary lookups; change
the component to only call useIdentity when a real profileHandle exists and the
avatar is needed (e.g., when pfp is not provided), and derive resolvedPfp
accordingly (pfp ?? profile?.pfp ?? null) only when the hook was invoked;
additionally, verify and/or update the useIdentity implementation so it
explicitly no-ops (returns null/idle state) when handleOrWallet is empty to
avoid accidental fetches — adjust ArtistPill to gate calling useIdentity by a
truthy profileHandle and avatar requirement and rely on pfp prop when present
(affecting components like NowMintingHeader and LatestDropNextMintSection that
may pass no handle or pass pfp directly).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3e8790a3-e874-40bf-bd49-433bbdd388e1
📒 Files selected for processing (7)
__tests__/components/home/LatestDropNextMintSubscribe.test.tsxcomponents/home/now-minting/ArtistPill.tsxcomponents/home/now-minting/LatestDropNextMintSection.tsxcomponents/home/now-minting/LatestDropNextMintSubscribe.tsxcomponents/home/now-minting/NowMintingHeader.tsxcomponents/user/subscriptions/MemeSubscriptionRow.tsxcomponents/user/subscriptions/UserPageSubscriptionsUpcoming.tsx
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/user/subscriptions/MemeSubscriptionRow.tsx (2)
91-107:⚠️ Potential issue | 🟠 MajorKeep
isSubmittingrecoverable if auth rejects.Lines 95-99 and 150-154 still run
requestAuth()outside thetry/finally. If it rejects instead of returning{ success: false },isSubmittingnever clears, the spinner stays up, and Line 202 also leaks that rejection through the fire-and-forget count update.💡 Suggested fix
const submit = async (): Promise<void> => { if (isSubmitting || props.minting_today) { return; } - setIsSubmitting(true); - const { success } = await requestAuth(); - if (!success) { - setIsSubmitting(false); - return; - } - const subscribe = !subscribed; interface SubscribeBody { contract: string; token_id: number; subscribed: boolean; } try { + setIsSubmitting(true); + const { success } = await requestAuth(); + if (!success) { + return; + } + const subscribe = !subscribed; const response = await commonApiPost<SubscribeBody, SubscribeBody>({Apply the same structure to
handleUpdateSubscriptionCount().Also applies to: 144-161, 199-202
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/user/subscriptions/MemeSubscriptionRow.tsx` around lines 91 - 107, The submit function currently sets isSubmitting before calling requestAuth() outside a try/finally, so if requestAuth throws the spinner never clears; update submit to call requestAuth() inside a try block and ensure setIsSubmitting(false) is executed in a finally so isSubmitting is always reset on success, failure, or thrown error; do the same for handleUpdateSubscriptionCount() (and any other spots around lines noted) so requestAuth() and any fire-and-forget updates are wrapped with try/catch/finally, catch/log errors appropriately and ensure setIsSubmitting is cleared in the finally block while preserving the existing subscribed toggle and side-effect behavior.
61-65:⚠️ Potential issue | 🟠 MajorHandle
eligibilityCount === 0as an ineligible state.Line 63 still clamps back to
1when there are zero eligible slots, sorenderCountSelector()can still render a controlled<select>with no options. Since the caller forwards0unchanged, and bothToggleinstances plussubmit()still allow the unsubscribed path, this component can still attempt a subscribe mutation with no legal quantity.💡 Suggested fix
useEffect(() => { if (selectedCount > props.eligibilityCount) { - setSelectedCount(Math.max(1, props.eligibilityCount)); + setSelectedCount( + props.eligibilityCount === 0 ? 0 : props.eligibilityCount + ); } }, [props.eligibilityCount, selectedCount]); const submit = async (): Promise<void> => { - if (isSubmitting || props.minting_today) { + if ( + isSubmitting || + props.minting_today || + (!subscribed && props.eligibilityCount < 1) + ) { return; }const renderCountSelector = ({ selectClassName, disableWhenSingleOption, }: { selectClassName: string; disableWhenSingleOption: boolean; }) => { - if (!subscribed) { + if (!subscribed || props.eligibilityCount < 1) { return null; }Mirror the same
(!subscribed && props.eligibilityCount < 1)guard in bothToggle.disabledexpressions.Also applies to: 91-99, 205-237, 249-255, 354-360
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/user/subscriptions/MemeSubscriptionRow.tsx` around lines 61 - 65, The useEffect that clamps selectedCount currently forces a minimum of 1 even when props.eligibilityCount === 0, enabling renderCountSelector to render a controlled <select> with no options and allowing subscribe paths; update the clamp in the useEffect (references: useEffect, selectedCount, setSelectedCount, props.eligibilityCount) to allow selectedCount to become 0 when eligibilityCount is 0 (e.g., clamp to Math.max(0, props.eligibilityCount)), and also add the same ineligibility guard to both Toggle.disabled expressions (references: Toggle.disabled, subscribed, props.eligibilityCount) so the toggles are disabled when (!subscribed && props.eligibilityCount < 1); apply the same pattern at the other affected blocks noted in the comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/user/subscriptions/MemeSubscriptionRow.tsx`:
- Around line 150-155: The optimistic update to selectedCount is happening
before authentication and isn't rolled back when requestAuth() returns {
success: false }; locate the calls to setSelectedCount / selectedCount updates
in MemeSubscriptionRow (the places that call setIsSubmitting(true) then await
requestAuth()) and either move the optimistic setSelectedCount so it happens
only after requestAuth() succeeds, or if you prefer to keep it optimistic,
restore subscribedCount/selectedCount by calling
setSelectedCount(subscribedCount) before returning when requestAuth() fails;
ensure both occurrences (the two spots where setSelectedCount is updated around
requestAuth()) are corrected and setIsSubmitting(false) is still called on the
failure path.
---
Duplicate comments:
In `@components/user/subscriptions/MemeSubscriptionRow.tsx`:
- Around line 91-107: The submit function currently sets isSubmitting before
calling requestAuth() outside a try/finally, so if requestAuth throws the
spinner never clears; update submit to call requestAuth() inside a try block and
ensure setIsSubmitting(false) is executed in a finally so isSubmitting is always
reset on success, failure, or thrown error; do the same for
handleUpdateSubscriptionCount() (and any other spots around lines noted) so
requestAuth() and any fire-and-forget updates are wrapped with
try/catch/finally, catch/log errors appropriately and ensure setIsSubmitting is
cleared in the finally block while preserving the existing subscribed toggle and
side-effect behavior.
- Around line 61-65: The useEffect that clamps selectedCount currently forces a
minimum of 1 even when props.eligibilityCount === 0, enabling
renderCountSelector to render a controlled <select> with no options and allowing
subscribe paths; update the clamp in the useEffect (references: useEffect,
selectedCount, setSelectedCount, props.eligibilityCount) to allow selectedCount
to become 0 when eligibilityCount is 0 (e.g., clamp to Math.max(0,
props.eligibilityCount)), and also add the same ineligibility guard to both
Toggle.disabled expressions (references: Toggle.disabled, subscribed,
props.eligibilityCount) so the toggles are disabled when (!subscribed &&
props.eligibilityCount < 1); apply the same pattern at the other affected blocks
noted in the comment.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3b21787a-fcc8-47b8-bfbf-6b2275ddd341
📒 Files selected for processing (1)
components/user/subscriptions/MemeSubscriptionRow.tsx
|



Summary by CodeRabbit
New Features
Tests