Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds comprehensive end-to-end workflow tests (AwardBounty, MoveGroup, SendCampaign), new Prisma-backed test utilities and E2E Prisma client, updates dev seed data, and refactors test helpers to use Prisma instead of HTTP. No runtime/public API changes. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/web/tests/workflows/send-campaign-workflow.test.ts`:
- Around line 274-349: The test "Cron processes eligible partner enrollment"
incorrectly sets the partner enrollment time to 18 hours ago
(prisma.programEnrollment.update with subHours) while the campaign trigger
expects partnerEnrolledDays >= 1, and it never asserts that an email was sent;
update the test to set a createdAt >24 hours ago (e.g., use subHours(..., 25) or
subDays(..., 2)) so the triggerCondition will be met, then add a post-cron
verification call such as verifyCampaignSent({ campaignId, partnerId: partner.id
}) after calling callCronWorkflow to assert the campaign email was actually
sent; ensure you reference the workflow from prisma.workflow.findFirst and keep
the existing cleanup onTestFinished.
- Around line 488-498: The test sets the enrollment time with
prisma.programEnrollment.update to subHours(new Date(), 18), which may be less
than the required partnerEnrolledDays >= 1 and cause the cron to skip sending
(making the dedup assertion vacuous); update the test to set the enrollment
earlier (e.g., subDays(new Date(), 1) or subHours(new Date(), 25)) so the
partner meets the 1-day threshold before pre-creating the notificationEmail and
running the dedup scenario involving E2E_PROGRAM and the existing
notificationEmail record.
🧹 Nitpick comments (6)
apps/web/tests/workflows/utils/verify-campaign-sent.ts (1)
27-32: Assertions are tautological given the query'swhereclause.The
findFirstquery already filters bycampaignId,type: "Campaign", andpartnerId, so asserting those same fields on the result will always pass. The only meaningful check is that the record exists (which theif (emailSent)guard already covers). Consider removing lines 28–31 to reduce noise, or keep them intentionally as documentation of the expected shape.Simplified return
if (emailSent) { - expect(emailSent).toBeDefined(); - expect(emailSent.type).toBe("Campaign"); - expect(emailSent.campaignId).toBe(campaignId); - expect(emailSent.partnerId).toBe(partnerId); return emailSent; }apps/web/tests/workflows/utils/verify-bounty-submission.ts (1)
22-22: Consider typinglastSubmissioninstead ofany.Using the Prisma-generated type (e.g.,
BountySubmission | null) would give you compile-time safety on the error-message field access (line 59:lastSubmission.status,lastSubmission.performanceCount).Type-safe alternative
+import { BountySubmission } from "@dub/prisma/client"; ... - let lastSubmission: any = null; + let lastSubmission: BountySubmission | null = null;apps/web/tests/workflows/award-bounty-workflow.test.ts (2)
10-42:trackLeadsis duplicated verbatim inmove-group-workflow.test.ts.This helper appears identically in both
award-bounty-workflow.test.ts(here) andmove-group-workflow.test.ts(lines 12–44). Extract it to a shared utility undertests/workflows/utils/to keep both files DRY and ensure future changes (e.g., adding delays, changing the tracking payload) are applied consistently.Extract to shared utility
Create
apps/web/tests/workflows/utils/track-leads.ts:import { expect } from "vitest"; import { E2E_TRACK_CLICK_HEADERS } from "../../utils/resource"; export async function trackLeads( http: any, partnerLink: { domain: string; key: string }, count: number, ) { for (let i = 0; i < count; i++) { const { status: clickStatus, data: clickData } = await http.post({ path: "/track/click", headers: E2E_TRACK_CLICK_HEADERS, body: { domain: partnerLink.domain, key: partnerLink.key, }, }); expect(clickStatus).toEqual(200); expect(clickData.clickId).toBeDefined(); const { status: leadStatus } = await http.post({ path: "/track/lead", body: { clickId: clickData.clickId, eventName: `Signup-${i}-${Date.now()}`, customerExternalId: `e2e-customer-${i}-${Date.now()}`, customerEmail: `customer${i}@example.com`, }, }); expect(leadStatus).toEqual(200); await new Promise((resolve) => setTimeout(resolve, 200)); } }
104-160: Hardcoded 10-second sleep is fragile.Line 147 uses a fixed 10-second wait to give the workflow time to not execute. If the system is slow, the workflow might still be processing, causing a false pass (draft found before workflow completes). If it's fast, 10 seconds is wasted CI time. Consider polling briefly (e.g., 2–3 iterations) and asserting draft status holds, or document why 10 seconds is a safe upper bound.
apps/web/tests/workflows/move-group-workflow.test.ts (2)
170-176:sourceGroupselection viaexistingGroups[0]is order-dependent and fragile.Multiple tests (lines 176, 262, 336, 420) grab
existingGroups[0]as the source group. The order of results fromGET /groupsmay not be deterministic, and if another test creates a group before this one runs (even with sequential execution), the first element could change. Consider filtering by the known default group ID or slug instead:Pin to the known default group
- const sourceGroup = existingGroups[0]; + const sourceGroup = existingGroups.find(g => g.slug === "default"); + expect(sourceGroup).toBeDefined();
492-540: AND-operator test validates configuration but not execution.This test verifies the workflow's
triggerConditionsare stored correctly with two rules, but doesn't test that both conditions must be satisfied for the move to occur (i.e., no execution test with one condition met and one not). Consider adding a follow-up test that exercises the AND semantics end-to-end.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/tests/workflows/move-group-workflow.test.ts`:
- Around line 394-399: The test currently silently returns when enrollment is
falsy which masks E2E seed/configuration failures; replace the early-return
block that checks enrollment (the conditional using enrollment, partner.id and
programId) with an explicit assertion such as expect(enrollment).not.toBeNull()
(or expect(enrollment).toBeDefined()) so the test fails loudly if enrollment is
missing, or if the intent is truly to skip, convert the test to use test.skip
with a clear reason referencing the missing enrollment.
🧹 Nitpick comments (2)
apps/web/tests/workflows/utils/verify-bounty-submission.ts (1)
34-52: Redundant assertions inside the already-matched condition block.Lines 40–46 re-assert conditions that were already verified by the
ifguard on lines 34–39. Theseexpectcalls will always pass at this point. They add noise without catching new failures. Consider removing them and keeping only thecompletedAtassertion (line 48–50), which checks something not in the guard.♻️ Suggested simplification
if ( submission && submission.status === expectedStatus && (minPerformanceCount === undefined || (submission.performanceCount ?? 0) >= minPerformanceCount) ) { - expect(submission.status).toBe(expectedStatus); - - if (minPerformanceCount !== undefined) { - expect(submission.performanceCount).toBeGreaterThanOrEqual( - minPerformanceCount, - ); - } - if (expectedStatus === "submitted") { expect(submission.completedAt).not.toBeNull(); } return submission; }apps/web/tests/workflows/move-group-workflow.test.ts (1)
12-13: Consider typing thehttpparameter.
http: anyloses type safety. If theIntegrationHarnessexposes a type for its HTTP client, using it here would catch mistakes at compile time.
1 participant
Add E2E tests covering all three workflow types (AwardBounty, SendCampaign, MoveGroup) across both trigger mechanisms (partnerMetricsUpdated, partnerEnrolled).
Summary by CodeRabbit
Tests
Chores