feat(onboarding): remove a social during verify-socials (chat#1889 row 9) - #1894
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 15 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d571997972
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| className="text-muted-foreground" | ||
| aria-label={`Remove ${platform} link`} | ||
| disabled={isSubmitting} | ||
| onClick={() => void onRemove(social.id)} |
There was a problem hiding this comment.
When this button removes an artist's final profile, useSocialRemove refreshes that artist to account_socials: [], but getOnboardingCheckpoints defines the socials checkpoint as complete only when every artist has at least one social. Consequently, the targeted user who removes their only incorrect auto-match can click “continue” but will be returned to an incomplete socials step on the next home landing and cannot complete onboarding without re-adding a profile or skipping. Persist an explicit verified-with-none state or change the checkpoint semantics before allowing the final profile to be removed.
Useful? React with 👍 / 👎.
| isFixing={ | ||
| fixingArtistId === artist.account_id || removingSocialId !== null | ||
| } |
There was a problem hiding this comment.
While a fix is pending for artist A, cards for every other artist still receive isFixing={false}, so their new Remove buttons remain enabled. A removal can therefore overlap useSocialFix; both hooks then call the same getArtists/fetchQuery, and if the second mutation completes while the first roster refetch is in flight, React Query can share that in-flight request and cache a snapshot taken before the second mutation. The removed row can reappear until a later refetch, and retrying it may produce a not-found error. Treat any non-null fixingArtistId as globally busy, as is already done for removals, or otherwise serialize/invalidate these mutations.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 6 files
Confidence score: 3/5
- In
components/Onboarding/SocialRow.tsx, removing an artist’s last social can flip the onboarding checkpoint to incomplete while navigation can still advance, creating inconsistent step state and a likely user-flow regression — block last-social removal or immediately enforce/add a replacement before allowing progression. - In
components/Onboarding/VerifySocialsStep.tsx, theremovingSocialId !== nullcondition disables allArtistSocialsCardinstances during one removal, so unrelated artists are temporarily blocked from fixes/removals and the step can feel frozen — scope disablement to the affected artist/card only. - In
hooks/onboarding/useSocialRemove.ts, a roster refresh failure after a successful DELETE is surfaced as a failed removal, leaving stale UI and misleading error state — treat delete success separately and update local roster state (with retry/fallback) even if the refresh call fails.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="hooks/onboarding/useSocialRemove.ts">
<violation number="1" location="hooks/onboarding/useSocialRemove.ts:34">
P2: A post-delete roster refresh failure is reported as a failed removal even though the DELETE already succeeded. The stale row remains visible and `false` leaves the UI in its pre-delete state; update the local roster optimistically (or handle refresh failure separately) once `deleteArtistSocial` resolves.</violation>
</file>
<file name="components/Onboarding/VerifySocialsStep.tsx">
<violation number="1" location="components/Onboarding/VerifySocialsStep.tsx:38">
P2: When a removal is in progress, `removingSocialId !== null` disables all `ArtistSocialsCard` components, not just the card for the artist whose social is being removed. During a fix, only the affected artist's card is disabled via `fixingArtistId === artist.account_id`. This inconsistency means removing from artist A also disables artist B's add and fix actions. Track the removing artist ID in `useSocialRemove` alongside (or instead of) the social ID so the disable can be per-artist like the fix path.</violation>
<violation number="2" location="components/Onboarding/VerifySocialsStep.tsx:39">
P2: The disabling logic is asymmetric: a pending removal globally disables all cards (`removingSocialId !== null`), but a pending fix only disables the card for the artist being fixed. This means Remove buttons on *other* artists remain clickable while a fix is in flight, and both mutations call `getArtists()` on completion — a concurrent refetch can cache stale data and briefly resurrect a removed row. Treat any non-null `fixingArtistId` as a global busy signal as well (i.e., `fixingArtistId !== null || removingSocialId !== null`) to serialize the mutations.</violation>
</file>
<file name="components/Onboarding/SocialRow.tsx">
<violation number="1" location="components/Onboarding/SocialRow.tsx:82">
P1: When this Remove button deletes the artist's only remaining social, the onboarding checkpoint (which gates on every artist having at least one social) will mark the socials step incomplete. The user can click "continue" but will be looped back on the next landing. Either guard this removal when it would leave `account_socials` empty (e.g., confirm or skip), persist an explicit "verified-with-none" state, or update the checkpoint semantics to treat an empty-after-removal list as complete.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| className="text-muted-foreground" | ||
| aria-label={`Remove ${platform} link`} | ||
| disabled={isSubmitting} | ||
| onClick={() => void onRemove(social.id)} |
There was a problem hiding this comment.
P1: When this Remove button deletes the artist's only remaining social, the onboarding checkpoint (which gates on every artist having at least one social) will mark the socials step incomplete. The user can click "continue" but will be looped back on the next landing. Either guard this removal when it would leave account_socials empty (e.g., confirm or skip), persist an explicit "verified-with-none" state, or update the checkpoint semantics to treat an empty-after-removal list as complete.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/Onboarding/SocialRow.tsx, line 82:
<comment>When this Remove button deletes the artist's only remaining social, the onboarding checkpoint (which gates on every artist having at least one social) will mark the socials step incomplete. The user can click "continue" but will be looped back on the next landing. Either guard this removal when it would leave `account_socials` empty (e.g., confirm or skip), persist an explicit "verified-with-none" state, or update the checkpoint semantics to treat an empty-after-removal list as complete.</comment>
<file context>
@@ -51,17 +60,30 @@ const SocialRow = ({ social, isSubmitting, onFix }: SocialRowProps) => {
+ className="text-muted-foreground"
+ aria-label={`Remove ${platform} link`}
+ disabled={isSubmitting}
+ onClick={() => void onRemove(social.id)}
+ >
+ <Trash2 className="size-4" />
</file context>
| throw new Error("Please sign in to remove a social"); | ||
| } | ||
| await deleteArtistSocial(accessToken, artist.account_id, socialId); | ||
| await getArtists(); |
There was a problem hiding this comment.
P2: A post-delete roster refresh failure is reported as a failed removal even though the DELETE already succeeded. The stale row remains visible and false leaves the UI in its pre-delete state; update the local roster optimistically (or handle refresh failure separately) once deleteArtistSocial resolves.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/onboarding/useSocialRemove.ts, line 34:
<comment>A post-delete roster refresh failure is reported as a failed removal even though the DELETE already succeeded. The stale row remains visible and `false` leaves the UI in its pre-delete state; update the local roster optimistically (or handle refresh failure separately) once `deleteArtistSocial` resolves.</comment>
<file context>
@@ -0,0 +1,47 @@
+ throw new Error("Please sign in to remove a social");
+ }
+ await deleteArtistSocial(accessToken, artist.account_id, socialId);
+ await getArtists();
+ return true;
+ } catch (error) {
</file context>
| key={artist.account_id} | ||
| artist={artist} | ||
| isFixing={fixingArtistId === artist.account_id} | ||
| isFixing={ |
There was a problem hiding this comment.
P2: When a removal is in progress, removingSocialId !== null disables all ArtistSocialsCard components, not just the card for the artist whose social is being removed. During a fix, only the affected artist's card is disabled via fixingArtistId === artist.account_id. This inconsistency means removing from artist A also disables artist B's add and fix actions. Track the removing artist ID in useSocialRemove alongside (or instead of) the social ID so the disable can be per-artist like the fix path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/Onboarding/VerifySocialsStep.tsx, line 38:
<comment>When a removal is in progress, `removingSocialId !== null` disables all `ArtistSocialsCard` components, not just the card for the artist whose social is being removed. During a fix, only the affected artist's card is disabled via `fixingArtistId === artist.account_id`. This inconsistency means removing from artist A also disables artist B's add and fix actions. Track the removing artist ID in `useSocialRemove` alongside (or instead of) the social ID so the disable can be per-artist like the fix path.</comment>
<file context>
@@ -33,8 +35,11 @@ const VerifySocialsStep = ({ onConfirmed }: { onConfirmed: () => void }) => {
key={artist.account_id}
artist={artist}
- isFixing={fixingArtistId === artist.account_id}
+ isFixing={
+ fixingArtistId === artist.account_id || removingSocialId !== null
+ }
</file context>
| artist={artist} | ||
| isFixing={fixingArtistId === artist.account_id} | ||
| isFixing={ | ||
| fixingArtistId === artist.account_id || removingSocialId !== null |
There was a problem hiding this comment.
P2: The disabling logic is asymmetric: a pending removal globally disables all cards (removingSocialId !== null), but a pending fix only disables the card for the artist being fixed. This means Remove buttons on other artists remain clickable while a fix is in flight, and both mutations call getArtists() on completion — a concurrent refetch can cache stale data and briefly resurrect a removed row. Treat any non-null fixingArtistId as a global busy signal as well (i.e., fixingArtistId !== null || removingSocialId !== null) to serialize the mutations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/Onboarding/VerifySocialsStep.tsx, line 39:
<comment>The disabling logic is asymmetric: a pending removal globally disables all cards (`removingSocialId !== null`), but a pending fix only disables the card for the artist being fixed. This means Remove buttons on *other* artists remain clickable while a fix is in flight, and both mutations call `getArtists()` on completion — a concurrent refetch can cache stale data and briefly resurrect a removed row. Treat any non-null `fixingArtistId` as a global busy signal as well (i.e., `fixingArtistId !== null || removingSocialId !== null`) to serialize the mutations.</comment>
<file context>
@@ -33,8 +35,11 @@ const VerifySocialsStep = ({ onConfirmed }: { onConfirmed: () => void }) => {
artist={artist}
- isFixing={fixingArtistId === artist.account_id}
+ isFixing={
+ fixingArtistId === artist.account_id || removingSocialId !== null
+ }
onFix={(url) => fixSocial(artist, url)}
</file context>
| fixingArtistId === artist.account_id || removingSocialId !== null | |
| fixingArtistId !== null || removingSocialId !== null |
chat#1889 matrix row 9 (onboarding blocker).
useSocialFix could only add or replace a profile. A user who added the wrong
social had no way to take it back and was left with a list they could not
correct, which reads as a dead end in the middle of setup.
- New deleteArtistSocial wraps the existing
DELETE /api/artists/{id}/socials/{socialId} endpoint.
- New useSocialRemove hook mutates + refreshes the roster, mirroring
useSocialFix.
- SocialRow gains a Remove affordance beside Edit; ArtistSocialsCard and
VerifySocialsStep thread it through.
- The no-matches copy now states adding is optional and the step can be
continued without it, instead of reading like an error.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
d571997 to
0635e4b
Compare
| > | ||
| <Pencil className="size-4" /> | ||
| </Button> | ||
| <div className="flex shrink-0 items-center"> |
There was a problem hiding this comment.
OCP
- actual: new component code added to existing component.
- required: new component code for the buttons section is defined in a new file.
There was a problem hiding this comment.
Done in ceda34ca — extracted to components/Onboarding/SocialRowActions.tsx (one exported component; Edit + Remove cluster with the a11y labels intact). SocialRow composes it and dropped its now-unused Button/icon imports. 272 tests pass; tsc at the 8 baseline; eslint clean.
Review feedback on chat#1894: the edit/remove button cluster was net-new component code inline in SocialRow. It now lives in SocialRowActions.tsx; SocialRow composes it, so a future per-social action is a new prop wire, not another inline block.
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Auto-approval blocked by 4 unresolved issues from previous reviews.
Re-trigger cubic
Preview verification — the dead end is gone, round-trip provenRebased onto latest
The instrumented walk
Incidental finding — narrows chat#1889 row 23's scopeThe re-added row carries the real handle and follower count ( Ready to merge. |
1 participant
Matrix row 9 in chat#1889 — flagged as an onboarding blocker. Independent of the convergence chain.
Why
useSocialFixexposed onlyfixSocial(add or replace). There was no delete. So a user who added the wrong profile — or who had a bad auto-match they wanted gone rather than corrected — was stuck with a list they could not fix, mid-setup. Combined with a no-matches state whose copy read like an error, the step felt like a dead end.What changed
lib/artists/deleteArtistSocial.ts— wraps the already existingDELETE /api/artists/{id}/socials/{socialId}endpoint. No api or docs change needed.hooks/onboarding/useSocialRemove.ts— mutate + refresh the roster, mirroringuseSocialFix's shape (own hook rather than a second verb onuseSocialFix, per one-export-per-file).SocialRowgains a Remove affordance beside Edit, with a per-platformaria-label.ArtistSocialsCard/VerifySocialsStepthread it through; rows disable while a removal is in flight.Verification
TDD, red → green:
```
RED — no remove affordance exists
× offers a remove affordance for a social the user does not want
Tests 1 failed | 1 passed (2)
GREEN
pnpm exec vitest run components/Onboarding
Test Files 1 passed (1)
Tests 2 passed (2)
```
The second test guards that Edit survives alongside Remove, so this can't silently replace the fix path.
pnpm exec tsc --noEmitclean for the touched files. Preview click-through pending (needs an authed roster with matched socials).Tracked in chat#1889 (matrix row 9).
Summary by cubic
Adds a Remove action to verify-socials so users can delete a wrongly matched profile and continue onboarding. Addresses chat#1889 (row 9) by unblocking the step.
New Features
deleteArtistSocialwrapsDELETE /api/artists/{id}/socials/{socialId}.useSocialRemovedeletes a social and refreshes the roster.SocialRowadds Remove beside Edit; wired throughArtistSocialsCardandVerifySocialsStep; rows disable during removal.Refactors
SocialRowActionsinto its own file;SocialRowcomposes it for Edit and Remove.Written for commit ceda34c. Summary will update on new commits.