Skip to content

feat: add option to skip CRM cancellation for blocklisted bookings#28492

Closed
joeauyeung wants to merge 3 commits intomainfrom
devin/1773886321-blocklist-skip-crm-cancellation
Closed

feat: add option to skip CRM cancellation for blocklisted bookings#28492
joeauyeung wants to merge 3 commits intomainfrom
devin/1773886321-blocklist-skip-crm-cancellation

Conversation

@joeauyeung
Copy link
Contributor

@joeauyeung joeauyeung commented Mar 19, 2026

What does this PR do?

Adds an organization-level setting blocklistSkipCrmOnCancel that, when enabled, signals that silently cancelled bookings from blocklisted emails/domains should skip the CRM cancellation flow.

Changes:

  • New blocklistSkipCrmOnCancel boolean field on OrganizationSettings (Prisma schema + migration)
  • tRPC update schema/handler support for the new field
  • OrganizationRepository.findCurrentOrg exposes the field
  • New BlocklistSkipCrmOnCancelSwitch toggle component on the org privacy/blocklist page
  • SpamCheckService accepts and threads the flag through to BlockingResult.skipCrmOnCancel
  • RegularBookingService fetches the org setting and passes it to startCheck
  • Unit tests for SpamCheckService covering the new skipCrmOnCancel flag behavior

⚠️ Important: Incomplete downstream consumption

The BlockingResult.skipCrmOnCancel flag is plumbed through but no downstream code currently reads it to conditionally skip CRM operations. A follow-up change is needed in the booking cancellation/decoy path (e.g., EventManager or handleCancelBooking) to actually honor this flag and skip CRM event deletion when it is true. This PR establishes the setting, UI, and data flow — the behavioral change is not yet wired end-to-end.

Items for reviewer attention

  1. Extra DB query on all org bookings: RegularBookingService now calls organizationSettings.findUnique for every org booking attempt, not just blocked ones. Consider whether this should be deferred until after spamCheckResult.isBlocked is confirmed, or batched with existing org data fetching.
  2. Cosmetic diff noise: The diff includes hasOwnPropertyObject.hasOwn refactor across all settings in update.handler.ts, import reordering (biome auto-fix), and a whitespace fix. These are cosmetic but inflate the diff.
  3. skipCrmOnCancel only set for org-level blocks: Global blocks won't carry this flag — intentional since the setting lives on OrganizationSettings.

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. N/A — no public API or doc changes.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  1. Navigate to an organization's Settings → Privacy page
  2. Scroll to the Organization blocklist section
  3. A new toggle "Skip CRM cancellation for blocked bookings" should appear below the blocklist table (only visible to users with edit permissions)
  4. Toggle it on → verify a success toast appears and the setting persists on page refresh
  5. Toggle it off → same verification
  6. Simulate a mutation error (e.g., network disconnect) → verify the toggle rolls back to its previous state
  7. Verify the setting is stored in OrganizationSettings.blocklistSkipCrmOnCancel in the database

Note: End-to-end behavioral verification (CRM events actually being skipped on blocked booking) requires downstream consumption code which is not yet implemented.

Checklist

  • My code follows the style guidelines of this project
  • I have checked if my changes generate no new warnings

Link to Devin session: https://app.devin.ai/sessions/28be88bf0b1d435993609f0ce3f9841a
Requested by: @joeauyeung

- Add blocklistSkipCrmOnCancel field to OrganizationSettings schema
- Add UI toggle on the organization blocklist privacy page
- Wire up setting through SpamCheckService and RegularBookingService
- Add BlockingResult.skipCrmOnCancel flag for downstream consumers
- Add i18n translation strings

Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@github-actions github-actions bot added the ❗️ migrations contains migration files label Mar 19, 2026
Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 11 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/features/bookings/lib/service/RegularBookingService.ts">

<violation number="1" location="packages/features/bookings/lib/service/RegularBookingService.ts:674">
P2: This adds an awaited organization settings query on every org booking attempt, even when the booking is not blocked. It creates avoidable latency/load in a hot path; defer this lookup until a blocked result actually needs the flag (or fetch it lazily inside spam-check handling).</violation>
</file>

<file name="apps/web/modules/ee/organizations/components/BlocklistSkipCrmOnCancelSwitch.tsx">

<violation number="1" location="apps/web/modules/ee/organizations/components/BlocklistSkipCrmOnCancelSwitch.tsx:17">
P2: The toggle uses optimistic local state but never rolls back or re-syncs from updated props after a failed mutation, so UI state can drift from the persisted organization setting.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

});

spamCheckService.startCheck({ email: bookerEmail, organizationId: eventOrganizationId });
const orgSettings = eventOrganizationId
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 19, 2026

Choose a reason for hiding this comment

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

P2: This adds an awaited organization settings query on every org booking attempt, even when the booking is not blocked. It creates avoidable latency/load in a hot path; defer this lookup until a blocked result actually needs the flag (or fetch it lazily inside spam-check handling).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/features/bookings/lib/service/RegularBookingService.ts, line 674:

<comment>This adds an awaited organization settings query on every org booking attempt, even when the booking is not blocked. It creates avoidable latency/load in a hot path; defer this lookup until a blocked result actually needs the flag (or fetch it lazily inside spam-check handling).</comment>

<file context>
@@ -671,7 +671,18 @@ async function handler(
   });
 
-  spamCheckService.startCheck({ email: bookerEmail, organizationId: eventOrganizationId });
+  const orgSettings = eventOrganizationId
+    ? await deps.prismaClient.organizationSettings.findUnique({
+        where: { organizationId: eventOrganizationId },
</file context>
Fix with Cubic

@github-actions
Copy link
Contributor

github-actions bot commented Mar 19, 2026

Devin AI is addressing Cubic AI's review feedback

New feedback has been sent to the existing Devin session.

View Devin Session


✅ Pushed commit 50e2608

…celSwitch

Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
@joeauyeung joeauyeung closed this Mar 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

❗️ migrations contains migration files size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant