Support multiple handles per platform type for partners#4109
Open
devkiran wants to merge 8 commits into
Open
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR updates partner-platform storage, verification, display, form handling, and validation to support multiple rows per platform type, keyed by ChangesMultiple partner platforms support
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/lib/social-utils.ts (1)
101-113: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject URL-like handles when no platform pattern matches.
If a user enters a URL for the wrong platform, no pattern has to match before Line 113 strips the path and invalid chars, so inputs like an Instagram URL in a Twitter field can become a bogus Twitter handle instead of failing validation.
Possible fix
+ const inputLooksLikeUrl = + /^https?:\/\//i.test(input.trim()) || + /^www\./i.test(input.trim()) || + /^[^/\s]+\.[^/\s]+\/.+/i.test(input.trim()); + let matchedPattern = false; + for (const pattern of patterns) { const match = handle.match(pattern); if (match) { handle = match[1]; + matchedPattern = true; break; } } + + if (inputLooksLikeUrl && !matchedPattern) { + return null; + } handle = handle.replace(/\/.*$/, "").replace(allowedChars, "");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/lib/social-utils.ts` around lines 101 - 113, The handle normalization logic in social-utils.ts currently strips paths and invalid characters even when no platform-specific pattern matches, which can let a URL for the wrong platform be treated as a valid handle. Update the normalization/validation flow around SOCIAL_PLATFORM_CONFIGS so that only recognized platform patterns are accepted; if no pattern matches and the input looks URL-like, reject it instead of falling through to the replace() cleanup. Use the existing handle parsing block to detect this case before the final cleanup step.
🧹 Nitpick comments (2)
apps/web/app/(ee)/api/cron/partner-platforms/enforce-limits/route.ts (2)
42-86: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winOne failing group aborts the whole run and can block subsequent groups.
If
deleteMany(orfindMany) throws for any single(partnerId, type)group,withCron's catch-all bubbles the error and terminates the entire run, leaving all groups after the failing one (per the deterministicpartnerId: "asc"order) unprocessed until the next scheduled run — and the failing group will keep blocking the same tail on every subsequent run too. Consider wrapping the per-group logic in a try/catch that logs and continues, so a single bad group doesn't stall enforcement for the rest of the backlog.♻️ Suggested per-group isolation
for (const { partnerId, type } of overLimitGroups) { + try { const platforms = await prisma.partnerPlatform.findMany({ where: { partnerId, type, }, select: { id: true, verifiedAt: true, createdAt: true, }, }); ... if (excessIds.length > 0) { await prisma.partnerPlatform.deleteMany({ where: { id: { in: excessIds } }, }); trimmedRows += excessIds.length; console.log(...); } + } catch (error) { + console.error( + `Failed to enforce platform limit for partner ${partnerId} (${type}):`, + error, + ); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/`(ee)/api/cron/partner-platforms/enforce-limits/route.ts around lines 42 - 86, The per-group enforcement loop in enforce-limits route currently lets a single failing partner/type group abort the whole cron run. Wrap the logic inside the overLimitGroups iteration (the prisma.partnerPlatform.findMany, sorting, and deleteMany work) in a try/catch so one bad group is logged and skipped while the loop continues processing later groups; keep the existing trimming behavior and logging in the successful path.
18-34: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valuePotential starvation with
take: 100+ deterministic ordering.Groups are always ordered by
partnerId: "asc"and capped at 100. If the backlog of over-limit groups ever exceeds 100, the same leading groups are processed every run while trailing groups can be starved indefinitely. Given this is meant as a safety net (should be rare), consider iterating until no groups remain over the limit, or use a less deterministic order (e.g., random/oldest-violation-first) to ensure fairness across runs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/`(ee)/api/cron/partner-platforms/enforce-limits/route.ts around lines 18 - 34, The partner-platform limit cron job is biased by the fixed `take: 100` plus `orderBy: { partnerId: "asc" }`, which can repeatedly process the same `groupBy` results in `route.ts`. Update the logic in the cron handler so `overLimitGroups` is drained fairly—either loop until `prisma.partnerPlatform.groupBy` returns no remaining over-limit groups, or replace the deterministic ordering with a fairer strategy such as oldest-violation-first or randomized selection. Keep the fix centered around the cron route’s over-limit query and processing flow so all `partnerId`/`type` groups eventually get handled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/app/`(ee)/api/partners/platforms/callback/route.ts:
- Around line 21-25: Runtime-validate the Redis OAuth state before destructuring
or using identifier, since redis.get<State>() only enforces TypeScript types and
can still return stale or malformed payloads. Update the callback route logic
around the State shape and the Redis read path to parse the stored state with
Zod before using it in the identifier-based lookup, and ensure the flow fails
safely if validation does not pass. Use the existing callback route handling and
the State interface as the main anchors when fixing this.
In `@apps/web/app/`(ee)/app.dub.co/embed/referrals/bounties/submission-form.tsx:
- Around line 207-208: The verified-platform check in the bounty submission flow
is too broad because SubmissionForm currently treats any verified partner
platform as sufficient. Update the logic around hasVerifiedPlatform to scope
verification to the bounty’s required platform type, using the matching platform
entry from partnerPlatforms instead of any verified item, so the “not verified”
warning still appears when the required type is unverified.
In `@apps/web/lib/actions/partners/update-partner-platforms.ts`:
- Around line 48-52: The updatePartnerPlatformsAction server action currently
relies only on partner membership auth, but it mutates partnerPlatform rows and
must also enforce partner_profile.update. Add a permission gate inside
updatePartnerPlatformsAction before any compute/delete/create logic, using the
existing auth context/permission pattern used elsewhere in the app, so users
without partner_profile.update cannot reach the row updates.
- Around line 87-138: The partner platform diff and limit validation in
updatePartnerPlatforms is happening before the write transaction, so concurrent
submissions can race and each pass the same baseline. Move the existing-platform
read, seen/existingKeys diffing, and per-type limit checks into the same
prisma.$transaction (or otherwise serialize by partnerId) so the
createMany/deleteMany decisions are made against a transactionally consistent
view.
In `@apps/web/lib/api/partner-profile/upsert-partner-platform.ts`:
- Around line 44-52: The limit enforcement in upsertPartnerPlatform is not
atomic because assertPartnerPlatformLimit runs before
prisma.partnerPlatform.create, allowing concurrent requests to تجاوز
MAX_PLATFORMS_PER_TYPE. Move the count check and creation into the same
transaction or row-locking flow in upsertPartnerPlatform/createPartnerPlatform
so the limit is enforced against a locked snapshot, and update any shared helper
used in the 55-83 path to use the same transactional strategy.
- Around line 27-40: The upsert logic in upsertPartnerPlatform is clearing an
already verified row by writing verifiedAt: null when verification is restarted
for the same handle. Update the existingPlatform branch to preserve the current
verifiedAt when the row is already verified and the incoming data.verifiedAt is
null or undefined, while still allowing metadata updates. Make this decision in
the existingPlatform update path by checking the current record’s verifiedAt
before building the prisma.partnerPlatform.update data.
In `@apps/web/lib/social-utils.ts`:
- Around line 4-7: The primary selection logic in social-utils currently breaks
ties by array order instead of actual recency, because PrimarySelectablePlatform
omits createdAt and the selection path in selectPrimary/related primary-ranking
logic can overwrite on equal rank. Update the selectable type to include
createdAt and compare it explicitly when choosing the “most recent” primary, or
else tighten the contract/comments so the input must already be pre-sorted by
recency. Use the existing primary-selection flow around
PrimarySelectablePlatform and the primary chooser methods in this file to keep
the tie-break behavior deterministic.
In `@apps/web/prisma/schema/platform.prisma`:
- Line 28: The schema change in the platform Prisma model removed the only guard
for the real platform key, so duplicate rows for the same
partnerId/type/identifier can now be created by concurrent upserts. Add a unique
constraint on the identifying trio used by upsertPartnerPlatform, and make sure
the migration first deduplicates any existing rows before applying the new
constraint. Keep the existing relaxed index for partnerId/type if still needed
for multiple handles.
In `@apps/web/ui/partners/bounties/bounty-social-content.tsx`:
- Around line 99-111: The social content requirements effect in
bounty-social-content.tsx is missing partner platform data in its dependency
list, so socialContentRequirementsMet can become stale when partner?.platforms
changes. Update the useEffect that calls evaluateSocialContentRequirements and
sets socialContentRequirementsMet to include partner?.platforms in the
dependency array, alongside data, bounty, and setSocialContentRequirementsMet,
so the checklist and submit gate recompute after the partner profile loads.
In `@apps/web/ui/partners/partner-platforms-form.tsx`:
- Around line 421-437: The add-more flow in PartnerPlatformsForm leaves newly
appended blank rows stuck because remove is only exposed for verified rows.
Update the row rendering in partner-platforms-form.tsx so PlatformFormRow can be
removed regardless of verification state, using the existing remove(index)
handler alongside fields.map, and make sure the same behavior is applied
anywhere else the add/remove row UI is duplicated (including the later
partner-platforms-form sections referenced by this diff).
---
Outside diff comments:
In `@apps/web/lib/social-utils.ts`:
- Around line 101-113: The handle normalization logic in social-utils.ts
currently strips paths and invalid characters even when no platform-specific
pattern matches, which can let a URL for the wrong platform be treated as a
valid handle. Update the normalization/validation flow around
SOCIAL_PLATFORM_CONFIGS so that only recognized platform patterns are accepted;
if no pattern matches and the input looks URL-like, reject it instead of falling
through to the replace() cleanup. Use the existing handle parsing block to
detect this case before the final cleanup step.
---
Nitpick comments:
In `@apps/web/app/`(ee)/api/cron/partner-platforms/enforce-limits/route.ts:
- Around line 42-86: The per-group enforcement loop in enforce-limits route
currently lets a single failing partner/type group abort the whole cron run.
Wrap the logic inside the overLimitGroups iteration (the
prisma.partnerPlatform.findMany, sorting, and deleteMany work) in a try/catch so
one bad group is logged and skipped while the loop continues processing later
groups; keep the existing trimming behavior and logging in the successful path.
- Around line 18-34: The partner-platform limit cron job is biased by the fixed
`take: 100` plus `orderBy: { partnerId: "asc" }`, which can repeatedly process
the same `groupBy` results in `route.ts`. Update the logic in the cron handler
so `overLimitGroups` is drained fairly—either loop until
`prisma.partnerPlatform.groupBy` returns no remaining over-limit groups, or
replace the deterministic ordering with a fairer strategy such as
oldest-violation-first or randomized selection. Keep the fix centered around the
cron route’s over-limit query and processing flow so all `partnerId`/`type`
groups eventually get handled.
🪄 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: 779f09d8-8f72-44e5-8dde-51eddb85225b
📒 Files selected for processing (23)
apps/web/app/(ee)/api/cron/partner-platforms/enforce-limits/route.tsapps/web/app/(ee)/api/partners/platforms/callback/route.tsapps/web/app/(ee)/app.dub.co/embed/referrals/bounties/submission-fields.tsxapps/web/app/(ee)/app.dub.co/embed/referrals/bounties/submission-form.tsxapps/web/app/(ee)/partners.dub.co/(onboarding)/onboarding/platforms/page.tsxapps/web/lib/actions/partners/start-partner-platform-verification.tsapps/web/lib/actions/partners/update-partner-platforms.tsapps/web/lib/actions/partners/verify-partner-website.tsapps/web/lib/actions/partners/verify-social-account-by-code.tsapps/web/lib/api/fraud/rules/check-partner-email-domain-mismatch.tsapps/web/lib/api/partner-profile/upsert-partner-platform.tsapps/web/lib/bounty/api/create-bounty-submission.tsapps/web/lib/firstpromoter/import-partners.tsapps/web/lib/partners/partner-platforms.tsapps/web/lib/social-utils.tsapps/web/lib/swr/use-partner-profile.tsapps/web/prisma/schema/platform.prismaapps/web/ui/modals/domain-verification-modal.tsxapps/web/ui/partners/bounties/bounty-social-content.tsxapps/web/ui/partners/bounties/evaluate-social-content-requirements.tsapps/web/ui/partners/partner-about.tsxapps/web/ui/partners/partner-platform-summary.tsxapps/web/ui/partners/partner-platforms-form.tsx
💤 Files with no reviewable changes (3)
- apps/web/lib/swr/use-partner-profile.ts
- apps/web/ui/partners/partner-about.tsx
- apps/web/lib/firstpromoter/import-partners.ts
Comment on lines
+44
to
+52
| return await prisma.partnerPlatform.create({ | ||
| data: { | ||
| partnerId, | ||
| type, | ||
| identifier, | ||
| verifiedAt: data?.verifiedAt, | ||
| metadata: data?.metadata, | ||
| }, | ||
| create: { | ||
| }); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make limit checks atomic with row creation.
assertPartnerPlatformLimit runs separately from the later create path, so concurrent requests can both observe fewer than MAX_PLATFORMS_PER_TYPE rows and create excess handles. Enforce the count in the same transaction/lock as creation, or add a database-backed constraint strategy for this invariant.
Also applies to: 55-83
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/lib/api/partner-profile/upsert-partner-platform.ts` around lines 44
- 52, The limit enforcement in upsertPartnerPlatform is not atomic because
assertPartnerPlatformLimit runs before prisma.partnerPlatform.create, allowing
concurrent requests to تجاوز MAX_PLATFORMS_PER_TYPE. Move the count check and
creation into the same transaction or row-locking flow in
upsertPartnerPlatform/createPartnerPlatform so the limit is enforced against a
locked snapshot, and update any shared helper used in the 55-83 path to use the
same transactional strategy.
Contributor
There was a problem hiding this comment.
Oops, something went wrong! Please try again later. 🐰 💔
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd start-partner-platform-verification.ts --exec cat -n {}Length of output: 11296
@devkiran, I took another look and wanted to flag something before marking this resolved.
Rate limiting (5 attempts/hour per partnerId:platform in startPartnerPlatformVerificationAction) does reduce the practical odds of a race, but the check-then-create pattern itself is still not atomic:
assertPartnerPlatformLimit(Lines 297-326) does afindManycount check on its own DB round trip.upsertPartnerPlatformthen separately doesfindFirst→create/update(Lines 12-29 inapps/web/lib/api/partner-profile/upsert-partner-platform.ts).- There's no shared transaction/lock spanning both, and the schema now uses
@@index([partnerId, type])(non-unique) rather than a DB-level constraint that could backstop the limit.
So two near-simultaneous verification requests for the same partner/platform could theoretically both pass the count check and both create rows, exceeding MAX_PLATFORMS_PER_TYPE. This is the same underlying issue as before, just less likely to trigger in practice now.
If you're comfortable with the residual (low-probability) risk given the rate limit, that's a reasonable trade-off to leave as-is. If you'd rather close the gap fully, you could wrap the count check and the create in a single prisma.$transaction(..., { isolationLevel: "Serializable" }), similar to the pattern already used in apps/web/lib/actions/partners/update-partner-platforms.ts.
Let me know how you'd like to proceed — happy to leave this as a known, low-risk trade-off if that's the intent.
🧠 Learnings used
Learnt from: devkiran
Repo: dubinc/dub PR: 3346
File: apps/web/lib/api/bounties/approve-bounty-submission.ts:165-165
Timestamp: 2026-01-13T12:06:42.476Z
Learning: Use 'noreply' as a sentinel for the replyTo email field: if replyTo === 'noreply', omit the replyTo property by spreading an empty object. Call sites can use replyTo: someEmail || 'noreply' to conditionally set replyTo without null/undefined. This pattern is broadly applicable across the TypeScript codebase; ensure this behavior is documented and that readability is preserved, using explicit conditional logic if the intent may be unclear.
Learnt from: devkiran
Repo: dubinc/dub PR: 3358
File: apps/web/app/(ee)/api/cron/partners/deactivate/route.ts:7-7
Timestamp: 2026-02-03T14:14:47.579Z
Learning: In all TypeScript files under apps/web, import zod using a namespace import: import * as z from 'zod/v4' and avoid using named import: import { z } from 'zod'. This ensures consistent API access and version specificity. During reviews, look for and replace any occurrences of import { z } from 'zod' with import * as z from 'zod/v4', and verify all zod usage continues to compile.
Learnt from: marcusljf
Repo: dubinc/dub PR: 3671
File: apps/web/ui/partners/program-card.tsx:93-93
Timestamp: 2026-03-31T20:45:16.716Z
Learning: In this codebase, Tailwind’s normal font weight (400) must use `font-normal`. Do not use `font-regular` (it is not a valid Tailwind utility here). When reviewing Tailwind class strings in `.ts`/`.tsx` files, flag any occurrence of `font-regular` and replace it with `font-normal` when the intent is weight 400.
Learnt from: devkiran
Repo: dubinc/dub PR: 3858
File: apps/web/lib/partner-referrals/create-referral-commission.ts:96-101
Timestamp: 2026-05-14T05:11:22.168Z
Learning: In this codebase, a customer belongs to exactly one program. During review of Prisma queries in apps/web/lib, if the query’s `where` clause already scopes by `customerId` (e.g., `prisma.commission.findFirst` with `where: { customerId, ... }`), then adding a redundant `programId` filter is unnecessary because the result is already implicitly scoped to the correct program/partner context. Only require `programId` when `customerId` is not part of the scoping (or is not reliably constrained), or when the query could otherwise return rows outside the customer’s unique program context.
Learnt from: devkiran
Repo: dubinc/dub PR: 3919
File: apps/web/app/(ee)/partners.dub.co/(apply)/[programSlug]/(default)/page.tsx:42-43
Timestamp: 2026-05-22T06:46:02.162Z
Learning: In the dubinc/dub codebase, `Project`/workspace Prisma model `environment` is defined as non-nullable (`default(production)`) and typed as `WorkspaceEnvironment`. Therefore, when you need to derive flags like `isNonProduction`, it’s safe to use a direct comparison such as `workspace.environment !== WorkspaceEnvironment.production` without guarding for `null`/`undefined`/unknown values. If the Prisma schema changes to make `environment` nullable or introduce an unknown state, then update the comparison logic accordingly.
Learnt from: pepeladeira
Repo: dubinc/dub PR: 4047
File: apps/web/lib/actions/partners/merge-partner-accounts.ts:343-355
Timestamp: 2026-06-17T21:48:06.200Z
Learning: In dubinc/dub code review, treat `triggerQStashWorkflow(...)` as a fire-and-forget call in production code: do not require capturing its return value or checking for `null` (or otherwise validating the return) and do not flag cases where the result is ignored. The only exception is e2e test helpers (e.g., files under `apps/web/**/api/e2e/**`): there, capturing the result and using it for assertions is expected—do flag when the e2e helper calls `triggerQStashWorkflow` but doesn’t capture/use the return value for the test checks.
Learnt from: devkiran
Repo: dubinc/dub PR: 4050
File: apps/web/lib/tapfiliate/import-customers.ts:77-83
Timestamp: 2026-06-19T06:11:40.095Z
Learning: In the dubinc/dub repo, if you query Prisma (e.g., `prisma.link.findMany`) using a `domain` filter, you can omit an additional `programId` filter **only when** the database/schema guarantees `domain` is unique per program (so the `domain` predicate already scopes results to the correct program). If that uniqueness guarantee is not enforced for the specific model/field set, include `programId` to prevent cross-program binding.
Learnt from: steven-tey
Repo: dubinc/dub PR: 4083
File: apps/web/app/(ee)/api/cron/webhooks/sync-click-workspaces/utils.ts:58-65
Timestamp: 2026-07-04T06:22:16.744Z
Learning: In the apps/web codebase (Prisma pinned to 6.19.1 in apps/web/package.json), it is supported to use the native `limit` argument in Prisma top-level `deleteMany()` and `updateMany()` calls (Prisma added this in v6.3.0). During review, do not flag `{ limit: ... }` usage inside `deleteMany`/`updateMany` filters (e.g., `prisma.model.deleteMany({ where, limit: 100 })` or `updateMany({ where, limit: 100, data: ... })`) as unsupported for this repository.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/app/`(ee)/api/admin/partners/[partnerId]/platforms/route.ts:
- Around line 93-134: The identifier-aware create/update flow in the partner
platform route is race-prone because `existingPlatform` is fetched with
`findFirst` and then conditionally created in separate steps. Update the
`prisma.partnerPlatform` write path to be atomic by enforcing a composite unique
constraint for the normalized `(partnerId, type, identifier)` boundary or by
using a transaction with duplicate handling around the existing `findFirst`,
`update`, and `create` logic so concurrent admin requests cannot create
duplicate rows.
In `@apps/web/app/`(ee)/api/admin/partners/[partnerId]/shared-platforms/route.ts:
- Around line 84-102: The shared-platform lookup in the route handler is
applying the `take: 10` limit too early, so broad `contains` website candidates
can crowd out exact matches before `platformMatches` does its
normalization/filtering. Update the `sharedPlatformMatches` query and the
`platformMatches` filtering logic so the limit is applied only after exact
website domain normalization (or filter per platform first), ensuring
`getDomainWithoutWWW`-based website matching does not lose valid results.
🪄 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: 09418bcf-21a0-4361-9260-32df02c4272a
📒 Files selected for processing (4)
apps/web/app/(ee)/api/admin/partners/[partnerId]/platforms/route.tsapps/web/app/(ee)/api/admin/partners/[partnerId]/shared-platforms/route.tsapps/web/lib/partners/partner-platforms.tsapps/web/ui/messages/messages-panel.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/web/ui/messages/messages-panel.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/lib/partners/partner-platforms.ts
Comment on lines
+84
to
+102
| let platformMatches: typeof sharedPlatformMatches; | ||
|
|
||
| // "contains" matches on the domain need exact verification | ||
| // (e.g. contains "example.com" would also match "notexample.com") | ||
| if (platform.type === "website") { | ||
| platformMatches = platformMatches.filter( | ||
| // "contains" matches on the domain need exact verification | ||
| // (e.g. contains "example.com" would also match "notexample.com") | ||
| const websiteDomain = getDomainWithoutWWW(platform.identifier); | ||
|
|
||
| platformMatches = sharedPlatformMatches.filter( | ||
| (match) => | ||
| match.type === platform.type && | ||
| websiteDomain !== null && | ||
| getDomainWithoutWWW(match.identifier) === websiteDomain, | ||
| ); | ||
| } else { | ||
| platformMatches = sharedPlatformMatches.filter( | ||
| (match) => | ||
| match.type === platform.type && | ||
| match.identifier === platform.identifier, | ||
| ); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don’t apply the global take: 10 before exact website filtering.
Website candidates are fetched with broad contains matching, then narrowed here. Because the DB query already applies take: 10, false positives like notexample.com can consume the limit before Line 91 filters them out, causing real shared platforms to be omitted. Move the limit after exact normalization, or query/filter per platform so each row gets complete exact matches.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/app/`(ee)/api/admin/partners/[partnerId]/shared-platforms/route.ts
around lines 84 - 102, The shared-platform lookup in the route handler is
applying the `take: 10` limit too early, so broad `contains` website candidates
can crowd out exact matches before `platformMatches` does its
normalization/filtering. Update the `sharedPlatformMatches` query and the
`platformMatches` filtering logic so the limit is applied only after exact
website domain normalization (or filter per platform first), ensuring
`getDomainWithoutWWW`-based website matching does not lose valid results.
Collaborator
Author
|
@coderabbitai, review again; fixed some of the issues mentioned above. |
Contributor
|
The PR description image shows the "Website and socials" settings form. It displays a title "Website and socials" with description text "Add your website and social accounts you use to share links. Verifying as many platforms as possible helps build trust with programs." On the right side there are input rows: "example.com" with a globe icon "Verify" button, two "youtube.com" rows (with "@ abc" and "@ hello" handles) each with a red YouTube "Verify" button, "x.com" with placeholder "handle" and an X icon "Verify" button, "instagram.com" with placeholder "handle" and Instagram icon "Verify" button, "tiktok.com" with placeholder "@ handle" and TikTok icon "Verify" button, and "linkedin.com/in" with placeholder "handle" and LinkedIn icon "Verify" button. Below is an "+ Add more" button, which is open showing a dropdown/popover menu listing "Website", "YouTube" (grayed out, labeled "Max 2" since two YouTube rows already exist), "X/Twitter", "Instagram", "TikTok", "LinkedIn". At the bottom right is a "Save changes" button. This confirms the multi-handle-per-platform feature (e.g., two YouTube rows) with a max-limit enforcement in the UI (YouTube disabled once limit of 2 reached), matching the
✅ Action performedReview finished.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit