Skip to content

Comments

fix: Support domains without @ prefix in watchlist#24476

Merged
hariombalhara merged 2 commits intomainfrom
10-15-fix_support_domains_without_prefix
Oct 15, 2025
Merged

fix: Support domains without @ prefix in watchlist#24476
hariombalhara merged 2 commits intomainfrom
10-15-fix_support_domains_without_prefix

Conversation

@hariombalhara
Copy link
Member

@hariombalhara hariombalhara commented Oct 15, 2025

What does this PR do?

Fixes domain matching in the watchlist spam detection system. Domains stored in the database without the @ prefix (e.g., qq.com) were not matching because the normalizeDomain() function was adding an @ prefix during lookups.

Problem

The normalizeDomain() function was returning domains with an @ prefix (e.g., @qq.com), but database records store domains WITHOUT the prefix (e.g., qq.com). This mismatch caused domain-based spam blocking to fail completely.

Example scenario:

  • Database has watchlist entry: qq.com
  • User books with email: user@qq.com
  • System extracts domain and normalizes to: @qq.com
  • Database query looks for: @qq.com
  • Result: No match found

Solution

Removed the @ prefix from the normalizeDomain() function return value to align with how domains are stored in the database.

After fix:

  • Database has watchlist entry: qq.com
  • User books with email: user@qq.com
  • System extracts domain and normalizes to: qq.com
  • Database query looks for: qq.com
  • Result: Match found

Changes Made

Core Logic

  • normalization.ts: Updated normalizeDomain() to return domain without @ prefix
  • Updated JSDoc comments to reflect the new behavior

Test Updates

Updated expectations in all test files to expect domains without @ prefix:

  • normalization.test.ts: 11 test expectations updated
  • GlobalBlockingService.test.ts: 10 domain expectations updated
  • OrganizationBlockingService.test.ts: 5 domain expectations updated
  • WatchlistService.test.ts: 3 domain expectations updated
  • spam-booking.integration-test.ts: 2 test cases fixed

Testing

All tests passing:

  • Normalization unit tests: 15/15 passed
  • Service layer tests: 30/30 passed
  • Spam booking integration tests: 8/8 passed
  • TypeScript type-check: No errors

How to test manually

  1. Add a domain to the watchlist without @ prefix:

    INSERT INTO Watchlist (type, value, action, isGlobal) 
    VALUES ('DOMAIN', 'testdomain.com', 'BLOCK', true);
  2. Try to create a booking with email user@testdomain.com

  3. Expected result: Booking should be blocked and return a decoy response

Impact

  • ✅ Global domain blocking now works correctly
  • ✅ Organization-specific domain blocking now works correctly
  • ✅ Existing database records with domains stored without @ prefix will now match properly
  • ✅ No breaking changes - only fixes broken functionality

Mandatory Tasks

  • I have self-reviewed the code
  • I have updated tests to cover the changes
  • I confirm automated tests are in place that prove my fix is effective
  • All existing tests still pass

@keithwillcode keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Oct 15, 2025
Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 15, 2025

Walkthrough

The change standardizes domain normalization to use plain domain strings without a leading @. The normalization utility now strips any leading @ and returns the domain. Services and their tests (GlobalBlockingService, OrganizationBlockingService, WatchlistService) are updated to pass domains without @ to repository methods. The free email domain check was simplified to use emailDomain directly (no slice) and to pass the plain domain to isFreeEmailDomain. Bookings spam test data now constructs emails as user@${blockedDomain}. No exported/public signatures changed.

Possibly related PRs

  • fix: Organization User Events' Spam #24468: Also modifies spam booking tests in packages/features/bookings/lib/handleNewBooking/test/spam-booking.integration-test.ts, adding organization-level spam cases alongside this PR’s domain normalization updates.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly describes the main change by indicating that the watchlist now supports domains without the “@” prefix, which directly aligns with the core purpose of the pull request.
Description Check ✅ Passed The description thoroughly outlines the problem, explains the solution, lists the affected files and tests, and includes both automated and manual testing steps, all of which align directly with the changes made in the pull request.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ 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 10-15-fix_support_domains_without_prefix

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 67843ad and bd1aa42.

📒 Files selected for processing (7)
  • packages/features/bookings/lib/handleNewBooking/test/spam-booking.integration-test.ts (2 hunks)
  • packages/features/watchlist/lib/freeEmailDomainCheck/checkIfFreeEmailDomain.ts (1 hunks)
  • packages/features/watchlist/lib/service/GlobalBlockingService.test.ts (7 hunks)
  • packages/features/watchlist/lib/service/OrganizationBlockingService.test.ts (4 hunks)
  • packages/features/watchlist/lib/service/WatchlistService.test.ts (5 hunks)
  • packages/features/watchlist/lib/utils/normalization.test.ts (3 hunks)
  • packages/features/watchlist/lib/utils/normalization.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/features/watchlist/lib/service/GlobalBlockingService.test.ts
  • packages/features/watchlist/lib/utils/normalization.test.ts
  • packages/features/watchlist/lib/freeEmailDomainCheck/checkIfFreeEmailDomain.ts
  • packages/features/bookings/lib/handleNewBooking/test/spam-booking.integration-test.ts
  • packages/features/watchlist/lib/service/OrganizationBlockingService.test.ts
  • packages/features/watchlist/lib/service/WatchlistService.test.ts
  • packages/features/watchlist/lib/utils/normalization.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • packages/features/watchlist/lib/service/GlobalBlockingService.test.ts
  • packages/features/watchlist/lib/utils/normalization.test.ts
  • packages/features/watchlist/lib/freeEmailDomainCheck/checkIfFreeEmailDomain.ts
  • packages/features/bookings/lib/handleNewBooking/test/spam-booking.integration-test.ts
  • packages/features/watchlist/lib/service/OrganizationBlockingService.test.ts
  • packages/features/watchlist/lib/service/WatchlistService.test.ts
  • packages/features/watchlist/lib/utils/normalization.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • packages/features/watchlist/lib/service/GlobalBlockingService.test.ts
  • packages/features/watchlist/lib/utils/normalization.test.ts
  • packages/features/watchlist/lib/freeEmailDomainCheck/checkIfFreeEmailDomain.ts
  • packages/features/bookings/lib/handleNewBooking/test/spam-booking.integration-test.ts
  • packages/features/watchlist/lib/service/OrganizationBlockingService.test.ts
  • packages/features/watchlist/lib/service/WatchlistService.test.ts
  • packages/features/watchlist/lib/utils/normalization.ts
🧠 Learnings (1)
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.

Applied to files:

  • packages/features/bookings/lib/handleNewBooking/test/spam-booking.integration-test.ts
🧬 Code graph analysis (2)
packages/features/watchlist/lib/utils/normalization.test.ts (1)
packages/features/watchlist/lib/utils/normalization.ts (2)
  • normalizeDomain (44-58)
  • extractDomainFromEmail (66-75)
packages/features/watchlist/lib/freeEmailDomainCheck/checkIfFreeEmailDomain.ts (1)
packages/features/di/watchlist/containers/watchlist.ts (1)
  • getWatchlistFeature (64-66)
🔇 Additional comments (14)
packages/features/bookings/lib/handleNewBooking/test/spam-booking.integration-test.ts (2)

246-247: LGTM! Test data construction is clearer.

The email construction user@${blockedDomain} is more readable now that blockedDomain contains the plain domain without the @ prefix. This aligns with the normalization changes where domains are stored and compared without the @ symbol.


499-500: LGTM! Consistent with the normalization changes.

The test data construction follows the same pattern as the global watchlist test, correctly building the email address from the normalized domain value.

packages/features/watchlist/lib/utils/normalization.ts (2)

35-57: LGTM! Documentation and behavior are now aligned.

The JSDoc correctly documents that domains are stored without the @ prefix, and the implementation ensures consistent normalization by stripping the @ if present. This change aligns the runtime behavior with the database storage convention.


60-75: LGTM! Domain extraction maintains consistency.

The extractDomainFromEmail function correctly passes the domain portion through normalizeDomain, ensuring the returned value has no @ prefix.

packages/features/watchlist/lib/freeEmailDomainCheck/checkIfFreeEmailDomain.ts (1)

15-24: LGTM! Excellent simplification.

Since extractDomainFromEmail now returns domains without the @ prefix, the intermediate domainWithoutAt variable is no longer needed. The code is cleaner and more straightforward.

packages/features/watchlist/lib/service/OrganizationBlockingService.test.ts (2)

55-55: LGTM! Test expectations align with normalization.

The test correctly expects findBlockedDomain to be called with the normalized domain "example.com" without the @ prefix.


62-62: LGTM! Consistent test updates across all test cases.

All test cases have been updated consistently to use and expect domain values without the @ prefix, matching the new normalization behavior.

Also applies to: 102-102, 135-135, 168-168

packages/features/watchlist/lib/service/WatchlistService.test.ts (2)

89-116: LGTM! Service normalization is correctly tested.

The test verifies that WatchlistService.createEntry correctly normalizes domain values (removing @ prefix) before passing them to the repository. The expectations match the new normalization behavior.


175-202: LGTM! Comprehensive coverage of domain normalization.

Test cases consistently verify that domain values are stored and retrieved without the @ prefix across different scenarios (manual entries, free domain policy, organization entries).

Also applies to: 329-329

packages/features/watchlist/lib/service/GlobalBlockingService.test.ts (2)

50-50: LGTM! isBlocked method tests correctly updated.

All test cases for the isBlocked method now expect findBlockedDomain to be called with normalized domains (no @ prefix), including tests for email matching, domain matching, and normalization.

Also applies to: 75-75, 96-96


57-57: LGTM! Comprehensive test coverage for domain normalization.

The test cases thoroughly verify that domains are normalized (@ prefix removed) across various scenarios including free email domain checks, @ prefix handling, and uppercase normalization.

Also applies to: 129-129, 154-154, 168-168, 177-177, 185-185, 193-193

packages/features/watchlist/lib/utils/normalization.test.ts (3)

20-41: LGTM! Normalization tests correctly reflect the new behavior.

The test cases verify that normalizeDomain correctly removes the @ prefix when present (line 22) and handles various domain formats including subdomains and multi-level TLDs without the @ prefix.


44-58: LGTM! Domain extraction tests are comprehensive.

The tests verify that extractDomainFromEmail correctly extracts and normalizes domains (without @ prefix) from email addresses, including multi-level TLDs.


78-82: LGTM! Edge case handling is verified.

The test confirms that international domains are correctly normalized without the @ prefix, maintaining consistency with the overall normalization behavior.


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.

@vercel
Copy link

vercel bot commented Oct 15, 2025

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

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Oct 15, 2025 0:44am
cal-eu Ignored Ignored Oct 15, 2025 0:44am

@hariombalhara hariombalhara force-pushed the 10-15-fix_support_domains_without_prefix branch from 44c7abe to bd1aa42 Compare October 15, 2025 12:44
@hariombalhara hariombalhara marked this pull request as ready for review October 15, 2025 12:56
@hariombalhara hariombalhara requested a review from a team as a code owner October 15, 2025 12:56
@graphite-app graphite-app bot requested a review from a team October 15, 2025 12:57
Copy link
Member

@alishaz-polymath alishaz-polymath left a comment

Choose a reason for hiding this comment

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

Legend! Thank you. 🫡

@dosubot dosubot bot added bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working labels Oct 15, 2025
Copy link
Contributor

@emrysal emrysal left a comment

Choose a reason for hiding this comment

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

lgtm

@github-actions
Copy link
Contributor

github-actions bot commented Oct 15, 2025

E2E results are ready!

@hariombalhara hariombalhara merged commit 37cec89 into main Oct 15, 2025
96 of 111 checks passed
@hariombalhara hariombalhara deleted the 10-15-fix_support_domains_without_prefix branch October 15, 2025 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO ready-for-e2e size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants