From fb1ef890e613df2e88fd2f079060567c4379dc55 Mon Sep 17 00:00:00 2001 From: Rohit Kumar Saini Date: Tue, 19 Nov 2024 13:41:58 +0100 Subject: [PATCH 01/27] feat(404): add additionalData in getAsset for special case --- app/modules/asset/service.server.ts | 39 ++++++++++++++++++- app/modules/organization/service.server.ts | 1 + .../_layout+/assets.$assetId.activity.tsx | 3 +- .../assets.$assetId.overview.duplicate.tsx | 11 ++++-- .../_layout+/assets.$assetId.overview.tsx | 3 +- ...sets.$assetId.overview.update-location.tsx | 4 +- app/routes/_layout+/assets.$assetId.tsx | 3 +- app/routes/_layout+/assets.$assetId_.edit.tsx | 14 ++++--- app/routes/api+/asset.scan.ts | 3 +- app/utils/roles.server.ts | 1 + 10 files changed, 65 insertions(+), 17 deletions(-) diff --git a/app/modules/asset/service.server.ts b/app/modules/asset/service.server.ts index ae02ecfb2..2f0ff97e7 100644 --- a/app/modules/asset/service.server.ts +++ b/app/modules/asset/service.server.ts @@ -11,6 +11,7 @@ import type { Booking, Kit, AssetIndexSettings, + UserOrganization, } from "@prisma/client"; import { AssetStatus, @@ -101,17 +102,47 @@ type AssetWithInclude = export async function getAsset({ id, organizationId, + userOrganizations, include, }: Pick & { organizationId: Asset["organizationId"]; + userOrganizations: Pick[]; include?: T; }): Promise> { try { + const otherOrganizationIds = userOrganizations.map( + (org) => org.organizationId + ); + const asset = await db.asset.findFirstOrThrow({ - where: { id, organizationId }, + where: { + OR: [ + { id, organizationId }, + { id, organizationId: { in: otherOrganizationIds } }, + ], + }, include: { ...include }, }); + /* User is accessing the asset in the wrong organization. In that case we need special 404 handling. */ + if ( + asset.organizationId !== organizationId && + otherOrganizationIds.includes(asset.organizationId) + ) { + throw new ShelfError({ + cause: null, + title: "Asset not found", + message: "", + additionalData: { + model: "asset", + assetOrganization: userOrganizations.find( + (org) => org.organizationId === asset.organizationId + ), + }, + label, + }); + } + return asset as AssetWithInclude; } catch (cause) { throw new ShelfError({ @@ -119,7 +150,11 @@ export async function getAsset({ title: "Asset not found", message: "The asset you are trying to access does not exist or you do not have permission to access it.", - additionalData: { id, organizationId }, + additionalData: { + id, + organizationId, + ...(cause instanceof ShelfError ? cause.additionalData : {}), + }, label, shouldBeCaptured: !isNotFoundError(cause), }); diff --git a/app/modules/organization/service.server.ts b/app/modules/organization/service.server.ts index f59658445..780dc316f 100644 --- a/app/modules/organization/service.server.ts +++ b/app/modules/organization/service.server.ts @@ -270,6 +270,7 @@ export async function getUserOrganizations({ userId }: { userId: string }) { return await db.userOrganization.findMany({ where: { userId }, select: { + organizationId: true, roles: true, organization: { select: { diff --git a/app/routes/_layout+/assets.$assetId.activity.tsx b/app/routes/_layout+/assets.$assetId.activity.tsx index 5f9b5aba6..0be205ff5 100644 --- a/app/routes/_layout+/assets.$assetId.activity.tsx +++ b/app/routes/_layout+/assets.$assetId.activity.tsx @@ -28,7 +28,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { }); try { - const { organizationId } = await requirePermission({ + const { organizationId, userOrganizations } = await requirePermission({ userId, request, entity: PermissionEntity.asset, @@ -38,6 +38,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { const asset = await getAsset({ id, organizationId, + userOrganizations, include: { notes: { orderBy: { createdAt: "desc" }, diff --git a/app/routes/_layout+/assets.$assetId.overview.duplicate.tsx b/app/routes/_layout+/assets.$assetId.overview.duplicate.tsx index ab101891a..b6ff87f53 100644 --- a/app/routes/_layout+/assets.$assetId.overview.duplicate.tsx +++ b/app/routes/_layout+/assets.$assetId.overview.duplicate.tsx @@ -33,14 +33,18 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { }); try { - const { organizationId } = await requirePermission({ + const { organizationId, userOrganizations } = await requirePermission({ userId, request, entity: PermissionEntity.asset, action: PermissionAction.create, }); - const asset = await getAsset({ id: assetId, organizationId }); + const asset = await getAsset({ + id: assetId, + organizationId, + userOrganizations, + }); return json( data({ @@ -75,7 +79,7 @@ export async function action({ context, request, params }: ActionFunctionArgs) { }); try { - const { organizationId } = await requirePermission({ + const { organizationId, userOrganizations } = await requirePermission({ userId, request, entity: PermissionEntity.asset, @@ -85,6 +89,7 @@ export async function action({ context, request, params }: ActionFunctionArgs) { const asset = await getAsset({ id: assetId, organizationId, + userOrganizations, include: { custody: { include: { custodian: true } }, tags: true, diff --git a/app/routes/_layout+/assets.$assetId.overview.tsx b/app/routes/_layout+/assets.$assetId.overview.tsx index 266c55124..9e8e224f0 100644 --- a/app/routes/_layout+/assets.$assetId.overview.tsx +++ b/app/routes/_layout+/assets.$assetId.overview.tsx @@ -70,7 +70,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { }); try { - const { organizationId } = await requirePermission({ + const { organizationId, userOrganizations } = await requirePermission({ userId, request, entity: PermissionEntity.asset, @@ -82,6 +82,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { const asset = await getAsset({ id, organizationId, + userOrganizations, include: ASSET_OVERVIEW_FIELDS, }); diff --git a/app/routes/_layout+/assets.$assetId.overview.update-location.tsx b/app/routes/_layout+/assets.$assetId.overview.update-location.tsx index 4baea8b09..90f66d248 100644 --- a/app/routes/_layout+/assets.$assetId.overview.update-location.tsx +++ b/app/routes/_layout+/assets.$assetId.overview.update-location.tsx @@ -30,13 +30,13 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { }); try { - const { organizationId } = await requirePermission({ + const { organizationId, userOrganizations } = await requirePermission({ userId, request, entity: PermissionEntity.asset, action: PermissionAction.update, }); - const asset = await getAsset({ organizationId, id }); + const asset = await getAsset({ organizationId, id, userOrganizations }); const { locations } = await getAllEntriesForCreateAndEdit({ organizationId, diff --git a/app/routes/_layout+/assets.$assetId.tsx b/app/routes/_layout+/assets.$assetId.tsx index 9b251b351..6167c79f4 100644 --- a/app/routes/_layout+/assets.$assetId.tsx +++ b/app/routes/_layout+/assets.$assetId.tsx @@ -55,7 +55,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { }); try { - const { organizationId } = await requirePermission({ + const { organizationId, userOrganizations } = await requirePermission({ userId, request, entity: PermissionEntity.asset, @@ -65,6 +65,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { const asset = await getAsset({ id, organizationId, + userOrganizations, include: { custody: { include: { custodian: true } }, kit: true, diff --git a/app/routes/_layout+/assets.$assetId_.edit.tsx b/app/routes/_layout+/assets.$assetId_.edit.tsx index 273f7ae03..0c8f157ac 100644 --- a/app/routes/_layout+/assets.$assetId_.edit.tsx +++ b/app/routes/_layout+/assets.$assetId_.edit.tsx @@ -52,17 +52,19 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { }); try { - const { organizationId, currentOrganization } = await requirePermission({ - userId, - request, - entity: PermissionEntity.asset, - action: PermissionAction.update, - }); + const { organizationId, currentOrganization, userOrganizations } = + await requirePermission({ + userId, + request, + entity: PermissionEntity.asset, + action: PermissionAction.update, + }); const asset = await getAsset({ organizationId, id, include: { tags: true, customFields: true }, + userOrganizations, }); const { categories, totalCategories, tags, locations, totalLocations } = diff --git a/app/routes/api+/asset.scan.ts b/app/routes/api+/asset.scan.ts index dbbd7b868..19d8477fe 100644 --- a/app/routes/api+/asset.scan.ts +++ b/app/routes/api+/asset.scan.ts @@ -19,7 +19,7 @@ export async function action({ context, request }: ActionFunctionArgs) { const { userId } = authSession; try { - const { organizationId } = await requirePermission({ + const { organizationId, userOrganizations } = await requirePermission({ userId, request, entity: PermissionEntity.asset, @@ -42,6 +42,7 @@ export async function action({ context, request }: ActionFunctionArgs) { const asset = await getAsset({ id: assetId, organizationId, + userOrganizations, include: { qrCodes: true }, }); /** WE get the first qrCode as the app only supports 1 code per asset for now */ diff --git a/app/utils/roles.server.ts b/app/utils/roles.server.ts index c7a7d3e58..7aebed967 100644 --- a/app/utils/roles.server.ts +++ b/app/utils/roles.server.ts @@ -90,6 +90,7 @@ export async function requirePermission({ isSelfServiceOrBase: role === OrganizationRoles.SELF_SERVICE || role === OrganizationRoles.BASE, + userOrganizations, }; } From 2514f869204c4517ba2d05846fc62c3cea126bd6 Mon Sep 17 00:00:00 2001 From: Rohit Kumar Saini Date: Tue, 19 Nov 2024 20:13:21 +0100 Subject: [PATCH 02/27] feat(special-error): create UI for handling special error cases --- app/components/errors/index.tsx | 7 +++ .../errors/special-error-handler.tsx | 57 +++++++++++++++++++ app/components/errors/utils.ts | 39 +++++++++++++ app/modules/asset/service.server.ts | 2 +- 4 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 app/components/errors/special-error-handler.tsx create mode 100644 app/components/errors/utils.ts diff --git a/app/components/errors/index.tsx b/app/components/errors/index.tsx index 3e221ac9c..4fb7f58f7 100644 --- a/app/components/errors/index.tsx +++ b/app/components/errors/index.tsx @@ -2,6 +2,8 @@ import { useLocation, useRouteError } from "@remix-run/react"; import { isRouteError } from "~/utils/http"; import { Button } from "../shared/button"; +import { parseSpecialErrorData } from "./utils"; +import SpecialErrorHandler from "./special-error-handler"; export const ErrorContent = () => { const loc = useLocation(); @@ -18,6 +20,11 @@ export const ErrorContent = () => { traceId = response.data.error.traceId; } + const specialError = parseSpecialErrorData(response); + if (specialError.success) { + return ; + } + // Creating a string with
tags for line breaks const messageHtml = { __html: message.split("\n").join("
") }; diff --git a/app/components/errors/special-error-handler.tsx b/app/components/errors/special-error-handler.tsx new file mode 100644 index 000000000..20c8f3ebe --- /dev/null +++ b/app/components/errors/special-error-handler.tsx @@ -0,0 +1,57 @@ +import { tw } from "~/utils/tw"; +import { SpecialErrorAdditionalData } from "./utils"; +import { useMemo } from "react"; +import { ErrorIcon } from "."; +import { Button } from "../shared/button"; + +export type SpecialErrorHandlerProps = { + className?: string; + style?: React.CSSProperties; + additionalData: SpecialErrorAdditionalData; +}; + +export default function SpecialErrorHandler({ + className, + style, + additionalData, +}: SpecialErrorHandlerProps) { + const content = useMemo(() => { + switch (additionalData.type) { + case "asset-from-other-org": { + return ( +
+

Asset belongs to other workspace.

+

+ The asset you are trying to view belongs to a different workspace + you are part of. Would you like to switch to workspace{" "} + + "{additionalData.assetOrganization.organization.name}" + {" "} + to view the asset? +

+ + +
+ ); + } + + default: { + return null; + } + } + }, []); + + return ( +
+
+ + + + {content} +
+
+ ); +} diff --git a/app/components/errors/utils.ts b/app/components/errors/utils.ts new file mode 100644 index 000000000..e6bb418f8 --- /dev/null +++ b/app/components/errors/utils.ts @@ -0,0 +1,39 @@ +import { z } from "zod"; +import { isRouteError } from "~/utils/http"; + +export const specialErrorAdditionalData = z.discriminatedUnion("type", [ + z.object({ + type: z.literal("asset-from-other-org"), + assetOrganization: z.object({ + organization: z.object({ + id: z.string(), + name: z.string(), + }), + }), + }), +]); + +export type SpecialErrorAdditionalData = z.infer< + typeof specialErrorAdditionalData +>; + +export function parseSpecialErrorData(response: unknown): + | { success: false; additionalData: null } + | { + success: true; + additionalData: SpecialErrorAdditionalData; + } { + if (!isRouteError(response)) { + return { success: false, additionalData: null }; + } + + const parsedDataResponse = specialErrorAdditionalData.safeParse( + response.data.error.additionalData + ); + + if (!parsedDataResponse.success) { + return { success: false, additionalData: null }; + } + + return { success: true, additionalData: parsedDataResponse.data }; +} diff --git a/app/modules/asset/service.server.ts b/app/modules/asset/service.server.ts index 2f0ff97e7..fc8439913 100644 --- a/app/modules/asset/service.server.ts +++ b/app/modules/asset/service.server.ts @@ -134,7 +134,7 @@ export async function getAsset({ title: "Asset not found", message: "", additionalData: { - model: "asset", + type: "asset-from-other-org", assetOrganization: userOrganizations.find( (org) => org.organizationId === asset.organizationId ), From fb73871a815e5c531f0b16dd4b2e4accd3701da2 Mon Sep 17 00:00:00 2001 From: Rohit Kumar Saini Date: Wed, 20 Nov 2024 19:11:36 +0100 Subject: [PATCH 03/27] pr review changes --- .../errors/special-error-handler.tsx | 62 +++++++++++-------- app/components/errors/utils.ts | 17 +++-- app/modules/asset/service.server.ts | 17 ++--- .../api+/user.change-current-organization.ts | 5 +- 4 files changed, 56 insertions(+), 45 deletions(-) diff --git a/app/components/errors/special-error-handler.tsx b/app/components/errors/special-error-handler.tsx index 20c8f3ebe..1ebf3914c 100644 --- a/app/components/errors/special-error-handler.tsx +++ b/app/components/errors/special-error-handler.tsx @@ -1,8 +1,9 @@ import { tw } from "~/utils/tw"; import { SpecialErrorAdditionalData } from "./utils"; -import { useMemo } from "react"; import { ErrorIcon } from "."; import { Button } from "../shared/button"; +import { useFetcher } from "@remix-run/react"; +import { isFormProcessing } from "~/utils/form"; export type SpecialErrorHandlerProps = { className?: string; @@ -15,31 +16,8 @@ export default function SpecialErrorHandler({ style, additionalData, }: SpecialErrorHandlerProps) { - const content = useMemo(() => { - switch (additionalData.type) { - case "asset-from-other-org": { - return ( -
-

Asset belongs to other workspace.

-

- The asset you are trying to view belongs to a different workspace - you are part of. Would you like to switch to workspace{" "} - - "{additionalData.assetOrganization.organization.name}" - {" "} - to view the asset? -

- - -
- ); - } - - default: { - return null; - } - } - }, []); + const fetcher = useFetcher(); + const disabled = isFormProcessing(fetcher.state); return (
- {content} +
+

+ {additionalData.model} belongs + to other workspace. +

+

+ The {additionalData.model} you are trying to view belongs to a + different workspace you are part of. Would you like to switch to + workspace{" "} + + "{additionalData.organization.organization.name}" + {" "} + to view the {additionalData.model}? +

+ + + + + +
); diff --git a/app/components/errors/utils.ts b/app/components/errors/utils.ts index e6bb418f8..a8e804521 100644 --- a/app/components/errors/utils.ts +++ b/app/components/errors/utils.ts @@ -1,17 +1,16 @@ import { z } from "zod"; import { isRouteError } from "~/utils/http"; -export const specialErrorAdditionalData = z.discriminatedUnion("type", [ - z.object({ - type: z.literal("asset-from-other-org"), - assetOrganization: z.object({ - organization: z.object({ - id: z.string(), - name: z.string(), - }), +export const specialErrorAdditionalData = z.object({ + model: z.enum(["assets"]), + id: z.string(), + organization: z.object({ + organization: z.object({ + id: z.string(), + name: z.string(), }), }), -]); +}); export type SpecialErrorAdditionalData = z.infer< typeof specialErrorAdditionalData diff --git a/app/modules/asset/service.server.ts b/app/modules/asset/service.server.ts index fc8439913..2139004f7 100644 --- a/app/modules/asset/service.server.ts +++ b/app/modules/asset/service.server.ts @@ -106,11 +106,11 @@ export async function getAsset({ include, }: Pick & { organizationId: Asset["organizationId"]; - userOrganizations: Pick[]; + userOrganizations?: Pick[]; include?: T; }): Promise> { try { - const otherOrganizationIds = userOrganizations.map( + const otherOrganizationIds = userOrganizations?.map( (org) => org.organizationId ); @@ -118,7 +118,9 @@ export async function getAsset({ where: { OR: [ { id, organizationId }, - { id, organizationId: { in: otherOrganizationIds } }, + ...(userOrganizations?.length + ? [{ id, organizationId: { in: otherOrganizationIds } }] + : []), ], }, include: { ...include }, @@ -126,16 +128,17 @@ export async function getAsset({ /* User is accessing the asset in the wrong organization. In that case we need special 404 handling. */ if ( + userOrganizations?.length && asset.organizationId !== organizationId && - otherOrganizationIds.includes(asset.organizationId) + otherOrganizationIds?.includes(asset.organizationId) ) { throw new ShelfError({ cause: null, title: "Asset not found", message: "", additionalData: { - type: "asset-from-other-org", - assetOrganization: userOrganizations.find( + model: "assets", + organization: userOrganizations.find( (org) => org.organizationId === asset.organizationId ), }, @@ -153,7 +156,7 @@ export async function getAsset({ additionalData: { id, organizationId, - ...(cause instanceof ShelfError ? cause.additionalData : {}), + ...(isLikeShelfError(cause) ? cause.additionalData : {}), }, label, shouldBeCaptured: !isNotFoundError(cause), diff --git a/app/routes/api+/user.change-current-organization.ts b/app/routes/api+/user.change-current-organization.ts index b0a62c1e3..b7eca4bee 100644 --- a/app/routes/api+/user.change-current-organization.ts +++ b/app/routes/api+/user.change-current-organization.ts @@ -10,14 +10,15 @@ export async function action({ context, request }: ActionFunctionArgs) { const { userId } = authSession; try { - const { organizationId } = parseData( + const { organizationId, redirectTo } = parseData( await request.formData(), z.object({ organizationId: z.string(), + redirectTo: z.string().optional(), }) ); - return redirect("/", { + return redirect(redirectTo ?? "/", { headers: [ setCookie(await setSelectedOrganizationIdCookie(organizationId)), ], From 82f31474b9d904bfd5dd07a0007dc3858c47a920 Mon Sep 17 00:00:00 2001 From: Rohit Kumar Saini Date: Fri, 22 Nov 2024 14:25:22 +0100 Subject: [PATCH 04/27] feat(improve-404): fix pr review issues --- ...rror-handler.tsx => error-404-handler.tsx} | 17 ++++++------- app/components/errors/index.tsx | 10 ++++---- app/components/errors/utils.ts | 25 +++++++++---------- app/modules/asset/service.server.ts | 11 +++++++- .../_layout+/assets.$assetId.overview.tsx | 1 + app/routes/api+/asset.scan.ts | 3 +-- .../api+/user.change-current-organization.ts | 4 +-- 7 files changed, 38 insertions(+), 33 deletions(-) rename app/components/errors/{special-error-handler.tsx => error-404-handler.tsx} (79%) diff --git a/app/components/errors/special-error-handler.tsx b/app/components/errors/error-404-handler.tsx similarity index 79% rename from app/components/errors/special-error-handler.tsx rename to app/components/errors/error-404-handler.tsx index 1ebf3914c..b84fc7a76 100644 --- a/app/components/errors/special-error-handler.tsx +++ b/app/components/errors/error-404-handler.tsx @@ -1,21 +1,21 @@ import { tw } from "~/utils/tw"; -import { SpecialErrorAdditionalData } from "./utils"; +import { Error404AdditionalData } from "./utils"; import { ErrorIcon } from "."; import { Button } from "../shared/button"; import { useFetcher } from "@remix-run/react"; import { isFormProcessing } from "~/utils/form"; -export type SpecialErrorHandlerProps = { +export type Error404HandlerProps = { className?: string; style?: React.CSSProperties; - additionalData: SpecialErrorAdditionalData; + additionalData: Error404AdditionalData; }; -export default function SpecialErrorHandler({ +export default function Error404Handler({ className, style, additionalData, -}: SpecialErrorHandlerProps) { +}: Error404HandlerProps) { const fetcher = useFetcher(); const disabled = isFormProcessing(fetcher.state); @@ -25,13 +25,10 @@ export default function SpecialErrorHandler({ style={style} >
- - -

{additionalData.model} belongs - to other workspace. + to another workspace.

The {additionalData.model} you are trying to view belongs to a @@ -54,7 +51,7 @@ export default function SpecialErrorHandler({ diff --git a/app/components/errors/index.tsx b/app/components/errors/index.tsx index 4fb7f58f7..e45f57727 100644 --- a/app/components/errors/index.tsx +++ b/app/components/errors/index.tsx @@ -2,8 +2,8 @@ import { useLocation, useRouteError } from "@remix-run/react"; import { isRouteError } from "~/utils/http"; import { Button } from "../shared/button"; -import { parseSpecialErrorData } from "./utils"; -import SpecialErrorHandler from "./special-error-handler"; +import { parse404ErrorData } from "./utils"; +import Error404Handler from "./error-404-handler"; export const ErrorContent = () => { const loc = useLocation(); @@ -20,9 +20,9 @@ export const ErrorContent = () => { traceId = response.data.error.traceId; } - const specialError = parseSpecialErrorData(response); - if (specialError.success) { - return ; + const error404 = parse404ErrorData(response); + if (error404.isError404) { + return ; } // Creating a string with
tags for line breaks diff --git a/app/components/errors/utils.ts b/app/components/errors/utils.ts index a8e804521..48249fdea 100644 --- a/app/components/errors/utils.ts +++ b/app/components/errors/utils.ts @@ -1,9 +1,10 @@ import { z } from "zod"; import { isRouteError } from "~/utils/http"; -export const specialErrorAdditionalData = z.object({ - model: z.enum(["assets"]), +export const error404AdditionalData = z.object({ + model: z.enum(["asset"]), id: z.string(), + redirectTo: z.string().optional(), organization: z.object({ organization: z.object({ id: z.string(), @@ -12,27 +13,25 @@ export const specialErrorAdditionalData = z.object({ }), }); -export type SpecialErrorAdditionalData = z.infer< - typeof specialErrorAdditionalData ->; +export type Error404AdditionalData = z.infer; -export function parseSpecialErrorData(response: unknown): - | { success: false; additionalData: null } +export function parse404ErrorData(response: unknown): + | { isError404: false; additionalData: null } | { - success: true; - additionalData: SpecialErrorAdditionalData; + isError404: true; + additionalData: Error404AdditionalData; } { if (!isRouteError(response)) { - return { success: false, additionalData: null }; + return { isError404: false, additionalData: null }; } - const parsedDataResponse = specialErrorAdditionalData.safeParse( + const parsedDataResponse = error404AdditionalData.safeParse( response.data.error.additionalData ); if (!parsedDataResponse.success) { - return { success: false, additionalData: null }; + return { isError404: false, additionalData: null }; } - return { success: true, additionalData: parsedDataResponse.data }; + return { isError404: true, additionalData: parsedDataResponse.data }; } diff --git a/app/modules/asset/service.server.ts b/app/modules/asset/service.server.ts index 2139004f7..74bcb1734 100644 --- a/app/modules/asset/service.server.ts +++ b/app/modules/asset/service.server.ts @@ -103,10 +103,12 @@ export async function getAsset({ id, organizationId, userOrganizations, + request, include, }: Pick & { organizationId: Asset["organizationId"]; userOrganizations?: Pick[]; + request?: Request; include?: T; }): Promise> { try { @@ -132,17 +134,24 @@ export async function getAsset({ asset.organizationId !== organizationId && otherOrganizationIds?.includes(asset.organizationId) ) { + const redirectTo = + typeof request !== "undefined" + ? new URL(request.url).pathname + : undefined; + throw new ShelfError({ cause: null, title: "Asset not found", message: "", additionalData: { - model: "assets", + model: "asset", organization: userOrganizations.find( (org) => org.organizationId === asset.organizationId ), + redirectTo, }, label, + status: 404, }); } diff --git a/app/routes/_layout+/assets.$assetId.overview.tsx b/app/routes/_layout+/assets.$assetId.overview.tsx index 9e8e224f0..00bb54aa8 100644 --- a/app/routes/_layout+/assets.$assetId.overview.tsx +++ b/app/routes/_layout+/assets.$assetId.overview.tsx @@ -83,6 +83,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { id, organizationId, userOrganizations, + request, include: ASSET_OVERVIEW_FIELDS, }); diff --git a/app/routes/api+/asset.scan.ts b/app/routes/api+/asset.scan.ts index 19d8477fe..dbbd7b868 100644 --- a/app/routes/api+/asset.scan.ts +++ b/app/routes/api+/asset.scan.ts @@ -19,7 +19,7 @@ export async function action({ context, request }: ActionFunctionArgs) { const { userId } = authSession; try { - const { organizationId, userOrganizations } = await requirePermission({ + const { organizationId } = await requirePermission({ userId, request, entity: PermissionEntity.asset, @@ -42,7 +42,6 @@ export async function action({ context, request }: ActionFunctionArgs) { const asset = await getAsset({ id: assetId, organizationId, - userOrganizations, include: { qrCodes: true }, }); /** WE get the first qrCode as the app only supports 1 code per asset for now */ diff --git a/app/routes/api+/user.change-current-organization.ts b/app/routes/api+/user.change-current-organization.ts index b7eca4bee..26463381d 100644 --- a/app/routes/api+/user.change-current-organization.ts +++ b/app/routes/api+/user.change-current-organization.ts @@ -3,7 +3,7 @@ import { z } from "zod"; import { setSelectedOrganizationIdCookie } from "~/modules/organization/context.server"; import { setCookie } from "~/utils/cookies.server"; import { makeShelfError } from "~/utils/error"; -import { error, parseData } from "~/utils/http.server"; +import { error, parseData, safeRedirect } from "~/utils/http.server"; export async function action({ context, request }: ActionFunctionArgs) { const authSession = context.getSession(); @@ -18,7 +18,7 @@ export async function action({ context, request }: ActionFunctionArgs) { }) ); - return redirect(redirectTo ?? "/", { + return redirect(safeRedirect(redirectTo), { headers: [ setCookie(await setSelectedOrganizationIdCookie(organizationId)), ], From 532f6d8ccd6d698007e93a6cc0f20ebc1d9103a3 Mon Sep 17 00:00:00 2001 From: Rohit Kumar Saini Date: Sat, 23 Nov 2024 12:42:28 +0100 Subject: [PATCH 05/27] feat(improve-404): add request in getAsset for proper redirecting --- app/components/errors/error-404-handler.tsx | 1 - app/routes/_layout+/assets.$assetId.activity.tsx | 1 + app/routes/_layout+/assets.$assetId.overview.duplicate.tsx | 1 + .../_layout+/assets.$assetId.overview.update-location.tsx | 7 ++++++- app/routes/_layout+/assets.$assetId.tsx | 1 + app/routes/_layout+/assets.$assetId_.edit.tsx | 1 + 6 files changed, 10 insertions(+), 2 deletions(-) diff --git a/app/components/errors/error-404-handler.tsx b/app/components/errors/error-404-handler.tsx index b84fc7a76..4b91e4165 100644 --- a/app/components/errors/error-404-handler.tsx +++ b/app/components/errors/error-404-handler.tsx @@ -1,6 +1,5 @@ import { tw } from "~/utils/tw"; import { Error404AdditionalData } from "./utils"; -import { ErrorIcon } from "."; import { Button } from "../shared/button"; import { useFetcher } from "@remix-run/react"; import { isFormProcessing } from "~/utils/form"; diff --git a/app/routes/_layout+/assets.$assetId.activity.tsx b/app/routes/_layout+/assets.$assetId.activity.tsx index 0be205ff5..20b8246f9 100644 --- a/app/routes/_layout+/assets.$assetId.activity.tsx +++ b/app/routes/_layout+/assets.$assetId.activity.tsx @@ -39,6 +39,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { id, organizationId, userOrganizations, + request, include: { notes: { orderBy: { createdAt: "desc" }, diff --git a/app/routes/_layout+/assets.$assetId.overview.duplicate.tsx b/app/routes/_layout+/assets.$assetId.overview.duplicate.tsx index b6ff87f53..4e94fc0b1 100644 --- a/app/routes/_layout+/assets.$assetId.overview.duplicate.tsx +++ b/app/routes/_layout+/assets.$assetId.overview.duplicate.tsx @@ -44,6 +44,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { id: assetId, organizationId, userOrganizations, + request, }); return json( diff --git a/app/routes/_layout+/assets.$assetId.overview.update-location.tsx b/app/routes/_layout+/assets.$assetId.overview.update-location.tsx index 90f66d248..d87d75f9f 100644 --- a/app/routes/_layout+/assets.$assetId.overview.update-location.tsx +++ b/app/routes/_layout+/assets.$assetId.overview.update-location.tsx @@ -36,7 +36,12 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { entity: PermissionEntity.asset, action: PermissionAction.update, }); - const asset = await getAsset({ organizationId, id, userOrganizations }); + const asset = await getAsset({ + organizationId, + id, + userOrganizations, + request, + }); const { locations } = await getAllEntriesForCreateAndEdit({ organizationId, diff --git a/app/routes/_layout+/assets.$assetId.tsx b/app/routes/_layout+/assets.$assetId.tsx index 6167c79f4..20bfa1da8 100644 --- a/app/routes/_layout+/assets.$assetId.tsx +++ b/app/routes/_layout+/assets.$assetId.tsx @@ -66,6 +66,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { id, organizationId, userOrganizations, + request, include: { custody: { include: { custodian: true } }, kit: true, diff --git a/app/routes/_layout+/assets.$assetId_.edit.tsx b/app/routes/_layout+/assets.$assetId_.edit.tsx index 0c8f157ac..de6d7a1af 100644 --- a/app/routes/_layout+/assets.$assetId_.edit.tsx +++ b/app/routes/_layout+/assets.$assetId_.edit.tsx @@ -65,6 +65,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { id, include: { tags: true, customFields: true }, userOrganizations, + request, }); const { categories, totalCategories, tags, locations, totalLocations } = From ca5fe651c0b573c5d741240dc8630282f8454c4c Mon Sep 17 00:00:00 2001 From: Rohit Kumar Saini Date: Sat, 23 Nov 2024 19:47:12 +0100 Subject: [PATCH 06/27] feat(improve-404): update getKit function to make it more generic --- app/modules/kit/service.server.ts | 15 ++++++++++++--- app/modules/qr/service.server.ts | 1 + 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/app/modules/kit/service.server.ts b/app/modules/kit/service.server.ts index 7a52cd756..6b66a1716 100644 --- a/app/modules/kit/service.server.ts +++ b/app/modules/kit/service.server.ts @@ -341,7 +341,14 @@ export async function getPaginatedAndFilterableKits< } } -export async function getKit({ +type KitWithInclude = + T extends Prisma.KitInclude + ? Prisma.KitGetPayload<{ + include: MergeInclude; + }> + : Prisma.KitGetPayload<{ include: typeof GET_KIT_STATIC_INCLUDES }>; + +export async function getKit({ id, organizationId, extraInclude, @@ -353,10 +360,12 @@ export async function getKit({ ...extraInclude, } as MergeInclude; - return (await db.kit.findUniqueOrThrow({ + const kit = await db.kit.findUniqueOrThrow({ where: { id, organizationId }, include: includes, - })) as Prisma.KitGetPayload<{ include: typeof includes }>; + }); + + return kit as KitWithInclude; } catch (cause) { throw new ShelfError({ cause, diff --git a/app/modules/qr/service.server.ts b/app/modules/qr/service.server.ts index ca01b7bd8..074ca8512 100644 --- a/app/modules/qr/service.server.ts +++ b/app/modules/qr/service.server.ts @@ -37,6 +37,7 @@ export async function getQrByAssetId({ assetId }: Pick) { }); } } + export async function getQrByKitId({ kitId }: Pick) { try { return await db.qr.findFirst({ From 7583eefffb019ab478eb420de7f759d7b2916c3c Mon Sep 17 00:00:00 2001 From: Rohit Kumar Saini Date: Mon, 25 Nov 2024 13:53:18 +0100 Subject: [PATCH 07/27] feat(improve-404): improve 404 handling for kit --- app/components/errors/utils.ts | 2 +- app/modules/kit/service.server.ts | 66 +++++++++++++++++-- .../_layout+/kits.$kitId.assign-custody.tsx | 16 +++-- .../kits.$kitId.create-new-booking.tsx | 20 ++++-- .../_layout+/kits.$kitId.release-custody.tsx | 9 ++- app/routes/_layout+/kits.$kitId.tsx | 4 +- app/routes/_layout+/kits.$kitId_.edit.tsx | 9 ++- 7 files changed, 101 insertions(+), 25 deletions(-) diff --git a/app/components/errors/utils.ts b/app/components/errors/utils.ts index 48249fdea..9fbe8abff 100644 --- a/app/components/errors/utils.ts +++ b/app/components/errors/utils.ts @@ -2,7 +2,7 @@ import { z } from "zod"; import { isRouteError } from "~/utils/http"; export const error404AdditionalData = z.object({ - model: z.enum(["asset"]), + model: z.enum(["asset", "kit"]), id: z.string(), redirectTo: z.string().optional(), organization: z.object({ diff --git a/app/modules/kit/service.server.ts b/app/modules/kit/service.server.ts index 6b66a1716..571349aea 100644 --- a/app/modules/kit/service.server.ts +++ b/app/modules/kit/service.server.ts @@ -6,6 +6,7 @@ import type { Qr, TeamMember, User, + UserOrganization, } from "@prisma/client"; import { AssetStatus, @@ -21,7 +22,12 @@ import { getDateTimeFormat } from "~/utils/client-hints"; import { updateCookieWithPerPage } from "~/utils/cookies.server"; import { dateTimeInUnix } from "~/utils/date-time-in-unix"; import type { ErrorLabel } from "~/utils/error"; -import { maybeUniqueConstraintViolation, ShelfError } from "~/utils/error"; +import { + isLikeShelfError, + isNotFoundError, + maybeUniqueConstraintViolation, + ShelfError, +} from "~/utils/error"; import { extractImageNameFromSupabaseUrl } from "~/utils/extract-image-name-from-supabase-url"; import { getCurrentSearchParams } from "~/utils/http.server"; import { id } from "~/utils/id/id.server"; @@ -352,19 +358,63 @@ export async function getKit({ id, organizationId, extraInclude, -}: Pick & { extraInclude?: T }) { + userOrganizations, + request, +}: Pick & { + extraInclude?: T; + userOrganizations?: Pick[]; + request?: Request; +}) { try { + const otherOrganizationIds = userOrganizations?.map( + (org) => org.organizationId + ); + // Merge static includes with dynamic includes const includes = { ...GET_KIT_STATIC_INCLUDES, ...extraInclude, } as MergeInclude; - const kit = await db.kit.findUniqueOrThrow({ - where: { id, organizationId }, + const kit = await db.kit.findFirstOrThrow({ + where: { + OR: [ + { id, organizationId }, + ...(userOrganizations?.length + ? [{ id, organizationId: { in: otherOrganizationIds } }] + : []), + ], + }, include: includes, }); + /* User is accessing the asset in the wrong organizations. In that case we need special 404 handlng. */ + if ( + userOrganizations?.length && + kit.organizationId !== organizationId && + otherOrganizationIds?.includes(kit.organizationId) + ) { + const redirectTo = + typeof request !== "undefined" + ? new URL(request.url).pathname + : undefined; + + throw new ShelfError({ + cause: null, + title: "Kit not found", + message: "", + additionalData: { + model: "kit", + organization: userOrganizations.find( + (org) => org.organizationId === kit.organizationId + ), + redirectTo, + }, + label, + status: 404, + }); + } + return kit as KitWithInclude; } catch (cause) { throw new ShelfError({ @@ -372,8 +422,12 @@ export async function getKit({ title: "Kit not found", message: "The kit you are trying to access does not exist or you do not have permission to access it.", - additionalData: { id }, - label, // Adjust the label as needed + additionalData: { + id, + ...(isLikeShelfError(cause) ? cause.additionalData : {}), + }, + label, + shouldBeCaptured: !isNotFoundError(cause), }); } } diff --git a/app/routes/_layout+/kits.$kitId.assign-custody.tsx b/app/routes/_layout+/kits.$kitId.assign-custody.tsx index c96139e46..be5d8981a 100644 --- a/app/routes/_layout+/kits.$kitId.assign-custody.tsx +++ b/app/routes/_layout+/kits.$kitId.assign-custody.tsx @@ -55,12 +55,14 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { }); try { - const { organizationId, role } = await requirePermission({ - userId, - request, - entity: PermissionEntity.kit, - action: PermissionAction.custody, - }); + const { organizationId, role, userOrganizations } = await requirePermission( + { + userId, + request, + entity: PermissionEntity.kit, + action: PermissionAction.custody, + } + ); const kit = await getKit({ id: kitId, @@ -81,6 +83,8 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { }, }, }, + userOrganizations, + request, }); if (kit.custody) { diff --git a/app/routes/_layout+/kits.$kitId.create-new-booking.tsx b/app/routes/_layout+/kits.$kitId.create-new-booking.tsx index c47d1c1cc..33b4631c7 100644 --- a/app/routes/_layout+/kits.$kitId.create-new-booking.tsx +++ b/app/routes/_layout+/kits.$kitId.create-new-booking.tsx @@ -31,13 +31,17 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { }); try { - const { organizationId, currentOrganization, isSelfServiceOrBase } = - await requirePermission({ - userId, - request, - entity: PermissionEntity.booking, - action: PermissionAction.create, - }); + const { + organizationId, + currentOrganization, + isSelfServiceOrBase, + userOrganizations, + } = await requirePermission({ + userId, + request, + entity: PermissionEntity.booking, + action: PermissionAction.create, + }); if (isPersonalOrg(currentOrganization)) { throw new ShelfError({ @@ -54,6 +58,8 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { id: kitId, organizationId, extraInclude: { assets: true }, + userOrganizations, + request, }); /* We need to fetch the team members to be able to display them in the custodian dropdown. */ diff --git a/app/routes/_layout+/kits.$kitId.release-custody.tsx b/app/routes/_layout+/kits.$kitId.release-custody.tsx index b7190eee1..5d266d3e7 100644 --- a/app/routes/_layout+/kits.$kitId.release-custody.tsx +++ b/app/routes/_layout+/kits.$kitId.release-custody.tsx @@ -31,14 +31,19 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { }); try { - const { organizationId } = await requirePermission({ + const { organizationId, userOrganizations } = await requirePermission({ userId, request, entity: PermissionEntity.kit, action: PermissionAction.custody, }); - const kit = await getKit({ id: kitId, organizationId }); + const kit = await getKit({ + id: kitId, + organizationId, + userOrganizations, + request, + }); if (!kit.custody) { return redirect(`/kits/${kitId}`); } diff --git a/app/routes/_layout+/kits.$kitId.tsx b/app/routes/_layout+/kits.$kitId.tsx index e2a855199..353d45dc5 100644 --- a/app/routes/_layout+/kits.$kitId.tsx +++ b/app/routes/_layout+/kits.$kitId.tsx @@ -75,7 +75,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { ); try { - const { organizationId } = await requirePermission({ + const { organizationId, userOrganizations } = await requirePermission({ userId, request, entity: PermissionEntity.kit, @@ -131,6 +131,8 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { }, qrCodes: true, }, + userOrganizations, + request, }), getAssetsForKits({ request, diff --git a/app/routes/_layout+/kits.$kitId_.edit.tsx b/app/routes/_layout+/kits.$kitId_.edit.tsx index fec0c6ff2..fb983c209 100644 --- a/app/routes/_layout+/kits.$kitId_.edit.tsx +++ b/app/routes/_layout+/kits.$kitId_.edit.tsx @@ -43,14 +43,19 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { }); try { - const { organizationId } = await requirePermission({ + const { organizationId, userOrganizations } = await requirePermission({ userId, request, entity: PermissionEntity.kit, action: PermissionAction.update, }); - const kit = await getKit({ id: kitId, organizationId }); + const kit = await getKit({ + id: kitId, + organizationId, + userOrganizations, + request, + }); const header: HeaderData = { title: `Edit | ${kit.name}`, From a1e3c3601666289ac141b0e4c0a5be8843512343 Mon Sep 17 00:00:00 2001 From: Rohit Kumar Saini Date: Tue, 26 Nov 2024 15:07:00 +0100 Subject: [PATCH 08/27] feat(improve-404): improve 404 handling for location --- app/components/errors/utils.ts | 2 +- app/modules/location/service.server.ts | 73 +++++++++++++++++-- app/routes/_layout+/locations.$locationId.tsx | 6 +- .../_layout+/locations.$locationId_.edit.tsx | 9 ++- 4 files changed, 79 insertions(+), 11 deletions(-) diff --git a/app/components/errors/utils.ts b/app/components/errors/utils.ts index 9fbe8abff..4b45330ca 100644 --- a/app/components/errors/utils.ts +++ b/app/components/errors/utils.ts @@ -2,7 +2,7 @@ import { z } from "zod"; import { isRouteError } from "~/utils/http"; export const error404AdditionalData = z.object({ - model: z.enum(["asset", "kit"]), + model: z.enum(["asset", "kit", "location"]), id: z.string(), redirectTo: z.string().optional(), organization: z.object({ diff --git a/app/modules/location/service.server.ts b/app/modules/location/service.server.ts index 70f43678b..bcb1eb755 100644 --- a/app/modules/location/service.server.ts +++ b/app/modules/location/service.server.ts @@ -1,8 +1,19 @@ -import type { Prisma, User, Location, Organization } from "@prisma/client"; +import type { + Prisma, + User, + Location, + Organization, + UserOrganization, +} from "@prisma/client"; import invariant from "tiny-invariant"; import { db } from "~/database/db.server"; import type { ErrorLabel } from "~/utils/error"; -import { ShelfError, maybeUniqueConstraintViolation } from "~/utils/error"; +import { + ShelfError, + isLikeShelfError, + isNotFoundError, + maybeUniqueConstraintViolation, +} from "~/utils/error"; import { ALL_SELECTED_KEY } from "~/utils/list"; import type { CreateAssetFromContentImportPayload } from "../asset/types"; @@ -16,11 +27,25 @@ export async function getLocation( /** Assets to be loaded per page with the location */ perPage?: number; search?: string | null; + userOrganizations?: Pick[]; + request?: Request; } ) { - const { organizationId, id, page = 1, perPage = 8, search } = params; + const { + organizationId, + id, + page = 1, + perPage = 8, + search, + userOrganizations, + request, + } = params; try { + const otherOrganizationIds = userOrganizations?.map( + (org) => org.organizationId + ); + const skip = page > 1 ? (page - 1) * perPage : 0; const take = perPage >= 1 ? perPage : 8; // min 1 and max 25 per page @@ -37,7 +62,14 @@ export async function getLocation( const [location, totalAssetsWithinLocation] = await Promise.all([ /** Get the items */ db.location.findFirstOrThrow({ - where: { id, organizationId }, + where: { + OR: [ + { id, organizationId }, + ...(userOrganizations?.length + ? [{ id, organizationId: { in: otherOrganizationIds } }] + : []), + ], + }, include: { image: { select: { @@ -64,13 +96,44 @@ export async function getLocation( }), ]); + /* User is accessing the asset in the wrong organization. In that case we need special 404 handling. */ + if ( + userOrganizations?.length && + location.organizationId !== organizationId && + otherOrganizationIds?.includes(location.organizationId) + ) { + const redirectTo = + typeof request !== "undefined" + ? new URL(request.url).pathname + : undefined; + + throw new ShelfError({ + cause: null, + title: "Location not found.", + message: "", + additionalData: { + model: "location", + organization: userOrganizations.find( + (org) => org.organizationId === location.organizationId + ), + redirectTo, + }, + label, + status: 404, + }); + } + return { location, totalAssetsWithinLocation }; } catch (cause) { throw new ShelfError({ cause, message: "Something went wrong while fetching location", - additionalData: { ...params }, + additionalData: { + ...params, + ...(isLikeShelfError(cause) ? cause.additionalData : {}), + }, label, + shouldBeCaptured: !isNotFoundError(cause), }); } } diff --git a/app/routes/_layout+/locations.$locationId.tsx b/app/routes/_layout+/locations.$locationId.tsx index 2365bade6..54776c626 100644 --- a/app/routes/_layout+/locations.$locationId.tsx +++ b/app/routes/_layout+/locations.$locationId.tsx @@ -62,7 +62,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { ); try { - const { organizationId } = await requirePermission({ + const { organizationId, userOrganizations } = await requirePermission({ userId: authSession.userId, request, entity: PermissionEntity.location, @@ -80,6 +80,8 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { page, perPage, search, + userOrganizations, + request, }); const totalItems = totalAssetsWithinLocation; @@ -358,5 +360,3 @@ const ListItemTagsColumn = ({ tags }: { tags: Tag[] | undefined }) => {

) : null; }; - -// export const ErrorBoundary = () => ; diff --git a/app/routes/_layout+/locations.$locationId_.edit.tsx b/app/routes/_layout+/locations.$locationId_.edit.tsx index c40e12387..6ab489b9d 100644 --- a/app/routes/_layout+/locations.$locationId_.edit.tsx +++ b/app/routes/_layout+/locations.$locationId_.edit.tsx @@ -43,14 +43,19 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { ); try { - const { organizationId } = await requirePermission({ + const { organizationId, userOrganizations } = await requirePermission({ userId: authSession.userId, request, entity: PermissionEntity.location, action: PermissionAction.update, }); - const { location } = await getLocation({ organizationId, id }); + const { location } = await getLocation({ + organizationId, + id, + userOrganizations, + request, + }); const header: HeaderData = { title: `Edit | ${location.name}`, From ff01befe77d081b3d38fd0b454556adfba0d7f83 Mon Sep 17 00:00:00 2001 From: Rohit Kumar Saini Date: Tue, 26 Nov 2024 20:08:41 +0100 Subject: [PATCH 09/27] feature(workflow): create separate function for team with 404 handling --- app/components/errors/utils.ts | 2 +- app/modules/user/service.server.ts | 105 +++++++++++++++++- app/modules/user/types.ts | 6 +- .../_layout+/settings.team.users.$userId.tsx | 44 ++++---- 4 files changed, 128 insertions(+), 29 deletions(-) diff --git a/app/components/errors/utils.ts b/app/components/errors/utils.ts index 4b45330ca..8d3772d14 100644 --- a/app/components/errors/utils.ts +++ b/app/components/errors/utils.ts @@ -2,7 +2,7 @@ import { z } from "zod"; import { isRouteError } from "~/utils/http"; export const error404AdditionalData = z.object({ - model: z.enum(["asset", "kit", "location"]), + model: z.enum(["asset", "kit", "location", "user"]), id: z.string(), redirectTo: z.string().optional(), organization: z.object({ diff --git a/app/modules/user/service.server.ts b/app/modules/user/service.server.ts index 50cb91a1c..85f6fb213 100644 --- a/app/modules/user/service.server.ts +++ b/app/modules/user/service.server.ts @@ -1,4 +1,4 @@ -import type { Organization, User } from "@prisma/client"; +import type { Organization, User, UserOrganization } from "@prisma/client"; import { Prisma, Roles, OrganizationRoles } from "@prisma/client"; import type { ITXClientDenyList } from "@prisma/client/runtime/library"; import { PrismaClientKnownRequestError } from "@prisma/client/runtime/library"; @@ -20,7 +20,7 @@ import { import { dateTimeInUnix } from "~/utils/date-time-in-unix"; import type { ErrorLabel } from "~/utils/error"; -import { ShelfError, isLikeShelfError } from "~/utils/error"; +import { ShelfError, isLikeShelfError, isNotFoundError } from "~/utils/error"; import type { ValidationError } from "~/utils/http"; import { getCurrentSearchParams } from "~/utils/http.server"; import { id as generateId } from "~/utils/id/id.server"; @@ -34,11 +34,12 @@ import { } from "~/utils/storage.server"; import { randomUsernameFromEmail } from "~/utils/user"; import { INCLUDE_SSO_DETAILS_VIA_USER_ORGANIZATION } from "./fields"; -import type { UpdateUserPayload } from "./types"; +import { type UpdateUserPayload, USER_STATIC_INCLUDE } from "./types"; import { defaultFields } from "../asset-index-settings/helpers"; import { defaultUserCategories } from "../category/default-categories"; import { getOrganizationsBySsoDomain } from "../organization/service.server"; import { createTeamMember } from "../team-member/service.server"; +import { MergeInclude } from "~/utils/utils"; const label: ErrorLabel = "User"; @@ -1238,3 +1239,101 @@ export async function transferEntitiesToNewOwner({ }, }); } + +type UserWithExtraInclude = + T extends Prisma.UserInclude + ? Prisma.UserGetPayload<{ + include: MergeInclude; + }> + : Prisma.UserGetPayload<{ include: typeof USER_STATIC_INCLUDE }>; + +export async function getUserFromOrg({ + id, + organizationId, + userOrganizations, + request, + extraInclude, +}: Pick & { + organizationId: Organization["id"]; + userOrganizations?: Pick[]; + request?: Request; + extraInclude?: T; +}) { + try { + const otherOrganizationIds = userOrganizations?.map( + (org) => org.organizationId + ); + + const mergedInclude = { + ...USER_STATIC_INCLUDE, + ...extraInclude, + } as MergeInclude; + + const user = (await db.user.findFirstOrThrow({ + where: { + OR: [ + { id, userOrganizations: { some: { organizationId } } }, + ...(userOrganizations?.length + ? [ + { + id, + userOrganizations: { + some: { organizationId: { in: otherOrganizationIds } }, + }, + }, + ] + : []), + ], + }, + include: mergedInclude, + })) as UserWithExtraInclude; + + /* User is accessing the User in the wrong organization */ + const isUserInCurrentOrg = !!user.userOrganizations.find( + (userOrg) => userOrg.organizationId === organizationId + ); + + const otherOrgOfUser = userOrganizations?.find( + (org) => + !!user.userOrganizations.find( + (userOrg) => userOrg.organizationId === org.organizationId + ) + ); + + if (userOrganizations?.length && !isUserInCurrentOrg && !!otherOrgOfUser) { + const redirectTo = + typeof request !== "undefined" + ? new URL(request.url).pathname + : undefined; + + throw new ShelfError({ + cause: null, + title: "User not found", + message: "", + additionalData: { + model: "user", + organization: otherOrgOfUser, + redirectTo, + }, + label, + status: 404, + }); + } + + return user; + } catch (cause) { + throw new ShelfError({ + cause, + title: "User not found.", + message: + "The user you are trying to access does not exists or you do not have permission to access it.", + additionalData: { + id, + organizationId, + ...(isLikeShelfError(cause) ? cause.additionalData : {}), + }, + label, + shouldBeCaptured: !isNotFoundError(cause), + }); + } +} diff --git a/app/modules/user/types.ts b/app/modules/user/types.ts index e15dbfcdd..ad75f9488 100644 --- a/app/modules/user/types.ts +++ b/app/modules/user/types.ts @@ -1,4 +1,4 @@ -import type { User } from "@prisma/client"; +import type { Prisma, User } from "@prisma/client"; export interface UpdateUserPayload { id: User["id"]; @@ -24,3 +24,7 @@ export interface UpdateUserResponse { /** Used when sending a pwd reset link for the user */ passwordReset?: boolean; } + +export const USER_STATIC_INCLUDE = { + userOrganizations: true, +} satisfies Prisma.UserInclude; diff --git a/app/routes/_layout+/settings.team.users.$userId.tsx b/app/routes/_layout+/settings.team.users.$userId.tsx index 833b08cde..394a52ee7 100644 --- a/app/routes/_layout+/settings.team.users.$userId.tsx +++ b/app/routes/_layout+/settings.team.users.$userId.tsx @@ -13,7 +13,7 @@ import { Badge } from "~/components/shared/badge"; import { Button } from "~/components/shared/button"; import When from "~/components/when/when"; import { TeamUsersActionsDropdown } from "~/components/workspace/users-actions-dropdown"; -import { getUserByID } from "~/modules/user/service.server"; +import { getUserFromOrg } from "~/modules/user/service.server"; import { resolveUserAction } from "~/modules/user/utils.server"; import { appendToMetaTitle } from "~/utils/append-to-meta-title"; import { makeShelfError } from "~/utils/error"; @@ -33,12 +33,13 @@ export const loader = async ({ const authSession = context.getSession(); const { userId } = authSession; try { - const { currentOrganization, organizationId } = await requirePermission({ - userId, - request, - entity: PermissionEntity.teamMemberProfile, - action: PermissionAction.read, - }); + const { currentOrganization, organizationId, userOrganizations } = + await requirePermission({ + userId, + request, + entity: PermissionEntity.teamMemberProfile, + action: PermissionAction.read, + }); const { userId: selectedUserId } = getParams( params, @@ -48,28 +49,23 @@ export const loader = async ({ } ); - const user = await getUserByID(selectedUserId, { - userOrganizations: { - where: { - organizationId, - }, - select: { - roles: true, - }, - }, - teamMembers: { - where: { - organizationId, - }, - include: { - receivedInvites: { - where: { - organizationId, + const user = await getUserFromOrg({ + id: selectedUserId, + organizationId, + userOrganizations, + request, + extraInclude: { + teamMembers: { + where: { organizationId }, + include: { + receivedInvites: { + where: { organizationId }, }, }, }, }, }); + const userName = (user.firstName ? user.firstName.trim() : "") + " " + From cf330c24f8bba5fa0042c6419b39249967c8cd21 Mon Sep 17 00:00:00 2001 From: Rohit Kumar Saini Date: Tue, 26 Nov 2024 20:42:22 +0100 Subject: [PATCH 10/27] feat(improve-404): handling special case of multiple organization for team member --- app/components/errors/error-404-handler.tsx | 140 +++++++++++++++----- app/components/errors/utils.ts | 31 +++-- app/modules/user/service.server.ts | 23 ++-- 3 files changed, 144 insertions(+), 50 deletions(-) diff --git a/app/components/errors/error-404-handler.tsx b/app/components/errors/error-404-handler.tsx index 4b91e4165..17ca763dc 100644 --- a/app/components/errors/error-404-handler.tsx +++ b/app/components/errors/error-404-handler.tsx @@ -1,8 +1,16 @@ +import { useMemo } from "react"; import { tw } from "~/utils/tw"; import { Error404AdditionalData } from "./utils"; import { Button } from "../shared/button"; import { useFetcher } from "@remix-run/react"; import { isFormProcessing } from "~/utils/form"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "../forms/select"; export type Error404HandlerProps = { className?: string; @@ -18,44 +26,110 @@ export default function Error404Handler({ const fetcher = useFetcher(); const disabled = isFormProcessing(fetcher.state); + const content = useMemo(() => { + switch (additionalData.model) { + case "asset": + case "kit": + case "location": { + return ( +
+
+

+ {additionalData.model}{" "} + belongs to another workspace. +

+

+ The {additionalData.model} you are trying to view belongs to a + different workspace you are part of. Would you like to switch to + workspace{" "} + + "{additionalData.organization.organization.name}" + {" "} + to view the {additionalData.model}? +

+ + + + + +
+
+ ); + } + + case "teamMember": { + return ( +
+
+

+ Team Member belongs to + another workspace(s). +

+

+ The team member you are trying to view belongs to one/some of + your different workspace you are part of. Would you like to + switch to workspace to view the team member? +

+ + + + + +
+
+ ); + } + + default: { + return null; + } + } + }, [additionalData]); + return (
-
-
-

- {additionalData.model} belongs - to another workspace. -

-

- The {additionalData.model} you are trying to view belongs to a - different workspace you are part of. Would you like to switch to - workspace{" "} - - "{additionalData.organization.organization.name}" - {" "} - to view the {additionalData.model}? -

- - - - - -
-
+ {content}
); } diff --git a/app/components/errors/utils.ts b/app/components/errors/utils.ts index 8d3772d14..c075c9232 100644 --- a/app/components/errors/utils.ts +++ b/app/components/errors/utils.ts @@ -1,19 +1,34 @@ import { z } from "zod"; import { isRouteError } from "~/utils/http"; -export const error404AdditionalData = z.object({ - model: z.enum(["asset", "kit", "location", "user"]), +const baseAdditionalDataSchema = z.object({ id: z.string(), redirectTo: z.string().optional(), +}); + +const organizationSchema = z.object({ organization: z.object({ - organization: z.object({ - id: z.string(), - name: z.string(), - }), + id: z.string(), + name: z.string(), }), }); -export type Error404AdditionalData = z.infer; +export const error404AdditionalDataSchema = z.discriminatedUnion("model", [ + /* For common and general use case */ + baseAdditionalDataSchema.extend({ + model: z.enum(["asset", "kit", "location"]), + organization: organizationSchema, + }), + /* A team member (user) can be in multiple organization's of user so we do this. */ + baseAdditionalDataSchema.extend({ + model: z.literal("teamMember"), + organizations: organizationSchema.array(), + }), +]); + +export type Error404AdditionalData = z.infer< + typeof error404AdditionalDataSchema +>; export function parse404ErrorData(response: unknown): | { isError404: false; additionalData: null } @@ -25,7 +40,7 @@ export function parse404ErrorData(response: unknown): return { isError404: false, additionalData: null }; } - const parsedDataResponse = error404AdditionalData.safeParse( + const parsedDataResponse = error404AdditionalDataSchema.safeParse( response.data.error.additionalData ); diff --git a/app/modules/user/service.server.ts b/app/modules/user/service.server.ts index 85f6fb213..5846962f7 100644 --- a/app/modules/user/service.server.ts +++ b/app/modules/user/service.server.ts @@ -1293,14 +1293,19 @@ export async function getUserFromOrg({ (userOrg) => userOrg.organizationId === organizationId ); - const otherOrgOfUser = userOrganizations?.find( - (org) => - !!user.userOrganizations.find( - (userOrg) => userOrg.organizationId === org.organizationId - ) - ); + const otherOrgsForUser = + userOrganizations?.filter( + (org) => + !!user.userOrganizations.find( + (userOrg) => userOrg.organizationId === org.organizationId + ) + ) ?? []; - if (userOrganizations?.length && !isUserInCurrentOrg && !!otherOrgOfUser) { + if ( + userOrganizations?.length && + !isUserInCurrentOrg && + otherOrgsForUser?.length + ) { const redirectTo = typeof request !== "undefined" ? new URL(request.url).pathname @@ -1311,8 +1316,8 @@ export async function getUserFromOrg({ title: "User not found", message: "", additionalData: { - model: "user", - organization: otherOrgOfUser, + model: "teamMember", + organizations: otherOrgsForUser, redirectTo, }, label, From e460b90beb7bf2f728005737a49f7b21d444547b Mon Sep 17 00:00:00 2001 From: Rohit Kumar Saini Date: Wed, 27 Nov 2024 11:45:36 +0100 Subject: [PATCH 11/27] feat(improve-404): fix style issue on ErrorBoundary for team settings --- app/components/errors/error-404-handler.tsx | 2 +- app/components/errors/index.tsx | 19 ++++++++++++++++--- .../_layout+/settings.team.users.$userId.tsx | 17 ++++++++++++----- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/app/components/errors/error-404-handler.tsx b/app/components/errors/error-404-handler.tsx index 17ca763dc..155331a99 100644 --- a/app/components/errors/error-404-handler.tsx +++ b/app/components/errors/error-404-handler.tsx @@ -88,7 +88,7 @@ export default function Error404Handler({ > + + + +
); diff --git a/app/components/errors/utils.ts b/app/components/errors/utils.ts index e6bb418f8..a8e804521 100644 --- a/app/components/errors/utils.ts +++ b/app/components/errors/utils.ts @@ -1,17 +1,16 @@ import { z } from "zod"; import { isRouteError } from "~/utils/http"; -export const specialErrorAdditionalData = z.discriminatedUnion("type", [ - z.object({ - type: z.literal("asset-from-other-org"), - assetOrganization: z.object({ - organization: z.object({ - id: z.string(), - name: z.string(), - }), +export const specialErrorAdditionalData = z.object({ + model: z.enum(["assets"]), + id: z.string(), + organization: z.object({ + organization: z.object({ + id: z.string(), + name: z.string(), }), }), -]); +}); export type SpecialErrorAdditionalData = z.infer< typeof specialErrorAdditionalData diff --git a/app/modules/asset/service.server.ts b/app/modules/asset/service.server.ts index fc8439913..2139004f7 100644 --- a/app/modules/asset/service.server.ts +++ b/app/modules/asset/service.server.ts @@ -106,11 +106,11 @@ export async function getAsset({ include, }: Pick & { organizationId: Asset["organizationId"]; - userOrganizations: Pick[]; + userOrganizations?: Pick[]; include?: T; }): Promise> { try { - const otherOrganizationIds = userOrganizations.map( + const otherOrganizationIds = userOrganizations?.map( (org) => org.organizationId ); @@ -118,7 +118,9 @@ export async function getAsset({ where: { OR: [ { id, organizationId }, - { id, organizationId: { in: otherOrganizationIds } }, + ...(userOrganizations?.length + ? [{ id, organizationId: { in: otherOrganizationIds } }] + : []), ], }, include: { ...include }, @@ -126,16 +128,17 @@ export async function getAsset({ /* User is accessing the asset in the wrong organization. In that case we need special 404 handling. */ if ( + userOrganizations?.length && asset.organizationId !== organizationId && - otherOrganizationIds.includes(asset.organizationId) + otherOrganizationIds?.includes(asset.organizationId) ) { throw new ShelfError({ cause: null, title: "Asset not found", message: "", additionalData: { - type: "asset-from-other-org", - assetOrganization: userOrganizations.find( + model: "assets", + organization: userOrganizations.find( (org) => org.organizationId === asset.organizationId ), }, @@ -153,7 +156,7 @@ export async function getAsset({ additionalData: { id, organizationId, - ...(cause instanceof ShelfError ? cause.additionalData : {}), + ...(isLikeShelfError(cause) ? cause.additionalData : {}), }, label, shouldBeCaptured: !isNotFoundError(cause), diff --git a/app/routes/api+/user.change-current-organization.ts b/app/routes/api+/user.change-current-organization.ts index b0a62c1e3..b7eca4bee 100644 --- a/app/routes/api+/user.change-current-organization.ts +++ b/app/routes/api+/user.change-current-organization.ts @@ -10,14 +10,15 @@ export async function action({ context, request }: ActionFunctionArgs) { const { userId } = authSession; try { - const { organizationId } = parseData( + const { organizationId, redirectTo } = parseData( await request.formData(), z.object({ organizationId: z.string(), + redirectTo: z.string().optional(), }) ); - return redirect("/", { + return redirect(redirectTo ?? "/", { headers: [ setCookie(await setSelectedOrganizationIdCookie(organizationId)), ], From 473c9dc3c8d73d0023c5ebae0085c3e2b4d2b78d Mon Sep 17 00:00:00 2001 From: Rohit Kumar Saini Date: Fri, 22 Nov 2024 14:25:22 +0100 Subject: [PATCH 16/27] feat(improve-404): fix pr review issues --- ...rror-handler.tsx => error-404-handler.tsx} | 17 ++++++------- app/components/errors/index.tsx | 10 ++++---- app/components/errors/utils.ts | 25 +++++++++---------- app/modules/asset/service.server.ts | 11 +++++++- .../_layout+/assets.$assetId.overview.tsx | 1 + app/routes/api+/asset.scan.ts | 3 +-- .../api+/user.change-current-organization.ts | 4 +-- 7 files changed, 38 insertions(+), 33 deletions(-) rename app/components/errors/{special-error-handler.tsx => error-404-handler.tsx} (79%) diff --git a/app/components/errors/special-error-handler.tsx b/app/components/errors/error-404-handler.tsx similarity index 79% rename from app/components/errors/special-error-handler.tsx rename to app/components/errors/error-404-handler.tsx index 1ebf3914c..b84fc7a76 100644 --- a/app/components/errors/special-error-handler.tsx +++ b/app/components/errors/error-404-handler.tsx @@ -1,21 +1,21 @@ import { tw } from "~/utils/tw"; -import { SpecialErrorAdditionalData } from "./utils"; +import { Error404AdditionalData } from "./utils"; import { ErrorIcon } from "."; import { Button } from "../shared/button"; import { useFetcher } from "@remix-run/react"; import { isFormProcessing } from "~/utils/form"; -export type SpecialErrorHandlerProps = { +export type Error404HandlerProps = { className?: string; style?: React.CSSProperties; - additionalData: SpecialErrorAdditionalData; + additionalData: Error404AdditionalData; }; -export default function SpecialErrorHandler({ +export default function Error404Handler({ className, style, additionalData, -}: SpecialErrorHandlerProps) { +}: Error404HandlerProps) { const fetcher = useFetcher(); const disabled = isFormProcessing(fetcher.state); @@ -25,13 +25,10 @@ export default function SpecialErrorHandler({ style={style} >
- - -

{additionalData.model} belongs - to other workspace. + to another workspace.

The {additionalData.model} you are trying to view belongs to a @@ -54,7 +51,7 @@ export default function SpecialErrorHandler({ diff --git a/app/components/errors/index.tsx b/app/components/errors/index.tsx index 4fb7f58f7..e45f57727 100644 --- a/app/components/errors/index.tsx +++ b/app/components/errors/index.tsx @@ -2,8 +2,8 @@ import { useLocation, useRouteError } from "@remix-run/react"; import { isRouteError } from "~/utils/http"; import { Button } from "../shared/button"; -import { parseSpecialErrorData } from "./utils"; -import SpecialErrorHandler from "./special-error-handler"; +import { parse404ErrorData } from "./utils"; +import Error404Handler from "./error-404-handler"; export const ErrorContent = () => { const loc = useLocation(); @@ -20,9 +20,9 @@ export const ErrorContent = () => { traceId = response.data.error.traceId; } - const specialError = parseSpecialErrorData(response); - if (specialError.success) { - return ; + const error404 = parse404ErrorData(response); + if (error404.isError404) { + return ; } // Creating a string with
tags for line breaks diff --git a/app/components/errors/utils.ts b/app/components/errors/utils.ts index a8e804521..48249fdea 100644 --- a/app/components/errors/utils.ts +++ b/app/components/errors/utils.ts @@ -1,9 +1,10 @@ import { z } from "zod"; import { isRouteError } from "~/utils/http"; -export const specialErrorAdditionalData = z.object({ - model: z.enum(["assets"]), +export const error404AdditionalData = z.object({ + model: z.enum(["asset"]), id: z.string(), + redirectTo: z.string().optional(), organization: z.object({ organization: z.object({ id: z.string(), @@ -12,27 +13,25 @@ export const specialErrorAdditionalData = z.object({ }), }); -export type SpecialErrorAdditionalData = z.infer< - typeof specialErrorAdditionalData ->; +export type Error404AdditionalData = z.infer; -export function parseSpecialErrorData(response: unknown): - | { success: false; additionalData: null } +export function parse404ErrorData(response: unknown): + | { isError404: false; additionalData: null } | { - success: true; - additionalData: SpecialErrorAdditionalData; + isError404: true; + additionalData: Error404AdditionalData; } { if (!isRouteError(response)) { - return { success: false, additionalData: null }; + return { isError404: false, additionalData: null }; } - const parsedDataResponse = specialErrorAdditionalData.safeParse( + const parsedDataResponse = error404AdditionalData.safeParse( response.data.error.additionalData ); if (!parsedDataResponse.success) { - return { success: false, additionalData: null }; + return { isError404: false, additionalData: null }; } - return { success: true, additionalData: parsedDataResponse.data }; + return { isError404: true, additionalData: parsedDataResponse.data }; } diff --git a/app/modules/asset/service.server.ts b/app/modules/asset/service.server.ts index 2139004f7..74bcb1734 100644 --- a/app/modules/asset/service.server.ts +++ b/app/modules/asset/service.server.ts @@ -103,10 +103,12 @@ export async function getAsset({ id, organizationId, userOrganizations, + request, include, }: Pick & { organizationId: Asset["organizationId"]; userOrganizations?: Pick[]; + request?: Request; include?: T; }): Promise> { try { @@ -132,17 +134,24 @@ export async function getAsset({ asset.organizationId !== organizationId && otherOrganizationIds?.includes(asset.organizationId) ) { + const redirectTo = + typeof request !== "undefined" + ? new URL(request.url).pathname + : undefined; + throw new ShelfError({ cause: null, title: "Asset not found", message: "", additionalData: { - model: "assets", + model: "asset", organization: userOrganizations.find( (org) => org.organizationId === asset.organizationId ), + redirectTo, }, label, + status: 404, }); } diff --git a/app/routes/_layout+/assets.$assetId.overview.tsx b/app/routes/_layout+/assets.$assetId.overview.tsx index 9e8e224f0..00bb54aa8 100644 --- a/app/routes/_layout+/assets.$assetId.overview.tsx +++ b/app/routes/_layout+/assets.$assetId.overview.tsx @@ -83,6 +83,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { id, organizationId, userOrganizations, + request, include: ASSET_OVERVIEW_FIELDS, }); diff --git a/app/routes/api+/asset.scan.ts b/app/routes/api+/asset.scan.ts index 19d8477fe..dbbd7b868 100644 --- a/app/routes/api+/asset.scan.ts +++ b/app/routes/api+/asset.scan.ts @@ -19,7 +19,7 @@ export async function action({ context, request }: ActionFunctionArgs) { const { userId } = authSession; try { - const { organizationId, userOrganizations } = await requirePermission({ + const { organizationId } = await requirePermission({ userId, request, entity: PermissionEntity.asset, @@ -42,7 +42,6 @@ export async function action({ context, request }: ActionFunctionArgs) { const asset = await getAsset({ id: assetId, organizationId, - userOrganizations, include: { qrCodes: true }, }); /** WE get the first qrCode as the app only supports 1 code per asset for now */ diff --git a/app/routes/api+/user.change-current-organization.ts b/app/routes/api+/user.change-current-organization.ts index b7eca4bee..26463381d 100644 --- a/app/routes/api+/user.change-current-organization.ts +++ b/app/routes/api+/user.change-current-organization.ts @@ -3,7 +3,7 @@ import { z } from "zod"; import { setSelectedOrganizationIdCookie } from "~/modules/organization/context.server"; import { setCookie } from "~/utils/cookies.server"; import { makeShelfError } from "~/utils/error"; -import { error, parseData } from "~/utils/http.server"; +import { error, parseData, safeRedirect } from "~/utils/http.server"; export async function action({ context, request }: ActionFunctionArgs) { const authSession = context.getSession(); @@ -18,7 +18,7 @@ export async function action({ context, request }: ActionFunctionArgs) { }) ); - return redirect(redirectTo ?? "/", { + return redirect(safeRedirect(redirectTo), { headers: [ setCookie(await setSelectedOrganizationIdCookie(organizationId)), ], From 5b1496bf9f1a61216a6d16b0ee6a8d4fa87c3737 Mon Sep 17 00:00:00 2001 From: Rohit Kumar Saini Date: Sat, 23 Nov 2024 12:42:28 +0100 Subject: [PATCH 17/27] feat(improve-404): add request in getAsset for proper redirecting --- app/components/errors/error-404-handler.tsx | 1 - app/routes/_layout+/assets.$assetId.activity.tsx | 1 + app/routes/_layout+/assets.$assetId.overview.duplicate.tsx | 1 + .../_layout+/assets.$assetId.overview.update-location.tsx | 7 ++++++- app/routes/_layout+/assets.$assetId.tsx | 1 + app/routes/_layout+/assets.$assetId_.edit.tsx | 1 + 6 files changed, 10 insertions(+), 2 deletions(-) diff --git a/app/components/errors/error-404-handler.tsx b/app/components/errors/error-404-handler.tsx index b84fc7a76..4b91e4165 100644 --- a/app/components/errors/error-404-handler.tsx +++ b/app/components/errors/error-404-handler.tsx @@ -1,6 +1,5 @@ import { tw } from "~/utils/tw"; import { Error404AdditionalData } from "./utils"; -import { ErrorIcon } from "."; import { Button } from "../shared/button"; import { useFetcher } from "@remix-run/react"; import { isFormProcessing } from "~/utils/form"; diff --git a/app/routes/_layout+/assets.$assetId.activity.tsx b/app/routes/_layout+/assets.$assetId.activity.tsx index 0be205ff5..20b8246f9 100644 --- a/app/routes/_layout+/assets.$assetId.activity.tsx +++ b/app/routes/_layout+/assets.$assetId.activity.tsx @@ -39,6 +39,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { id, organizationId, userOrganizations, + request, include: { notes: { orderBy: { createdAt: "desc" }, diff --git a/app/routes/_layout+/assets.$assetId.overview.duplicate.tsx b/app/routes/_layout+/assets.$assetId.overview.duplicate.tsx index b6ff87f53..4e94fc0b1 100644 --- a/app/routes/_layout+/assets.$assetId.overview.duplicate.tsx +++ b/app/routes/_layout+/assets.$assetId.overview.duplicate.tsx @@ -44,6 +44,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { id: assetId, organizationId, userOrganizations, + request, }); return json( diff --git a/app/routes/_layout+/assets.$assetId.overview.update-location.tsx b/app/routes/_layout+/assets.$assetId.overview.update-location.tsx index 90f66d248..d87d75f9f 100644 --- a/app/routes/_layout+/assets.$assetId.overview.update-location.tsx +++ b/app/routes/_layout+/assets.$assetId.overview.update-location.tsx @@ -36,7 +36,12 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { entity: PermissionEntity.asset, action: PermissionAction.update, }); - const asset = await getAsset({ organizationId, id, userOrganizations }); + const asset = await getAsset({ + organizationId, + id, + userOrganizations, + request, + }); const { locations } = await getAllEntriesForCreateAndEdit({ organizationId, diff --git a/app/routes/_layout+/assets.$assetId.tsx b/app/routes/_layout+/assets.$assetId.tsx index 6167c79f4..20bfa1da8 100644 --- a/app/routes/_layout+/assets.$assetId.tsx +++ b/app/routes/_layout+/assets.$assetId.tsx @@ -66,6 +66,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { id, organizationId, userOrganizations, + request, include: { custody: { include: { custodian: true } }, kit: true, diff --git a/app/routes/_layout+/assets.$assetId_.edit.tsx b/app/routes/_layout+/assets.$assetId_.edit.tsx index 0c8f157ac..de6d7a1af 100644 --- a/app/routes/_layout+/assets.$assetId_.edit.tsx +++ b/app/routes/_layout+/assets.$assetId_.edit.tsx @@ -65,6 +65,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { id, include: { tags: true, customFields: true }, userOrganizations, + request, }); const { categories, totalCategories, tags, locations, totalLocations } = From 9cb7f0236b293341157f98862a2dff04abc8d990 Mon Sep 17 00:00:00 2001 From: Rohit Kumar Saini Date: Sat, 23 Nov 2024 19:47:12 +0100 Subject: [PATCH 18/27] feat(improve-404): update getKit function to make it more generic --- app/modules/kit/service.server.ts | 15 ++++++++++++--- app/modules/qr/service.server.ts | 1 + 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/app/modules/kit/service.server.ts b/app/modules/kit/service.server.ts index 7a52cd756..6b66a1716 100644 --- a/app/modules/kit/service.server.ts +++ b/app/modules/kit/service.server.ts @@ -341,7 +341,14 @@ export async function getPaginatedAndFilterableKits< } } -export async function getKit({ +type KitWithInclude = + T extends Prisma.KitInclude + ? Prisma.KitGetPayload<{ + include: MergeInclude; + }> + : Prisma.KitGetPayload<{ include: typeof GET_KIT_STATIC_INCLUDES }>; + +export async function getKit({ id, organizationId, extraInclude, @@ -353,10 +360,12 @@ export async function getKit({ ...extraInclude, } as MergeInclude; - return (await db.kit.findUniqueOrThrow({ + const kit = await db.kit.findUniqueOrThrow({ where: { id, organizationId }, include: includes, - })) as Prisma.KitGetPayload<{ include: typeof includes }>; + }); + + return kit as KitWithInclude; } catch (cause) { throw new ShelfError({ cause, diff --git a/app/modules/qr/service.server.ts b/app/modules/qr/service.server.ts index ca01b7bd8..074ca8512 100644 --- a/app/modules/qr/service.server.ts +++ b/app/modules/qr/service.server.ts @@ -37,6 +37,7 @@ export async function getQrByAssetId({ assetId }: Pick) { }); } } + export async function getQrByKitId({ kitId }: Pick) { try { return await db.qr.findFirst({ From 3bedc338f9c8acd6848344f28123f7c8dc5b61b7 Mon Sep 17 00:00:00 2001 From: Rohit Kumar Saini Date: Mon, 25 Nov 2024 13:53:18 +0100 Subject: [PATCH 19/27] feat(improve-404): improve 404 handling for kit --- app/components/errors/utils.ts | 2 +- app/modules/kit/service.server.ts | 66 +++++++++++++++++-- .../_layout+/kits.$kitId.assign-custody.tsx | 16 +++-- .../kits.$kitId.create-new-booking.tsx | 20 ++++-- .../_layout+/kits.$kitId.release-custody.tsx | 9 ++- app/routes/_layout+/kits.$kitId.tsx | 4 +- app/routes/_layout+/kits.$kitId_.edit.tsx | 9 ++- 7 files changed, 101 insertions(+), 25 deletions(-) diff --git a/app/components/errors/utils.ts b/app/components/errors/utils.ts index 48249fdea..9fbe8abff 100644 --- a/app/components/errors/utils.ts +++ b/app/components/errors/utils.ts @@ -2,7 +2,7 @@ import { z } from "zod"; import { isRouteError } from "~/utils/http"; export const error404AdditionalData = z.object({ - model: z.enum(["asset"]), + model: z.enum(["asset", "kit"]), id: z.string(), redirectTo: z.string().optional(), organization: z.object({ diff --git a/app/modules/kit/service.server.ts b/app/modules/kit/service.server.ts index 6b66a1716..571349aea 100644 --- a/app/modules/kit/service.server.ts +++ b/app/modules/kit/service.server.ts @@ -6,6 +6,7 @@ import type { Qr, TeamMember, User, + UserOrganization, } from "@prisma/client"; import { AssetStatus, @@ -21,7 +22,12 @@ import { getDateTimeFormat } from "~/utils/client-hints"; import { updateCookieWithPerPage } from "~/utils/cookies.server"; import { dateTimeInUnix } from "~/utils/date-time-in-unix"; import type { ErrorLabel } from "~/utils/error"; -import { maybeUniqueConstraintViolation, ShelfError } from "~/utils/error"; +import { + isLikeShelfError, + isNotFoundError, + maybeUniqueConstraintViolation, + ShelfError, +} from "~/utils/error"; import { extractImageNameFromSupabaseUrl } from "~/utils/extract-image-name-from-supabase-url"; import { getCurrentSearchParams } from "~/utils/http.server"; import { id } from "~/utils/id/id.server"; @@ -352,19 +358,63 @@ export async function getKit({ id, organizationId, extraInclude, -}: Pick & { extraInclude?: T }) { + userOrganizations, + request, +}: Pick & { + extraInclude?: T; + userOrganizations?: Pick[]; + request?: Request; +}) { try { + const otherOrganizationIds = userOrganizations?.map( + (org) => org.organizationId + ); + // Merge static includes with dynamic includes const includes = { ...GET_KIT_STATIC_INCLUDES, ...extraInclude, } as MergeInclude; - const kit = await db.kit.findUniqueOrThrow({ - where: { id, organizationId }, + const kit = await db.kit.findFirstOrThrow({ + where: { + OR: [ + { id, organizationId }, + ...(userOrganizations?.length + ? [{ id, organizationId: { in: otherOrganizationIds } }] + : []), + ], + }, include: includes, }); + /* User is accessing the asset in the wrong organizations. In that case we need special 404 handlng. */ + if ( + userOrganizations?.length && + kit.organizationId !== organizationId && + otherOrganizationIds?.includes(kit.organizationId) + ) { + const redirectTo = + typeof request !== "undefined" + ? new URL(request.url).pathname + : undefined; + + throw new ShelfError({ + cause: null, + title: "Kit not found", + message: "", + additionalData: { + model: "kit", + organization: userOrganizations.find( + (org) => org.organizationId === kit.organizationId + ), + redirectTo, + }, + label, + status: 404, + }); + } + return kit as KitWithInclude; } catch (cause) { throw new ShelfError({ @@ -372,8 +422,12 @@ export async function getKit({ title: "Kit not found", message: "The kit you are trying to access does not exist or you do not have permission to access it.", - additionalData: { id }, - label, // Adjust the label as needed + additionalData: { + id, + ...(isLikeShelfError(cause) ? cause.additionalData : {}), + }, + label, + shouldBeCaptured: !isNotFoundError(cause), }); } } diff --git a/app/routes/_layout+/kits.$kitId.assign-custody.tsx b/app/routes/_layout+/kits.$kitId.assign-custody.tsx index c96139e46..be5d8981a 100644 --- a/app/routes/_layout+/kits.$kitId.assign-custody.tsx +++ b/app/routes/_layout+/kits.$kitId.assign-custody.tsx @@ -55,12 +55,14 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { }); try { - const { organizationId, role } = await requirePermission({ - userId, - request, - entity: PermissionEntity.kit, - action: PermissionAction.custody, - }); + const { organizationId, role, userOrganizations } = await requirePermission( + { + userId, + request, + entity: PermissionEntity.kit, + action: PermissionAction.custody, + } + ); const kit = await getKit({ id: kitId, @@ -81,6 +83,8 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { }, }, }, + userOrganizations, + request, }); if (kit.custody) { diff --git a/app/routes/_layout+/kits.$kitId.create-new-booking.tsx b/app/routes/_layout+/kits.$kitId.create-new-booking.tsx index c47d1c1cc..33b4631c7 100644 --- a/app/routes/_layout+/kits.$kitId.create-new-booking.tsx +++ b/app/routes/_layout+/kits.$kitId.create-new-booking.tsx @@ -31,13 +31,17 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { }); try { - const { organizationId, currentOrganization, isSelfServiceOrBase } = - await requirePermission({ - userId, - request, - entity: PermissionEntity.booking, - action: PermissionAction.create, - }); + const { + organizationId, + currentOrganization, + isSelfServiceOrBase, + userOrganizations, + } = await requirePermission({ + userId, + request, + entity: PermissionEntity.booking, + action: PermissionAction.create, + }); if (isPersonalOrg(currentOrganization)) { throw new ShelfError({ @@ -54,6 +58,8 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { id: kitId, organizationId, extraInclude: { assets: true }, + userOrganizations, + request, }); /* We need to fetch the team members to be able to display them in the custodian dropdown. */ diff --git a/app/routes/_layout+/kits.$kitId.release-custody.tsx b/app/routes/_layout+/kits.$kitId.release-custody.tsx index b7190eee1..5d266d3e7 100644 --- a/app/routes/_layout+/kits.$kitId.release-custody.tsx +++ b/app/routes/_layout+/kits.$kitId.release-custody.tsx @@ -31,14 +31,19 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { }); try { - const { organizationId } = await requirePermission({ + const { organizationId, userOrganizations } = await requirePermission({ userId, request, entity: PermissionEntity.kit, action: PermissionAction.custody, }); - const kit = await getKit({ id: kitId, organizationId }); + const kit = await getKit({ + id: kitId, + organizationId, + userOrganizations, + request, + }); if (!kit.custody) { return redirect(`/kits/${kitId}`); } diff --git a/app/routes/_layout+/kits.$kitId.tsx b/app/routes/_layout+/kits.$kitId.tsx index e2a855199..353d45dc5 100644 --- a/app/routes/_layout+/kits.$kitId.tsx +++ b/app/routes/_layout+/kits.$kitId.tsx @@ -75,7 +75,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { ); try { - const { organizationId } = await requirePermission({ + const { organizationId, userOrganizations } = await requirePermission({ userId, request, entity: PermissionEntity.kit, @@ -131,6 +131,8 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { }, qrCodes: true, }, + userOrganizations, + request, }), getAssetsForKits({ request, diff --git a/app/routes/_layout+/kits.$kitId_.edit.tsx b/app/routes/_layout+/kits.$kitId_.edit.tsx index fec0c6ff2..fb983c209 100644 --- a/app/routes/_layout+/kits.$kitId_.edit.tsx +++ b/app/routes/_layout+/kits.$kitId_.edit.tsx @@ -43,14 +43,19 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { }); try { - const { organizationId } = await requirePermission({ + const { organizationId, userOrganizations } = await requirePermission({ userId, request, entity: PermissionEntity.kit, action: PermissionAction.update, }); - const kit = await getKit({ id: kitId, organizationId }); + const kit = await getKit({ + id: kitId, + organizationId, + userOrganizations, + request, + }); const header: HeaderData = { title: `Edit | ${kit.name}`, From 8cfd653161f44612b984ecbcd47db40d40f56b34 Mon Sep 17 00:00:00 2001 From: Rohit Kumar Saini Date: Tue, 26 Nov 2024 15:07:00 +0100 Subject: [PATCH 20/27] feat(improve-404): improve 404 handling for location --- app/components/errors/utils.ts | 2 +- app/modules/location/service.server.ts | 73 +++++++++++++++++-- app/routes/_layout+/locations.$locationId.tsx | 6 +- .../_layout+/locations.$locationId_.edit.tsx | 9 ++- 4 files changed, 79 insertions(+), 11 deletions(-) diff --git a/app/components/errors/utils.ts b/app/components/errors/utils.ts index 9fbe8abff..4b45330ca 100644 --- a/app/components/errors/utils.ts +++ b/app/components/errors/utils.ts @@ -2,7 +2,7 @@ import { z } from "zod"; import { isRouteError } from "~/utils/http"; export const error404AdditionalData = z.object({ - model: z.enum(["asset", "kit"]), + model: z.enum(["asset", "kit", "location"]), id: z.string(), redirectTo: z.string().optional(), organization: z.object({ diff --git a/app/modules/location/service.server.ts b/app/modules/location/service.server.ts index 70f43678b..bcb1eb755 100644 --- a/app/modules/location/service.server.ts +++ b/app/modules/location/service.server.ts @@ -1,8 +1,19 @@ -import type { Prisma, User, Location, Organization } from "@prisma/client"; +import type { + Prisma, + User, + Location, + Organization, + UserOrganization, +} from "@prisma/client"; import invariant from "tiny-invariant"; import { db } from "~/database/db.server"; import type { ErrorLabel } from "~/utils/error"; -import { ShelfError, maybeUniqueConstraintViolation } from "~/utils/error"; +import { + ShelfError, + isLikeShelfError, + isNotFoundError, + maybeUniqueConstraintViolation, +} from "~/utils/error"; import { ALL_SELECTED_KEY } from "~/utils/list"; import type { CreateAssetFromContentImportPayload } from "../asset/types"; @@ -16,11 +27,25 @@ export async function getLocation( /** Assets to be loaded per page with the location */ perPage?: number; search?: string | null; + userOrganizations?: Pick[]; + request?: Request; } ) { - const { organizationId, id, page = 1, perPage = 8, search } = params; + const { + organizationId, + id, + page = 1, + perPage = 8, + search, + userOrganizations, + request, + } = params; try { + const otherOrganizationIds = userOrganizations?.map( + (org) => org.organizationId + ); + const skip = page > 1 ? (page - 1) * perPage : 0; const take = perPage >= 1 ? perPage : 8; // min 1 and max 25 per page @@ -37,7 +62,14 @@ export async function getLocation( const [location, totalAssetsWithinLocation] = await Promise.all([ /** Get the items */ db.location.findFirstOrThrow({ - where: { id, organizationId }, + where: { + OR: [ + { id, organizationId }, + ...(userOrganizations?.length + ? [{ id, organizationId: { in: otherOrganizationIds } }] + : []), + ], + }, include: { image: { select: { @@ -64,13 +96,44 @@ export async function getLocation( }), ]); + /* User is accessing the asset in the wrong organization. In that case we need special 404 handling. */ + if ( + userOrganizations?.length && + location.organizationId !== organizationId && + otherOrganizationIds?.includes(location.organizationId) + ) { + const redirectTo = + typeof request !== "undefined" + ? new URL(request.url).pathname + : undefined; + + throw new ShelfError({ + cause: null, + title: "Location not found.", + message: "", + additionalData: { + model: "location", + organization: userOrganizations.find( + (org) => org.organizationId === location.organizationId + ), + redirectTo, + }, + label, + status: 404, + }); + } + return { location, totalAssetsWithinLocation }; } catch (cause) { throw new ShelfError({ cause, message: "Something went wrong while fetching location", - additionalData: { ...params }, + additionalData: { + ...params, + ...(isLikeShelfError(cause) ? cause.additionalData : {}), + }, label, + shouldBeCaptured: !isNotFoundError(cause), }); } } diff --git a/app/routes/_layout+/locations.$locationId.tsx b/app/routes/_layout+/locations.$locationId.tsx index 2365bade6..54776c626 100644 --- a/app/routes/_layout+/locations.$locationId.tsx +++ b/app/routes/_layout+/locations.$locationId.tsx @@ -62,7 +62,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { ); try { - const { organizationId } = await requirePermission({ + const { organizationId, userOrganizations } = await requirePermission({ userId: authSession.userId, request, entity: PermissionEntity.location, @@ -80,6 +80,8 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { page, perPage, search, + userOrganizations, + request, }); const totalItems = totalAssetsWithinLocation; @@ -358,5 +360,3 @@ const ListItemTagsColumn = ({ tags }: { tags: Tag[] | undefined }) => {

) : null; }; - -// export const ErrorBoundary = () => ; diff --git a/app/routes/_layout+/locations.$locationId_.edit.tsx b/app/routes/_layout+/locations.$locationId_.edit.tsx index c40e12387..6ab489b9d 100644 --- a/app/routes/_layout+/locations.$locationId_.edit.tsx +++ b/app/routes/_layout+/locations.$locationId_.edit.tsx @@ -43,14 +43,19 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) { ); try { - const { organizationId } = await requirePermission({ + const { organizationId, userOrganizations } = await requirePermission({ userId: authSession.userId, request, entity: PermissionEntity.location, action: PermissionAction.update, }); - const { location } = await getLocation({ organizationId, id }); + const { location } = await getLocation({ + organizationId, + id, + userOrganizations, + request, + }); const header: HeaderData = { title: `Edit | ${location.name}`, From 92bc9aa462ef35104bc037da970ee3c2ed1bf9ae Mon Sep 17 00:00:00 2001 From: Rohit Kumar Saini Date: Tue, 26 Nov 2024 20:08:41 +0100 Subject: [PATCH 21/27] feature(workflow): create separate function for team with 404 handling --- app/components/errors/utils.ts | 2 +- app/modules/user/service.server.ts | 105 +++++++++++++++++- app/modules/user/types.ts | 6 +- .../_layout+/settings.team.users.$userId.tsx | 44 ++++---- 4 files changed, 128 insertions(+), 29 deletions(-) diff --git a/app/components/errors/utils.ts b/app/components/errors/utils.ts index 4b45330ca..8d3772d14 100644 --- a/app/components/errors/utils.ts +++ b/app/components/errors/utils.ts @@ -2,7 +2,7 @@ import { z } from "zod"; import { isRouteError } from "~/utils/http"; export const error404AdditionalData = z.object({ - model: z.enum(["asset", "kit", "location"]), + model: z.enum(["asset", "kit", "location", "user"]), id: z.string(), redirectTo: z.string().optional(), organization: z.object({ diff --git a/app/modules/user/service.server.ts b/app/modules/user/service.server.ts index 50cb91a1c..85f6fb213 100644 --- a/app/modules/user/service.server.ts +++ b/app/modules/user/service.server.ts @@ -1,4 +1,4 @@ -import type { Organization, User } from "@prisma/client"; +import type { Organization, User, UserOrganization } from "@prisma/client"; import { Prisma, Roles, OrganizationRoles } from "@prisma/client"; import type { ITXClientDenyList } from "@prisma/client/runtime/library"; import { PrismaClientKnownRequestError } from "@prisma/client/runtime/library"; @@ -20,7 +20,7 @@ import { import { dateTimeInUnix } from "~/utils/date-time-in-unix"; import type { ErrorLabel } from "~/utils/error"; -import { ShelfError, isLikeShelfError } from "~/utils/error"; +import { ShelfError, isLikeShelfError, isNotFoundError } from "~/utils/error"; import type { ValidationError } from "~/utils/http"; import { getCurrentSearchParams } from "~/utils/http.server"; import { id as generateId } from "~/utils/id/id.server"; @@ -34,11 +34,12 @@ import { } from "~/utils/storage.server"; import { randomUsernameFromEmail } from "~/utils/user"; import { INCLUDE_SSO_DETAILS_VIA_USER_ORGANIZATION } from "./fields"; -import type { UpdateUserPayload } from "./types"; +import { type UpdateUserPayload, USER_STATIC_INCLUDE } from "./types"; import { defaultFields } from "../asset-index-settings/helpers"; import { defaultUserCategories } from "../category/default-categories"; import { getOrganizationsBySsoDomain } from "../organization/service.server"; import { createTeamMember } from "../team-member/service.server"; +import { MergeInclude } from "~/utils/utils"; const label: ErrorLabel = "User"; @@ -1238,3 +1239,101 @@ export async function transferEntitiesToNewOwner({ }, }); } + +type UserWithExtraInclude = + T extends Prisma.UserInclude + ? Prisma.UserGetPayload<{ + include: MergeInclude; + }> + : Prisma.UserGetPayload<{ include: typeof USER_STATIC_INCLUDE }>; + +export async function getUserFromOrg({ + id, + organizationId, + userOrganizations, + request, + extraInclude, +}: Pick & { + organizationId: Organization["id"]; + userOrganizations?: Pick[]; + request?: Request; + extraInclude?: T; +}) { + try { + const otherOrganizationIds = userOrganizations?.map( + (org) => org.organizationId + ); + + const mergedInclude = { + ...USER_STATIC_INCLUDE, + ...extraInclude, + } as MergeInclude; + + const user = (await db.user.findFirstOrThrow({ + where: { + OR: [ + { id, userOrganizations: { some: { organizationId } } }, + ...(userOrganizations?.length + ? [ + { + id, + userOrganizations: { + some: { organizationId: { in: otherOrganizationIds } }, + }, + }, + ] + : []), + ], + }, + include: mergedInclude, + })) as UserWithExtraInclude; + + /* User is accessing the User in the wrong organization */ + const isUserInCurrentOrg = !!user.userOrganizations.find( + (userOrg) => userOrg.organizationId === organizationId + ); + + const otherOrgOfUser = userOrganizations?.find( + (org) => + !!user.userOrganizations.find( + (userOrg) => userOrg.organizationId === org.organizationId + ) + ); + + if (userOrganizations?.length && !isUserInCurrentOrg && !!otherOrgOfUser) { + const redirectTo = + typeof request !== "undefined" + ? new URL(request.url).pathname + : undefined; + + throw new ShelfError({ + cause: null, + title: "User not found", + message: "", + additionalData: { + model: "user", + organization: otherOrgOfUser, + redirectTo, + }, + label, + status: 404, + }); + } + + return user; + } catch (cause) { + throw new ShelfError({ + cause, + title: "User not found.", + message: + "The user you are trying to access does not exists or you do not have permission to access it.", + additionalData: { + id, + organizationId, + ...(isLikeShelfError(cause) ? cause.additionalData : {}), + }, + label, + shouldBeCaptured: !isNotFoundError(cause), + }); + } +} diff --git a/app/modules/user/types.ts b/app/modules/user/types.ts index e15dbfcdd..ad75f9488 100644 --- a/app/modules/user/types.ts +++ b/app/modules/user/types.ts @@ -1,4 +1,4 @@ -import type { User } from "@prisma/client"; +import type { Prisma, User } from "@prisma/client"; export interface UpdateUserPayload { id: User["id"]; @@ -24,3 +24,7 @@ export interface UpdateUserResponse { /** Used when sending a pwd reset link for the user */ passwordReset?: boolean; } + +export const USER_STATIC_INCLUDE = { + userOrganizations: true, +} satisfies Prisma.UserInclude; diff --git a/app/routes/_layout+/settings.team.users.$userId.tsx b/app/routes/_layout+/settings.team.users.$userId.tsx index 833b08cde..394a52ee7 100644 --- a/app/routes/_layout+/settings.team.users.$userId.tsx +++ b/app/routes/_layout+/settings.team.users.$userId.tsx @@ -13,7 +13,7 @@ import { Badge } from "~/components/shared/badge"; import { Button } from "~/components/shared/button"; import When from "~/components/when/when"; import { TeamUsersActionsDropdown } from "~/components/workspace/users-actions-dropdown"; -import { getUserByID } from "~/modules/user/service.server"; +import { getUserFromOrg } from "~/modules/user/service.server"; import { resolveUserAction } from "~/modules/user/utils.server"; import { appendToMetaTitle } from "~/utils/append-to-meta-title"; import { makeShelfError } from "~/utils/error"; @@ -33,12 +33,13 @@ export const loader = async ({ const authSession = context.getSession(); const { userId } = authSession; try { - const { currentOrganization, organizationId } = await requirePermission({ - userId, - request, - entity: PermissionEntity.teamMemberProfile, - action: PermissionAction.read, - }); + const { currentOrganization, organizationId, userOrganizations } = + await requirePermission({ + userId, + request, + entity: PermissionEntity.teamMemberProfile, + action: PermissionAction.read, + }); const { userId: selectedUserId } = getParams( params, @@ -48,28 +49,23 @@ export const loader = async ({ } ); - const user = await getUserByID(selectedUserId, { - userOrganizations: { - where: { - organizationId, - }, - select: { - roles: true, - }, - }, - teamMembers: { - where: { - organizationId, - }, - include: { - receivedInvites: { - where: { - organizationId, + const user = await getUserFromOrg({ + id: selectedUserId, + organizationId, + userOrganizations, + request, + extraInclude: { + teamMembers: { + where: { organizationId }, + include: { + receivedInvites: { + where: { organizationId }, }, }, }, }, }); + const userName = (user.firstName ? user.firstName.trim() : "") + " " + From e023233e71749acec208d4239923c342dcd6c531 Mon Sep 17 00:00:00 2001 From: Rohit Kumar Saini Date: Tue, 26 Nov 2024 20:42:22 +0100 Subject: [PATCH 22/27] feat(improve-404): handling special case of multiple organization for team member --- app/components/errors/error-404-handler.tsx | 140 +++++++++++++++----- app/components/errors/utils.ts | 31 +++-- app/modules/user/service.server.ts | 23 ++-- 3 files changed, 144 insertions(+), 50 deletions(-) diff --git a/app/components/errors/error-404-handler.tsx b/app/components/errors/error-404-handler.tsx index 4b91e4165..17ca763dc 100644 --- a/app/components/errors/error-404-handler.tsx +++ b/app/components/errors/error-404-handler.tsx @@ -1,8 +1,16 @@ +import { useMemo } from "react"; import { tw } from "~/utils/tw"; import { Error404AdditionalData } from "./utils"; import { Button } from "../shared/button"; import { useFetcher } from "@remix-run/react"; import { isFormProcessing } from "~/utils/form"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "../forms/select"; export type Error404HandlerProps = { className?: string; @@ -18,44 +26,110 @@ export default function Error404Handler({ const fetcher = useFetcher(); const disabled = isFormProcessing(fetcher.state); + const content = useMemo(() => { + switch (additionalData.model) { + case "asset": + case "kit": + case "location": { + return ( +
+
+

+ {additionalData.model}{" "} + belongs to another workspace. +

+

+ The {additionalData.model} you are trying to view belongs to a + different workspace you are part of. Would you like to switch to + workspace{" "} + + "{additionalData.organization.organization.name}" + {" "} + to view the {additionalData.model}? +

+ + + + + +
+
+ ); + } + + case "teamMember": { + return ( +
+
+

+ Team Member belongs to + another workspace(s). +

+

+ The team member you are trying to view belongs to one/some of + your different workspace you are part of. Would you like to + switch to workspace to view the team member? +

+ + + + + +
+
+ ); + } + + default: { + return null; + } + } + }, [additionalData]); + return (
-
-
-

- {additionalData.model} belongs - to another workspace. -

-

- The {additionalData.model} you are trying to view belongs to a - different workspace you are part of. Would you like to switch to - workspace{" "} - - "{additionalData.organization.organization.name}" - {" "} - to view the {additionalData.model}? -

- - - - - -
-
+ {content}
); } diff --git a/app/components/errors/utils.ts b/app/components/errors/utils.ts index 8d3772d14..c075c9232 100644 --- a/app/components/errors/utils.ts +++ b/app/components/errors/utils.ts @@ -1,19 +1,34 @@ import { z } from "zod"; import { isRouteError } from "~/utils/http"; -export const error404AdditionalData = z.object({ - model: z.enum(["asset", "kit", "location", "user"]), +const baseAdditionalDataSchema = z.object({ id: z.string(), redirectTo: z.string().optional(), +}); + +const organizationSchema = z.object({ organization: z.object({ - organization: z.object({ - id: z.string(), - name: z.string(), - }), + id: z.string(), + name: z.string(), }), }); -export type Error404AdditionalData = z.infer; +export const error404AdditionalDataSchema = z.discriminatedUnion("model", [ + /* For common and general use case */ + baseAdditionalDataSchema.extend({ + model: z.enum(["asset", "kit", "location"]), + organization: organizationSchema, + }), + /* A team member (user) can be in multiple organization's of user so we do this. */ + baseAdditionalDataSchema.extend({ + model: z.literal("teamMember"), + organizations: organizationSchema.array(), + }), +]); + +export type Error404AdditionalData = z.infer< + typeof error404AdditionalDataSchema +>; export function parse404ErrorData(response: unknown): | { isError404: false; additionalData: null } @@ -25,7 +40,7 @@ export function parse404ErrorData(response: unknown): return { isError404: false, additionalData: null }; } - const parsedDataResponse = error404AdditionalData.safeParse( + const parsedDataResponse = error404AdditionalDataSchema.safeParse( response.data.error.additionalData ); diff --git a/app/modules/user/service.server.ts b/app/modules/user/service.server.ts index 85f6fb213..5846962f7 100644 --- a/app/modules/user/service.server.ts +++ b/app/modules/user/service.server.ts @@ -1293,14 +1293,19 @@ export async function getUserFromOrg({ (userOrg) => userOrg.organizationId === organizationId ); - const otherOrgOfUser = userOrganizations?.find( - (org) => - !!user.userOrganizations.find( - (userOrg) => userOrg.organizationId === org.organizationId - ) - ); + const otherOrgsForUser = + userOrganizations?.filter( + (org) => + !!user.userOrganizations.find( + (userOrg) => userOrg.organizationId === org.organizationId + ) + ) ?? []; - if (userOrganizations?.length && !isUserInCurrentOrg && !!otherOrgOfUser) { + if ( + userOrganizations?.length && + !isUserInCurrentOrg && + otherOrgsForUser?.length + ) { const redirectTo = typeof request !== "undefined" ? new URL(request.url).pathname @@ -1311,8 +1316,8 @@ export async function getUserFromOrg({ title: "User not found", message: "", additionalData: { - model: "user", - organization: otherOrgOfUser, + model: "teamMember", + organizations: otherOrgsForUser, redirectTo, }, label, From 1476c9d1fec79fce12f374b05119d0548b7e3e92 Mon Sep 17 00:00:00 2001 From: Rohit Kumar Saini Date: Wed, 27 Nov 2024 11:45:36 +0100 Subject: [PATCH 23/27] feat(improve-404): fix style issue on ErrorBoundary for team settings --- app/components/errors/error-404-handler.tsx | 2 +- app/components/errors/index.tsx | 19 ++++++++++++++++--- .../_layout+/settings.team.users.$userId.tsx | 17 ++++++++++++----- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/app/components/errors/error-404-handler.tsx b/app/components/errors/error-404-handler.tsx index 17ca763dc..155331a99 100644 --- a/app/components/errors/error-404-handler.tsx +++ b/app/components/errors/error-404-handler.tsx @@ -88,7 +88,7 @@ export default function Error404Handler({ > - + { return (
diff --git a/app/modules/asset/service.server.ts b/app/modules/asset/service.server.ts index bfd30d3ca..c9dcedc77 100644 --- a/app/modules/asset/service.server.ts +++ b/app/modules/asset/service.server.ts @@ -56,6 +56,7 @@ import { isNotFoundError, maybeUniqueConstraintViolation, } from "~/utils/error"; +import { getRedirectUrlFromRequest } from "~/utils/http"; import { getCurrentSearchParams } from "~/utils/http.server"; import { id } from "~/utils/id/id.server"; import { ALL_SELECTED_KEY, getParamsValues } from "~/utils/list"; @@ -91,7 +92,6 @@ import { createKitsIfNotExists } from "../kit/service.server"; import { createNote } from "../note/service.server"; import { getUserByID } from "../user/service.server"; -import { getRedirectUrlFromRequest } from "~/utils/http"; const label: ErrorLabel = "Assets"; diff --git a/app/modules/booking/service.server.ts b/app/modules/booking/service.server.ts index cf9740128..70e8a1730 100644 --- a/app/modules/booking/service.server.ts +++ b/app/modules/booking/service.server.ts @@ -16,10 +16,12 @@ import { calcTimeDifference } from "~/utils/date-fns"; import { sendNotification } from "~/utils/emitter/send-notification.server"; import type { ErrorLabel } from "~/utils/error"; import { isLikeShelfError, isNotFoundError, ShelfError } from "~/utils/error"; +import { getRedirectUrlFromRequest } from "~/utils/http"; import { getCurrentSearchParams } from "~/utils/http.server"; import { ALL_SELECTED_KEY } from "~/utils/list"; import { Logger } from "~/utils/logger"; import { scheduler } from "~/utils/scheduler.server"; +import type { MergeInclude } from "~/utils/utils"; import { bookingSchedulerEventsEnum, schedulerKeys } from "./constants"; import { assetReservedEmailContent, @@ -34,8 +36,6 @@ import { getBookingWhereInput } from "./utils.server"; import { createNotes } from "../note/service.server"; import { getOrganizationAdminsEmails } from "../organization/service.server"; import { getUserByID } from "../user/service.server"; -import { MergeInclude } from "~/utils/utils"; -import { getRedirectUrlFromRequest } from "~/utils/http"; const label: ErrorLabel = "Booking"; diff --git a/app/modules/custom-field/service.server.ts b/app/modules/custom-field/service.server.ts index 26ee933b2..0ea8e5369 100644 --- a/app/modules/custom-field/service.server.ts +++ b/app/modules/custom-field/service.server.ts @@ -13,6 +13,7 @@ import { isLikeShelfError, maybeUniqueConstraintViolation, } from "~/utils/error"; +import { getRedirectUrlFromRequest } from "~/utils/http"; import type { CustomFieldDraftPayload } from "./types"; import type { CreateAssetFromContentImportPayload } from "../asset/types"; import type { Column } from "../asset-index-settings/helpers"; @@ -20,7 +21,6 @@ import { updateAssetIndexSettingsAfterCfUpdate, updateAssetIndexSettingsWithNewCustomFields, } from "../asset-index-settings/service.server"; -import { getRedirectUrlFromRequest } from "~/utils/http"; const label: ErrorLabel = "Custom fields"; diff --git a/app/modules/kit/service.server.ts b/app/modules/kit/service.server.ts index c014c69e5..db050e3e5 100644 --- a/app/modules/kit/service.server.ts +++ b/app/modules/kit/service.server.ts @@ -29,6 +29,7 @@ import { ShelfError, } from "~/utils/error"; import { extractImageNameFromSupabaseUrl } from "~/utils/extract-image-name-from-supabase-url"; +import { getRedirectUrlFromRequest } from "~/utils/http"; import { getCurrentSearchParams } from "~/utils/http.server"; import { id } from "~/utils/id/id.server"; import { ALL_SELECTED_KEY, getParamsValues } from "~/utils/list"; @@ -44,7 +45,6 @@ import { createNote } from "../note/service.server"; import { getQr } from "../qr/service.server"; import { getUserByID } from "../user/service.server"; -import { getRedirectUrlFromRequest } from "~/utils/http"; const label: ErrorLabel = "Kit"; diff --git a/app/modules/location/service.server.ts b/app/modules/location/service.server.ts index 48067b623..62f29535e 100644 --- a/app/modules/location/service.server.ts +++ b/app/modules/location/service.server.ts @@ -14,9 +14,9 @@ import { isNotFoundError, maybeUniqueConstraintViolation, } from "~/utils/error"; +import { getRedirectUrlFromRequest } from "~/utils/http"; import { ALL_SELECTED_KEY } from "~/utils/list"; import type { CreateAssetFromContentImportPayload } from "../asset/types"; -import { getRedirectUrlFromRequest } from "~/utils/http"; const label: ErrorLabel = "Location"; diff --git a/app/modules/user/service.server.ts b/app/modules/user/service.server.ts index 04cd8064c..09bd3c66b 100644 --- a/app/modules/user/service.server.ts +++ b/app/modules/user/service.server.ts @@ -33,13 +33,13 @@ import { parseFileFormData, } from "~/utils/storage.server"; import { randomUsernameFromEmail } from "~/utils/user"; +import type { MergeInclude } from "~/utils/utils"; import { INCLUDE_SSO_DETAILS_VIA_USER_ORGANIZATION } from "./fields"; import { type UpdateUserPayload, USER_STATIC_INCLUDE } from "./types"; import { defaultFields } from "../asset-index-settings/helpers"; import { defaultUserCategories } from "../category/default-categories"; import { getOrganizationsBySsoDomain } from "../organization/service.server"; import { createTeamMember } from "../team-member/service.server"; -import { MergeInclude } from "~/utils/utils"; const label: ErrorLabel = "User"; diff --git a/app/routes/_layout+/bookings.$bookingId.cal[.ics].ts b/app/routes/_layout+/bookings.$bookingId.cal[.ics].ts index 475e87a13..23107cba6 100644 --- a/app/routes/_layout+/bookings.$bookingId.cal[.ics].ts +++ b/app/routes/_layout+/bookings.$bookingId.cal[.ics].ts @@ -21,13 +21,12 @@ export async function loader({ request, context, params }: LoaderFunctionArgs) { try { /** Check if the current user is allowed to read booking */ - const { organizationId, isSelfServiceOrBase, userOrganizations } = - await requirePermission({ - userId: authSession.userId, - request, - entity: PermissionEntity.booking, - action: PermissionAction.read, - }); + const { organizationId, isSelfServiceOrBase } = await requirePermission({ + userId: authSession.userId, + request, + entity: PermissionEntity.booking, + action: PermissionAction.read, + }); const booking = await getBooking({ id: bookingId, organizationId }); diff --git a/app/routes/_layout+/settings.team.users.$userId.tsx b/app/routes/_layout+/settings.team.users.$userId.tsx index 80e488a87..912d17897 100644 --- a/app/routes/_layout+/settings.team.users.$userId.tsx +++ b/app/routes/_layout+/settings.team.users.$userId.tsx @@ -24,7 +24,6 @@ import { } from "~/utils/permissions/permission.data"; import { requirePermission } from "~/utils/roles.server"; import { organizationRolesMap } from "./settings.team"; -import { ErrorContent } from "~/components/errors"; export const loader = async ({ request,