fix(internal-notes): handle unique constraint violation for presets

[bandhan-majumder]

What does this PR do?

Handles unique constraint violation cause by creating internal note preset creation/update with same name.

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

Before:

before.mp4

After:

after.mp4

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
• My PR is too large (>500 lines or >10 files) and should be split into smaller PRs

[bandhan-majumder]

I have not create an issue for it yet. LMK if it is necessary for this fix.

[coderabbitai]

📝 Walkthrough

Walkthrough

A test suite was added for the updateInternalNotesPresets handler that validates authorization checks, organization admin bypass behavior, conflict handling for database constraint violations, and CRUD operations. The handler implementation was updated to catch Prisma unique constraint errors during database writes and convert them into a CONFLICT error with a descriptive message, while re-throwing all other errors. The core functionality for updating and creating presets remains unchanged.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: handling unique constraint violations for internal notes presets.
Description check	✅ Passed	The description relates to the changeset by explaining the purpose of handling unique constraint violations for preset creation/update, though it lacks concrete testing details.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (3)

packages/trpc/server/routers/viewer/teams/updateInternalNotesPresets.handler.test.ts (2)

203-203: Minor: Mock data has inconsistent teamId values.

The mock returns teamId: 1 but the test input uses teamId: 50. While this doesn't affect test correctness (mocks don't validate this), consistent values would improve test readability and make the test scenarios clearer.

✨ Suggested fix for consistency

-      vi.mocked(prisma.internalNotePreset.findMany).mockResolvedValue([{ id: 1, cancellationReason: '', createdAt: new Date(), name: '', teamId: 1 }]);
+      vi.mocked(prisma.internalNotePreset.findMany).mockResolvedValue([{ id: 1, cancellationReason: '', createdAt: new Date(), name: '', teamId: 50 }]);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/trpc/server/routers/viewer/teams/updateInternalNotesPresets.handler.test.ts`
at line 203, The test's mock data for prisma.internalNotePreset.findMany uses
teamId: 1 while the test input uses teamId: 50; update the mocked object in
updateInternalNotesPresets.handler.test.ts (the mocked call
prisma.internalNotePreset.findMany) to use teamId: 50 (or alternatively change
the test input teamId to 1) so the mock and test input are consistent for
readability.

---

11-32: Unused mock: Prisma class in this mock is never used.

The Prisma object defined here (lines 20-31) is never utilized because:

• The handler imports Prisma from @calcom/prisma/client (not @calcom/prisma)
• The test file also imports Prisma from @calcom/prisma/client on line 5

The test works correctly because both use the real Prisma.PrismaClientKnownRequestError class, but this mock definition is dead code.

🧹 Remove unused mock

 vi.mock("@calcom/prisma", () => ({
   prisma: {
     internalNotePreset: {
       findMany: vi.fn(),
       deleteMany: vi.fn(),
       create: vi.fn(),
       update: vi.fn(),
     },
   },
-  Prisma: {
-    PrismaClientKnownRequestError: class extends Error {
-      code: string;
-      meta: unknown;
-      constructor(message: string, options: { code: string; clientVersion: string; meta?: unknown }) {
-        super(message);
-        this.name = "PrismaClientKnownRequestError";
-        this.code = options.code;
-        this.meta = options.meta;
-      }
-    },
-  },
 }));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/trpc/server/routers/viewer/teams/updateInternalNotesPresets.handler.test.ts`
around lines 11 - 32, Remove the unused Prisma mock inside the
vi.mock("@calcom/prisma") block: the test and handler import Prisma from
"@calcom/prisma/client", so the mocked PrismaClientKnownRequestError class (the
Prisma object and its PrismaClientKnownRequestError) is dead code; either delete
that Prisma mock entirely from the vi.mock block or move/duplicate the mock to
the same module being imported ("@calcom/prisma/client") so that
PrismaClientKnownRequestError is actually mocked where referenced (look for the
vi.mock call and the Prisma/PrismaClientKnownRequestError symbols).

packages/trpc/server/routers/viewer/teams/updateInternalNotesPresets.handler.ts (1)

6-9: Import organization: Move Prisma import to group with other @calcom imports.

The Prisma import is placed after the @trpc/server import, breaking the typical grouping pattern. Consider moving it to be adjacent to the other @calcom/prisma imports for consistency.

✨ Suggested reordering

 import { PermissionCheckService } from "@calcom/features/pbac/services/permission-check.service";
 import { prisma } from "@calcom/prisma";
+import { Prisma } from "@calcom/prisma/client";
 import { MembershipRole } from "@calcom/prisma/enums";
 import type { TrpcSessionUser } from "@calcom/trpc/server/types";

 import { TRPCError } from "@trpc/server";

 import type { TUpdateInternalNotesPresetsInputSchema } from "./updateInternalNotesPresets.schema";
-import { Prisma } from "@calcom/prisma/client";

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/trpc/server/routers/viewer/teams/updateInternalNotesPresets.handler.ts`
around lines 6 - 9, Move the Prisma import so it sits with the other `@calcom`
imports for consistent grouping: relocate the line importing Prisma from
"@calcom/prisma/client" to be adjacent to any existing `@calcom` imports (e.g.,
near the import of TUpdateInternalNotesPresetsInputSchema if it's from a `@calcom`
module), keeping the TRPCError import from "@trpc/server" separate; no code
changes other than reordering the import statements.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@packages/trpc/server/routers/viewer/teams/updateInternalNotesPresets.handler.test.ts`:
- Line 203: The test's mock data for prisma.internalNotePreset.findMany uses
teamId: 1 while the test input uses teamId: 50; update the mocked object in
updateInternalNotesPresets.handler.test.ts (the mocked call
prisma.internalNotePreset.findMany) to use teamId: 50 (or alternatively change
the test input teamId to 1) so the mock and test input are consistent for
readability.
- Around line 11-32: Remove the unused Prisma mock inside the
vi.mock("@calcom/prisma") block: the test and handler import Prisma from
"@calcom/prisma/client", so the mocked PrismaClientKnownRequestError class (the
Prisma object and its PrismaClientKnownRequestError) is dead code; either delete
that Prisma mock entirely from the vi.mock block or move/duplicate the mock to
the same module being imported ("@calcom/prisma/client") so that
PrismaClientKnownRequestError is actually mocked where referenced (look for the
vi.mock call and the Prisma/PrismaClientKnownRequestError symbols).

In
`@packages/trpc/server/routers/viewer/teams/updateInternalNotesPresets.handler.ts`:
- Around line 6-9: Move the Prisma import so it sits with the other `@calcom`
imports for consistent grouping: relocate the line importing Prisma from
"@calcom/prisma/client" to be adjacent to any existing `@calcom` imports (e.g.,
near the import of TUpdateInternalNotesPresetsInputSchema if it's from a `@calcom`
module), keeping the TRPCError import from "@trpc/server" separate; no code
changes other than reordering the import statements.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: be262e95-f706-40b5-a91a-6a56a013b629

📥 Commits

Reviewing files that changed from the base of the PR and between c0d105e and a57ca7b.

📒 Files selected for processing (2)

• packages/trpc/server/routers/viewer/teams/updateInternalNotesPresets.handler.test.ts
• packages/trpc/server/routers/viewer/teams/updateInternalNotesPresets.handler.ts

[romitg2]

why are we showing both toast if it fails, shouldn't we just show error one in case of failure? wdyt

edit: seems like it's from earlier success. nevermind if that's the case

[bandhan-majumder]

@romitg2 yup.. it's from earlier ones

beforev2.mp4