-
Notifications
You must be signed in to change notification settings - Fork 12k
refactor: static methods of HostRepository & PrismaAttributeRepository #23175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,9 +1,9 @@ | ||||||||||
| import { HostRepository } from "@calcom/lib/server/repository/host"; | ||||||||||
| import { prisma } from "@calcom/prisma"; | ||||||||||
|
|
||||||||||
|
Comment on lines
+1
to
3
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainImport style consistency for prisma This file uses a named import Run this to audit usage: 🏁 Script executed: #!/bin/bash
rg -n --type=ts -C1 -e $'^import\\s+\\{\\s*prisma\\s*\\}\\s+from\\s+"@calcom/prisma"' -e $'^import\\s+prisma\\s+from\\s+"@calcom/prisma"'Length of output: 203787 Standardize prisma import style --- packages/trpc/server/routers/viewer/availability/schedule/delete.handler.ts
@@
-import { prisma } from "@calcom/prisma";
+import prisma from "@calcom/prisma";• Only one import style should be used across the codebase—prefer the default import ( 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
| import { TRPCError } from "@trpc/server"; | ||||||||||
|
|
||||||||||
| import type { TrpcSessionUser } from "../../../../types"; | ||||||||||
| import { updateHostsWithNewDefaultSchedule } from "../util"; | ||||||||||
| import type { TDeleteInputSchema } from "./delete.schema"; | ||||||||||
|
|
||||||||||
| type DeleteOptions = { | ||||||||||
|
|
@@ -15,6 +15,7 @@ type DeleteOptions = { | |||||||||
|
|
||||||||||
| export const deleteHandler = async ({ input, ctx }: DeleteOptions) => { | ||||||||||
| const { user } = ctx; | ||||||||||
| const hostRepo = new HostRepository(prisma); | ||||||||||
|
|
||||||||||
| const scheduleToDelete = await prisma.schedule.findUnique({ | ||||||||||
| where: { | ||||||||||
|
|
@@ -46,7 +47,7 @@ export const deleteHandler = async ({ input, ctx }: DeleteOptions) => { | |||||||||
| // to throw the error if there arent any other schedules | ||||||||||
| if (!scheduleToSetAsDefault) throw new TRPCError({ code: "BAD_REQUEST" }); | ||||||||||
|
|
||||||||||
| await updateHostsWithNewDefaultSchedule(user.id, input.scheduleId, scheduleToSetAsDefault.id); | ||||||||||
| await hostRepo.updateHostsSchedule(user.id, input.scheduleId, scheduleToSetAsDefault.id); | ||||||||||
|
|
||||||||||
| await prisma.user.update({ | ||||||||||
| where: { | ||||||||||
|
|
@@ -57,7 +58,7 @@ export const deleteHandler = async ({ input, ctx }: DeleteOptions) => { | |||||||||
| }, | ||||||||||
| }); | ||||||||||
| } else if (user.defaultScheduleId) { | ||||||||||
| await updateHostsWithNewDefaultSchedule(user.id, input.scheduleId, user.defaultScheduleId); | ||||||||||
| await hostRepo.updateHostsSchedule(user.id, input.scheduleId, user.defaultScheduleId); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| await prisma.schedule.delete({ | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,9 @@ | ||
| import { describe, expect, it, vi, beforeEach } from "vitest"; | ||
|
|
||
| import { HostRepository } from "@calcom/lib/server/repository/host"; | ||
| import { PrismaClient } from "@calcom/prisma"; | ||
|
|
||
| import { | ||
| getDefaultScheduleId, | ||
| hasDefaultSchedule, | ||
| setupDefaultSchedule, | ||
| updateHostsWithNewDefaultSchedule, | ||
| } from "./util"; | ||
| import { getDefaultScheduleId, hasDefaultSchedule, setupDefaultSchedule } from "./util"; | ||
|
|
||
| vi.mock("@calcom/prisma", () => { | ||
| const mockPrisma = { | ||
|
|
@@ -31,9 +27,11 @@ vi.mock("@calcom/prisma", () => { | |
|
|
||
| describe("Availability Utils", () => { | ||
| let prisma: PrismaClient; | ||
| let hostRepo: HostRepository; | ||
|
|
||
| beforeEach(() => { | ||
| prisma = new PrismaClient(); | ||
| hostRepo = new HostRepository(prisma); | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
|
|
@@ -158,7 +156,7 @@ describe("Availability Utils", () => { | |
|
|
||
| (prisma.host.updateMany as any).mockResolvedValue(updateResult); | ||
|
|
||
| const result = await updateHostsWithNewDefaultSchedule(userId, oldScheduleId, newScheduleId, prisma); | ||
| const result = await hostRepo.updateHostsSchedule(userId, oldScheduleId, newScheduleId); | ||
|
|
||
| expect(prisma.host.updateMany).toHaveBeenCalledWith({ | ||
| where: { | ||
|
|
@@ -180,7 +178,7 @@ describe("Availability Utils", () => { | |
|
|
||
| (prisma.host.updateMany as any).mockResolvedValue(updateResult); | ||
|
|
||
| const result = await updateHostsWithNewDefaultSchedule(userId, oldScheduleId, newScheduleId, prisma); | ||
| const result = await hostRepo.updateHostsSchedule(userId, oldScheduleId, newScheduleId); | ||
|
|
||
| expect(prisma.host.updateMany).toHaveBeenCalledWith({ | ||
| where: { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tempted to fully remove those tests, basically these tests are just testing that a method calling prisma is calling prisma, no real value added
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah would be better as integration tests |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,5 @@ | ||
| import type { User } from "@prisma/client"; | ||
|
|
||
| import { HostRepository } from "@calcom/lib/server/repository/host"; | ||
| import type { PrismaClient } from "@calcom/prisma"; | ||
|
|
||
| export const getDefaultScheduleId = async (userId: number, prisma: PrismaClient) => { | ||
|
|
@@ -54,11 +53,3 @@ export const setupDefaultSchedule = async (userId: number, scheduleId: number, p | |
| }, | ||
| }); | ||
| }; | ||
|
|
||
| export const updateHostsWithNewDefaultSchedule = async ( | ||
| userId: number, | ||
| defaultScheduleId: number, | ||
| scheduleId: number | ||
| ) => { | ||
| return await HostRepository.updateHostsSchedule(userId, defaultScheduleId, scheduleId); | ||
| }; | ||
|
Comment on lines
-58
to
-64
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not useful method, just calling repository, deleted it |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Instance-based repository usage looks good; ensure downstream repo query uses
selectnotincludeThe switch to
new PrismaAttributeRepository(prisma)is on point. However,findAllByOrgIdWithOptionsin the repo usesinclude: { options: true }. Our guideline is “only select data you need; never use include”. Please update the repo method to useselectand return only the necessary fields (id, name, slug, type, options[id,value,slug,(isGroup?),(contains?)]).Run this script to (a) ensure no static uses of
PrismaAttributeRepositoryremain and (b) locateinclude:usage inside the repository:🏁 Script executed:
Length of output: 500
🏁 Script executed:
Length of output: 217
🏁 Script executed:
Length of output: 1319
Refactor
findAllByOrgIdWithOptionsto useselectinstead ofincludeThe method in packages/lib/server/repository/PrismaAttributeRepository.ts still uses:
Please replace it with a
selectblock that returns only the fields you need, for example:This ensures we “only select data you need; never use
include.”🤖 Prompt for AI Agents