Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/improve 404 handling #1433

Merged
merged 36 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
fb1ef89
feat(404): add additionalData in getAsset for special case
rockingrohit9639 Nov 19, 2024
2514f86
feat(special-error): create UI for handling special error cases
rockingrohit9639 Nov 19, 2024
74fe491
Merge branch 'main' of github.com:Shelf-nu/shelf.nu into feature/impr…
rockingrohit9639 Nov 19, 2024
9d580a2
Merge branch 'main' of github.com:Shelf-nu/shelf.nu into feature/impr…
rockingrohit9639 Nov 20, 2024
fb73871
pr review changes
rockingrohit9639 Nov 20, 2024
bbe1653
Merge branch 'main' of github.com:Shelf-nu/shelf.nu into feature/impr…
rockingrohit9639 Nov 22, 2024
82f3147
feat(improve-404): fix pr review issues
rockingrohit9639 Nov 22, 2024
c08bcb1
Merge branch 'main' into feature/improve-404-handling
DonKoko Nov 22, 2024
532f6d8
feat(improve-404): add request in getAsset for proper redirecting
rockingrohit9639 Nov 23, 2024
ca5fe65
feat(improve-404): update getKit function to make it more generic
rockingrohit9639 Nov 23, 2024
7583eef
feat(improve-404): improve 404 handling for kit
rockingrohit9639 Nov 25, 2024
96f723b
Merge branch 'main' of github.com:Shelf-nu/shelf.nu into feature/impr…
rockingrohit9639 Nov 26, 2024
a1e3c36
feat(improve-404): improve 404 handling for location
rockingrohit9639 Nov 26, 2024
ff01bef
feature(workflow): create separate function for team with 404 handling
rockingrohit9639 Nov 26, 2024
cf330c2
feat(improve-404): handling special case of multiple organization for…
rockingrohit9639 Nov 26, 2024
e460b90
feat(improve-404): fix style issue on ErrorBoundary for team settings
rockingrohit9639 Nov 27, 2024
5ec5d9e
feat(improve-404): improve 404 handling for bookings
rockingrohit9639 Nov 27, 2024
bf91979
feat(404): add additionalData in getAsset for special case
rockingrohit9639 Nov 19, 2024
a708ae4
feat(special-error): create UI for handling special error cases
rockingrohit9639 Nov 19, 2024
b9f50e6
pr review changes
rockingrohit9639 Nov 20, 2024
473c9dc
feat(improve-404): fix pr review issues
rockingrohit9639 Nov 22, 2024
5b1496b
feat(improve-404): add request in getAsset for proper redirecting
rockingrohit9639 Nov 23, 2024
9cb7f02
feat(improve-404): update getKit function to make it more generic
rockingrohit9639 Nov 23, 2024
3bedc33
feat(improve-404): improve 404 handling for kit
rockingrohit9639 Nov 25, 2024
8cfd653
feat(improve-404): improve 404 handling for location
rockingrohit9639 Nov 26, 2024
92bc9aa
feature(workflow): create separate function for team with 404 handling
rockingrohit9639 Nov 26, 2024
e023233
feat(improve-404): handling special case of multiple organization for…
rockingrohit9639 Nov 26, 2024
1476c9d
feat(improve-404): fix style issue on ErrorBoundary for team settings
rockingrohit9639 Nov 27, 2024
bcea05b
feat(improve-404): improve 404 handling for bookings
rockingrohit9639 Nov 27, 2024
6342658
Merge branch 'main' into feature/improve-404-handling
rockingrohit9639 Nov 27, 2024
c15bcae
Merge branch 'feature/improve-404-handling' of github.com:rockingrohi…
rockingrohit9639 Nov 28, 2024
64607bd
Merge branch 'main' of github.com:Shelf-nu/shelf.nu into feature/impr…
rockingrohit9639 Nov 28, 2024
9145221
feat(improve-404): done pr review changes
rockingrohit9639 Nov 28, 2024
260e1e7
feat(improve-404): improve 404 handling for customField
rockingrohit9639 Nov 28, 2024
331623f
Merge branch 'main' of github.com:Shelf-nu/shelf.nu into feature/impr…
rockingrohit9639 Nov 29, 2024
c3afdb5
feat(improve-404): fix eslint issues
rockingrohit9639 Nov 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions app/components/errors/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -18,6 +20,11 @@ export const ErrorContent = () => {
traceId = response.data.error.traceId;
}

const specialError = parseSpecialErrorData(response);
rockingrohit9639 marked this conversation as resolved.
Show resolved Hide resolved
if (specialError.success) {
return <SpecialErrorHandler additionalData={specialError.additionalData} />;
rockingrohit9639 marked this conversation as resolved.
Show resolved Hide resolved
}

// Creating a string with <br/> tags for line breaks
const messageHtml = { __html: message.split("\n").join("<br/>") };

Expand Down
57 changes: 57 additions & 0 deletions app/components/errors/special-error-handler.tsx
Original file line number Diff line number Diff line change
@@ -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": {
rockingrohit9639 marked this conversation as resolved.
Show resolved Hide resolved
return (
<div className="w-full md:max-w-screen-sm">
<h2 className="mb-2">Asset belongs to other workspace.</h2>
<p className="mb-4">
The asset you are trying to view belongs to a different workspace
you are part of. Would you like to switch to workspace{" "}
<span className="font-bold">
"{additionalData.assetOrganization.organization.name}"
</span>{" "}
to view the asset?
</p>

<Button>Switch workspace</Button>
rockingrohit9639 marked this conversation as resolved.
Show resolved Hide resolved
</div>
);
}

default: {
return null;
}
}
}, []);

return (
<div
rockingrohit9639 marked this conversation as resolved.
Show resolved Hide resolved
className={tw("flex size-full items-center justify-center", className)}
style={style}
>
<div className="flex flex-col items-center text-center">
<span className="mb-5 size-[56px] text-primary">
rockingrohit9639 marked this conversation as resolved.
Show resolved Hide resolved
<ErrorIcon />
</span>
{content}
</div>
</div>
);
}
39 changes: 39 additions & 0 deletions app/components/errors/utils.ts
Original file line number Diff line number Diff line change
@@ -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 }
DonKoko marked this conversation as resolved.
Show resolved Hide resolved
| {
success: true;
rockingrohit9639 marked this conversation as resolved.
Show resolved Hide resolved
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 };
}
39 changes: 37 additions & 2 deletions app/modules/asset/service.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {
Booking,
Kit,
AssetIndexSettings,
UserOrganization,
} from "@prisma/client";
import {
AssetStatus,
Expand Down Expand Up @@ -101,25 +102,59 @@ type AssetWithInclude<T extends Prisma.AssetInclude | undefined> =
export async function getAsset<T extends Prisma.AssetInclude | undefined>({
id,
organizationId,
userOrganizations,
include,
}: Pick<Asset, "id"> & {
organizationId: Asset["organizationId"];
userOrganizations: Pick<UserOrganization, "organizationId">[];
include?: T;
}): Promise<AssetWithInclude<T>> {
try {
const otherOrganizationIds = userOrganizations.map(
rockingrohit9639 marked this conversation as resolved.
Show resolved Hide resolved
(org) => org.organizationId
);

const asset = await db.asset.findFirstOrThrow({
where: { id, organizationId },
where: {
OR: [
{ id, organizationId },
{ id, organizationId: { in: otherOrganizationIds } },
rockingrohit9639 marked this conversation as resolved.
Show resolved Hide resolved
],
},
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,
rockingrohit9639 marked this conversation as resolved.
Show resolved Hide resolved
title: "Asset not found",
message: "",
additionalData: {
type: "asset-from-other-org",
assetOrganization: userOrganizations.find(
rockingrohit9639 marked this conversation as resolved.
Show resolved Hide resolved
(org) => org.organizationId === asset.organizationId
),
},
label,
});
}

return asset as AssetWithInclude<T>;
} catch (cause) {
throw new ShelfError({
cause,
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 : {}),
rockingrohit9639 marked this conversation as resolved.
Show resolved Hide resolved
},
label,
shouldBeCaptured: !isNotFoundError(cause),
});
Expand Down
1 change: 1 addition & 0 deletions app/modules/organization/service.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
3 changes: 2 additions & 1 deletion app/routes/_layout+/assets.$assetId.activity.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -38,6 +38,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) {
const asset = await getAsset({
id,
organizationId,
userOrganizations,
include: {
notes: {
orderBy: { createdAt: "desc" },
Expand Down
11 changes: 8 additions & 3 deletions app/routes/_layout+/assets.$assetId.overview.duplicate.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion app/routes/_layout+/assets.$assetId.overview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -82,6 +82,7 @@ export async function loader({ context, request, params }: LoaderFunctionArgs) {
const asset = await getAsset({
id,
organizationId,
userOrganizations,
include: ASSET_OVERVIEW_FIELDS,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion app/routes/_layout+/assets.$assetId.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
14 changes: 8 additions & 6 deletions app/routes/_layout+/assets.$assetId_.edit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 } =
Expand Down
3 changes: 2 additions & 1 deletion app/routes/api+/asset.scan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -42,6 +42,7 @@ export async function action({ context, request }: ActionFunctionArgs) {
const asset = await getAsset({
id: assetId,
organizationId,
userOrganizations,
rockingrohit9639 marked this conversation as resolved.
Show resolved Hide resolved
include: { qrCodes: true },
});
/** WE get the first qrCode as the app only supports 1 code per asset for now */
Expand Down
1 change: 1 addition & 0 deletions app/utils/roles.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export async function requirePermission({
isSelfServiceOrBase:
role === OrganizationRoles.SELF_SERVICE ||
role === OrganizationRoles.BASE,
userOrganizations,
};
}

Expand Down
Loading