Limit bounty eligibility to selected partner tags#4045
Open
devkiran wants to merge 24 commits into
Open
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds partner tag-based bounty eligibility alongside the existing partner group mechanism. Introduces ChangesPartner Tag Bounty Eligibility
Sequence Diagram(s)sequenceDiagram
participant Partner
participant BountyRoute as Partner bounty route
participant getProgramEnrollmentOrThrow
participant getBountyOrThrow
participant isPartnerEligibleForBounty
Partner->>BountyRoute: GET bounty details
BountyRoute->>getProgramEnrollmentOrThrow: include programPartnerTags
getProgramEnrollmentOrThrow-->>BountyRoute: partnerGroupId + partnerTagIds
BountyRoute->>getBountyOrThrow: include bountyEligibilityIncludes
getBountyOrThrow-->>BountyRoute: bountyGroupIds + bountyTagIds
BountyRoute->>isPartnerEligibleForBounty: evaluate eligibility
isPartnerEligibleForBounty-->>BountyRoute: true / false
sequenceDiagram
participant Trigger as triggerDraftBountySubmissionCreation
participant Prisma
participant isPartnerEligibleForBounty
participant QStash
Trigger->>Prisma: load eligible bounties + enrollments
Prisma-->>Trigger: bounty and partner tag data
loop per bounty
Trigger->>isPartnerEligibleForBounty: group IDs + tag IDs
isPartnerEligibleForBounty-->>Trigger: eligible partner IDs
Trigger->>QStash: publishJSON draft submissions
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![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: 5
🧹 Nitpick comments (1)
apps/web/lib/api/tags/throw-if-invalid-partner-tag-ids.ts (1)
24-26: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winUse a
Setfor invalid-tag detection to avoid quadratic scans.The current
filter + somecheck is O(n*m). ASetmakes this O(n) and keeps behavior identical.♻️ Suggested refactor
- const invalidPartnerTagIds = partnerTagIds?.filter( - (partnerTagId) => !partnerTags?.some((tag) => tag.id === partnerTagId), - ); + const validPartnerTagIdSet = new Set(partnerTags.map((tag) => tag.id)); + const invalidPartnerTagIds = partnerTagIds.filter( + (partnerTagId) => !validPartnerTagIdSet.has(partnerTagId), + );🤖 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/tags/throw-if-invalid-partner-tag-ids.ts` around lines 24 - 26, The current implementation of invalidPartnerTagIds has O(n*m) complexity due to the nested filter and some operations. To optimize this, create a Set from the valid partnerTags IDs first (by mapping tag.id from the partnerTags array into a Set), then use that Set in the filter operation to check if each partnerTagId exists. This changes the complexity to O(n) while maintaining identical behavior. Replace the filter and some check with a simple Set.has() lookup.
🤖 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/cron/bounties/notify-partners/route.ts:
- Around line 16-18: The TODO comment near the top of the route.ts file is vague
and provides no actionable context about what needs to be fixed. Either replace
the comment with a specific, detailed description of what issue needs to be
addressed (including relevant details like function names, expected behavior, or
known problems), or remove the comment entirely if the issue has already been
resolved or is no longer relevant.
In
`@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/program/bounties/add-edit-bounty/bounty-eligibility.tsx:
- Around line 164-170: The useEffect hook that checks the threshold condition
for `groups` (and similarly for `partnerTags` at line 230) includes `useAsync`
in its dependency array, causing a flip-flop effect. When the threshold is met
and `useAsync` is set to true via `setUseAsync`, this change triggers the effect
again, but now the condition evaluates differently and sets it back to false.
Remove `useAsync` from the dependency array in both effects and instead use a
functional updater pattern with `setUseAsync` that checks the previous state
value to prevent this repeating cycle. Apply this fix to both occurrences: the
one with `groups` and `GROUPS_MAX_PAGE_SIZE` around line 166, and the one with
`partnerTags` and `PARTNER_TAGS_MAX_PAGE_SIZE` around line 230.
In `@apps/web/lib/bounty/api/bounty-eligibility.ts`:
- Around line 75-85: The isPartnerEligibleForBounty function can return null
instead of a boolean because when partnerGroupId is null/falsy, the expression
(partnerGroupId && bountyTagIds.includes(partnerGroupId)) evaluates to null
rather than false. Fix this by explicitly converting the inGroup assignment to
always return a boolean value, either by using the double negation operator (!!)
on the inGroup expression or by ensuring the logical AND operation in the return
statement at line 84 always produces a proper boolean result.
In `@apps/web/lib/bounty/api/get-bounty-submission-upload-url.ts`:
- Around line 16-21: The `ProgramEnrollmentWithPartnerTags` type incorrectly
references `BountyPartnerTag` instead of `ProgramPartnerTag` for the
`programPartnerTags` field. Since `ProgramEnrollment` has a relation to
`ProgramPartnerTag` and the actual data returned from
`getProgramEnrollmentOrThrow` provides `ProgramPartnerTag[]`, change the type
definition to use `ProgramPartnerTag` instead of `BountyPartnerTag` in the Pick
statement. Apply the same fix in the other location mentioned in
`apps/web/lib/embed/referrals/auth.ts` at line 32 to maintain type consistency
across the codebase.
In `@apps/web/ui/partners/tags/partner-tags-multi-select.tsx`:
- Around line 55-65: The useEffect hook in partner-tags-multi-select.tsx is
creating an oscillation loop due to the `!useAsync` condition in the Boolean
expression. The effect sets useAsync to true when partnerTags.length exceeds
PARTNER_TAGS_MAX_PAGE_SIZE, but then immediately toggles it back to false on the
next run because useAsync is now true. Remove the `!useAsync` check from the
condition so that the effect only enables useAsync when the threshold is met
without toggling it back off. The condition should simply be
`Boolean(partnerTags && partnerTags.length >= PARTNER_TAGS_MAX_PAGE_SIZE)`.
---
Nitpick comments:
In `@apps/web/lib/api/tags/throw-if-invalid-partner-tag-ids.ts`:
- Around line 24-26: The current implementation of invalidPartnerTagIds has
O(n*m) complexity due to the nested filter and some operations. To optimize
this, create a Set from the valid partnerTags IDs first (by mapping tag.id from
the partnerTags array into a Set), then use that Set in the filter operation to
check if each partnerTagId exists. This changes the complexity to O(n) while
maintaining identical behavior. Replace the filter and some check with a simple
Set.has() lookup.
🪄 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: 1e1f6c78-ed99-42ae-ab4c-b251134211aa
📒 Files selected for processing (35)
apps/web/app/(ee)/api/bounties/[bountyId]/route.tsapps/web/app/(ee)/api/bounties/[bountyId]/submissions/route.tsapps/web/app/(ee)/api/bounties/route.tsapps/web/app/(ee)/api/cron/bounties/create-draft-submissions/route.tsapps/web/app/(ee)/api/cron/bounties/notify-partners/route.tsapps/web/app/(ee)/api/embed/referrals/bounties/[bountyId]/social-content-stats/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/bounties/[bountyId]/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/bounties/[bountyId]/social-content-stats/route.tsapps/web/app/(ee)/api/partner-profile/programs/[programId]/bounties/route.tsapps/web/app/(ee)/app.dub.co/embed/referrals/get-referrals-embed-data.tsapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/bounties/add-edit-bounty/add-edit-bounty-sheet.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/bounties/add-edit-bounty/bounty-eligibility.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/bounties/add-edit-bounty/confirm-create-bounty-modal.tsxapps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/bounties/add-edit-bounty/use-add-edit-bounty-form.tsapps/web/lib/actions/partners/tags/update-program-partner-tags.tsapps/web/lib/actions/partners/upload-bounty-submission-file.tsapps/web/lib/api/tags/throw-if-invalid-partner-tag-ids.tsapps/web/lib/api/workflows/execute-complete-bounty-workflow.tsapps/web/lib/api/workflows/execute-workflows.tsapps/web/lib/bounty/api/bounty-eligibility.tsapps/web/lib/bounty/api/create-bounty-submission.tsapps/web/lib/bounty/api/get-bounties-by-groups.tsapps/web/lib/bounty/api/get-bounties-for-partner.tsapps/web/lib/bounty/api/get-bounty-submission-upload-url.tsapps/web/lib/bounty/api/get-bounty-with-details.tsapps/web/lib/bounty/api/trigger-draft-bounty-submissions.tsapps/web/lib/embed/referrals/auth.tsapps/web/lib/types.tsapps/web/lib/webhook/sample-events/bounty-created.jsonapps/web/lib/webhook/sample-events/bounty-updated.jsonapps/web/lib/zod/schemas/bounties.tsapps/web/lib/zod/schemas/partner-profile.tsapps/web/prisma/schema/bounty.prismaapps/web/prisma/schema/tag.prismaapps/web/ui/partners/tags/partner-tags-multi-select.tsx
💤 Files with no reviewable changes (2)
- apps/web/lib/bounty/api/get-bounties-by-groups.ts
- apps/web/app/(ee)/api/bounties/[bountyId]/submissions/route.ts
Treat null eligibility selections as empty so PATCH clears existing restrictions, and refresh the eligibility picker when selections change.
Collaborator
Author
|
@coderabbitai review. Look for breaking changes and security issues. |
Contributor
🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== bounty-eligibility.ts ==="
cat apps/web/lib/bounty/api/bounty-eligibility.ts
echo ""
echo "=== throw-if-invalid-partner-tag-ids.ts ==="
cat apps/web/lib/api/tags/throw-if-invalid-partner-tag-ids.ts
echo ""
echo "=== bounty.prisma ==="
cat apps/web/prisma/schema/bounty.prisma
echo ""
echo "=== get-bounties-for-partner.ts ==="
cat apps/web/lib/bounty/api/get-bounties-for-partner.ts
echo ""
echo "=== trigger-draft-bounty-submissions.ts ==="
cat apps/web/lib/bounty/api/trigger-draft-bounty-submissions.tsLength of output: 12953 🏁 Script executed: #!/bin/bash
echo "=== create-draft-submissions cron route ==="
cat apps/web/app/\(ee\)/api/cron/bounties/create-draft-submissions/route.ts
echo ""
echo "=== notify-partners cron route ==="
cat apps/web/app/\(ee\)/api/cron/bounties/notify-partners/route.ts
echo ""
echo "=== bounties route (main) ==="
cat apps/web/app/\(ee\)/api/bounties/route.ts
echo ""
echo "=== bountyId route ==="
cat apps/web/app/\(ee\)/api/bounties/\[bountyId\]/route.ts
echo ""
echo "=== execute-complete-bounty-workflow.ts ==="
cat apps/web/lib/api/workflows/execute-complete-bounty-workflow.tsLength of output: 45592 🏁 Script executed: #!/bin/bash
echo "=== create-bounty-submission.ts ==="
cat apps/web/lib/bounty/api/create-bounty-submission.ts
echo ""
echo "=== get-bounty-submission-upload-url.ts ==="
cat apps/web/lib/bounty/api/get-bounty-submission-upload-url.ts
echo ""
echo "=== partner-profile bounties route ==="
cat apps/web/app/\(ee\)/api/partner-profile/programs/\[programId\]/bounties/route.ts
echo ""
echo "=== partner-profile bountyId route ==="
cat apps/web/app/\(ee\)/api/partner-profile/programs/\[programId\]/bounties/\[bountyId\]/route.ts
echo ""
echo "=== bounty-eligibility.tsx ==="
cat apps/web/app/app.dub.co/\(dashboard\)/\[slug\]/\(ee\)/program/bounties/add-edit-bounty/bounty-eligibility.tsx
echo ""
echo "=== bounties schema ==="
cat apps/web/lib/zod/schemas/bounties.ts
echo ""
echo "=== partner-profile schema ==="
cat apps/web/lib/zod/schemas/partner-profile.ts
echo ""
echo "=== Check for Prisma migration files ==="
fd -e sql . apps/web/prisma/migrations --exec echo {} | sort | tail -5
echo ""
echo "=== Check for any callers of getBountiesByGroups ==="
rg "getBountiesByGroups" --type ts -n
echo ""
echo "=== Check for any callers of getBountiesForPartner ==="
rg "getBountiesForPartner" --type ts -n -A3
echo ""
echo "=== update-program-partner-tags.ts ==="
cat apps/web/lib/actions/partners/tags/update-program-partner-tags.ts
echo ""
echo "=== upload-bounty-submission-file.ts ==="
cat apps/web/lib/actions/partners/upload-bounty-submission-file.ts
echo ""
echo "=== sample bounty-created.json ==="
cat apps/web/lib/webhook/sample-events/bounty-created.json
echo ""
echo "=== embed referrals auth ==="
cat apps/web/lib/embed/referrals/auth.tsLength of output: 64349
🔴 Security / Correctness1.
|
| # | Severity | Area | Description |
|---|---|---|---|
| 1 | 🔴 Correctness | Cron routes | defaultGroupId fallback missing → silent partner exclusion |
| 2 | 🟡 Breaking | DB | No migration for BountyPartnerTag |
| 3 | 🟡 Breaking | API | getBountiesForPartner signature change (callers updated ✅) |
| 4 | 🟡 Breaking | Schema | PartnerBountySchema drops partnerTags (intentional?) |
| 5 | 🔵 Minor | Workflow | Dead groups variable |
| 6 | 🔵 Minor | API | TOCTOU on tag validation |
✅ Action performed
Review finished.
Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
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
Release Notes
New Features
Bug Fixes
Chores