Skip to content

Comments

chore: Watchlist schema update#24246

Merged
alishaz-polymath merged 8 commits intomainfrom
feat/watchlist-schema-update
Oct 4, 2025
Merged

chore: Watchlist schema update#24246
alishaz-polymath merged 8 commits intomainfrom
feat/watchlist-schema-update

Conversation

@alishaz-polymath
Copy link
Member

What does this PR do?

Updates the Watchlist Schema to include Reporting & Blocking feature changes, and an audit trail

  • Fixes #XXXX (GitHub issue number)
  • Fixes CAL-XXXX (Linear issue number - should be visible at the bottom of the GitHub issue description)

Visual Demo (For contributors especially)

A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).

Video Demo (if applicable):

  • Show screen recordings of the issue or feature.
  • Demonstrate how to reproduce the issue, the behavior before and after the change.

Image Demo (if applicable):

  • Add side-by-side screenshots of the original and updated change.
  • Highlight any significant change(s).

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. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • Are there environment variables that should be set?
  • What are the minimal test data to have?
  • What is expected (happy path) to have (input and output)?
  • Any other important info that could help to test that PR

Checklist

  • I haven't read the contributing guide
  • My code doesn't follow the style guidelines of this project
  • I haven't commented my code, particularly in hard-to-understand areas
  • I haven't checked if my changes generate no new warnings

@github-actions github-actions bot added the ❗️ migrations contains migration files label Oct 3, 2025
@keithwillcode keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Oct 3, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 3, 2025

Walkthrough

Adds a new enum WatchlistSource and the ALERT value to WatchlistAction. Migrates Watchlist.id from string to UUID by adding a temporary uid, backfilling with gen_random_uuid(), swapping columns, and re-creating the primary key (handling dependent constraints). Removes createdAt, createdById, severity, updatedAt, updatedById and the WatchlistSeverity type; adds isGlobal, lastUpdatedAt, source. Creates WatchlistAudit and WatchlistEventAudit tables, drops BlockedBookingLog, and adds multiple indexes. Updates Prisma schema and service code to stop using WatchlistSeverity.

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title “chore: Watchlist schema update” accurately reflects the primary change in this PR, which is a comprehensive update to the Watchlist schema; it is concise, clear, and directly related to the main purpose of the changeset.
Description Check ✅ Passed The description clearly states that the PR updates the Watchlist schema to add reporting, blocking, and an audit trail, and it is directly related to the changes implemented; although some placeholders remain, it is on-topic and describes the intent of the changes.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/watchlist-schema-update

📜 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 243a4af and 9c3b2bb.

📒 Files selected for processing (1)
  • packages/prisma/migrations/20251003103832_upsert_watchlist_audit/migration.sql (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/prisma/migrations/20251003103832_upsert_watchlist_audit/migration.sql

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.

volnei
volnei previously requested changes Oct 3, 2025
Copy link
Contributor

@volnei volnei left a comment

Choose a reason for hiding this comment

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

Small change requested

@vercel
Copy link

vercel bot commented Oct 3, 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 3, 2025 7:36pm
cal-eu Ignored Ignored Oct 3, 2025 7:36pm

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: 5

🧹 Nitpick comments (3)
packages/prisma/migrations/20251003103832_upsert_watchlist_audit/migration.sql (2)

61-69: Consider auto-updating lastUpdatedAt at DB level.

If app forgets to set it, it will drift. Add an UPDATE trigger.

-- Optional helper to keep lastUpdatedAt current
CREATE OR REPLACE FUNCTION set_watchlist_last_updated()
RETURNS trigger LANGUAGE plpgsql AS $$
BEGIN
  NEW."lastUpdatedAt" := CURRENT_TIMESTAMP;
  RETURN NEW;
END$$;

DROP TRIGGER IF EXISTS watchlist_last_updated_trg ON "Watchlist";
CREATE TRIGGER watchlist_last_updated_trg
BEFORE UPDATE ON "Watchlist"
FOR EACH ROW EXECUTE FUNCTION set_watchlist_last_updated();

92-92: Redundant unique index on primary key.

"WatchlistAudit_id_key" duplicates the PK. Drop this line to avoid unnecessary index bloat.

-CREATE UNIQUE INDEX "WatchlistAudit_id_key" ON "WatchlistAudit"("id");
packages/prisma/schema.prisma (1)

2281-2294: Watchlist model: add update semantics and validate org/global relation.

  • lastUpdatedAt isn’t @updatedat; consider using @updatedat for auto-update or ensure app sets it.
  • organizationId has no relation; consider linking to Team to enforce integrity.
 model Watchlist {
   id             String          @id @default(uuid()) @db.Uuid
   type           WatchlistType
   value          String
   description    String?
   isGlobal       Boolean         @default(false)
-  organizationId Int?
+  organizationId Int?
+  // Optional: enforce referential integrity at Prisma level
+  // organization   Team?           @relation(fields: [organizationId], references: [id], onDelete: SetNull)
   action         WatchlistAction @default(REPORT)
   source         WatchlistSource @default(MANUAL)
-  lastUpdatedAt  DateTime        @default(now())
+  lastUpdatedAt  DateTime        @default(now())
+  // Optional: prefer @updatedAt if DB trigger is not used
+  // lastUpdatedAt  DateTime        @updatedAt
   @@unique([type, value, organizationId])
   @@index([type, value, organizationId, action])
 }
📜 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 c11814c and 176ac9e.

📒 Files selected for processing (2)
  • packages/prisma/migrations/20251003103832_upsert_watchlist_audit/migration.sql (1 hunks)
  • packages/prisma/schema.prisma (1 hunks)
🔇 Additional comments (5)
packages/prisma/schema.prisma (3)

2276-2279: WatchlistSource enum — LGTM.

Defaults and values align with the migration.


2273-2274: Enum value ALERT added — looks good.
No switch/case references found in TS; manually verify any code paths handling WatchlistAction (JS/TS/other) include the new ALERT value.


2296-2298: Prisma UUIDv7 Support: Prisma supports @default(uuid(7)) as of version 5.18.0; ensure your project uses Prisma ≥ 5.18.0 or fall back to @default(uuid()) or app‐generated IDs. Applies to lines 2296–2298 and 2313–2314.

packages/prisma/migrations/20251003103832_upsert_watchlist_audit/migration.sql (2)

23-37: No additional FKs referencing Watchlist.id
Only createdBy/updatedBy constraints are dropped; no other references exist.


70-77: BlockedBookingLog removal verified: no references found outside migrations; safe to drop.

volnei
volnei previously approved these changes Oct 3, 2025
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: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
packages/features/watchlist/watchlist.repository.ts (3)

12-19: Add explicit select to the Prisma query.

The query returns all fields from the Watchlist table. Per coding guidelines, Prisma queries should only select the data you need using select rather than returning entire records.

As per coding guidelines, consider specifying only the required fields:

 const emailInWatchlist = await db.watchlist.findFirst({
   where: {
     OR: [
       { type: WatchlistType.EMAIL, value: email },
       { type: WatchlistType.DOMAIN, value: domain },
     ],
   },
+  select: {
+    id: true,
+    type: true,
+    value: true,
+    // Add other fields as needed by callers
+  },
 });

29-34: Add explicit select to the Prisma query.

This query also returns all fields from the Watchlist table. Apply the same pattern as suggested for getBlockedEmailInWatchlist to select only required fields.

As per coding guidelines:

 const domainInWatchWatchlist = await db.watchlist.findFirst({
   where: {
     type: WatchlistType.DOMAIN,
     value: emailDomain,
   },
+  select: {
+    id: true,
+    type: true,
+    value: true,
+    // Add other fields as needed by callers
+  },
 });

53-88: Add explicit select to the Prisma query.

This findMany query returns all fields from each Watchlist record. Specify only the fields needed by callers.

As per coding guidelines:

 const blockedRecords = await db.watchlist.findMany({
   where: {
     OR: [
       ...(usernames.length > 0
         ? [
             {
               type: WatchlistType.USERNAME,
               value: {
                 in: usernames,
               },
             },
           ]
         : []),
       ...(emails.length > 0
         ? [
             {
               type: WatchlistType.EMAIL,
               value: {
                 in: emails,
               },
             },
           ]
         : []),
       ...(domains.length > 0
         ? [
             {
               type: WatchlistType.DOMAIN,
               value: {
                 in: domains,
               },
             },
           ]
         : []),
     ],
   },
+  select: {
+    id: true,
+    type: true,
+    value: true,
+    // Add other fields as needed by callers
+  },
 });
📜 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 176ac9e and 3334a8f.

📒 Files selected for processing (2)
  • packages/features/watchlist/watchlist.repository.ts (1 hunks)
  • packages/prisma/migrations/20251003103832_upsert_watchlist_audit/migration.sql (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{service,repository}.ts

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

Avoid dot-suffixes like .service.ts or .repository.ts for new files; reserve .test.ts, .spec.ts, .types.ts for their specific purposes

Files:

  • packages/features/watchlist/watchlist.repository.ts
**/*.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/watchlist.repository.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/watchlist.repository.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/watchlist.repository.ts
🔇 Additional comments (1)
packages/features/watchlist/watchlist.repository.ts (1)

4-4: LGTM! Import correctly updated.

The removal of WatchlistSeverity from imports aligns with the schema migration that drops this enum type.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2025

E2E results are ready!

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.

Nice! LGTM

@alishaz-polymath alishaz-polymath merged commit 7cd8dbf into main Oct 4, 2025
38 checks passed
@alishaz-polymath alishaz-polymath deleted the feat/watchlist-schema-update branch October 4, 2025 00:44
@coderabbitai coderabbitai bot mentioned this pull request Oct 14, 2025
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO ❗️ migrations contains migration files ready-for-e2e size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants