Skip to content

Unify partner approval on bulk approve modal#3736

Open
pepeladeira wants to merge 3 commits intomainfrom
unify-partner-approve-bulk-modal
Open

Unify partner approval on bulk approve modal#3736
pepeladeira wants to merge 3 commits intomainfrom
unify-partner-approve-bulk-modal

Conversation

@pepeladeira
Copy link
Copy Markdown
Collaborator

@pepeladeira pepeladeira commented Apr 10, 2026

Summary by CodeRabbit

  • New Features

    • Bulk approval modal for partner applications, callable for selected rows or from an application detail sheet.
    • Keyboard shortcut ("A") to trigger approve when modal is open.
  • Improvements

    • Unified approval UX across lists and detail views.
    • Improved selection preview, smarter default group assignment, and consolidated success messaging after approvals.

@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Apr 10, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
dub Ready Ready Preview Apr 10, 2026 11:47pm

Request Review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 10, 2026

📝 Walkthrough

Walkthrough

Replaces per-row single-approve modal/state with a payload-driven bulk-approve modal. Pages and sheets call an exported openBulkApprove(payload) to open the modal for one or many partners; the modal handles single vs bulk approval, keyboard shortcuts, selection/grouping, and post-approve mutations/toast.

Changes

Cohort / File(s) Summary
Bulk Approval Modal
apps/web/ui/modals/bulk-approve-partners-modal.tsx
Added BulkApprovePartnersPayload type; refactored hook to useBulkApprovePartnersModal() returning openBulkApprove(payload) and BulkApprovePartnersModal. Modal derives visibility from bulkApprovePayload, supports single vs bulk approval paths (including single-partner approvePartnerAction), keyboard shortcut binding, selectedGroupId init, and unified post-approve behavior (mutations + toast).
Applications — Active Page
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/partners/applications/page-client.tsx
Removed per-row modal/state (useApprovePartnerApplicationModal, pendingApprovePartners) and integrated useBulkApprovePartnersModal(); table row and bulk selection now call openBulkApprove({ partners }). Passed onOpenBulkApprove={openBulkApprove} into PartnerApplicationSheet.
Applications — Rejected Page
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/partners/applications/rejected/page-client.tsx
Instantiated bulk-approve hook, renders BulkApprovePartnersModal at page level, and passes onOpenBulkApprove={openBulkApprove} into PartnerApplicationSheet.
Partner Application Sheet
apps/web/ui/partners/partner-application-sheet.tsx
Removed local single-approve hook/modal; added required prop onOpenBulkApprove: (payload: BulkApprovePartnersPayload) => void. Replaced local approve handlers with memoized openApproveModal that invokes onOpenBulkApprove with a single-partner payload (includes initialGroupId, onApproveSuccess, and keyboard options). Adjusted effect dependencies.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  rect rgba(200,200,255,0.5)
    participant Page as Page (Table / Row)
    participant Sheet as PartnerApplicationSheet
    participant Hook as useBulkApprovePartnersModal
    participant Modal as BulkApprovePartnersModal
    participant API as Server/API
    participant Cache as SWR/Cache
  end

  Page->>Hook: openBulkApprove({ partners })
  Hook->>Modal: set bulkApprovePayload -> open modal
  Sheet->>Hook: openBulkApprove(single-partner payload)
  Modal->>Modal: user confirms (Approve)
  Modal->>API: bulkApprovePartnersAction or approvePartnerAction
  API-->>Modal: success
  Modal->>Cache: mutate("/api/partners") & mutate("/api/partners/count")
  Modal->>Modal: clear bulkApprovePayload (close)
  Modal->>Page: show success toast / call onApproveSuccess
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

  • devkiran

Poem

🐰 Hopping through code with a payload in paw,
One open call and the modal is law,
Single or many, groups line the trail,
Shortcuts and toasts sing the success tale,
From a rabbit's desk — approve without fail!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main objective of the PR: refactoring partner approval flows to use a unified bulk approve modal instead of multiple separate approval mechanisms.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch unify-partner-approve-bulk-modal

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/partners/applications/rejected/page-client.tsx (1)

139-140: Wire the row menu into openBulkApprove too.

This page now mounts the payload-based approve modal, but only the sheet can open it. PartnerRowMenuButton in this same file still approves through useConfirmModal, so rejected applications keep a second flow that bypasses the shared group-assignment/success handling.

Also applies to: 385-406

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/program/partners/applications/rejected/page-client.tsx
around lines 139 - 140, The PartnerRowMenuButton flow still uses useConfirmModal
instead of the shared bulk-approve flow, so update the row menu to call
openBulkApprove from useBulkApprovePartnersModal (the same hook that provides
BulkApprovePartnersModal) instead of invoking useConfirmModal; pass a
single-item payload (the partner id/object) into openBulkApprove so the modal
receives the same payload-based shape as the sheet, and remove/replace the old
confirm modal success handling so all approvals go through the shared
group-assignment/success logic found in the bulk-approve modal; make this change
for the PartnerRowMenuButton usages in this file (including the other instance
around the 385-406 area) so both sheet and row-menu trigger the identical
openBulkApprove path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/web/ui/partners/partner-application-sheet.tsx`:
- Around line 238-247: openApproveModal is passing initialGroupId but the
sheet's selectedGroupId is only reset when partner.groupId changes, causing a
previous unsaved selection to leak to a new partner; update the logic that
initializes/reset selectedGroupId so it runs when the partner identity changes
(e.g. watch partner or partner.id instead of partner.groupId) and set
selectedGroupId = partner.groupId (or null) on partner change, and ensure any
useCallback/useEffect that reads selectedGroupId or initialGroupId (such as
openApproveModal and its dependency array) includes the correct dependencies
(partner or partner.id and selectedGroupId) so each partner gets its own fresh
group selection.

---

Nitpick comments:
In
`@apps/web/app/app.dub.co/`(dashboard)/[slug]/(ee)/program/partners/applications/rejected/page-client.tsx:
- Around line 139-140: The PartnerRowMenuButton flow still uses useConfirmModal
instead of the shared bulk-approve flow, so update the row menu to call
openBulkApprove from useBulkApprovePartnersModal (the same hook that provides
BulkApprovePartnersModal) instead of invoking useConfirmModal; pass a
single-item payload (the partner id/object) into openBulkApprove so the modal
receives the same payload-based shape as the sheet, and remove/replace the old
confirm modal success handling so all approvals go through the shared
group-assignment/success logic found in the bulk-approve modal; make this change
for the PartnerRowMenuButton usages in this file (including the other instance
around the 385-406 area) so both sheet and row-menu trigger the identical
openBulkApprove path.
🪄 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: a648d801-b4a8-4064-b4ad-fd973fbff8c7

📥 Commits

Reviewing files that changed from the base of the PR and between e70888a and bf7a215.

📒 Files selected for processing (4)
  • apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/partners/applications/page-client.tsx
  • apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/partners/applications/rejected/page-client.tsx
  • apps/web/ui/modals/bulk-approve-partners-modal.tsx
  • apps/web/ui/partners/partner-application-sheet.tsx

Comment thread apps/web/ui/partners/partner-application-sheet.tsx
@steven-tey
Copy link
Copy Markdown
Collaborator

@coderabbitai full review, any breaking changes?

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 11, 2026

🧩 Analysis chain

Let me inspect the codebase for all usages of the changed APIs before giving a definitive answer.


🏁 Script executed:

#!/bin/bash
echo "=== useBulkApprovePartnersModal usages ==="
rg -n "useBulkApprovePartnersModal" --type=tsx --type=ts -C2

echo ""
echo "=== PartnerApplicationSheet usages ==="
rg -n "PartnerApplicationSheet" --type=tsx --type=ts -C2

echo ""
echo "=== useApprovePartnerApplicationModal usages ==="
rg -n "useApprovePartnerApplicationModal" --type=tsx --type=ts -C2

echo ""
echo "=== ApprovePartnerApplicationModal usages ==="
rg -n "ApprovePartnerApplicationModal" --type=tsx --type=ts -C2

echo ""
echo "=== onOpenBulkApprove usages ==="
rg -n "onOpenBulkApprove" --type=tsx --type=ts -C2

echo ""
echo "=== BulkApprovePartnersPayload usages ==="
rg -n "BulkApprovePartnersPayload" --type=tsx --type=ts -C2

Length of output: 424


🏁 Script executed:

#!/bin/bash
echo "=== useBulkApprovePartnersModal usages ==="
rg -n "useBulkApprovePartnersModal" -g "*.tsx" -g "*.ts" -C2

echo ""
echo "=== PartnerApplicationSheet render/import usages ==="
rg -n "PartnerApplicationSheet" -g "*.tsx" -g "*.ts" -C2

echo ""
echo "=== useApprovePartnerApplicationModal usages ==="
rg -n "useApprovePartnerApplicationModal" -g "*.tsx" -g "*.ts" -C2

echo ""
echo "=== ApprovePartnerApplicationModal usages ==="
rg -n "ApprovePartnerApplicationModal" -g "*.tsx" -g "*.ts" -C2

echo ""
echo "=== onOpenBulkApprove usages ==="
rg -n "onOpenBulkApprove" -g "*.tsx" -g "*.ts" -C2

echo ""
echo "=== BulkApprovePartnersPayload usages ==="
rg -n "BulkApprovePartnersPayload" -g "*.tsx" -g "*.ts" -C2

echo ""
echo "=== approvePartnerAction import/usage ==="
rg -n "approvePartnerAction" -g "*.tsx" -g "*.ts" -C2

Length of output: 28154


@steven-tey, here's the full review of PR #3736. No breaking changes remain after this PR's updates — all call sites for the changed APIs have been updated within the PR itself. Here's the detailed breakdown:


✅ No Breaking Changes

Changed API Old New All callers updated?
useBulkApprovePartnersModal hook signature ({ partners }) () + openBulkApprove(payload) ✅ Yes
PartnerApplicationSheetProps no onOpenBulkApprove onOpenBulkApprove required ✅ Yes (both render sites pass it)
BulkApprovePartnersModal props showBulkApprovePartnersModal / partners bulkApprovePayload / setBulkApprovePayload ✅ Internal only

⚠️ Issues Found

1. Dead code — approve-partner-application-modal.tsx is now orphaned

useApprovePartnerApplicationModal and ApprovePartnerApplicationModal are no longer imported anywhere in the codebase. The entire file should be deleted to avoid confusion and bundle bloat.

2. Dual approval paths in rejected/page-client.tsx

The rejected page now has two separate approval mechanisms:

  • Line 140: useBulkApprovePartnersModal() → for the sheet's approve button
  • Line 465: A direct useAction(approvePartnerAction, ...) → still wired to something in the same file

If the second one is for a row-level "quick approve" action not going through the modal, this should be intentional and consistent. But if it's leftover from the previous approach, it's dead code that should be removed.

3. Branching approval logic in bulk-approve-partners-modal.tsx

The new handleApprove function now routes to different server actions based on payload:

  • Single partner + status === "rejected"approvePartnerAction (single action)
  • Everything elsebulkApprovePartnersAction

This branching adds complexity. Edge cases to verify:

  • A single partner with status "pending" routes to bulkApprovePartnersAction — is that intentional?
  • The runAfterApproveSuccess callback fires from both paths — ensure toast messages are accurate for both.

4. BulkApprovePartnersPayload import — type-only import hygiene

In applications/page-client.tsx, BulkApprovePartnersPayload is correctly imported with type, which is good for tree-shaking. Verify partner-application-sheet.tsx also uses import type:

// partner-application-sheet.tsx — Line 4
import type { BulkApprovePartnersPayload } from "@/ui/modals/bulk-approve-partners-modal";

✅ It does — this is fine.


Summary

Status
Breaking changes ✅ None
Dead code (approve-partner-application-modal.tsx) ⚠️ Should be deleted
Dual approval in rejected page ⚠️ Needs clarification
Approval branching logic ⚠️ Needs test coverage

The PR's unification goal is solid — routing both single and bulk approvals through one modal reduces duplication. The main follow-up action is deleting approve-partner-application-modal.tsx and clarifying the second approvePartnerAction usage in the rejected page.

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
apps/web/ui/partners/partner-application-sheet.tsx (1)

35-35: Delete the unused approve-partner-application-modal.tsx file.

This modal has no remaining callers in the codebase. The PartnerApproval component in partner-application-sheet.tsx has migrated entirely to use onOpenBulkApprove, making the legacy modal dead code. Removing it prevents accidental reuse and keeps the codebase clean.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/ui/partners/partner-application-sheet.tsx` at line 35, The legacy
modal file approve-partner-application-modal.tsx is unused and should be
removed: delete that file and any imports referencing it; verify PartnerApproval
usage in partner-application-sheet.tsx now goes through onOpenBulkApprove (the
onOpenBulkApprove: (payload: BulkApprovePartnersPayload) => void signature) and
remove or update any leftover references (types, exports, or stories/tests) that
point to approve-partner-application-modal.tsx to avoid dangling imports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@apps/web/ui/partners/partner-application-sheet.tsx`:
- Line 35: The legacy modal file approve-partner-application-modal.tsx is unused
and should be removed: delete that file and any imports referencing it; verify
PartnerApproval usage in partner-application-sheet.tsx now goes through
onOpenBulkApprove (the onOpenBulkApprove: (payload: BulkApprovePartnersPayload)
=> void signature) and remove or update any leftover references (types, exports,
or stories/tests) that point to approve-partner-application-modal.tsx to avoid
dangling imports.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 52fc40ec-f28c-4d29-9c6a-47ee368c6934

📥 Commits

Reviewing files that changed from the base of the PR and between e70888a and 6c74f13.

📒 Files selected for processing (4)
  • apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/partners/applications/page-client.tsx
  • apps/web/app/app.dub.co/(dashboard)/[slug]/(ee)/program/partners/applications/rejected/page-client.tsx
  • apps/web/ui/modals/bulk-approve-partners-modal.tsx
  • apps/web/ui/partners/partner-application-sheet.tsx

Comment on lines +139 to +140
const { openBulkApprove, BulkApprovePartnersModal } =
useBulkApprovePartnersModal();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Rejected-page approval is still split across two code paths.

These additions wire the bulk-approve modal into the sheet, but PartnerRowMenuButton below still approves directly via approvePartnerAction/useConfirmModal on Lines 464-488. Rejected applications can therefore still bypass the shared group-selection and post-approve flow, so this page is not actually unified yet. Please thread openBulkApprove into the row menu as well and remove the legacy direct-approve path here.

Also applies to: 385-406

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants