Skip to content

feat: add multi-domain filters to webhooks#361

Merged
KMKoushik merged 5 commits intomainfrom
feat/webhook-domain-filter
Mar 7, 2026
Merged

feat: add multi-domain filters to webhooks#361
KMKoushik merged 5 commits intomainfrom
feat/webhook-domain-filter

Conversation

@KMKoushik
Copy link
Member

@KMKoushik KMKoushik commented Feb 27, 2026

Summary

  • Add webhook domain scoping with support for selecting multiple domains per endpoint (domainIds), where empty means all domains.
  • Update webhook dispatch logic so events only trigger for matching domains (or globally scoped endpoints), and validate selected domains belong to the team.
  • Extend webhook create/edit UI and TRPC inputs to configure domain filters and display selected domains on webhook details.

Testing

  • Not run (not requested).

Summary by cubic

Added multi-domain filters to webhooks so endpoints can target specific domains or all. Domain filtering applies only when a domainId is present; null or missing domain context sends events to all matching endpoints.

  • New Features

    • Prisma: add Webhook.domainIds Int[] with default [].
    • Dispatch: filter by domain only when domainId is present; include global endpoints; no filtering for events without domain context (or when domainId is null).
    • Emit: pass domainId from domain events and from the SES parser when available.
    • API: TRPC create/update accept optional domainIds; service dedupes IDs and validates they belong to the team.
    • UI: domain selector in create/edit; details show “All domains” or first two domain badges with “+N more”.
    • Tests: router tests for domainIds passthrough; unit tests for emit filters and domain validation.
  • Migration

    • Run Prisma migrate to add domainIds.
    • No backfill; existing webhooks remain global (domainIds=[]).

Written for commit 508e3d0. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • Webhooks can target specific domains or "All domains"; UI lets you select domains and shows domain badges with a "+N more" indicator.
  • Chores

    • Backend now persists webhook domain selections and applies domain-based filtering when dispatching webhook events; schema updated to store domain IDs.
  • Tests

    • Added tests covering domain selection on webhook create/update and domain-aware event dispatching.

@vercel
Copy link

vercel bot commented Feb 27, 2026

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

Project Deployment Actions Updated (UTC)
unsend-marketing Ready Ready Preview, Comment Mar 7, 2026 9:57pm

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Feb 27, 2026

Deploying usesend with  Cloudflare Pages  Cloudflare Pages

Latest commit: 508e3d0
Status: ✅  Deploy successful!
Preview URL: https://89ab6e27.usesend.pages.dev
Branch Preview URL: https://feat-webhook-domain-filter.usesend.pages.dev

View logs

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 27, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds domain-scoped webhook support by introducing domainIds (Int[] default []) to the Webhook Prisma model and DB migration. Propagates domainIds through API routes and WebhookService (create, update, emit) with normalization and validation that domains belong to the team, and applies domain filtering when emitting events. Includes domainId in emitted webhook payloads from SES and DomainService. Front-end: domain selection/UI and display added to create, edit, and info views. Tests added for router handling and emit filtering behavior.

Possibly related PRs

  • usesend/useSend PR 334 — Prior work that modified the webhook model, service, router, and UI flows; shares direct code-level overlap with adding domainIds and related wiring.
🚥 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
Title check ✅ Passed The title clearly and accurately describes the main feature being added: multi-domain filtering capability for webhooks. It is specific, concise, and directly reflects the changeset's primary objective.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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


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

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Copy link
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.

🧹 Nitpick comments (1)
apps/web/src/app/(dashboard)/webhooks/webhook-update-dialog.tsx (1)

317-395: Consider extracting domain selection into a shared component.

The domain selection UI (lines 317-395) is nearly identical to the implementation in add-webhook.tsx (lines 323-401). Consider extracting this into a reusable DomainSelector component to reduce duplication.

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

In `@apps/web/src/app/`(dashboard)/webhooks/webhook-update-dialog.tsx around lines
317 - 395, Extract the repeated domain-selection UI into a reusable
DomainSelector component and replace the inline FormField render block with it:
create DomainSelector that accepts props { domains, selectedDomainIds, onChange
} (or accept form field callbacks and integrate with FormField) and encapsulates
the DropdownMenu, DropdownMenuTrigger/Content, DropdownMenuCheckboxItem list,
the "All domains" checkbox, selectedDomainsLabel logic, and handleToggleDomain
behavior; then update both webhook-update-dialog.tsx and add-webhook.tsx to use
<DomainSelector .../> (wired to form.control or field.onChange) to remove
duplication while preserving the existing behaviors and callbacks (FormField,
DropdownMenuCheckboxItem, handleToggleDomain, selectedDomainsLabel).
🤖 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/src/app/`(dashboard)/webhooks/webhook-update-dialog.tsx:
- Around line 317-395: Extract the repeated domain-selection UI into a reusable
DomainSelector component and replace the inline FormField render block with it:
create DomainSelector that accepts props { domains, selectedDomainIds, onChange
} (or accept form field callbacks and integrate with FormField) and encapsulates
the DropdownMenu, DropdownMenuTrigger/Content, DropdownMenuCheckboxItem list,
the "All domains" checkbox, selectedDomainsLabel logic, and handleToggleDomain
behavior; then update both webhook-update-dialog.tsx and add-webhook.tsx to use
<DomainSelector .../> (wired to form.control or field.onChange) to remove
duplication while preserving the existing behaviors and callbacks (FormField,
DropdownMenuCheckboxItem, handleToggleDomain, selectedDomainsLabel).

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b2ed09e and 3404f38.

📒 Files selected for processing (8)
  • apps/web/prisma/schema.prisma
  • apps/web/src/app/(dashboard)/webhooks/[webhookId]/webhook-info.tsx
  • apps/web/src/app/(dashboard)/webhooks/add-webhook.tsx
  • apps/web/src/app/(dashboard)/webhooks/webhook-update-dialog.tsx
  • apps/web/src/server/api/routers/webhook.ts
  • apps/web/src/server/service/domain-service.ts
  • apps/web/src/server/service/ses-hook-parser.ts
  • apps/web/src/server/service/webhook-service.ts

Copy link
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.

🧹 Nitpick comments (1)
apps/web/src/server/api/routers/webhook-domain-filter.trpc.test.ts (1)

77-115: Consider adding edge case tests.

The current tests verify that domainIds is passed through correctly, which is good. For more comprehensive coverage, consider adding tests for:

  • Empty domainIds: [] (global webhook behavior)
  • Omitted domainIds (should remain undefined in the service call)

This would help document the expected behavior for global vs. domain-scoped webhooks.

💡 Example additional test cases
it("passes empty domainIds for global webhooks", async () => {
  const caller = createCaller(getContext());

  await caller.create({
    url: "https://example.com/webhook",
    eventTypes: ["email.sent"],
    domainIds: [],
  });

  expect(mockWebhookService.createWebhook).toHaveBeenCalledWith(
    expect.objectContaining({
      domainIds: [],
    })
  );
});

it("omits domainIds when not provided", async () => {
  const caller = createCaller(getContext());

  await caller.create({
    url: "https://example.com/webhook",
    eventTypes: ["email.sent"],
  });

  expect(mockWebhookService.createWebhook).toHaveBeenCalledWith(
    expect.objectContaining({
      domainIds: undefined,
    })
  );
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/server/api/routers/webhook-domain-filter.trpc.test.ts` around
lines 77 - 115, Add two edge-case tests to the existing
webhook-domain-filter.trpc.test.ts: one that calls create (via
createCaller(getContext())) with domainIds: [] and asserts
mockWebhookService.createWebhook was called containing domainIds: [], and
another that calls create without providing domainIds and asserts the service
call contains domainIds: undefined; follow the pattern used in the existing
tests (use expect.objectContaining and the same caller/getContext setup) and
mirror the existing assertions for update/create so the new tests reference
mockWebhookService.createWebhook and createCaller/getContext.
🤖 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/src/server/api/routers/webhook-domain-filter.trpc.test.ts`:
- Around line 77-115: Add two edge-case tests to the existing
webhook-domain-filter.trpc.test.ts: one that calls create (via
createCaller(getContext())) with domainIds: [] and asserts
mockWebhookService.createWebhook was called containing domainIds: [], and
another that calls create without providing domainIds and asserts the service
call contains domainIds: undefined; follow the pattern used in the existing
tests (use expect.objectContaining and the same caller/getContext setup) and
mirror the existing assertions for update/create so the new tests reference
mockWebhookService.createWebhook and createCaller/getContext.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3404f38 and a01ad82.

📒 Files selected for processing (2)
  • apps/web/prisma/migrations/20260227040924_add_webhook_domain_filters/migration.sql
  • apps/web/src/server/api/routers/webhook-domain-filter.trpc.test.ts

Copy link
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.

🧹 Nitpick comments (2)
apps/web/src/server/service/webhook-service.unit.test.ts (1)

120-186: LGTM! Thorough validation of domain-filtered emission.

The test comprehensively verifies the query structure when a domain context is provided, including both branches of the OR condition (global webhooks and domain-specific matches).

Optional: Consider adding a test case for { domainId: null } to verify it behaves the same as omitting the option entirely, matching the == null check in the implementation.

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

In `@apps/web/src/server/service/webhook-service.unit.test.ts` around lines 120 -
186, Add a new unit test in webhook-service.unit.test.ts (next to the existing
"filters webhooks by domain when the event has a domain context" test) that
calls WebhookService.emit with the same payload but options { domainId: null }
and asserts that mockDb.webhook.findMany is called with the identical where
clause (including the AND with the domainIds isEmpty OR has 42 behavior when
applicable), and that mockDb.webhookCall.create and mockQueueAdd are invoked the
same number of times with the same args; reference WebhookService.emit,
mockDb.webhook.findMany, mockDb.webhookCall.create and mockQueueAdd when
locating the code to mirror the existing test setup and expectations.
apps/web/src/server/service/webhook-service.ts (1)

354-376: Correct domain status validation in the query.

The validation ensures domains belong to the team but doesn't check the domain's status field. This allows scoping webhooks to domains with PENDING or other non-active statuses. While this behavior may be intentional, consider restricting selection to verified (SUCCESS) domains only if webhooks should only trigger for ready domains.

Note: The DomainStatus enum uses SUCCESS for verified domains (not VERIFIED).

If restricting to active domains is desired:
 private static async assertDomainsBelongToTeam(
   domainIds: number[],
   teamId: number,
 ) {
   const matchingDomains = await db.domain.findMany({
     where: {
       id: {
         in: domainIds,
       },
       teamId,
+      status: "SUCCESS",
     },
     select: {
       id: true,
     },
   });

   if (matchingDomains.length !== domainIds.length) {
     throw new UnsendApiError({
       code: "NOT_FOUND",
-      message: "One or more domains were not found",
+      message: "One or more domains were not found or are not verified",
     });
   }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/server/service/webhook-service.ts` around lines 354 - 376, The
current assertDomainsBelongToTeam (used in webhook-service.ts) only verifies
team ownership but not domain status; to restrict to verified/active domains
update the db.domain.findMany query filter to include status:
DomainStatus.SUCCESS (using the existing DomainStatus enum) so only domains with
verified status count toward matchingDomains, and keep the existing
UnsendApiError throw when counts mismatch; ensure DomainStatus is
imported/available in the file before using it.
🤖 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/src/server/service/webhook-service.ts`:
- Around line 354-376: The current assertDomainsBelongToTeam (used in
webhook-service.ts) only verifies team ownership but not domain status; to
restrict to verified/active domains update the db.domain.findMany query filter
to include status: DomainStatus.SUCCESS (using the existing DomainStatus enum)
so only domains with verified status count toward matchingDomains, and keep the
existing UnsendApiError throw when counts mismatch; ensure DomainStatus is
imported/available in the file before using it.

In `@apps/web/src/server/service/webhook-service.unit.test.ts`:
- Around line 120-186: Add a new unit test in webhook-service.unit.test.ts (next
to the existing "filters webhooks by domain when the event has a domain context"
test) that calls WebhookService.emit with the same payload but options {
domainId: null } and asserts that mockDb.webhook.findMany is called with the
identical where clause (including the AND with the domainIds isEmpty OR has 42
behavior when applicable), and that mockDb.webhookCall.create and mockQueueAdd
are invoked the same number of times with the same args; reference
WebhookService.emit, mockDb.webhook.findMany, mockDb.webhookCall.create and
mockQueueAdd when locating the code to mirror the existing test setup and
expectations.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 53cf3f4f-dfed-4be3-96c9-7210887ba03f

📥 Commits

Reviewing files that changed from the base of the PR and between a01ad82 and 303c368.

📒 Files selected for processing (2)
  • apps/web/src/server/service/webhook-service.ts
  • apps/web/src/server/service/webhook-service.unit.test.ts

Copy link
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: 2

🧹 Nitpick comments (1)
apps/web/src/app/(dashboard)/webhooks/webhook-update-dialog.tsx (1)

55-56: Treat domainIds as required in this dialog.

Webhook.domainIds is a required field in the schema (Int[] @default([])), but the local type override makes it optional. This allows missing data to silently default to [] ("All domains"), which on save will clear the webhook's domain scope instead of surfacing the integration bug. Remove the optional override.

♻️ Suggested cleanup
 type EditWebhookFormValues = z.infer<typeof editWebhookSchema>;
-type WebhookWithDomainIds = Webhook & { domainIds?: number[] };
 
 export function EditWebhookDialog({
   webhook,
   open,
   onOpenChange,
 }: {
-  webhook: WebhookWithDomainIds;
+  webhook: Webhook;
   open: boolean;
   onOpenChange: (open: boolean) => void;
 }) {
@@
     defaultValues: {
       url: webhook.url,
       eventTypes: initialHasAllEvents
         ? []
         : (webhook.eventTypes as WebhookEventType[]),
-      domainIds: webhook.domainIds ?? [],
+      domainIds: webhook.domainIds,
     },
   });
@@
       form.reset({
         url: webhook.url,
         eventTypes: hasAllEvents
           ? []
           : (webhook.eventTypes as WebhookEventType[]),
-        domainIds: webhook.domainIds ?? [],
+        domainIds: webhook.domainIds,
       });

Also applies to: 71-71, 90-103, 122-122 in webhook-update-dialog.tsx and the same pattern in webhook-info.tsx lines 13–93.

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

In `@apps/web/src/app/`(dashboard)/webhooks/webhook-update-dialog.tsx around lines
55 - 56, The local type override makes domainIds optional which masks missing
data and can clear domain scope on save; update the WebhookWithDomainIds type to
make domainIds required (change to Webhook & { domainIds: number[] }) and remove
any code paths that rely on domainIds being undefined; then audit and update all
usages in webhook-update-dialog (the WebhookWithDomainIds declaration and places
that assume optionality) and the corresponding pattern in webhook-info (replace
optional handling with required handling or explicit validation/error reporting)
so missing domainIds surface as an integration bug rather than defaulting to [].
🤖 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/prisma/migrations/20260227040924_add_webhook_domain_filters/migration.sql`:
- Line 2: The migration adds the Webhook.domainIds column but leaves it
nullable; update the migration so the "domainIds" column on table "Webhook" is
created as NOT NULL with the same default empty integer array
(ARRAY[]::INTEGER[]), or add an immediate ALTER TABLE ... ALTER COLUMN ... SET
NOT NULL after creation; ensure the SQL references the "Webhook" table and
"domainIds" column exactly and preserves the default empty array to match the
Prisma model Int[] (required) semantics.

In `@apps/web/src/app/`(dashboard)/webhooks/add-webhook.tsx:
- Around line 394-397: Update the FormDescription copy in add-webhook.tsx to
clarify that the domain filter only applies to domain-aware events (those that
include a domainId) and does not prevent team- or global-level events; use
similar wording in the edit dialog so both UIs match. Locate the FormDescription
node in this file (and the corresponding edit dialog component) and replace the
current text with a succinct message stating that leaving it blank receives all
events, while specifying a domain only filters events that carry a domainId.

---

Nitpick comments:
In `@apps/web/src/app/`(dashboard)/webhooks/webhook-update-dialog.tsx:
- Around line 55-56: The local type override makes domainIds optional which
masks missing data and can clear domain scope on save; update the
WebhookWithDomainIds type to make domainIds required (change to Webhook & {
domainIds: number[] }) and remove any code paths that rely on domainIds being
undefined; then audit and update all usages in webhook-update-dialog (the
WebhookWithDomainIds declaration and places that assume optionality) and the
corresponding pattern in webhook-info (replace optional handling with required
handling or explicit validation/error reporting) so missing domainIds surface as
an integration bug rather than defaulting to [].

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4f9adfb1-9bbb-4382-a452-966273e55060

📥 Commits

Reviewing files that changed from the base of the PR and between 303c368 and efbba79.

📒 Files selected for processing (11)
  • apps/web/prisma/migrations/20260227040924_add_webhook_domain_filters/migration.sql
  • apps/web/prisma/schema.prisma
  • apps/web/src/app/(dashboard)/webhooks/[webhookId]/webhook-info.tsx
  • apps/web/src/app/(dashboard)/webhooks/add-webhook.tsx
  • apps/web/src/app/(dashboard)/webhooks/webhook-update-dialog.tsx
  • apps/web/src/server/api/routers/webhook-domain-filter.trpc.test.ts
  • apps/web/src/server/api/routers/webhook.ts
  • apps/web/src/server/service/domain-service.ts
  • apps/web/src/server/service/ses-hook-parser.ts
  • apps/web/src/server/service/webhook-service.ts
  • apps/web/src/server/service/webhook-service.unit.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • apps/web/prisma/schema.prisma
  • apps/web/src/server/service/webhook-service.unit.test.ts
  • apps/web/src/server/api/routers/webhook-domain-filter.trpc.test.ts
  • apps/web/src/server/service/domain-service.ts

@@ -0,0 +1,2 @@
-- AlterTable
ALTER TABLE "Webhook" ADD COLUMN "domainIds" INTEGER[] DEFAULT ARRAY[]::INTEGER[];
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Webhook model ==="
sed -n '/model Webhook/,/}/p' apps/web/prisma/schema.prisma

echo
echo "=== Migration ==="
sed -n '1,20p' apps/web/prisma/migrations/20260227040924_add_webhook_domain_filters/migration.sql

Repository: usesend/useSend

Length of output: 1825


🏁 Script executed:

cat -n apps/web/src/server/service/webhook-service.ts | head -100

Repository: usesend/useSend

Length of output: 3476


🏁 Script executed:

rg "domainIds" apps/web/src/server/service/webhook-service.ts -B 2 -A 2

Repository: usesend/useSend

Length of output: 1715


🏁 Script executed:

rg "isEmpty|\.has" apps/web/src/server/service/ -B 2 -A 2 --type ts --max-count 20

Repository: usesend/useSend

Length of output: 5309


Add NOT NULL constraint to match the Prisma model definition.

The Prisma schema declares domainIds as a required field (Int[] without ?) with default [], but the migration allows NULL at the database level. This mismatch means if NULL ever enters the column (via raw SQL, data corruption, or other means), the filter logic using isEmpty and has would not behave as expected, potentially silencing webhooks from domain filtering.

🛠️ Suggested migration change
-ALTER TABLE "Webhook" ADD COLUMN     "domainIds" INTEGER[] DEFAULT ARRAY[]::INTEGER[];
+ALTER TABLE "Webhook" ADD COLUMN "domainIds" INTEGER[] NOT NULL DEFAULT ARRAY[]::INTEGER[];
📝 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
ALTER TABLE "Webhook" ADD COLUMN "domainIds" INTEGER[] DEFAULT ARRAY[]::INTEGER[];
ALTER TABLE "Webhook" ADD COLUMN "domainIds" INTEGER[] NOT NULL DEFAULT ARRAY[]::INTEGER[];
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/web/prisma/migrations/20260227040924_add_webhook_domain_filters/migration.sql`
at line 2, The migration adds the Webhook.domainIds column but leaves it
nullable; update the migration so the "domainIds" column on table "Webhook" is
created as NOT NULL with the same default empty integer array
(ARRAY[]::INTEGER[]), or add an immediate ALTER TABLE ... ALTER COLUMN ... SET
NOT NULL after creation; ensure the SQL references the "Webhook" table and
"domainIds" column exactly and preserves the default empty array to match the
Prisma model Int[] (required) semantics.

Comment on lines +394 to +397
<FormDescription>
Leave this as all domains to receive events from every
domain.
</FormDescription>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Clarify that this only filters domain-aware events.

The backend only narrows dispatch when the emitted event carries a domainId, so this copy currently reads stronger than the actual behavior and can surprise users when team-level events still arrive. Please mirror the same wording in the edit dialog too.

✏️ Suggested copy
-                        Leave this as all domains to receive events from every
-                        domain.
+                        Leave this as all domains to receive events from every
+                        domain. Domain filters apply only to events that include
+                        a domain.
📝 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
<FormDescription>
Leave this as all domains to receive events from every
domain.
</FormDescription>
<FormDescription>
Leave this as all domains to receive events from every
domain. Domain filters apply only to events that include
a domain.
</FormDescription>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/app/`(dashboard)/webhooks/add-webhook.tsx around lines 394 -
397, Update the FormDescription copy in add-webhook.tsx to clarify that the
domain filter only applies to domain-aware events (those that include a
domainId) and does not prevent team- or global-level events; use similar wording
in the edit dialog so both UIs match. Locate the FormDescription node in this
file (and the corresponding edit dialog component) and replace the current text
with a succinct message stating that leaving it blank receives all events, while
specifying a domain only filters events that carry a domainId.

Copy link
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/src/app/(dashboard)/webhooks/webhook-update-dialog.tsx (1)

317-395: Consider extracting a shared DomainSelector component.

The domain selection UI and logic are nearly identical between add-webhook.tsx and this file. A shared component could reduce duplication and ensure consistency.

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

In `@apps/web/src/app/`(dashboard)/webhooks/webhook-update-dialog.tsx around lines
317 - 395, Extract the duplicated domain selection UI/logic into a shared
DomainSelector component: move the FormField render block that handles
"domainIds" (including selectedDomainIds calculation, selectedDomainsLabel,
handleToggleDomain, and the DropdownMenu with DropdownMenuCheckboxItem entries)
into a new reusable DomainSelector component that accepts props { value,
onChange, domains } (or integrates with react-hook-form via a controlled prop)
and replace the inline FormField render in both webhook-update-dialog.tsx and
add-webhook.tsx with <FormField name="domainIds" control={form.control}
render={({field}) => <DomainSelector value={field.value}
onChange={field.onChange} domains={domainsQuery.data} />} /> so the shared logic
(selectedDomainsLabel, toggling, "All domains" checkbox behavior, and aria/text
layout) lives in one place.
🤖 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/src/app/`(dashboard)/webhooks/webhook-update-dialog.tsx:
- Around line 388-391: Update the FormDescription used in the webhook update
dialog (inside the WebhookUpdateDialog component where FormDescription is
rendered) to match the add-webhook clarification: change the text to state that
leaving it as "all domains" will receive events from every domain, but that
domain filtering only applies to events that include a domainId and team-level
events will still be delivered regardless of this setting; replace the existing
sentence in the FormDescription node accordingly to reflect that distinction.

---

Nitpick comments:
In `@apps/web/src/app/`(dashboard)/webhooks/webhook-update-dialog.tsx:
- Around line 317-395: Extract the duplicated domain selection UI/logic into a
shared DomainSelector component: move the FormField render block that handles
"domainIds" (including selectedDomainIds calculation, selectedDomainsLabel,
handleToggleDomain, and the DropdownMenu with DropdownMenuCheckboxItem entries)
into a new reusable DomainSelector component that accepts props { value,
onChange, domains } (or integrates with react-hook-form via a controlled prop)
and replace the inline FormField render in both webhook-update-dialog.tsx and
add-webhook.tsx with <FormField name="domainIds" control={form.control}
render={({field}) => <DomainSelector value={field.value}
onChange={field.onChange} domains={domainsQuery.data} />} /> so the shared logic
(selectedDomainsLabel, toggling, "All domains" checkbox behavior, and aria/text
layout) lives in one place.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fda410bb-db3b-435e-b3b6-114211ffa5e0

📥 Commits

Reviewing files that changed from the base of the PR and between efbba79 and b740c95.

📒 Files selected for processing (3)
  • apps/web/src/app/(dashboard)/campaigns/schedule-campaign.tsx
  • apps/web/src/app/(dashboard)/webhooks/add-webhook.tsx
  • apps/web/src/app/(dashboard)/webhooks/webhook-update-dialog.tsx

Comment on lines +388 to +391
<FormDescription>
Leave this as all domains to receive events from every
domain.
</FormDescription>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Update FormDescription to match add-webhook.tsx clarification.

Per the past review comment on add-webhook.tsx, this text should clarify that domain filtering only applies to events that include a domainId. Team-level events will still be delivered regardless of this setting.

✏️ Suggested copy
                      <FormDescription>
-                       Leave this as all domains to receive events from every
-                       domain.
+                       Leave this as all domains to receive events from every
+                       domain. Domain filters apply only to events that include
+                       a domain.
                      </FormDescription>
📝 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
<FormDescription>
Leave this as all domains to receive events from every
domain.
</FormDescription>
<FormDescription>
Leave this as all domains to receive events from every
domain. Domain filters apply only to events that include
a domain.
</FormDescription>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/app/`(dashboard)/webhooks/webhook-update-dialog.tsx around lines
388 - 391, Update the FormDescription used in the webhook update dialog (inside
the WebhookUpdateDialog component where FormDescription is rendered) to match
the add-webhook clarification: change the text to state that leaving it as "all
domains" will receive events from every domain, but that domain filtering only
applies to events that include a domainId and team-level events will still be
delivered regardless of this setting; replace the existing sentence in the
FormDescription node accordingly to reflect that distinction.

@KMKoushik KMKoushik merged commit 3c2d379 into main Mar 7, 2026
6 checks passed
@KMKoushik KMKoushik deleted the feat/webhook-domain-filter branch March 7, 2026 22:01
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.

1 participant