Improve delete-group feature to be more robust

[steven-tey]

Summary by CodeRabbit

• Refactor
  • Partner reassignment during group deletion is now batched and processed in background; final audit logging simplified.
• Bug Fixes / Cleanup
  • Discount remapping and cleanup moved out of the immediate delete transaction into the background job; unused discounts are removed after reassignment when applicable.
• Chore
  • A deletion flag is propagated through partner-move and remap workflows.

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
dub	Ready	Preview	Feb 11, 2026 3:07am

[coderabbitai]

📝 Walkthrough

Walkthrough

Batch-driven group deletion now iteratively moves program enrollments to the default group (via movePartnersToGroup with isGroupDeleted), removes inline discount/code handling from the delete flow, adds isGroupDeleted to remap cron payloads, and reduces post-deletion side effects to a single audit log entry.

Changes

Cohort / File(s)	Summary
Group DELETE handler apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts	Reworks DELETE to batch-fetch program enrollments (limit 100) and call movePartnersToGroup until none remain; removes direct discount/code handling and other post-delete side effects; retains deletion of group rewards, workflow, and the group, and writes a single audit log.
Remap cron scheduling apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts	Forwards isGroupDeleted from the incoming body into the scheduled remap-discount-codes job payload.
Remap cron processor apps/web/app/(ee)/api/cron/groups/remap-discount-codes/route.ts	Adds optional isGroupDeleted to input schema; captures oldDiscount and, when isGroupDeleted is true and no discount codes remain after remapping, deletes the discount and records the deletion.
Partner migration helper apps/web/lib/api/groups/move-partners-to-group.ts	Adds optional isGroupDeleted?: boolean to MovePartnersToGroupParams, defaults it to false, and includes it in the payload published to the cron endpoint.

Sequence Diagram

sequenceDiagram
    participant Client
    participant API as DELETE API Handler
    participant DB as Database
    participant Migrate as movePartnersToGroup
    participant Cron as Remap Cron
    participant Audit as Audit Service

    Client->>API: DELETE /api/groups/[groupIdOrSlug]
    
    loop Batch fetch & migrate (<=100)
        API->>DB: Query program enrollments by groupId (limit 100)
        DB-->>API: Batch of enrollments
        API->>Migrate: POST movePartnersToGroup (isGroupDeleted=true)
        Migrate->>Cron: Enqueue remap-discount-codes job (payload includes isGroupDeleted)
        Cron->>DB: Remap discount codes for moved enrollments
        DB-->>Cron: Updated counts
        alt isGroupDeleted && no codes remain
            Cron->>DB: Delete old discount
            DB-->>Cron: Discount deleted
        end
    end

    API->>DB: Delete group rewards & workflow (transaction)
    API->>DB: Delete group (transaction)
    DB-->>API: Confirm deletion

    API->>Audit: Record group deletion audit log
    Audit-->>API: Recorded

    API-->>Client: 200 OK

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Skip /remap-default-links if no partners exist for a group #2909 — Related adjustments to the group DELETE flow and handling/scheduling of remap-default-links cron triggers.
• Fix partners ban cron crash when link has no discount code #3428 — Related changes touching discount-code deletion and input handling for remapping/cleanup.

Poem

🐰 I hopped through code with careful paws,
Batching partners, skipping old applause,
Discounts checked, then gently freed,
One audit note, a tidy deed,
🥕✨ — a rabbit's quiet cause.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.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 clearly summarizes the main change: improving the delete-group feature's robustness through batching and deferred processing.

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

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch improve-delete-group

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts (2)

327-327: Replace console.log with structured logging.

Bare console.log in a production API route makes it harder to filter/search in observability tools and can be noisy. Consider using the project's structured logger (if available) or removing the log, since the count is not actionable at this level.

---

366-369: deletedGroup is always true — the guard is redundant.

The transaction either succeeds (returning true) or throws, so if (deletedGroup) on line 369 can never be false. This is harmless but slightly misleading.

Simplify by removing the guard

-    const deletedGroup = await prisma.$transaction(async (tx) => {
+    await prisma.$transaction(async (tx) => {
       // ... transaction body ...
-      return true;
     });

-    if (deletedGroup) {
-      waitUntil(
-        recordAuditLog({ ... }),
-      );
-    }
+    waitUntil(
+      recordAuditLog({ ... }),
+    );

apps/web/app/(ee)/api/cron/groups/remap-discount-codes/route.ts (1)

45-49: Nit: move oldDiscount capture after the early-return guard.

oldDiscount is only relevant when programEnrollments is non-empty. Moving it after the guard on line 47-49 avoids a dead computation on the empty path and places it closer to the mutation/cleanup logic that uses it.

Suggested change

-  const oldDiscount = programEnrollments[0]?.discountCodes[0]?.discount;
-
   if (programEnrollments.length === 0) {
     return logAndRespond("No program enrollments found.");
   }
+
+  const oldDiscount = programEnrollments[0]?.discountCodes[0]?.discount;

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@apps/web/app/`(ee)/api/groups/[groupIdOrSlug]/route.ts:
- Around line 345-353: Add a deferred cleanup job to remove orphaned discounts
after movePartnersToGroup/remap-discount-codes async work completes: when
handling group deletion in route.ts, keep the current guard that skips immediate
deletion for group.discountId but enqueue a delayed task (via qstash or your
existing job runner) that runs after a safe delay to call the discount deletion
logic for the captured group.discountId; reference the same discount deletion
code path used in the commented-out block and ensure the job payload includes
groupId/group.discountId and idempotency checks so the background handler (the
qstash consumer or a new handler function) safely deletes the discount if it’s
still orphaned.
- Around line 309-327: The current while(true) loop in the route handler that
repeatedly calls prisma.programEnrollment.findMany and movePartnersToGroup can
run indefinitely and block the HTTP request; add a max-iteration safeguard
(e.g., maxBatches or maxIterations) and break when reached, and also detect
repeated/unmoved partnerIds to avoid spinning if movePartnersToGroup doesn't
clear groupId; additionally, for large workloads refactor to enqueue a
background job (use the existing qstash/job dispatching pattern) from the route
instead of processing all batches synchronously — update the loop around
prisma.programEnrollment.findMany and movePartnersToGroup to honor the iteration
cap and enqueue remaining work when cap is hit.

🧹 Nitpick comments (1)

apps/web/app/(ee)/api/groups/[groupIdOrSlug]/route.ts (1)

310-326: Minor: narrow the query and use structured logging.

The findMany fetches all columns but only partnerId is used. Adding a select clause reduces data transfer. Also, console.log is fine for debugging but consider using structured logging for production observability.

Suggested select clause

       const programEnrollments = await prisma.programEnrollment.findMany({
         where: {
           groupId: group.id,
         },
         take: 100,
+        select: { partnerId: true },
       });

[steven-tey]

@coderabbitai full review, are there any breaking changes?

[coderabbitai]

🧩 Analysis chain

I'll analyze the PR for breaking changes and trigger a full review. Let me first examine the actual code changes to provide you with a thorough analysis.

---

🏁 Script executed:

#!/bin/bash
# Fetch the diff for the PR
gh pr diff 3454 --repo dubinc/dub

Length of output: 10650

---

🏁 Script executed:

#!/bin/bash
# Get the full content of the changed file to understand the new implementation
cat apps/web/app/\(ee\)/api/groups/\[groupIdOrSlug\]/route.ts

Length of output: 11104

---

🏁 Script executed:

#!/bin/bash
# Check if there are any tests for this route that might indicate expected behavior
fd -e test.ts -e spec.ts . | xargs rg -l "groups.*delete|deleteGroup" 2>/dev/null || echo "No test files found"

Length of output: 43

---

🏁 Script executed:

#!/bin/bash
# Look for any API documentation or OpenAPI specs that define this endpoint
fd -g "*openapi*" -g "*swagger*" -g "*api-spec*" . 2>/dev/null || echo "No API spec files found"

Length of output: 62

---

🏁 Script executed:

#!/bin/bash
# Check for any migration files that might indicate database schema changes
fd -e sql -e prisma . | head -20

Length of output: 764

---

🏁 Script executed:

#!/bin/bash
# Check the test file to understand expected behavior
cat apps/web/tests/partner-groups/index.test.ts

Length of output: 7888

---

🏁 Script executed:

#!/bin/bash
# Look at movePartnersToGroup implementation to understand what it does
cat apps/web/lib/api/groups/move-partners-to-group.ts

Length of output: 4812

---

🏁 Script executed:

#!/bin/bash
# Check if there are any API clients or SDK that might be affected
fd -e ts -e js . -g "*client*" -g "*sdk*" | xargs rg -l "groups.*delete" 2>/dev/null | head -10

Length of output: 373

---

🏁 Script executed:

#!/bin/bash
# Look for any webhook or event handlers that might depend on group deletion
rg -n "group\.deleted|groups.*deleted" --type ts -C3 | head -50

Length of output: 1595

---

@steven-tey, I'll provide a comprehensive analysis of breaking changes and trigger a full review.

Breaking Changes Analysis

After examining the PR diff, I've identified several behavioral changes that could be breaking depending on how systems interact with this endpoint:

✅ Not Breaking - API Contract Unchanged

• Response structure: Still returns { id: string } with 200 status
• Request signature: No changes to endpoint path or required parameters
• Error responses: Error handling appears unchanged

⚠️ Potentially Breaking - Behavioral Changes

1. Loss of Atomicity for Large Groups

Atomic transactions ensure operations either all succeed or all fail together, preventing partial changes.

Before: All partner moves and deletions happened in a single database transaction
After: Partners moved in batches of 100 BEFORE the deletion transaction

Impact: If the process fails mid-batch, some partners may be moved to the default group but the group itself won't be deleted. This creates an inconsistent state.

2. Discount Deletion Timing Changed

Before: Discount deleted immediately within the transaction
After: Discount deletion deferred to async remap-discount-codes cron job

Impact: Brief period where:

• The group is deleted but its discount still exists
• Queries for the discount immediately after group deletion may find orphaned records
• External systems expecting immediate cleanup could encounter unexpected state

3. Side Effect Timing Completely Restructured

Before: Side effects (link remapping, notifications, audit logs) triggered AFTER deletion in waitUntil
After: Most side effects (via movePartnersToGroup) happen BEFORE deletion, only audit log happens AFTER

Impact:

• Changes to timing and latency could affect downstream monitoring and alerting
• Webhook consumers or event listeners may receive notifications before deletion completes
• For large groups, the DELETE request will take significantly longer to complete

4. Non-Idempotent Retry Behavior

Before: Single transaction made retries relatively safe
After: Batched approach means retrying a failed DELETE could process some partners twice

Impact: Idempotent APIs produce the same result no matter how many times performed, important for maintaining data integrity. The new implementation is less idempotent.

🔍 Edge Cases to Consider

1. Very Large Groups: DELETE requests may now timeout if groups have thousands of partners (processing 100 at a time)
2. Concurrent Operations: No locking mechanism prevents concurrent modifications during the batch loop
3. Partial Failures: If movePartnersToGroup fails for a batch, unclear how to recover

✅ Improvements (Not Breaking)

• Scalability: Better handling of large groups that would previously fail
• Robustness: Leverages existing movePartnersToGroup logic (DRY principle)
• Cleaner Code: Reduced complexity (-112/+42 lines)

📋 Recommendations

1. Add safeguards for the batch loop (timeout, max iterations, error handling)
2. Document the behavioral changes - especially non-atomic nature and timing
3. Consider adding a flag to track "deletion in progress" state
4. Test with large groups to understand real-world performance impact
5. Add idempotency key support if not already present

For most typical API consumers who simply call DELETE and check the 200 response, this should work fine. However, systems that:

• Expect atomic group deletion
• Rely on immediate discount cleanup
• Have tight timing dependencies on side effects
• Monitor deletion performance

...may need adjustments.

✅ Actions performed

Full review triggered.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts (1)

80-95: ⚠️ Potential issue | 🟠 Major

Deletion path can miss default links due to the take limit.
When isGroupDeleted is true, you include all links but still take: MAX_DEFAULT_LINKS_PER_GROUP (Line 94). If a partner has many links, the default links might not fall within the first N and won’t be remapped. Consider removing the cap for deletion runs or adding pagination.

✅ Suggested fix (limit only for non-deletion path)

           links: {
             // if this was invoked from the DELETE /groups/[groupId] route, the partnerGroupDefaultLinkId will be null
             // due to Prisma cascade SetNull on delete – therefore we should take all links and remap them instead.
             ...(isGroupDeleted
               ? {}
               : {
                   where: {
                     partnerGroupDefaultLinkId: {
                       not: null,
                     },
                   },
+                  take: MAX_DEFAULT_LINKS_PER_GROUP,
                 }),
             orderBy: {
               createdAt: "asc",
             },
-            take: MAX_DEFAULT_LINKS_PER_GROUP, // there can only be up to MAX_DEFAULT_LINKS_PER_GROUP default links per group
           },

apps/web/app/(ee)/api/cron/groups/remap-discount-codes/route.ts (1)

45-183: ⚠️ Potential issue | 🟡 Minor

Cleanup can be skipped when the first enrollment has no discount codes.
Line 45 takes oldDiscount from programEnrollments[0], so if the first enrollment has no codes (even when later ones do), the deletion cleanup won’t run. Consider deriving oldDiscount from any discount code in the batch. Also, if there are no codes but a deleted group discount still exists, you may want to pass the old discount id in the payload to guarantee cleanup.

✅ Suggested fix (derive from any code)

-  const oldDiscount = programEnrollments[0]?.discountCodes[0]?.discount;
+  const oldDiscount = programEnrollments
+    .flatMap(({ discountCodes }) => discountCodes)
+    .find((code) => code.discount)?.discount;

🤖 Fix all issues with AI agents

In `@apps/web/app/`(ee)/api/groups/[groupIdOrSlug]/route.ts:
- Around line 309-325: When moving partners out of a deleting group, include
isGroupDeleted: true in the payload to ensure downstream remap jobs run deletion
logic; update the call to movePartnersToGroup (inside the loop that queries
prisma.programEnrollment.findMany and constructs partnerIds) to pass
isGroupDeleted: true, and if the movePartnersToGroup signature or type
definitions don't accept that flag, add it to the function/method signature and
propagate it to any remap/cleanup logic that relies on partnerGroupDefaultLinkId
or discount cleanup.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@apps/web/app/`(ee)/api/cron/groups/remap-discount-codes/route.ts:
- Around line 166-183: The cleanup block that runs when isGroupDeleted &&
oldDiscount currently calls prisma.discount.delete and can raise a
RecordNotFound error under concurrent runs; change that to
prisma.discount.deleteMany({ where: { id: oldDiscount.id } }) so the operation
is idempotent (no-op if already deleted), and adjust any surrounding
logic/logging in the remap-discount-codes job to check the deleteMany count if
you need to log whether a row was removed; this touches the conditional that
references isGroupDeleted, oldDiscount and the prisma.discount.delete call
(triggered by movePartnersToGroup batches).

🧹 Nitpick comments (1)

apps/web/app/(ee)/api/cron/groups/remap-discount-codes/route.ts (1)

45-45: oldDiscount derivation assumes uniform discount across enrollments.

programEnrollments[0]?.discountCodes[0]?.discount takes the discount from the first enrollment's first code. This works correctly when all partners in the batch come from the same deleted group (same discount), but is fragile if this code path is ever reused for mixed-origin batches. A brief inline comment would help future readers understand the assumption.