feat(providers): add dodopayments provider adapter#167
Draft
ayushsharma74 wants to merge 3 commits intogetpaykit:mainfrom
Draft
feat(providers): add dodopayments provider adapter#167ayushsharma74 wants to merge 3 commits intogetpaykit:mainfrom
ayushsharma74 wants to merge 3 commits intogetpaykit:mainfrom
Conversation
|
@ayushsharma74 is attempting to deploy a commit to the maxktz Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis change introduces comprehensive support for DodoPayments as a new payment provider. It includes a complete provider implementation with subscription management, webhook processing, and customer portal functionality, alongside E2E testing infrastructure (harness, configuration, webhook support) and test execution configuration for provider-specific serialized runs. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.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: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/test-utils/harness/dodopayments.ts`:
- Around line 78-81: The current catch on page.waitForURL("**/success**")
swallows timeouts and treats them as success; change the logic in the
dodopayments harness so a timed-out URL wait does not silently pass: remove the
empty catch and instead attempt a deterministic fallback (e.g. call
page.waitForSelector or page.locator(...) for the known confirmation DOM
element/text used by the checkout flow), and if both
page.waitForURL("**/success**") and the fallback wait time out, throw an error
(or return a failed/promise-rejection) so the test fails immediately; reference
the existing page.waitForURL call and add a page.waitForSelector/page.locator
check and explicit throw when neither condition is met.
- Around line 30-32: The harness hard-codes headed mode in async
completeCheckout by calling chromium.launch({ headless: false }); change this to
run headless by default (e.g., chromium.launch({ headless: true })) and make
headed mode opt-in for local debugging (toggle via an environment variable or
parameter passed into completeCheckout). Update the chromium.launch call in
completeCheckout to read the flag (e.g., process.env.PLAYWRIGHT_HEADLESS or a
function argument) so CI uses headless=true while developers can opt into
headful mode when needed.
In `@e2e/test-utils/hub.ts`:
- Around line 40-46: In extractDodoCustomerId, after JSON.parse and before
returning parsed.data?.customer?.customer_id, validate that the extracted
customer_id is a string; if it's missing or not a string return null to avoid
creating invalid buffer keys. Update the function to cast parsed as before, pull
const id = parsed.data?.customer?.customer_id, check typeof id === "string" and
return id or null, leaving the existing try/catch intact to handle parse errors.
In `@packages/dodopayments/src/dodopayments-provider.ts`:
- Around line 425-483: The cleanup currently archives any non-archived Dodo
product not touched in this sync, which can remove unrelated catalog items;
restrict archival to only PayKit-managed products by checking
product.metadata.paykit_product_id before archiving. Update the logic around
dodoProductsMap/cleanup (references: dodoProductsMap, activeProductIds,
client.products.archive, and the metadata key "paykit_product_id") so you either
build dodoProductsMap only from items that have metadata.paykit_product_id or,
at minimum, in the cleanup loop skip archiving unless the product's
metadata.paykit_product_id exists (and optionally matches an incoming
data.products id), preserving the existing creation path that sets metadata when
creating new products.
- Around line 143-145: The current branch that maps priceInterval to an interval
string (the block beginning with if (priceInterval) { const interval =
priceInterval === "year" ? "Year" : "Month"; return { ...) silently maps any
non-"year" value to "Month"; change it to validate allowed values explicitly
(e.g., only "year" and "month"), and when priceInterval is non-null but not one
of those, throw or return an error (or reject the request) instead of defaulting
to Month; update the mapping logic in the dodopayments-provider.ts code that
uses priceInterval to perform an explicit switch/check and surface a clear error
for unknown intervals.
- Around line 315-334: The scheduleSubscriptionChange function unconditionally
calls client.subscriptions.changePlan using data.providerProduct!.productId!,
which will throw when providerProduct is null; update the logic to branch on
data.providerProduct: if it's null, and there's an existing scheduled change
(current.scheduled_change) call
client.subscriptions.cancelChangePlan(data.providerSubscriptionId) (or ensure
any scheduled change is cleared) and return; if it's non-null, call
client.subscriptions.changePlan with the safe product id (avoid non-null
assertions). Reference: scheduleSubscriptionChange, data.providerProduct,
client.subscriptions.changePlan, and client.subscriptions.cancelChangePlan.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d2509c42-2237-482b-a30e-463ae0a5f8e7
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
e2e/core/checkout/resubscribe-after-cancel.test.tse2e/package.jsone2e/test-utils/env.tse2e/test-utils/harness/dodopayments.tse2e/test-utils/harness/index.tse2e/test-utils/hub.tse2e/vitest.config.tspackages/dodopayments/package.jsonpackages/dodopayments/src/dodopayments-provider.tspackages/dodopayments/src/index.tspackages/dodopayments/tsconfig.jsonpackages/dodopayments/tsdown.config.ts
Comment on lines
+30
to
+32
| async completeCheckout(url: string) { | ||
| const browser = await chromium.launch({ headless: false }); | ||
| const page = await browser.newPage(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect whether this repo already provisions a headed Playwright environment.
rg -n --iglob '*.{ts,json,yml,yaml}' 'chromium\.launch|playwright|xvfb|DISPLAY'Repository: getpaykit/paykit
Length of output: 645
🏁 Script executed:
#!/bin/bash
# Check for CI configuration files
fd -t f '\.(yml|yaml|json)$' -e yml -e yaml -e json | grep -E '(github|gitlab|circle|travis|actions|ci|test)' | head -20Repository: getpaykit/paykit
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Check for playwright configuration files and env setup
fd -t f 'playwright.config' -o '.env*' 2>/dev/null | head -10
# Also search for any existing Playwright env var patterns
rg -n 'process\.env.*PLAYWRIGHT|PLAYWRIGHT.*process\.env' --type ts --type js 2>/dev/nullRepository: getpaykit/paykit
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Understand the context around the dodopayments completeCheckout method
cat -n e2e/test-utils/harness/dodopayments.ts | sed -n '1,50p'Repository: getpaykit/paykit
Length of output: 2438
🏁 Script executed:
#!/bin/bash
# Get the complete completeCheckout method
cat -n e2e/test-utils/harness/dodopayments.ts | sed -n '30,100p'Repository: getpaykit/paykit
Length of output: 3362
🏁 Script executed:
#!/bin/bash
# Check for any .github or workflows directory structure
find . -type d -name '.github' -o -name 'workflows' 2>/dev/null | head -5
# Also look for any root-level config files
ls -la | grep -E '\.yml|\.yaml|package\.json|tsconfig' | head -10Repository: getpaykit/paykit
Length of output: 292
🏁 Script executed:
#!/bin/bash
# Search for comments or discussion around the headless setting
rg -B3 -A3 'headless.*false|headed.*mode' --type ts --type js 2>/dev/nullRepository: getpaykit/paykit
Length of output: 506
🏁 Script executed:
#!/bin/bash
# Check GitHub workflows for e2e test execution
find .github/workflows -type f -name '*.yml' -o -name '*.yaml' 2>/dev/null | xargs grep -l 'e2e\|playwright' 2>/dev/nullRepository: getpaykit/paykit
Length of output: 86
🏁 Script executed:
#!/bin/bash
# Read the e2e workflow configuration
cat .github/workflows/e2e.ymlRepository: getpaykit/paykit
Length of output: 9247
Run Playwright headless by default.
Hard-coding headless: false on line 31 will break this harness on CI runners without a display server (like ubuntu-latest in your GitHub Actions workflow). All other payment harnesses (stripe, polar) use headless: true and work correctly. Headed mode is not needed for automated form filling and payment submission—make it opt-in for local debugging instead.
🔧 Suggested change
- const browser = await chromium.launch({ headless: false });
+ const browser = await chromium.launch({
+ headless: process.env.PLAYWRIGHT_HEADLESS !== "false",
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async completeCheckout(url: string) { | |
| const browser = await chromium.launch({ headless: false }); | |
| const page = await browser.newPage(); | |
| async completeCheckout(url: string) { | |
| const browser = await chromium.launch({ | |
| headless: process.env.PLAYWRIGHT_HEADLESS !== "false", | |
| }); | |
| const page = await browser.newPage(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/test-utils/harness/dodopayments.ts` around lines 30 - 32, The harness
hard-codes headed mode in async completeCheckout by calling chromium.launch({
headless: false }); change this to run headless by default (e.g.,
chromium.launch({ headless: true })) and make headed mode opt-in for local
debugging (toggle via an environment variable or parameter passed into
completeCheckout). Update the chromium.launch call in completeCheckout to read
the flag (e.g., process.env.PLAYWRIGHT_HEADLESS or a function argument) so CI
uses headless=true while developers can opt into headful mode when needed.
Comment on lines
+78
to
+81
| // Wait for redirect to success URL or confirmation | ||
| await page.waitForURL("**/success**", { timeout: 30_000 }).catch(() => { | ||
| // Some checkouts show a confirmation page rather than redirecting | ||
| }); |
There was a problem hiding this comment.
Don't treat a timed-out success wait as a passed checkout.
If waitForURL("**/success**") times out, the catch block currently returns control as if payment completed. That turns real checkout failures into false positives until a later assertion hangs or flakes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/test-utils/harness/dodopayments.ts` around lines 78 - 81, The current
catch on page.waitForURL("**/success**") swallows timeouts and treats them as
success; change the logic in the dodopayments harness so a timed-out URL wait
does not silently pass: remove the empty catch and instead attempt a
deterministic fallback (e.g. call page.waitForSelector or page.locator(...) for
the known confirmation DOM element/text used by the checkout flow), and if both
page.waitForURL("**/success**") and the fallback wait time out, throw an error
(or return a failed/promise-rejection) so the test fails immediately; reference
the existing page.waitForURL call and add a page.waitForSelector/page.locator
check and explicit throw when neither condition is met.
Comment on lines
+40
to
+46
| function extractDodoCustomerId(body: string): string | null { | ||
| try { | ||
| const parsed = JSON.parse(body) as { | ||
| data?: { customer?: { customer_id?: string } }; | ||
| }; | ||
| return parsed.data?.customer?.customer_id ?? null; | ||
| } catch { |
There was a problem hiding this comment.
Validate Dodo customer_id type before returning it.
Line 45 currently trusts parsed shape; a non-string customer_id can create unrouteable buffer keys and dropped forwarding behavior.
Proposed fix
function extractDodoCustomerId(body: string): string | null {
try {
const parsed = JSON.parse(body) as {
data?: { customer?: { customer_id?: string } };
};
- return parsed.data?.customer?.customer_id ?? null;
+ const customerId = parsed.data?.customer?.customer_id;
+ return typeof customerId === "string" ? customerId : null;
} catch {
return null;
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/test-utils/hub.ts` around lines 40 - 46, In extractDodoCustomerId, after
JSON.parse and before returning parsed.data?.customer?.customer_id, validate
that the extracted customer_id is a string; if it's missing or not a string
return null to avoid creating invalid buffer keys. Update the function to cast
parsed as before, pull const id = parsed.data?.customer?.customer_id, check
typeof id === "string" and return id or null, leaving the existing try/catch
intact to handle parse errors.
Comment on lines
+143
to
+145
| if (priceInterval) { | ||
| const interval = priceInterval === "year" ? "Year" : "Month"; | ||
| return { |
There was a problem hiding this comment.
Reject unknown billing intervals instead of defaulting them to monthly.
Any non-null value other than "year" becomes "Month" here, so a typo like "monthly" or a future interval like "week" silently creates the wrong billing cadence.
🔧 Suggested guard
if (priceInterval) {
- const interval = priceInterval === "year" ? "Year" : "Month";
+ if (priceInterval !== "month" && priceInterval !== "year") {
+ throw new Error(`Unsupported DodoPayments interval: ${priceInterval}`);
+ }
+ const interval = priceInterval === "year" ? "Year" : "Month";
return {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (priceInterval) { | |
| const interval = priceInterval === "year" ? "Year" : "Month"; | |
| return { | |
| if (priceInterval) { | |
| if (priceInterval !== "month" && priceInterval !== "year") { | |
| throw new Error(`Unsupported DodoPayments interval: ${priceInterval}`); | |
| } | |
| const interval = priceInterval === "year" ? "Year" : "Month"; | |
| return { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dodopayments/src/dodopayments-provider.ts` around lines 143 - 145,
The current branch that maps priceInterval to an interval string (the block
beginning with if (priceInterval) { const interval = priceInterval === "year" ?
"Year" : "Month"; return { ...) silently maps any non-"year" value to "Month";
change it to validate allowed values explicitly (e.g., only "year" and "month"),
and when priceInterval is non-null but not one of those, throw or return an
error (or reject the request) instead of defaulting to Month; update the mapping
logic in the dodopayments-provider.ts code that uses priceInterval to perform an
explicit switch/check and surface a clear error for unknown intervals.
Comment on lines
+315
to
+334
| async scheduleSubscriptionChange(data) { | ||
| const current = await client.subscriptions.retrieve(data.providerSubscriptionId); | ||
| const wasCanceled = current.cancel_at_next_billing_date; | ||
|
|
||
| if (wasCanceled) { | ||
| await client.subscriptions.update(data.providerSubscriptionId, { | ||
| cancel_at_next_billing_date: false, | ||
| }); | ||
| } | ||
|
|
||
| if (current.scheduled_change) { | ||
| await client.subscriptions.cancelChangePlan(data.providerSubscriptionId); | ||
| } | ||
|
|
||
| await client.subscriptions.changePlan(data.providerSubscriptionId, { | ||
| product_id: data.providerProduct!.productId!, | ||
| effective_at: "next_billing_date", | ||
| quantity: 1, | ||
| proration_billing_mode: "full_immediately", | ||
| }); |
There was a problem hiding this comment.
Handle the providerProduct: null branch in scheduled changes.
The provider contract allows providerProduct to be null, but Line 330 non-null-asserts it and always calls changePlan. That will throw for callers trying to clear an existing scheduled change instead of scheduling a new plan.
🔧 Suggested branch
- await client.subscriptions.changePlan(data.providerSubscriptionId, {
- product_id: data.providerProduct!.productId!,
- effective_at: "next_billing_date",
- quantity: 1,
- proration_billing_mode: "full_immediately",
- });
+ if (!data.providerProduct?.productId) {
+ if (current.scheduled_change) {
+ await client.subscriptions.cancelChangePlan(data.providerSubscriptionId);
+ }
+ } else {
+ await client.subscriptions.changePlan(data.providerSubscriptionId, {
+ product_id: data.providerProduct.productId,
+ effective_at: "next_billing_date",
+ quantity: 1,
+ proration_billing_mode: "full_immediately",
+ });
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dodopayments/src/dodopayments-provider.ts` around lines 315 - 334,
The scheduleSubscriptionChange function unconditionally calls
client.subscriptions.changePlan using data.providerProduct!.productId!, which
will throw when providerProduct is null; update the logic to branch on
data.providerProduct: if it's null, and there's an existing scheduled change
(current.scheduled_change) call
client.subscriptions.cancelChangePlan(data.providerSubscriptionId) (or ensure
any scheduled change is cleared) and return; if it's non-null, call
client.subscriptions.changePlan with the safe product id (avoid non-null
assertions). Reference: scheduleSubscriptionChange, data.providerProduct,
client.subscriptions.changePlan, and client.subscriptions.cancelChangePlan.
Comment on lines
+425
to
+483
| const allDodoProducts = await client.products.list({ archived: false }); | ||
| const dodoProductsMap = new Map((allDodoProducts.items ?? []).map((p) => [p.product_id, p])); | ||
|
|
||
| const activeProductIds = new Set<string>(); | ||
|
|
||
| const results = await Promise.all( | ||
| data.products.map(async (product) => { | ||
| const existingProductId = product.existingProviderProduct?.productId ?? null; | ||
| const existingDodoProduct = existingProductId | ||
| ? dodoProductsMap.get(existingProductId) | ||
| : null; | ||
|
|
||
| const desiredInterval = product.priceInterval ?? null; | ||
| const existingPrice = existingDodoProduct?.price_detail; | ||
|
|
||
| const intervalMatches = | ||
| (desiredInterval === null && existingPrice?.type === "one_time_price") || | ||
| (desiredInterval !== null && | ||
| existingPrice?.type === "recurring_price" && | ||
| existingPrice.payment_frequency_interval.toLowerCase() === desiredInterval); | ||
|
|
||
| if (existingDodoProduct && intervalMatches) { | ||
| await client.products.update(existingDodoProduct.product_id, { | ||
| name: product.name, | ||
| price: buildDodoPrice(product.priceAmount, desiredInterval), | ||
| }); | ||
| activeProductIds.add(existingDodoProduct.product_id); | ||
| return { | ||
| id: product.id, | ||
| providerProduct: { productId: existingDodoProduct.product_id }, | ||
| }; | ||
| } | ||
|
|
||
| // Interval changed or no existing product — archive old, create new | ||
| if (existingDodoProduct) { | ||
| await client.products.archive(existingDodoProduct.product_id).catch(() => {}); | ||
| activeProductIds.delete(existingDodoProduct.product_id); | ||
| } | ||
|
|
||
| const created = await client.products.create({ | ||
| name: product.name, | ||
| price: buildDodoPrice(product.priceAmount, desiredInterval), | ||
| tax_category: options.taxCategory ?? "saas", | ||
| metadata: { paykit_product_id: product.id }, | ||
| }); | ||
|
|
||
| activeProductIds.add(created.product_id); | ||
| return { id: product.id, providerProduct: { productId: created.product_id } }; | ||
| }), | ||
| ); | ||
|
|
||
| // Archive orphans (ignore errors for already-deleted products) | ||
| const cleanup: Promise<unknown>[] = []; | ||
| for (const [dodoId] of dodoProductsMap) { | ||
| if (!activeProductIds.has(dodoId)) { | ||
| cleanup.push(client.products.archive(dodoId).catch(() => {})); | ||
| } | ||
| } | ||
| await Promise.all(cleanup); |
There was a problem hiding this comment.
Only archive products that this adapter owns.
syncProducts() builds dodoProductsMap from every non-archived Dodo product, then cleanup archives every entry not touched in this call. That will archive unrelated catalog items in the same Dodo account, not just PayKit-managed products.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dodopayments/src/dodopayments-provider.ts` around lines 425 - 483,
The cleanup currently archives any non-archived Dodo product not touched in this
sync, which can remove unrelated catalog items; restrict archival to only
PayKit-managed products by checking product.metadata.paykit_product_id before
archiving. Update the logic around dodoProductsMap/cleanup (references:
dodoProductsMap, activeProductIds, client.products.archive, and the metadata key
"paykit_product_id") so you either build dodoProductsMap only from items that
have metadata.paykit_product_id or, at minimum, in the cleanup loop skip
archiving unless the product's metadata.paykit_product_id exists (and optionally
matches an incoming data.products id), preserving the existing creation path
that sets metadata when creating new products.
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
@paykitjs/dodopaymentspackage for DodoPayments payment processingTests