-
Notifications
You must be signed in to change notification settings - Fork 12k
refactor: circular deps between app store and lib [2] #23734
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
56e22e7
151705f
7a00fff
6bd7c6f
0dc03b0
be3f5f4
88ca269
0ac2fc1
e35bcb5
30dcaf9
975887f
fb0accf
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,10 +1,9 @@ | ||
| import type { LocationObject } from "@calcom/app-store/locations"; | ||
| import { getAppFromSlug } from "@calcom/app-store/utils"; | ||
| import type { PrismaClient } from "@calcom/prisma"; | ||
| import type { User } from "@calcom/prisma/client"; | ||
| import { userMetadata as userMetadataSchema } from "@calcom/prisma/zod-utils"; | ||
|
|
||
| import { TRPCError } from "@trpc/server"; | ||
| import type { LocationObject } from "../locations"; | ||
| import { getAppFromSlug } from "../utils"; | ||
|
|
||
| export const bulkUpdateEventsToDefaultLocation = async ({ | ||
| eventTypeIds, | ||
|
|
@@ -18,19 +17,13 @@ export const bulkUpdateEventsToDefaultLocation = async ({ | |
| const defaultApp = userMetadataSchema.parse(user.metadata)?.defaultConferencingApp; | ||
|
|
||
| if (!defaultApp) { | ||
| throw new TRPCError({ | ||
|
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. An app store utility function shouldn't be throwing a TRPC error |
||
| code: "BAD_REQUEST", | ||
| message: "Default conferencing app not set", | ||
| }); | ||
| throw new Error("Default conferencing app not set"); | ||
| } | ||
|
|
||
| const foundApp = getAppFromSlug(defaultApp.appSlug); | ||
| const appType = foundApp?.appData?.location?.type; | ||
| if (!appType) { | ||
| throw new TRPCError({ | ||
| code: "BAD_REQUEST", | ||
| message: `Default conferencing app '${defaultApp.appSlug}' doesnt exist.`, | ||
| }); | ||
| throw new Error(`Default conferencing app '${defaultApp.appSlug}' doesnt exist.`); | ||
| } | ||
|
|
||
| const credential = await prisma.credential.findFirst({ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,8 @@ | ||
| import type { LocationObject } from "@calcom/app-store/locations"; | ||
|
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. What a nasty import. Love this change
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. Indeed! |
||
| import { getAppFromSlug } from "@calcom/app-store/utils"; | ||
| import type { PrismaClient } from "@calcom/prisma"; | ||
| import { teamMetadataSchema } from "@calcom/prisma/zod-utils"; | ||
|
|
||
| import { TRPCError } from "@trpc/server"; | ||
| import type { LocationObject } from "../locations"; | ||
| import { getAppFromSlug } from "../utils"; | ||
|
|
||
| export const bulkUpdateTeamEventsToDefaultLocation = async ({ | ||
| eventTypeIds, | ||
|
|
@@ -18,22 +17,17 @@ export const bulkUpdateTeamEventsToDefaultLocation = async ({ | |
| where: { id: teamId }, | ||
| select: { metadata: true }, | ||
| }); | ||
| const defaultApp = teamMetadataSchema.parse(team?.metadata)?.defaultConferencingApp; | ||
| const metadataResult = teamMetadataSchema.safeParse(team?.metadata); | ||
| const defaultApp = metadataResult.success ? metadataResult.data?.defaultConferencingApp : null; | ||
|
|
||
| if (!defaultApp) { | ||
| throw new TRPCError({ | ||
|
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. An app store utility function shouldn't be throwing a TRPC error |
||
| code: "BAD_REQUEST", | ||
| message: "Default conferencing app not set", | ||
| }); | ||
| throw new Error("Default conferencing app not set"); | ||
| } | ||
|
|
||
| const foundApp = getAppFromSlug(defaultApp.appSlug); | ||
| const appType = foundApp?.appData?.location?.type; | ||
| if (!appType) { | ||
| throw new TRPCError({ | ||
| code: "BAD_REQUEST", | ||
| message: `Default conferencing app '${defaultApp.appSlug}' doesnt exist.`, | ||
| }); | ||
| throw new Error(`Default conferencing app '${defaultApp.appSlug}' doesnt exist.`); | ||
| } | ||
|
|
||
| const credential = await prisma.credential.findFirst({ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,13 +4,12 @@ import { getGoogleMeetCredential, TestData } from "@calcom/web/test/utils/bookin | |
|
|
||
| import { describe, expect, it } from "vitest"; | ||
|
|
||
| import { DailyLocationType, MeetLocationType } from "@calcom/app-store/locations"; | ||
|
|
||
| import { DailyLocationType, MeetLocationType } from "../locations"; | ||
| import { getDefaultLocations } from "./getDefaultLocations"; | ||
|
|
||
| type User = { | ||
| id: number; | ||
| email?: string; | ||
|
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. Shouldn't be optional since |
||
| email: string; | ||
| name?: string; | ||
| metadata: { | ||
| defaultConferencingApp?: { | ||
|
|
@@ -34,6 +33,7 @@ describe("getDefaultLocation ", async () => { | |
| it("should return location based on user default conferencing app", async () => { | ||
| const user: User = { | ||
| id: 101, | ||
| email: "test@example.com", | ||
| metadata: { | ||
| defaultConferencingApp: { | ||
| appSlug: "google-meet", | ||
|
|
@@ -53,6 +53,7 @@ describe("getDefaultLocation ", async () => { | |
| it("should return calvideo when default conferencing app is not set", async () => { | ||
| const user: User = { | ||
| id: 101, | ||
| email: "test@example.com", | ||
| metadata: {}, | ||
| }; | ||
| await mockUser(user); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| import type { ConnectedApps } from "_utils/getConnectedApps"; | ||
|
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 chainPotential module resolution issue: use a relative path for intra-package type import. -import type { ConnectedApps } from "_utils/getConnectedApps";
+import type { ConnectedApps } from "./_utils/getConnectedApps";To verify, check whether 🏁 Script executed: #!/bin/bash
# Inspect TS config for app-store package
fd tsconfig.json packages/app-store | xargs -I{} sh -c 'echo "== {} =="; cat "{}" | jq "{baseUrl: .compilerOptions.baseUrl, paths: .compilerOptions.paths}"'Length of output: 95 🏁 Script executed: #!/bin/bash
file="packages/app-store/tsconfig.json"
echo "== Inspecting $file =="
# Check for baseUrl
grep -n '"baseUrl"' "$file" || echo "baseUrl not found"
# Check for paths block
grep -nC3 '"paths"' "$file" || echo "paths not found"
# Check for _utils alias specifically
grep -nC2 '"_utils/*"' "$file" || echo "no '_utils/*' alias defined"Length of output: 397 Use a relative import for ConnectedApps in types.d.ts import type { ConnectedApps } from "_utils/getConnectedApps";will fail resolution. Change to: -import type { ConnectedApps } from "_utils/getConnectedApps";
+import type { ConnectedApps } from "./_utils/getConnectedApps";🤖 Prompt for AI Agents |
||
| import type React from "react"; | ||
| import type { z } from "zod"; | ||
|
|
||
| import type { ConnectedApps } from "@calcom/lib/getConnectedApps"; | ||
| import type { EventTypeFormMetadataSchema } from "@calcom/prisma/zod-utils"; | ||
| import type { ButtonProps } from "@calcom/ui/components/button"; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,8 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { bulkUpdateEventsToDefaultLocation } from "@calcom/lib/bulkUpdateEventsToDefaultLocation"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { bulkUpdateEventsToDefaultLocation } from "@calcom/app-store/_utils/bulkUpdateEventsToDefaultLocation"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { prisma } from "@calcom/prisma"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { TRPCError } from "@trpc/server"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { TrpcSessionUser } from "../../../types"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { TBulkUpdateToDefaultLocationInputSchema } from "./bulkUpdateToDefaultLocation.schema"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -13,9 +15,19 @@ type BulkUpdateToDefaultLocationOptions = { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export const bulkUpdateToDefaultLocationHandler = ({ ctx, input }: BulkUpdateToDefaultLocationOptions) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { eventTypeIds } = input; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return bulkUpdateEventsToDefaultLocation({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| eventTypeIds, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| user: ctx.user, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| prisma, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return bulkUpdateEventsToDefaultLocation({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| eventTypeIds, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| user: ctx.user, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| prisma, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (error instanceof Error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new TRPCError({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| code: "BAD_REQUEST", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| message: error.message, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw error; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+18
to
+32
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. Bug: try/catch won’t catch async rejection (missing await). -export const bulkUpdateToDefaultLocationHandler = ({ ctx, input }: BulkUpdateToDefaultLocationOptions) => {
+export const bulkUpdateToDefaultLocationHandler = async ({ ctx, input }: BulkUpdateToDefaultLocationOptions) => {
const { eventTypeIds } = input;
try {
- return bulkUpdateEventsToDefaultLocation({
+ return await bulkUpdateEventsToDefaultLocation({
eventTypeIds,
user: ctx.user,
prisma,
});
} catch (error) {
if (error instanceof Error) {
throw new TRPCError({
code: "BAD_REQUEST",
message: error.message,
});
}
throw error;
}
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.