diff --git a/.claude/rules/cross-app-mirrors-need-provenance.md b/.claude/rules/cross-app-mirrors-need-provenance.md new file mode 100644 index 000000000..764021010 --- /dev/null +++ b/.claude/rules/cross-app-mirrors-need-provenance.md @@ -0,0 +1,39 @@ +--- +description: Hand-copied webapp logic in the companion (or any second app) must be marked as a mirror, point at its source, and prefer extraction to packages/* +globs: apps/companion/** +--- + +# Cross-App Mirrors Need Provenance + +The companion cannot import from `apps/webapp/app/**` (Remix-internal paths, +server-adjacent imports — Metro can't consume them). When it needs webapp +truth (permission matrices, enums, business constants), a hand-copied mirror +is sometimes the pragmatic choice — but every mirror MUST: + +1. **Declare itself a mirror, never a source** — file-level JSDoc stating the + canonical file it mirrors and that the server enforces the real rules. +2. **Mirror the EFFECTIVE behavior, not the raw data** — e.g. the server's + `hasPermission()` short-circuits ADMIN/OWNER to allow-all; a copy of the + raw `Role2PermissionMap` alone is wrong. Say so in a comment at the spot. +3. **Be UI-cosmetic only** — if a client copy ever gates anything the server + does not independently enforce, that is a bug, not a mirror. +4. **Carry an extraction path** — when the mirrored thing is behavioral + (matrix + resolution logic), the durable fix is a shared workspace package + (`packages/*`, like `@shelf/database`). Note the intended package in the + JSDoc so reviewers see the debt is tracked, not accidental. + +```ts +// ❌ Bad — silent copy; reviewer can't tell drift from design +const ROLE_PERMISSIONS = { OWNER: { qr: ["read", "update"] } }; + +// ✅ Good — provenance + effective-behavior note + extraction path +/** + * MIRROR of apps/webapp .../permission.data.ts — cosmetic UI gating only; + * server enforces via requireMobilePermission. Encodes the EFFECTIVE result + * (matrix + ADMIN/OWNER allow-all short-circuit). Extraction target: + * @shelf/permissions (see PR #2753 discussion). + */ +``` + +Existing mirrors: `apps/companion/lib/permissions.ts`. When you touch one, +diff it against its canonical source before shipping. diff --git a/apps/companion/app/(tabs)/assets/_layout.tsx b/apps/companion/app/(tabs)/assets/_layout.tsx index 728862082..6577d7a39 100644 --- a/apps/companion/app/(tabs)/assets/_layout.tsx +++ b/apps/companion/app/(tabs)/assets/_layout.tsx @@ -31,6 +31,9 @@ export default function AssetsLayout() { + {/* Asset picker for linking a scanned (claimed, unlinked) QR code — + reached from the scanner's Unclaimed Code / No Asset Linked cards. */} + {/* Kits live in this stack so the segmented Assets|Kits switcher swaps lists in place and the tab bar keeps Assets active. */} diff --git a/apps/companion/app/(tabs)/assets/link-qr.tsx b/apps/companion/app/(tabs)/assets/link-qr.tsx new file mode 100644 index 000000000..58b4e122a --- /dev/null +++ b/apps/companion/app/(tabs)/assets/link-qr.tsx @@ -0,0 +1,603 @@ +/** + * Link-QR asset picker screen. + * + * Reached from the scanner after an unclaimed QR code is claimed into the + * current workspace (or directly for a code the org already claimed but never + * linked). Presents a searchable, paginated list of the workspace's assets; + * selecting one links the scanned QR to it via `POST /api/mobile/qr/link-asset` + * and navigates to the asset's detail. + * + * Web parity: mirrors `qr+/_private+/$qrId_.link.asset.tsx`, whose list is + * `getPaginatedAndFilterableAssets` with NO exclusions — assets that already + * carry a QR code are listed too, because linking REPLACES an asset's current + * codes. The confirm dialog carries the same "current QR code will be + * unlinked" warning as the web dialog. + * + * Admin/owner only in practice (the server gates the link endpoint on + * `qr:update`); the scanner never routes other roles here, and the server + * would 403 them anyway. + */ + +import { useState, useEffect, useCallback, useRef } from "react"; +import { + View, + Text, + FlatList, + TextInput, + TouchableOpacity, + ActivityIndicator, + Alert, +} from "react-native"; +import * as Haptics from "expo-haptics"; +import { Image } from "expo-image"; +import { useRouter, useLocalSearchParams } from "expo-router"; +import { Ionicons } from "@expo/vector-icons"; +import { + api, + type AssetListItem, + type QrResolveFailureReason, +} from "@/lib/api"; +import { useOrg } from "@/lib/org-context"; +import { + fontSize, + spacing, + borderRadius, + formatStatus, + hitSlop, +} from "@/lib/constants"; +import { useTheme } from "@/lib/theme-context"; +import { createStyles } from "@/lib/create-styles"; +import { ErrorBoundary } from "@/components/error-boundary"; +import { AssetListSkeleton } from "@/components/skeleton-loader"; +import { playScanSound } from "@/lib/scan-sound"; +import { announce } from "@/lib/a11y"; + +const PAGE_SIZE = 20; +const keyExtractor = (item: AssetListItem) => item.id; + +/** + * Screen wrapper — hosts the picker inside the shared error boundary, matching + * the other asset-stack screens. + */ +export default function LinkQrScreen() { + return ( + + + + ); +} + +/** + * The picker itself: search + paginated asset list + confirm-and-link flow. + * Split from the default export so the error boundary wraps all hook usage. + */ +function LinkQrContent() { + const router = useRouter(); + const { qrId } = useLocalSearchParams<{ qrId?: string }>(); + const { currentOrg } = useOrg(); + const { colors, statusBadge } = useTheme(); + const styles = useStyles(); + + const [assets, setAssets] = useState([]); + const [isLoading, setIsLoading] = useState(true); + const [isLoadingMore, setIsLoadingMore] = useState(false); + const [error, setError] = useState(null); + const [searchInput, setSearchInput] = useState(""); + const [debouncedSearch, setDebouncedSearch] = useState(""); + /** Asset id currently being linked; disables rows while the POST runs. */ + const [linkingAssetId, setLinkingAssetId] = useState(null); + /** + * True from the moment a confirm dialog opens until it is cancelled or its + * link attempt fails. `linkingAssetId` only guards while the POST runs, so + * two quick taps on different rows would queue two confirm Alerts — + * confirming both would double-POST and surface a bogus "Couldn't Link" + * for a flow that succeeded. Stays set after a successful link: the screen + * is done and gets replaced by the asset detail. + */ + const confirmingRef = useRef(false); + const [totalPages, setTotalPages] = useState(0); + const nextPage = useRef(1); + + useEffect(() => { + const timer = setTimeout(() => setDebouncedSearch(searchInput.trim()), 400); + return () => clearTimeout(timer); + }, [searchInput]); + + /** + * Monotonic token identifying the newest fetch. Search/workspace changes + * rebuild `fetchAssets` (new deps) and the list effect starts a fresh + * request without aborting the old one — if the older response lands + * last it must not overwrite the state the UI now reflects. + */ + const fetchVersionRef = useRef(0); + + const fetchAssets = useCallback( + async (pageNum: number, reset: boolean) => { + if (!currentOrg) return; + const version = ++fetchVersionRef.current; + const { data, error: fetchErr } = await api.assets(currentOrg.id, { + search: debouncedSearch || undefined, + page: pageNum, + perPage: PAGE_SIZE, + }); + // A newer fetch started while this one was in flight (search typed, + // workspace switched) — discard this stale response entirely. + if (version !== fetchVersionRef.current) return; + if (!data && !fetchErr) return; // Request cancelled (navigation) — ignore + if (fetchErr || !data) { + // Only a first-page failure is fatal for the screen; a failed + // load-more must not discard the rows already on screen. + if (reset) setError(fetchErr || "Failed to load assets"); + return; + } + setError(null); + setTotalPages(data.totalPages); + nextPage.current = pageNum + 1; + if (reset) setAssets(data.assets); + else + setAssets((prev) => { + const existingIds = new Set(prev.map((a) => a.id)); + const newItems = data.assets.filter((a) => !existingIds.has(a.id)); + return [...prev, ...newItems]; + }); + }, + [currentOrg, debouncedSearch] + ); + + // Initial load + reload on search change. No focus-refetch here — the + // picker is a short-lived flow step, not a browsable list. + useEffect(() => { + if (!currentOrg) return; + setIsLoading(true); + nextPage.current = 1; + fetchAssets(1, true).finally(() => setIsLoading(false)); + }, [currentOrg, fetchAssets]); + + const onEndReached = async () => { + if (isLoadingMore || isLoading || nextPage.current > totalPages) return; + setIsLoadingMore(true); + await fetchAssets(nextPage.current, false); + setIsLoadingMore(false); + }; + + /** + * POST the link, recovering once from a claim that didn't stick: a 400 with + * `reason: "unclaimed"` means the QR lost (or never got) its org — re-run + * the claim and retry the link a single time before surfacing the error. + * + * @returns The final `{ error }` string, or `null` on success. + */ + const linkWithClaimRecovery = useCallback( + async (linkQrId: string, assetId: string): Promise => { + if (!currentOrg) return "No workspace selected."; + const first = await api.linkQrToAsset(currentOrg.id, linkQrId, assetId); + if (!first.error) return null; + // `satisfies` ties the literal to the wire contract type, so a typo + // (or a server-side rename) fails to compile. + if ( + first.errorDetails?.reason !== + ("unclaimed" satisfies QrResolveFailureReason) + ) { + return first.error; + } + + // Claim didn't stick — claim into the current org and retry once. + // The claim's error is deliberately NOT short-circuited: its 403 is + // generic ("Failed to claim qr code") and also covers a + // timed-out-but-landed claim or a same-org teammate winning the race — + // cases where the code IS now claimed by this org and the retry + // succeeds (mirrors the scanner's re-resolve recovery). The link + // endpoint's own guards (unclaimed / wrong-org / already-linked) + // produce the definitive, accurate error either way. + await api.claimQr(currentOrg.id, linkQrId); + const second = await api.linkQrToAsset(currentOrg.id, linkQrId, assetId); + return second.error; + }, + [currentOrg] + ); + + const handleSelect = useCallback( + (asset: AssetListItem) => { + if (!qrId || !currentOrg || linkingAssetId || confirmingRef.current) { + return; + } + confirmingRef.current = true; + + Alert.alert( + "Link QR Code", + `Link the scanned QR code to "${asset.title}"? If this asset already has a QR code, it will be unlinked and replaced.`, + [ + { + text: "Cancel", + style: "cancel", + onPress: () => { + confirmingRef.current = false; + }, + }, + { + text: "Link", + onPress: async () => { + setLinkingAssetId(asset.id); + const linkError = await linkWithClaimRecovery(qrId, asset.id); + setLinkingAssetId(null); + + if (linkError) { + // Failed link: release the guard so another asset can be + // picked. (On success it stays set — see its JSDoc.) + confirmingRef.current = false; + Haptics.notificationAsync( + Haptics.NotificationFeedbackType.Error + ); + Alert.alert("Couldn't Link", linkError); + return; + } + + Haptics.notificationAsync( + Haptics.NotificationFeedbackType.Success + ); + playScanSound(); + announce(`QR code linked to ${asset.title}`); + Alert.alert( + "Linked", + `The QR code is now linked to "${asset.title}".`, + [ + { + text: "View Asset", + // replace: the stale picker must not remain under the + // detail — back should return to the assets list. + onPress: () => router.replace(`/(tabs)/assets/${asset.id}`), + }, + ], + // why: Android alerts dismiss on tap-outside/back BY DEFAULT + // without firing any button — that would skip the navigation + // and strand the user on the done picker (confirmingRef stays + // set). Force iOS-style modality so "View Asset" is the only + // exit. + { cancelable: false } + ); + }, + }, + ], + // why: Android alerts dismiss on tap-outside/back BY DEFAULT without + // firing any button's onPress — confirmingRef would stay true forever + // and soft-lock the picker (rows silently ignore taps). Forcing + // iOS-style modality guarantees Cancel/Link is the only way out, so + // the ref is always released. + { cancelable: false } + ); + }, + [qrId, currentOrg, linkingAssetId, linkWithClaimRecovery, router] + ); + + const renderAsset = useCallback( + ({ item }: { item: AssetListItem }) => { + const badge = statusBadge[item.status] ?? { + bg: colors.backgroundTertiary, + text: colors.muted, + }; + const isLinkingThis = linkingAssetId === item.id; + + return ( + handleSelect(item)} + disabled={linkingAssetId !== null} + activeOpacity={0.6} + accessibilityLabel={`Link QR code to ${item.title}, ${formatStatus( + item.status + )}${item.category ? `, ${item.category.name}` : ""}`} + accessibilityRole="button" + > + {item.thumbnailImage || item.mainImage ? ( + + ) : ( + + + + )} + + + + {item.title} + + {item.category && ( + + {item.category.name} + + )} + + + {isLinkingThis ? ( + + ) : ( + + + + {formatStatus(item.status)} + + + )} + + ); + }, + [colors, statusBadge, styles, linkingAssetId, handleSelect] + ); + + // Opened without a QR id (should never happen from the scanner) — there is + // nothing to link, so say so instead of rendering a lying picker. + if (!qrId) { + return ( + + + No QR code to link + + Scan an unlinked QR code from the scanner to start the link flow. + + + ); + } + + return ( + + {/* Context banner — which code we're linking, mirroring the create + form's "QR Code Ready to Link" banner */} + + + + Link Scanned QR Code + + Select the asset to link. Its current QR code (if any) will be + replaced. + + + + + {/* Search bar */} + + + + {searchInput.trim() !== debouncedSearch && searchInput.length > 0 && ( + + )} + {searchInput.length > 0 && ( + setSearchInput("")} + hitSlop={hitSlop.md} + accessibilityLabel="Clear search" + accessibilityRole="button" + > + + + )} + + + {error ? ( + + + {error} + { + setIsLoading(true); + nextPage.current = 1; + fetchAssets(1, true).finally(() => setIsLoading(false)); + }} + accessibilityLabel="Retry loading assets" + accessibilityRole="button" + > + Retry + + + ) : isLoading && assets.length === 0 ? ( + + ) : assets.length === 0 ? ( + + + + {debouncedSearch ? "No results found" : "No assets yet"} + + + {debouncedSearch + ? "Try a different search term" + : "Create an asset first, then link this QR code to it."} + + + ) : ( + + ) : null + } + /> + )} + + ); +} + +const useStyles = createStyles((colors, shadows) => ({ + container: { + flex: 1, + backgroundColor: colors.backgroundSecondary, + }, + centered: { + flex: 1, + justifyContent: "center", + alignItems: "center", + gap: spacing.md, + backgroundColor: colors.backgroundSecondary, + }, + + // QR context banner (mirrors the create form's banner styling) + qrBanner: { + flexDirection: "row", + alignItems: "center", + backgroundColor: colors.white, + marginHorizontal: spacing.lg, + marginTop: spacing.md, + padding: spacing.md, + borderRadius: borderRadius.md, + borderWidth: 1, + borderColor: colors.border, + gap: spacing.md, + ...shadows.sm, + }, + qrBannerText: { + flex: 1, + }, + qrBannerTitle: { + fontSize: fontSize.base, + fontWeight: "600", + color: colors.foreground, + }, + qrBannerSubtitle: { + fontSize: fontSize.sm, + color: colors.muted, + marginTop: 2, + }, + + // Search + searchContainer: { + flexDirection: "row", + alignItems: "center", + backgroundColor: colors.white, + marginHorizontal: spacing.lg, + marginTop: spacing.md, + marginBottom: spacing.sm, + paddingHorizontal: 14, + paddingVertical: 10, + borderRadius: borderRadius.sm, + borderWidth: 1, + borderColor: colors.gray300, + gap: spacing.sm, + ...shadows.sm, + }, + searchInput: { + flex: 1, + fontSize: fontSize.lg, + color: colors.foreground, + }, + + // List + list: { + paddingHorizontal: spacing.lg, + paddingBottom: spacing.xxl, + }, + assetCard: { + flexDirection: "row", + alignItems: "center", + backgroundColor: colors.white, + borderRadius: borderRadius.md, + padding: spacing.md, + marginBottom: spacing.sm, + borderWidth: 1, + borderColor: colors.border, + gap: spacing.md, + }, + assetImage: { + width: 44, + height: 44, + borderRadius: borderRadius.sm, + }, + assetImagePlaceholder: { + backgroundColor: colors.backgroundTertiary, + justifyContent: "center", + alignItems: "center", + }, + assetInfo: { + flex: 1, + gap: 3, + }, + assetTitle: { + fontSize: fontSize.base, + fontWeight: "600", + color: colors.foreground, + }, + assetCategory: { + fontSize: fontSize.sm, + color: colors.muted, + }, + + // Status badge — pill shape like the assets list + statusBadge: { + flexDirection: "row", + alignItems: "center", + paddingHorizontal: 6, + paddingVertical: 2, + borderRadius: borderRadius.pill, + gap: 4, + }, + statusDot: { + width: 6, + height: 6, + borderRadius: 3, + }, + statusText: { + fontSize: fontSize.xs, + fontWeight: "500", + }, + + // Empty / error states + emptyTitle: { + fontSize: fontSize.lg, + fontWeight: "600", + color: colors.foreground, + textAlign: "center", + }, + emptyText: { + fontSize: fontSize.base, + color: colors.muted, + textAlign: "center", + paddingHorizontal: spacing.xxxl, + }, + retryButton: { + backgroundColor: colors.primary, + paddingHorizontal: spacing.xxl, + paddingVertical: 10, + borderRadius: borderRadius.md, + marginTop: spacing.sm, + }, + retryText: { + color: colors.primaryForeground, + fontWeight: "600", + fontSize: fontSize.base, + }, + footer: { + paddingVertical: spacing.lg, + }, +})); diff --git a/apps/companion/app/(tabs)/scanner.tsx b/apps/companion/app/(tabs)/scanner.tsx index a0093d0a1..52766ca5b 100644 --- a/apps/companion/app/(tabs)/scanner.tsx +++ b/apps/companion/app/(tabs)/scanner.tsx @@ -24,7 +24,11 @@ import { openShelfWebUrl, pushIntoTab } from "@/lib/navigation"; import { TeamMemberPicker } from "@/components/team-member-picker"; import { LocationPicker } from "@/components/location-picker"; import type { TeamMember, Location as LocationType } from "@/lib/api"; -import type { BookingAsset, ScannedKit } from "@/lib/api/types"; +import type { + BookingAsset, + QrResolveFailureReason, + ScannedKit, +} from "@/lib/api/types"; import { fontSize, spacing, borderRadius } from "@/lib/constants"; import { useTheme } from "@/lib/theme-context"; import { createStyles } from "@/lib/create-styles"; @@ -48,6 +52,7 @@ import { useScannerGestures } from "@/hooks/use-scanner-gestures"; import { useScanProcessing } from "@/hooks/use-scan-processing"; import { ScanFrame } from "@/components/scanner/scan-frame"; import { ScanResultCard } from "@/components/scanner/scan-result-card"; +import type { IoniconName } from "@/components/scanner/scan-result-card"; import { ActionPills, ModeDots } from "@/components/scanner/action-pills"; import { ActionPillsCoachmark } from "@/components/scanner/action-pills-coachmark"; import { BatchDrawer } from "@/components/scanner/batch-drawer"; @@ -217,6 +222,15 @@ function ScannerContent() { // web scanner, which pre-selects self and disables the custodian picker). const isSelfService = currentOrg?.roles?.includes("SELF_SERVICE") ?? false; + // Native claim / link-existing is gated on qr:update — effectively + // ADMIN/OWNER only (the server short-circuits those roles to allow-all; + // BASE/SELF_SERVICE only hold qr:read). Non-admins keep the web bridge. + const canManageQrCodes = userHasPermission({ + roles: currentOrg?.roles, + entity: "qr", + action: "update", + }); + // Action state const [action, setAction] = useState("view"); @@ -386,6 +400,12 @@ function ScannerContent() { // Scan processing state (extracted hook) -- created first so // its setIsPaused can be referenced by the inactivity callback. const setIsPausedRef = useRef<(v: boolean) => void>(() => {}); + /** + * Live mirror of the active org id, written by the org-change effect. + * Async continuations (claim → navigate) compare their originating org + * against this to detect a mid-flight workspace switch. + */ + const activeOrgIdRef = useRef(undefined); const onInactivityTimeout = useCallback( () => setIsPausedRef.current(true), [] @@ -430,6 +450,29 @@ function ScannerContent() { lastScanRef.current = ""; }, [dismissResultBase, lastScanRef]); + /** + * A workspace switch invalidates any on-screen result card: the scanner tab + * keeps its state while unfocused, and the card's action closures captured + * the org active at SCAN time — most dangerously the Unclaimed Code card, + * whose claim would land in the PREVIOUS workspace (permanently, there is + * no unclaim) while the card copy promises "this workspace". Clearing the + * card and the scan-dedup memory on org change means every action the user + * can see always targets the workspace they see. + */ + useEffect(() => { + setScanResult(null); + lastScanRef.current = ""; + // Also invalidates any in-flight claim continuation: claimQrAndProceed + // compares its originating org against this ref after its awaits and + // drops the follow-up navigation when they differ (the claim itself may + // have landed in the old org — irreversible — but we must not open the + // create/link flow under the newly active workspace). + activeOrgIdRef.current = currentOrg?.id; + // setScanResult / lastScanRef are stable identities (setState / ref), so + // this effectively runs only when the active org changes (the mount run + // is a no-op — the card starts null and the dedup memory empty). + }, [currentOrg?.id, setScanResult, lastScanRef]); + // Animation for scan line (shared hook) const scanLineAnim = useScanLineAnimation(isFocused, isPaused); @@ -482,6 +525,104 @@ function ScannerContent() { startCooldown(); }; + /** + * Claim an unclaimed QR into the current workspace, then continue into the + * chosen follow-up: create a new asset (the create form links the QR on + * submit) or pick an existing asset to link. Mirrors the web + * claim → new / claim → link flow; mobile always claims into the ACTIVE + * workspace — the server refuses any body-supplied org. + * + * A failed claim re-resolves the code once before erroring: a + * timed-out-but-landed claim (the request isn't retried — see + * `api.claimQr`) or a teammate claiming the same label seconds earlier both + * leave the code claimed by this org and unlinked, in which case the flow + * proceeds as if our claim had succeeded. Any other failure (someone else's + * org won the race, permission revoked) surfaces as an error card. + * + * @param claimQrId - The unclaimed QR id (from the resolve error payload). + * @param next - Which follow-up to open once the code is claimed. + */ + const claimQrAndProceed = useCallback( + async (claimQrId: string, next: "create" | "link") => { + // Same lock discipline as handleBarCodeScanned: the result card's two + // buttons are plain touchables, so a rapid double-tap (or one tap on + // each) would otherwise start two concurrent claim flows and stack two + // navigations — the second claim 403s, its recovery re-resolve + // "succeeds", and both invocations navigate. + if (!currentOrg || isProcessingRef.current) return; + + // Take the processing lock so further camera scans are ignored and the + // spinner shows while the claim is in flight. + isProcessingRef.current = true; + setIsProcessing(true); + setScanResult(null); + + // Pin the originating org: if the user switches workspaces while the + // claim is in flight, the continuation below must not run (the claim + // may have landed in this org, but navigating would open the + // create/link flow under the NEW org, where createAsset silently mints + // a different QR and strands the scanned label unlinked). + const originOrgId = currentOrg.id; + + const { error: claimError } = await api.claimQr(currentOrg.id, claimQrId); + + let claimed = !claimError; + if (claimError) { + // Recovery resolve — the server's claim 403 is deliberately generic + // ("already claimed or not allowed"), so the fresh resolve is the + // source of truth for whether the code ended up ours and unlinked. + // Non-recording resolve: this is an internal identify step, not a + // real field scan, so it must not write scan provenance (the normal + // claim-success path records nothing either). + const { data: recheck } = await api.getScannedItem( + claimQrId, + currentOrg.id + ); + claimed = Boolean( + recheck?.qr && + recheck.qr.organizationId === currentOrg.id && + !recheck.qr.assetId && + !recheck.qr.kitId + ); + } + + finalizeScan(); + + // Workspace switched while the claim was in flight — drop the + // continuation. The org-change effect has already cleared the card; + // showing a stale success/error for the previous workspace (or worse, + // navigating) would act on a workspace the user no longer sees. + if (activeOrgIdRef.current !== originOrgId) return; + + if (!claimed) { + Haptics.notificationAsync(Haptics.NotificationFeedbackType.Error); + setScanResult({ + type: "error", + title: "Couldn't Claim Code", + message: + claimError || + "This QR code could not be claimed into your workspace.", + }); + return; + } + + Haptics.notificationAsync(Haptics.NotificationFeedbackType.Success); + // Clear the result card + dedup memory so returning to the scanner is + // a clean slate (and re-scanning the same label resolves fresh). + dismissResult(); + pushIntoTab("/(tabs)/assets", { + pathname: + next === "create" ? "/(tabs)/assets/new" : "/(tabs)/assets/link-qr", + params: { qrId: claimQrId }, + }); + }, + // why: finalizeScan, isProcessingRef, setIsProcessing, and setScanResult + // are stable across renders (refs / setState identities) or plain + // functions over refs; intentionally excluded to match the scan handler + // eslint-disable-next-line react-hooks/exhaustive-deps + [currentOrg, dismissResult] + ); + // ── Scan Handler ──────────────────────────────────── const handleBarCodeScanned = useCallback( @@ -556,20 +697,57 @@ function ScannerContent() { // orgId is only consumed by the server's SAM branch; on the QR path // the org is derived from the QR record and this is ignored. - const { data: qrData, error } = await api.qr( - qrLookupId, - currentOrg?.id - ); + const { + data: qrData, + error, + errorDetails, + } = await api.qr(qrLookupId, currentOrg?.id); if (error || !qrData) { flashFrame("error"); Haptics.notificationAsync(Haptics.NotificationFeedbackType.Error); - // Detect unclaimed QR codes — offer browser link instead of generic error - const isUnclaimed = - error === "This QR code is not linked to any organization"; - - if (isUnclaimed) { + // Unclaimed QR codes carry the structured `reason` discriminator + // (never branch on the message text — it is not the contract). + // Plain not-found / wrong-org failures carry no reason and MUST + // keep the generic error below: claiming is not an option there. + // `satisfies` ties the literal to the wire contract type, so a + // typo (or a server-side rename) fails to compile. + const unclaimedQrId = + errorDetails?.reason === + ("unclaimed" satisfies QrResolveFailureReason) + ? errorDetails.qrId ?? qrId + : null; + + if (unclaimedQrId && canManageQrCodes) { + // Admin/owner: native takeover of the web claim flow. Both + // actions claim the code into the CURRENT workspace first, + // then continue in-app (create form links on submit; the + // picker links an existing asset). + setScanResult({ + type: "not_found", + title: "Unclaimed Code", + message: + "This QR code isn't claimed yet. Claim it into this workspace by creating a new asset or linking an existing one.", + action: { + label: "Create New Asset", + icon: "add-circle-outline", + onPress: () => { + void claimQrAndProceed(unclaimedQrId, "create"); + }, + }, + secondaryAction: { + label: "Link Existing Asset", + icon: "link-outline", + onPress: () => { + void claimQrAndProceed(unclaimedQrId, "link"); + }, + }, + }); + } else if (unclaimedQrId) { + // Non-admin/owner: claiming is not allowed for their role — + // keep the web bridge. Loop-safe in-app browser via + // openShelfWebUrl, never Linking.openURL (claimed App Link). setScanResult({ type: "not_found", title: "No Asset Linked", @@ -579,7 +757,9 @@ function ScannerContent() { label: "Link in Browser", icon: "open-outline", onPress: () => { - void openShelfWebUrl(`https://app.shelf.nu/qr/${qrId}`); + void openShelfWebUrl( + `https://app.shelf.nu/qr/${unclaimedQrId}` + ); dismissResult(); }, }, @@ -906,17 +1086,18 @@ function ScannerContent() { flashFrame("error"); Haptics.notificationAsync(Haptics.NotificationFeedbackType.Error); - // For Shelf QR codes, offer a path to link the QR to a new asset: - // - If claimed to current org → navigate to in-app asset creation - // - If unclaimed → bridge to web for the claim+link flow + // For Shelf QR codes, offer a path forward for the unlinked code: + // - If claimed to current org → in-app asset creation and/or + // linking an existing asset (admins) — no claim step needed + // - Otherwise → bridge to web // Barcodes don't have an external link flow, so no action for those - let unlinkedQrAction: - | { - label: string; - icon: string; - onPress: () => void; - } - | undefined; + type UnlinkedQrAction = { + label: string; + icon: IoniconName; + onPress: () => void; + }; + let unlinkedQrAction: UnlinkedQrAction | undefined; + let unlinkedQrSecondaryAction: UnlinkedQrAction | undefined; const canCreateAsset = userHasPermission({ roles: currentOrg?.roles, @@ -931,6 +1112,13 @@ function ScannerContent() { // "this QR will be linked" promise. Bridge those to the web kit view. const isKitLinked = Boolean(kitId); + // Claimed to the current org, truly unlinked, and the caller can + // act on it in-app (create is admin/owner + qr:update is too, but + // gate each action on its own permission for correctness). + const canActOnUnlinked = + codeOrgId === currentOrg?.id && + (canCreateAsset || canManageQrCodes); + if (qrId && isKitLinked) { // QR belongs to a kit — open the web app to view the kit unlinkedQrAction = { @@ -941,21 +1129,40 @@ function ScannerContent() { dismissResult(); }, }; - } else if (qrId && codeOrgId === currentOrg?.id && canCreateAsset) { - // QR is claimed to current org, truly unlinked, and user can create — create in-app - unlinkedQrAction = { - label: "Create Asset", - icon: "add-circle-outline", - onPress: () => { - pushIntoTab("/(tabs)/assets", { - pathname: "/(tabs)/assets/new", - params: { qrId }, - }); - dismissResult(); - }, - }; + } else if (qrId && canActOnUnlinked) { + // Already claimed by this org — the claim step is skipped; the + // create form / link picker attach this exact QR directly. + const createAction: UnlinkedQrAction | undefined = canCreateAsset + ? { + label: "Create Asset", + icon: "add-circle-outline", + onPress: () => { + pushIntoTab("/(tabs)/assets", { + pathname: "/(tabs)/assets/new", + params: { qrId }, + }); + dismissResult(); + }, + } + : undefined; + const linkAction: UnlinkedQrAction | undefined = canManageQrCodes + ? { + label: "Link Existing Asset", + icon: "link-outline", + onPress: () => { + pushIntoTab("/(tabs)/assets", { + pathname: "/(tabs)/assets/link-qr", + params: { qrId }, + }); + dismissResult(); + }, + } + : undefined; + unlinkedQrAction = createAction ?? linkAction; + unlinkedQrSecondaryAction = createAction ? linkAction : undefined; } else if (qrId) { - // QR is unclaimed — bridge to web for claim flow + // Unclaimed (for roles without the native claim flow) or claimed + // by this org while the caller can't act — bridge to web unlinkedQrAction = { label: "Link in Browser", icon: "open-outline", @@ -972,11 +1179,12 @@ function ScannerContent() { message: qrId ? isKitLinked ? "This QR code is linked to a kit, not an asset. Open the web app to view the kit." - : codeOrgId === currentOrg?.id && canCreateAsset - ? "This QR code is not linked to any asset. Create one now." + : canActOnUnlinked + ? "This QR code is not linked to any asset. Create a new asset or link an existing one." : "This QR code is not linked to any asset. Open the web app to link it." : "This code exists but is not linked to any asset.", action: unlinkedQrAction, + secondaryAction: unlinkedQrSecondaryAction, }); finalizeScan(); return; @@ -1199,6 +1407,8 @@ function ScannerContent() { router, action, currentOrg, + canManageQrCodes, + claimQrAndProceed, scannedItems, isBookingMode, bookingCheckinItems, @@ -2067,7 +2277,9 @@ function ScannerContent() { (apps/webapp/app/components/scanner/code-scanner.tsx). NOTE: testIDs/state keep the `dev-scan`/`devScan` prefix from this control's origin so the existing e2e flows stay stable. */} - {manualEntryBottom !== null && ( + {/* Hidden while a result card is shown — the card's actions must never + be occluded (the unclaimed card is tall enough to reach this pill). */} + {manualEntryBottom !== null && !scanResult && ( ["name"]; + /** * Optional action button displayed on the scan result card. * Used to provide a path forward when a QR code is unlinked. */ type ScanResultAction = { label: string; - icon?: string; + icon?: IoniconName; onPress: () => void; }; @@ -19,6 +26,12 @@ export type ScanResult = { message: string; /** Optional action button (e.g., "Link in Browser" for unlinked QR codes) */ action?: ScanResultAction; + /** + * Optional second action, rendered below the primary one (e.g. the + * unclaimed-QR card offers "Create New Asset" and "Link Existing Asset"). + * Only meaningful when `action` is also set. + */ + secondaryAction?: ScanResultAction; }; type ScanResultCardProps = { @@ -26,7 +39,7 @@ type ScanResultCardProps = { onDismiss: () => void; }; -const ICON_MAP: Record = { +const ICON_MAP: Record = { success: "checkmark-circle", error: "alert-circle", not_found: "help-circle", @@ -56,7 +69,7 @@ export function ScanResultCard({ result, onDismiss }: ScanResultCardProps) { accessibilityRole="button" accessibilityLabel={`${result.title}. ${result.message}. Tap to dismiss.`} > - + {result.title} {result.message} @@ -76,7 +89,7 @@ export function ScanResultCard({ result, onDismiss }: ScanResultCardProps) { > {result.action.icon && ( {result.action.label} )} + + {result.secondaryAction && ( + + {result.secondaryAction.icon && ( + + )} + {result.secondaryAction.label} + + )} ); } diff --git a/apps/companion/ios/Shelf.xcodeproj/project.pbxproj b/apps/companion/ios/Shelf.xcodeproj/project.pbxproj index 40ff7aa75..ed21a72f3 100644 --- a/apps/companion/ios/Shelf.xcodeproj/project.pbxproj +++ b/apps/companion/ios/Shelf.xcodeproj/project.pbxproj @@ -348,7 +348,7 @@ CLANG_ENABLE_MODULES = YES; CODE_SIGN_ENTITLEMENTS = Shelf/Shelf.entitlements; CURRENT_PROJECT_VERSION = 1; - DEVELOPMENT_TEAM = 3V6BGGX7JS; + DEVELOPMENT_TEAM = 27Q4MHFB8K; ENABLE_BITCODE = NO; GCC_PREPROCESSOR_DEFINITIONS = ( "$(inherited)", diff --git a/apps/companion/lib/api/assets.ts b/apps/companion/lib/api/assets.ts index f50924c3e..6c3936e64 100644 --- a/apps/companion/lib/api/assets.ts +++ b/apps/companion/lib/api/assets.ts @@ -5,6 +5,7 @@ import type { AssetDetail, AssetNote, QrResponse, + QrMutationResponse, BarcodeResponse, TeamMembersResponse, LocationsResponse, @@ -91,6 +92,55 @@ export const assetsApi = { }` ), + /** + * Claim an unclaimed QR code into the caller's CURRENT organization + * (`POST /api/mobile/qr/claim`) — the mobile twin of the web claim route. + * The server claims into the org resolved from `orgId` only; mobile + * deliberately offers no org picker (web does). + * + * Admin/owner only: the server gates on `qr:update`, which no role below + * ADMIN holds. A 403 also covers the already-claimed race (the server wraps + * it with a generic "Failed to claim qr code"), so callers should re-resolve + * the code on failure instead of trusting the message text. + * + * @param orgId - Caller's current workspace id (the claim target). + * @param qrId - The unclaimed QR id (echoed by the resolve error payload). + * @returns `{ qr }` summary on success (assetId/kitId null) or `{ error }`. + */ + claimQr: (orgId: string, qrId: string) => + apiFetch(`/api/mobile/qr/claim?orgId=${orgId}`, { + method: "POST", + body: JSON.stringify({ qrId }), + // why: not retried — a timed-out-but-landed claim would 403 on the + // retry ("already claimed"), turning a success into a scary error. + // The caller's re-resolve fallback recovers the landed case instead. + retry: false, + }), + + /** + * Link a claimed-but-unlinked QR code to an existing asset + * (`POST /api/mobile/qr/link-asset`). Replaces the asset's current QR codes + * (web parity — the web confirm dialog carries the same warning), so the + * picker's confirm step must warn about that. A 400 whose + * `errorDetails.reason === "unclaimed"` means the claim didn't stick — + * re-run {@link claimQr} and retry. + * + * Admin/owner only (server gates on `qr:update`, same as the web link flow). + * + * @param orgId - Caller's current workspace id (must own the QR). + * @param qrId - The claimed, unlinked QR id. + * @param assetId - The asset (in the caller's org) to link the code to. + * @returns `{ qr }` summary on success (assetId set) or `{ error }`. + */ + linkQrToAsset: (orgId: string, qrId: string, assetId: string) => + apiFetch(`/api/mobile/qr/link-asset?orgId=${orgId}`, { + method: "POST", + body: JSON.stringify({ qrId, assetId }), + // why: not retried — a timed-out-but-landed link would 403 on the + // retry ("already linked"), reporting failure for a landed success. + retry: false, + }), + /** Resolve a barcode (additional code) to an asset */ barcode: (value: string, orgId: string) => apiFetch( diff --git a/apps/companion/lib/api/client.ts b/apps/companion/lib/api/client.ts index bacff3005..c0bdcbc41 100644 --- a/apps/companion/lib/api/client.ts +++ b/apps/companion/lib/api/client.ts @@ -72,10 +72,55 @@ export async function getAccessToken(): Promise { */ export type ApiFetchOptions = RequestInit & { retry?: boolean }; +/** + * Structured error payload from the mobile API's `{ error: { … } }` envelope. + * `message` mirrors the flat `error` string consumers already display. + * `reason` is an additive machine-readable discriminator some endpoints emit + * (today: `"unclaimed"` on the QR resolve / link routes), with `qrId` echoing + * the scanned code id whenever `reason` is present. Branch on `reason`, never + * on `message` text — messages are human copy and can change; the reason + * field is the wire contract. + */ +export type ApiErrorDetails = { + message: string; + reason?: string; + qrId?: string; +}; + +/** + * Extracts the structured error payload from a parsed non-OK response body. + * + * @param json - The parsed response body (unknown: may be an HTML error page + * coerced to null, an empty body, or a proxy's own JSON). + * @returns The typed error payload, or `null` when the body doesn't match the + * mobile API's `{ error: { message } }` envelope. + */ +function extractErrorDetails(json: unknown): ApiErrorDetails | null { + if (typeof json !== "object" || json === null) return null; + const err = (json as { error?: unknown }).error; + if (typeof err !== "object" || err === null) return null; + const { message, reason, qrId } = err as { + message?: unknown; + reason?: unknown; + qrId?: unknown; + }; + if (typeof message !== "string") return null; + return { + message, + ...(typeof reason === "string" ? { reason } : {}), + ...(typeof qrId === "string" ? { qrId } : {}), + }; +} + /** * Makes an authenticated API call to the Shelf webapp. * Automatically attaches the current Supabase session JWT. * - Returns structured { data, error } -- never throws. + * - On HTTP errors carrying the mobile API's `{ error: { … } }` envelope, + * `errorDetails` additionally exposes the structured payload (message + + * optional machine-readable `reason` / `qrId`) so callers can branch on + * contract fields instead of message strings. Absent for transport-level + * failures (timeout, network, non-JSON bodies). * - Detects 401/session-expired and notifies global auth listeners. * - Enforces a request timeout to avoid hanging on slow networks. */ @@ -83,7 +128,11 @@ export async function apiFetch( path: string, options: ApiFetchOptions = {}, _retryCount = 0 -): Promise<{ data: T | null; error: string | null }> { +): Promise<{ + data: T | null; + error: string | null; + errorDetails?: ApiErrorDetails | null; +}> { // Declared outside try so catch block can read it let timedOut = false; @@ -148,6 +197,7 @@ export async function apiFetch( error: "Session expired. Please sign in again.", }; } + const errorDetails = extractErrorDetails(json); // 403 = forbidden → user lacks permission, but session is valid if (response.status === 403) { return { @@ -155,11 +205,13 @@ export async function apiFetch( error: json?.error?.message || "You don't have permission to perform this action.", + errorDetails, }; } return { data: null, error: json?.error?.message || `Request failed (${response.status})`, + errorDetails, }; } diff --git a/apps/companion/lib/api/types.ts b/apps/companion/lib/api/types.ts index 8e34e6db2..fb2767316 100644 --- a/apps/companion/lib/api/types.ts +++ b/apps/companion/lib/api/types.ts @@ -257,6 +257,36 @@ export type QrResponse = { }; }; +/** + * Machine-readable failure discriminator carried by the QR resolve / link + * error payloads (`{ error: { message, reason?, qrId? } }`), surfaced client + * side via `apiFetch`'s `errorDetails`. Mirrors the server's + * `ResolveMobileCodeFailureReason`. + * + * `"unclaimed"` — the QR row exists, has no organization yet (a printed + * Shelf code nobody claimed) and is not linked to an asset or kit. The + * scanner offers the native claim → create / link flow for it. Absence of a + * reason (plain not-found 404, wrong-org 403, or an orgless-but-linked + * corrupted row the web claim flow refuses) MUST keep the existing dead-end / + * web-bridge behaviour — never offer claim for those. + */ +export type QrResolveFailureReason = "unclaimed"; + +/** + * Response of `POST /api/mobile/qr/claim` and `POST /api/mobile/qr/link-asset`: + * the mutated QR summary. After a claim, `assetId`/`kitId` are both `null` + * (freshly claimed codes are unlinked); after a link, `assetId` is the linked + * asset's id (navigate straight to its detail) and `kitId` stays `null`. + */ +export type QrMutationResponse = { + qr: { + id: string; + organizationId: string; + assetId: string | null; + kitId: string | null; + }; +}; + export type BarcodeResponse = { barcode: { id: string; diff --git a/apps/companion/lib/permissions.ts b/apps/companion/lib/permissions.ts index d7f2b1232..6f8537113 100644 --- a/apps/companion/lib/permissions.ts +++ b/apps/companion/lib/permissions.ts @@ -7,7 +7,7 @@ * via requireMobilePermission on every API call. */ -type PermissionEntity = "asset" | "booking" | "audit" | "kit"; +type PermissionEntity = "asset" | "booking" | "audit" | "kit" | "qr"; type PermissionAction = | "read" | "create" @@ -30,24 +30,33 @@ const ROLE_PERMISSIONS: Record< booking: ["read", "create", "update", "delete", "checkout", "checkin"], audit: ["read", "create", "update", "delete"], kit: ["read", "create", "update", "delete", "custody"], + // qr:update gates the native claim / link-existing flows. The server + // short-circuits ADMIN/OWNER to allow-all, so listing it here mirrors + // the effective server behaviour rather than the literal map. + qr: ["read", "update"], }, ADMIN: { asset: ["read", "create", "update", "delete", "custody"], booking: ["read", "create", "update", "delete", "checkout", "checkin"], audit: ["read", "create", "update", "delete"], kit: ["read", "create", "update", "delete", "custody"], + qr: ["read", "update"], }, SELF_SERVICE: { asset: ["read", "custody"], booking: ["read", "create", "update", "checkout", "checkin"], audit: ["read", "update"], kit: ["read", "custody"], + // Web's Role2PermissionMap grants BASE / SELF_SERVICE qr:read only — + // they never see the native claim / link actions. + qr: ["read"], }, BASE: { asset: ["read"], booking: ["read"], audit: ["read"], kit: ["read"], + qr: ["read"], }, }; diff --git a/apps/webapp/app/modules/api/mobile-code-resolve.server.test.ts b/apps/webapp/app/modules/api/mobile-code-resolve.server.test.ts new file mode 100644 index 000000000..bcfbccc1d --- /dev/null +++ b/apps/webapp/app/modules/api/mobile-code-resolve.server.test.ts @@ -0,0 +1,173 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { db } from "~/database/db.server"; +import { resolveMobileScannedCode } from "~/modules/api/mobile-code-resolve.server"; + +// why: importing the module transitively loads `~/database/db.server`, which +// instantiates a real Prisma client and tries to connect at module load — +// under `pnpm test:run` (no DB available) that fails the suite. The resolver +// only touches `qr.findUnique`, `userOrganization.findUnique`, +// `asset.findFirst` and `kit.findFirst`, so we stub exactly those. +vi.mock("~/database/db.server", () => ({ + db: { + qr: { findUnique: vi.fn() }, + userOrganization: { findUnique: vi.fn() }, + asset: { findFirst: vi.fn() }, + kit: { findFirst: vi.fn() }, + }, +})); + +// why: `mobile-auth.server` transitively loads the Supabase admin client +// (needs env + network wiring we don't have in unit tests). The resolver only +// needs the select constants and the shape helpers from it; pass-through +// stubs keep the branching under test observable without the heavy imports. +vi.mock("~/modules/api/mobile-auth.server", () => ({ + requireOrganizationAccess: vi.fn(), + MOBILE_ASSET_SELECT: {}, + MOBILE_KIT_SELECT: {}, + shapeMobileAssetResponse: (asset: unknown) => asset, + shapeMobileKitResponse: (kit: unknown) => kit, +})); + +/** + * Tests for `resolveMobileScannedCode`'s failure discrimination — most + * importantly the additive `reason: "unclaimed"` discriminator that lets the + * companion take over the native claim flow. The pre-existing contract + * (status codes + messages) must stay byte-identical so the audit scanner + * and older app builds keep behaving unchanged. + * + * @see {@link file://./mobile-code-resolve.server.ts} + */ + +/** Builds loader-shaped args for the resolver with a plain QR id param. */ +function resolveArgs(qrId = "abcdefghij") { + return { + request: new Request(`http://localhost/api/mobile/qr/${qrId}`), + params: { qrId }, + user: { id: "user-1" }, + }; +} + +const qrFindUnique = vi.mocked(db.qr.findUnique); +const membershipFindUnique = vi.mocked(db.userOrganization.findUnique); + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("resolveMobileScannedCode failure discrimination", () => { + it("returns a plain 404 (no reason) when the QR does not exist", async () => { + // why: unknown code → nothing actionable; the companion must NOT offer + // the claim flow for codes that aren't Shelf codes at all. + qrFindUnique.mockResolvedValue(null); + + const result = await resolveMobileScannedCode(resolveArgs()); + + expect(result).toEqual({ + ok: false, + status: 404, + message: "QR code not found", + }); + }); + + it("returns 404 with reason 'unclaimed' + the qr id for an unclaimed code", async () => { + // why: this is THE branch the native takeover keys off — QR row exists + // but organizationId is null (printed, never claimed). + // why: cast — the resolver selects a narrow shape, not the full Qr row + // (same pattern as the note service tests' Prisma mocks). + qrFindUnique.mockResolvedValue({ + id: "qr-unclaimed", + assetId: null, + kitId: null, + organizationId: null, + } as any); + + const result = await resolveMobileScannedCode(resolveArgs("qr-unclaimed")); + + expect(result).toEqual({ + ok: false, + status: 404, + // Message must stay byte-identical: older companion builds string-match + // it and the audit scanner surfaces it verbatim. + message: "This QR code is not linked to any organization", + reason: "unclaimed", + qrId: "qr-unclaimed", + }); + }); + + it("returns a plain 404 (no reason) for an orgless code that is linked to an asset", async () => { + // why: orgless-but-linked is the corrupted state createAsset's loose + // QR-connect branch can produce. The web claim loader refuses these + // (`assetId: null, kitId: null` guard), so the resolver must NOT + // advertise the claim flow — same status/message, just no reason. + // why: cast — narrow selected shape, not the full Qr row. + qrFindUnique.mockResolvedValue({ + id: "qr-orgless-linked", + assetId: "asset-1", + kitId: null, + organizationId: null, + } as any); + + const result = await resolveMobileScannedCode( + resolveArgs("qr-orgless-linked") + ); + + expect(result).toEqual({ + ok: false, + status: 404, + message: "This QR code is not linked to any organization", + }); + // No claim offer for a code the web claim flow would refuse. + expect(result.ok === false && result.reason).toBeFalsy(); + }); + + it("returns a plain 403 (no reason) when the code belongs to another org", async () => { + // why: cast — narrow selected shape, not the full Qr row. + qrFindUnique.mockResolvedValue({ + id: "qr-foreign", + assetId: null, + kitId: null, + organizationId: "org-other", + } as any); + // Caller is not a member of org-other. + membershipFindUnique.mockResolvedValue(null); + + const result = await resolveMobileScannedCode(resolveArgs("qr-foreign")); + + expect(result).toEqual({ + ok: false, + status: 403, + message: "This QR code belongs to a different organization", + }); + // No claim offer for someone else's code. + expect(result.ok === false && result.reason).toBeFalsy(); + }); + + it("resolves ok with null asset/kit for a claimed-but-unlinked code in the caller's org", async () => { + // why: this drives the companion's "claimed but unlinked → offer + // create/link (skipping the claim call)" branch. + // why: casts — narrow selected shapes, not full Prisma rows. + qrFindUnique.mockResolvedValue({ + id: "qr-claimed", + assetId: null, + kitId: null, + organizationId: "org-1", + } as any); + membershipFindUnique.mockResolvedValue({ id: "membership-1" } as any); + + const result = await resolveMobileScannedCode(resolveArgs("qr-claimed")); + + expect(result).toEqual({ + ok: true, + recordableQrId: "qr-claimed", + qr: { + id: "qr-claimed", + assetId: null, + kitId: null, + organizationId: "org-1", + asset: null, + kit: null, + }, + }); + }); +}); diff --git a/apps/webapp/app/modules/api/mobile-code-resolve.server.ts b/apps/webapp/app/modules/api/mobile-code-resolve.server.ts index 7a2bb7e50..229403562 100644 --- a/apps/webapp/app/modules/api/mobile-code-resolve.server.ts +++ b/apps/webapp/app/modules/api/mobile-code-resolve.server.ts @@ -43,15 +43,43 @@ type ResolvedCode = { kit: unknown; }; +/** + * Structured discriminator for actionable not-ok resolves. + * + * `"unclaimed"` — the QR row exists, has no `organizationId` (a printed + * Shelf code nobody claimed yet) AND is not linked to an asset or kit. The + * companion uses this to offer the native claim → link flow instead of + * string-matching the 404 message. Orgless-but-linked rows (a corrupted + * state `createAsset`'s loose QR-connect branch can produce) deliberately + * carry NO reason: the web claim loader refuses them (`assetId: null, + * kitId: null` guard), so the companion must not offer claim either. + * Additive: the status/message of the not-ok result are unchanged, so + * existing consumers (audit scanner included) keep behaving exactly as + * before. + */ +type ResolveMobileCodeFailureReason = "unclaimed"; + /** * Discriminated result of {@link resolveMobileScannedCode}. * * On success, `recordableQrId` is the QR id a recording caller may attribute a * scan to, or `null` for a SAM resolve (no backing QR record, so nothing to * record, matching the web). + * + * On failure, `reason`/`qrId` are only present for actionable cases (see + * {@link ResolveMobileCodeFailureReason}); plain not-found / wrong-org + * failures carry the message alone. */ export type ResolveMobileCodeResult = - | { ok: false; status: number; message: string } + | { + ok: false; + status: number; + message: string; + /** Structured failure discriminator — present only when actionable. */ + reason?: ResolveMobileCodeFailureReason; + /** The scanned QR id, echoed back when `reason` is set. */ + qrId?: string; + } | { ok: true; qr: ResolvedCode; recordableQrId: string | null }; /** @@ -125,10 +153,20 @@ export async function resolveMobileScannedCode({ // Require organization membership — deny unowned QR codes. if (!qr.organizationId) { + // Only a truly unclaimed AND unlinked code is claimable — the web claim + // loader enforces `assetId: null, kitId: null`, so an orgless-but-linked + // row (corrupted state) must not advertise the claim flow. It falls back + // to the plain 404 (companion dead-ends, matching web's refusal). + const claimable = !qr.assetId && !qr.kitId; return { ok: false, status: 404, message: "This QR code is not linked to any organization", + // Structured discriminator so the companion can take over the native + // claim → link flow (mirroring web's `/qr/:qrId/claim`) without + // string-matching the message. Status + message stay unchanged so the + // audit scanner and older app builds see the exact same 404. + ...(claimable ? { reason: "unclaimed" as const, qrId: qr.id } : {}), }; } diff --git a/apps/webapp/app/modules/qr/service.server.test.ts b/apps/webapp/app/modules/qr/service.server.test.ts index 9022f0e33..48c8c7729 100644 --- a/apps/webapp/app/modules/qr/service.server.test.ts +++ b/apps/webapp/app/modules/qr/service.server.test.ts @@ -1,16 +1,19 @@ import { describe, expect, it, vitest, beforeEach } from "vitest"; import { db } from "~/database/db.server"; import { ShelfError } from "~/utils/error"; -import { parseQrCodesFromImportData } from "./service.server"; +import { claimQrCode, parseQrCodesFromImportData } from "./service.server"; // why: parseQrCodesFromImportData reads QR rows from the database to detect -// invalid imports; mock the client so the tests exercise the validation -// branches without a real DB. +// invalid imports, and claimQrCode reads (findUniqueOrThrow via getQr) then +// atomically updates a single row; mock the client so the tests exercise the +// validation/claim branches without a real DB. vitest.mock("~/database/db.server", () => ({ db: { qr: { findMany: vitest.fn().mockResolvedValue([]), updateMany: vitest.fn().mockResolvedValue({ count: 0 }), + findUniqueOrThrow: vitest.fn(), + update: vitest.fn(), }, }, })); @@ -92,3 +95,97 @@ describe("parseQrCodesFromImportData — import validation errors", () => { expect(err.shouldBeCaptured).toBe(false); }); }); + +describe("claimQrCode", () => { + const claimArgs = { id: "qr-1", organizationId, userId }; + + /** An unclaimed, unlinked QR row as returned by the pre-check read. */ + const unclaimedQr = { + id: "qr-1", + organizationId: null, + userId: null, + assetId: null, + kitId: null, + }; + + beforeEach(() => { + vitest.clearAllMocks(); + }); + + /** + * Runs claimQrCode and returns the thrown ShelfError, failing the test if + * it unexpectedly resolves. + */ + async function captureClaimThrow() { + try { + await claimQrCode(claimArgs); + throw new Error("expected claimQrCode to throw"); + } catch (err) { + expect(err).toBeInstanceOf(ShelfError); + return err as ShelfError; + } + } + + it("rejects an already-claimed code with a 403 without writing", async () => { + // why: the pre-check read must see a row that already belongs to an org + // to exercise the early "already claimed" branch. + (db.qr.findUniqueOrThrow as ReturnType).mockResolvedValue( + { ...unclaimedQr, organizationId: "other-org" } + ); + + const err = await captureClaimThrow(); + + expect(err.status).toBe(403); + expect(err.message).toBe("Failed to claim qr code"); + expect(db.qr.update).not.toHaveBeenCalled(); + }); + + it("maps a lost claim race (P2025 on the atomic update) to a 403, not a 404", async () => { + // why: the pre-check must pass (unclaimed row) so the test drives the + // window AFTER the guard — a concurrent claim/link wins the atomic + // update and Prisma raises P2025 for the loser. + (db.qr.findUniqueOrThrow as ReturnType).mockResolvedValue( + unclaimedQr + ); + // why: simulate the losing side of the race; Prisma signals "no row + // matched the constrained WHERE" as a P2025 known request error. + (db.qr.update as ReturnType).mockRejectedValue({ + code: "P2025", + }); + + const err = await captureClaimThrow(); + + // The lost race must surface as "already claimed" (403), never as a + // not-found (404) — makeShelfError would collapse a propagated P2025 + // to a 404 if the mapping branch were removed. + expect(err.status).toBe(403); + expect(err.message).toBe("Failed to claim qr code"); + }); + + it("claims atomically: the update WHERE requires the unclaimed AND unlinked state", async () => { + (db.qr.findUniqueOrThrow as ReturnType).mockResolvedValue( + unclaimedQr + ); + const claimedQr = { ...unclaimedQr, organizationId, userId }; + // why: resolve the write so the success branch returns the claimed row. + (db.qr.update as ReturnType).mockResolvedValue(claimedQr); + + const result = await claimQrCode(claimArgs); + + expect(result).toEqual(claimedQr); + // Guard the atomicity constraint itself: dropping any of these WHERE + // conditions would let a lost race silently re-assign the code's org + // (or claim a code createAsset just linked to another org's asset). + expect(db.qr.update).toHaveBeenCalledWith( + expect.objectContaining({ + where: { + id: "qr-1", + organizationId: null, + assetId: null, + kitId: null, + }, + data: { organizationId, userId }, + }) + ); + }); +}); diff --git a/apps/webapp/app/modules/qr/service.server.ts b/apps/webapp/app/modules/qr/service.server.ts index 687471eec..0e54cb2fb 100644 --- a/apps/webapp/app/modules/qr/service.server.ts +++ b/apps/webapp/app/modules/qr/service.server.ts @@ -453,16 +453,51 @@ export async function claimQrCode({ }); } - return await db.qr.update({ - // eslint-disable-next-line local-rules/require-org-scope-on-id-queries -- idor-safe: claiming only proceeds for unclaimed codes (verified at line 442: throws if qr.organizationId already set), so the code has no organizationId to scope by — this assigns its first org - where: { - id, - }, - data: { - organizationId, - userId, - }, - }); + try { + return await db.qr.update({ + // eslint-disable-next-line local-rules/require-org-scope-on-id-queries -- idor-safe: claiming only proceeds for unclaimed codes (verified above: throws if qr.organizationId already set), so the code has no organizationId to scope by — this assigns its first org + where: { + id, + // why: the check above is not atomic with this write — two + // concurrent claims could both pass it and the last write would + // silently re-assign the code's org (cross-tenant: org B's claim + // overwriting a code org A already linked to an asset). + // Constraining the WHERE to the still-unclaimed state makes the + // first claim win; the loser gets a P2025, mapped below to the + // same 403 the pre-check produces. + organizationId: null, + // why: createAsset's loose QR-connect branch can link an orgless + // code to an asset between the callers' availability guards and + // this write. Also requiring the still-unlinked state here keeps + // claim atomic with "not linked to anything" — a loser on any + // condition gets the same P2025 → 403 mapping, instead of ending + // up with a code owned by org B but attached to org A's asset. + assetId: null, + kitId: null, + }, + data: { + organizationId, + userId, + }, + }); + } catch (updateCause) { + if (isNotFoundError(updateCause)) { + throw new ShelfError({ + // why: cause deliberately null — makeShelfError collapses any + // P2025 anywhere in the cause chain to a 404, which would turn + // this lost-race "already claimed" outcome into a not-found. + cause: null, + message: + "This QR code has already been claimed or linked so you cannot claim it.", + title: "QR code already claimed", + status: 403, + additionalData: { id, organizationId, userId }, + label, + shouldBeCaptured: false, + }); + } + throw updateCause; + } } catch (cause) { throw new ShelfError({ cause, diff --git a/apps/webapp/app/routes/api+/mobile+/get-scanned-item.$qrId.test.ts b/apps/webapp/app/routes/api+/mobile+/get-scanned-item.$qrId.test.ts new file mode 100644 index 000000000..c7f349a8d --- /dev/null +++ b/apps/webapp/app/routes/api+/mobile+/get-scanned-item.$qrId.test.ts @@ -0,0 +1,93 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// why: `mobile-auth.server` transitively loads the Supabase admin client and +// the real Prisma client (no DB / env in unit tests); the loader only calls +// requireMobileAuth from it. +vi.mock("~/modules/api/mobile-auth.server", () => ({ + requireMobileAuth: vi.fn(), +})); + +// why: the resolver's own branching is covered by its co-located test; here +// we only assert how the audit-scanner route propagates the discriminated +// results — the reason field must be additive (same status + message). +vi.mock("~/modules/api/mobile-code-resolve.server", () => ({ + resolveMobileScannedCode: vi.fn(), +})); + +import { requireMobileAuth } from "~/modules/api/mobile-auth.server"; +import { resolveMobileScannedCode } from "~/modules/api/mobile-code-resolve.server"; +import { loader } from "./get-scanned-item.$qrId"; + +/** + * Tests for GET /api/mobile/get-scanned-item/:qrId error-payload propagation. + * The audit scanner consumes this route: an unclaimed code must still be the + * exact same 404 + message it always was, with `reason`/`qrId` strictly + * additive on top. + * + * @see {@link file://./get-scanned-item.$qrId.ts} + */ + +/** Shape of the `data()` result the route loader returns. */ +type DataResult = { data: T; init: ResponseInit | null }; + +/** Runs the loader and unwraps the data() envelope. */ +async function callLoader(qrId = "qr-1") { + const request = new Request( + `http://localhost/api/mobile/get-scanned-item/${qrId}` + ); + const result = await loader({ + request, + params: { qrId }, + context: {}, + } as never); + const { data, init } = result as unknown as DataResult<{ + qr?: unknown; + error?: { message: string; reason?: string; qrId?: string }; + }>; + return { body: data, status: init?.status ?? 200 }; +} + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(requireMobileAuth).mockResolvedValue({ + user: { id: "user-1" }, + } as any); +}); + +describe("GET /api/mobile/get-scanned-item/:qrId error payload", () => { + it("keeps the audit-scanner 404 contract for unclaimed codes, with reason additive", async () => { + vi.mocked(resolveMobileScannedCode).mockResolvedValue({ + ok: false, + status: 404, + message: "This QR code is not linked to any organization", + reason: "unclaimed", + qrId: "qr-1", + }); + + const { body, status } = await callLoader(); + + // Unchanged for existing callers: same status, same message. + expect(status).toBe(404); + expect(body.error?.message).toBe( + "This QR code is not linked to any organization" + ); + // Additive discriminator for reason-aware callers. + expect(body.error?.reason).toBe("unclaimed"); + expect(body.error?.qrId).toBe("qr-1"); + }); + + it("omits reason/qrId entirely for plain failures (additive contract)", async () => { + vi.mocked(resolveMobileScannedCode).mockResolvedValue({ + ok: false, + status: 404, + message: "QR code not found", + }); + + const { body, status } = await callLoader(); + + expect(status).toBe(404); + expect(body.error).toEqual({ message: "QR code not found" }); + expect(body.error).not.toHaveProperty("reason"); + expect(body.error).not.toHaveProperty("qrId"); + }); +}); diff --git a/apps/webapp/app/routes/api+/mobile+/get-scanned-item.$qrId.ts b/apps/webapp/app/routes/api+/mobile+/get-scanned-item.$qrId.ts index 6454c23de..23df512f2 100644 --- a/apps/webapp/app/routes/api+/mobile+/get-scanned-item.$qrId.ts +++ b/apps/webapp/app/routes/api+/mobile+/get-scanned-item.$qrId.ts @@ -27,7 +27,17 @@ export async function loader({ request, params }: LoaderFunctionArgs) { const result = await resolveMobileScannedCode({ request, params, user }); if (!result.ok) { return data( - { error: { message: result.message } }, + { + error: { + message: result.message, + // Additive structured discriminator (e.g. "unclaimed") + the + // scanned QR id. Audit-scanner behavior is unchanged for callers + // that ignore it — still the same status and message. + ...(result.reason + ? { reason: result.reason, qrId: result.qrId } + : {}), + }, + }, { status: result.status } ); } diff --git a/apps/webapp/app/routes/api+/mobile+/qr.$qrId.test.ts b/apps/webapp/app/routes/api+/mobile+/qr.$qrId.test.ts new file mode 100644 index 000000000..991c772f8 --- /dev/null +++ b/apps/webapp/app/routes/api+/mobile+/qr.$qrId.test.ts @@ -0,0 +1,98 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// why: `mobile-auth.server` transitively loads the Supabase admin client and +// the real Prisma client (no DB / env in unit tests); the loader only calls +// requireMobileAuth from it. +vi.mock("~/modules/api/mobile-auth.server", () => ({ + requireMobileAuth: vi.fn(), +})); + +// why: the resolver's own branching is covered by its co-located test; here +// we only assert how THIS route propagates the resolver's discriminated +// results into the wire payload the companion consumes. +vi.mock("~/modules/api/mobile-code-resolve.server", () => ({ + resolveMobileScannedCode: vi.fn(), +})); + +// why: scan provenance writes hit the DB; the route's contract under test is +// the error payload shape, not recording (which stays unchanged). +vi.mock("~/modules/scan/service.server", () => ({ + createScan: vi.fn(), +})); + +import { requireMobileAuth } from "~/modules/api/mobile-auth.server"; +import { resolveMobileScannedCode } from "~/modules/api/mobile-code-resolve.server"; +import { createScan } from "~/modules/scan/service.server"; +import { loader } from "./qr.$qrId"; + +/** + * Tests for GET /api/mobile/qr/:qrId error-payload propagation — the + * structured `reason`/`qrId` discriminator must reach the wire for unclaimed + * codes, and must be absent for plain failures (additive contract). + * + * @see {@link file://./qr.$qrId.ts} + */ + +/** Shape of the `data()` result the route loader returns. */ +type DataResult = { data: T; init: ResponseInit | null }; + +/** Runs the loader and unwraps the data() envelope. */ +async function callLoader(qrId = "qr-1") { + const request = new Request(`http://localhost/api/mobile/qr/${qrId}`); + const result = await loader({ + request, + params: { qrId }, + context: {}, + } as never); + const { data, init } = result as unknown as DataResult<{ + qr?: unknown; + error?: { message: string; reason?: string; qrId?: string }; + }>; + return { body: data, status: init?.status ?? 200 }; +} + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(requireMobileAuth).mockResolvedValue({ + user: { id: "user-1" }, + } as any); +}); + +describe("GET /api/mobile/qr/:qrId error payload", () => { + it("propagates reason + qrId for an unclaimed code and records nothing", async () => { + vi.mocked(resolveMobileScannedCode).mockResolvedValue({ + ok: false, + status: 404, + message: "This QR code is not linked to any organization", + reason: "unclaimed", + qrId: "qr-1", + }); + + const { body, status } = await callLoader(); + + expect(status).toBe(404); + expect(body.error).toEqual({ + message: "This QR code is not linked to any organization", + reason: "unclaimed", + qrId: "qr-1", + }); + expect(createScan).not.toHaveBeenCalled(); + }); + + it("omits reason/qrId entirely for plain failures (additive contract)", async () => { + vi.mocked(resolveMobileScannedCode).mockResolvedValue({ + ok: false, + status: 403, + message: "This QR code belongs to a different organization", + }); + + const { body, status } = await callLoader(); + + expect(status).toBe(403); + expect(body.error).toEqual({ + message: "This QR code belongs to a different organization", + }); + expect(body.error).not.toHaveProperty("reason"); + expect(body.error).not.toHaveProperty("qrId"); + }); +}); diff --git a/apps/webapp/app/routes/api+/mobile+/qr.$qrId.ts b/apps/webapp/app/routes/api+/mobile+/qr.$qrId.ts index eaa199f67..53ec8592e 100644 --- a/apps/webapp/app/routes/api+/mobile+/qr.$qrId.ts +++ b/apps/webapp/app/routes/api+/mobile+/qr.$qrId.ts @@ -32,7 +32,17 @@ export async function loader({ request, params }: LoaderFunctionArgs) { const result = await resolveMobileScannedCode({ request, params, user }); if (!result.ok) { return data( - { error: { message: result.message } }, + { + error: { + message: result.message, + // Additive structured discriminator (e.g. "unclaimed") + the + // scanned QR id, so the companion can branch into the native + // claim flow without string-matching the message. + ...(result.reason + ? { reason: result.reason, qrId: result.qrId } + : {}), + }, + }, { status: result.status } ); } diff --git a/apps/webapp/app/routes/api+/mobile+/qr.claim.test.ts b/apps/webapp/app/routes/api+/mobile+/qr.claim.test.ts new file mode 100644 index 000000000..881475d75 --- /dev/null +++ b/apps/webapp/app/routes/api+/mobile+/qr.claim.test.ts @@ -0,0 +1,207 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// why: the route pre-reads the QR state via `db.qr.findUnique` (the web claim +// loader's `assetId: null, kitId: null` availability guard); stubbing just +// that avoids the real Prisma client (no DB in unit tests). +vi.mock("~/database/db.server", () => ({ + db: { + qr: { findUnique: vi.fn() }, + }, +})); + +// why: `mobile-auth.server` transitively loads the Supabase admin client and +// the real Prisma client (no DB / env in unit tests). The route only calls +// these three gate functions, so we stub exactly those and drive the +// permission/org branching through them. +vi.mock("~/modules/api/mobile-auth.server", () => ({ + requireMobileAuth: vi.fn(), + requireOrganizationAccess: vi.fn(), + requireMobilePermission: vi.fn(), +})); + +// why: `claimQrCode` is the web-parity service (its own semantics are the +// web claim route's semantics). The route's observable job is gating + +// envelope shaping, so the service is stubbed and its failure modes are +// simulated as the ShelfErrors it really throws. +vi.mock("~/modules/qr/service.server", () => ({ + claimQrCode: vi.fn(), +})); + +import { db } from "~/database/db.server"; +import { + requireMobileAuth, + requireMobilePermission, + requireOrganizationAccess, +} from "~/modules/api/mobile-auth.server"; +import { claimQrCode } from "~/modules/qr/service.server"; +import { ShelfError } from "~/utils/error"; +import { action } from "./qr.claim"; + +/** + * Tests for POST /api/mobile/qr/claim — the native takeover of the web claim + * flow. Asserts observable behavior: status codes, error envelopes, + * permission denials, and that the claim is always bound to the caller's + * current org (never a body-supplied one). + * + * @see {@link file://./qr.claim.ts} + */ + +/** Shape of the `data()` result the route action returns. */ +type DataResult = { data: T; init: ResponseInit | null }; + +/** Runs the action with a JSON body and unwraps the data() envelope. */ +async function callAction(body: unknown) { + const request = new Request( + "http://localhost/api/mobile/qr/claim?orgId=org-1", + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + } + ); + const result = await action({ request, params: {}, context: {} } as never); + const { data, init } = result as unknown as DataResult<{ + qr?: { id: string; organizationId: string | null }; + error?: { message: string }; + }>; + return { body: data, status: init?.status ?? 200 }; +} + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(requireMobileAuth).mockResolvedValue({ + user: { id: "user-1" }, + } as any); + vi.mocked(requireOrganizationAccess).mockResolvedValue("org-1"); + vi.mocked(requireMobilePermission).mockResolvedValue(undefined); + // why: cast — the route selects a narrow shape, not the full Qr row. + // Default: an unlinked code, so the availability guard passes and each + // denial test overrides only the branch it exercises. + vi.mocked(db.qr.findUnique).mockResolvedValue({ + id: "qr-1", + assetId: null, + kitId: null, + } as any); +}); + +describe("POST /api/mobile/qr/claim", () => { + it("claims the code into the caller's current org and returns the qr summary", async () => { + vi.mocked(claimQrCode).mockResolvedValue({ + id: "qr-1", + organizationId: "org-1", + assetId: null, + kitId: null, + } as any); + + const { body, status } = await callAction({ qrId: "qr-1" }); + + expect(status).toBe(200); + expect(body).toEqual({ + qr: { id: "qr-1", organizationId: "org-1", assetId: null, kitId: null }, + }); + // The claim is bound to the resolved caller org + caller user — the body + // can never smuggle a different org (org-scope contract). + expect(claimQrCode).toHaveBeenCalledWith({ + id: "qr-1", + organizationId: "org-1", + userId: "user-1", + }); + }); + + it("returns 403 and never claims when the caller lacks qr:update (non-admin/owner)", async () => { + vi.mocked(requireMobilePermission).mockRejectedValue( + new ShelfError({ + cause: null, + message: "You have no permission to perform this action", + label: "Permission", + status: 403, + }) + ); + + const { body, status } = await callAction({ qrId: "qr-1" }); + + expect(status).toBe(403); + expect(body.error?.message).toBe( + "You have no permission to perform this action" + ); + expect(claimQrCode).not.toHaveBeenCalled(); + }); + + it("returns 403 when the code is already claimed by an organization", async () => { + // why: simulate exactly what claimQrCode throws for an already-claimed + // code — a 403 ShelfError whose status survives the service's re-wrap. + vi.mocked(claimQrCode).mockRejectedValue( + new ShelfError({ + cause: null, + message: "Failed to claim qr code", + label: "QR", + status: 403, + }) + ); + + const { body, status } = await callAction({ qrId: "qr-1" }); + + expect(status).toBe(403); + expect(body.error?.message).toBe("Failed to claim qr code"); + }); + + it("returns 400 for a body without a qrId", async () => { + const { body, status } = await callAction({}); + + expect(status).toBe(400); + expect(body.error?.message).toBe("Invalid request body"); + expect(claimQrCode).not.toHaveBeenCalled(); + }); + + it("returns 400 (not 500) for a non-JSON body", async () => { + // why: raw Request — `request.json()` must throw a SyntaxError here, and + // the route has to funnel it into the same 400 as a Zod failure instead + // of leaking a 500 through makeShelfError's unknown-error branch. + const request = new Request( + "http://localhost/api/mobile/qr/claim?orgId=org-1", + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: "not-json", + } + ); + const result = await action({ request, params: {}, context: {} } as never); + const { data, init } = result as unknown as DataResult<{ + error?: { message: string }; + }>; + + expect(init?.status).toBe(400); + expect(data.error?.message).toBe("Invalid request body"); + expect(claimQrCode).not.toHaveBeenCalled(); + }); + + it("returns 404 when the QR does not exist", async () => { + vi.mocked(db.qr.findUnique).mockResolvedValue(null); + + const { body, status } = await callAction({ qrId: "qr-missing" }); + + expect(status).toBe(404); + expect(body.error?.message).toBe("QR code not found"); + expect(claimQrCode).not.toHaveBeenCalled(); + }); + + it("returns 403 and never claims an orgless code that is linked to an asset/kit", async () => { + // why: an orgless-but-linked row is the corrupted state createAsset's + // loose QR-connect branch can produce; the web claim loader refuses it + // (`assetId: null, kitId: null` guard) so mobile must too — claiming it + // would let "Create New Asset" re-parent the QR off its existing asset. + vi.mocked(db.qr.findUnique).mockResolvedValue({ + id: "qr-1", + assetId: "asset-linked", + kitId: null, + } as any); + + const { body, status } = await callAction({ qrId: "qr-1" }); + + expect(status).toBe(403); + expect(body.error?.message).toBe( + "This QR code is not available for claiming." + ); + expect(claimQrCode).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/webapp/app/routes/api+/mobile+/qr.claim.ts b/apps/webapp/app/routes/api+/mobile+/qr.claim.ts new file mode 100644 index 000000000..9c1ee872d --- /dev/null +++ b/apps/webapp/app/routes/api+/mobile+/qr.claim.ts @@ -0,0 +1,157 @@ +/** + * Mobile QR Claim API + * + * Endpoint backing the companion scanner's native "Unclaimed Code" takeover. + * Claims an unclaimed Shelf QR code (a printed code with no `organizationId`) + * into the caller's current organization — the mobile twin of the web claim + * route (`qr+/_private+/$qrId_.claim.tsx`). It reuses the exact same service + * function (`claimQrCode`), so claim semantics (unclaimed-only check, 403 on + * already-claimed, org + user assignment) and any notes/events stay in parity + * with web for free. + * + * Permission gate: `qr` / `update` — Role2PermissionMap only grants BASE and + * SELF_SERVICE `qr:read`, and ADMIN/OWNER short-circuit to allow-all in + * `hasPermission`, so this is effectively ADMIN/OWNER only, exactly like the + * web claim route's `requirePermission` gate. + * + * @see {@link file://./../../qr+/_private+/$qrId_.claim.tsx} — the mirrored web route + * @see {@link file://./../../../modules/qr/service.server.ts} — claimQrCode + * @see {@link file://./qr.link-asset.ts} — the follow-up link-existing step + */ + +import { data, type ActionFunctionArgs } from "react-router"; +import { z } from "zod"; +import { db } from "~/database/db.server"; +import { + requireMobileAuth, + requireMobilePermission, + requireOrganizationAccess, +} from "~/modules/api/mobile-auth.server"; +import { claimQrCode } from "~/modules/qr/service.server"; +import { makeShelfError, ShelfError } from "~/utils/error"; +import { + PermissionAction, + PermissionEntity, +} from "~/utils/permissions/permission.data"; + +/** Zod schema for the claim JSON body. */ +const ClaimQrSchema = z.object({ + qrId: z.string().min(1, "QR ID is required"), +}); + +/** + * POST /api/mobile/qr/claim + * + * Claims an unclaimed QR code into the caller's current organization. + * + * Body: `{ qrId: string }` + * Org: `?orgId=` query param or `x-shelf-organization` header. + * + * Success envelope: `{ qr: { id, organizationId, assetId, kitId } }` — the + * claimed code summary (assetId/kitId are null for a freshly claimed code), + * so the app can proceed straight to create-new / link-existing. + * + * @param args - React Router action args (carrying the incoming request). + * @returns A JSON response with the claimed QR summary on success, or + * `{ error: { message } }` with an appropriate HTTP status on failure: + * - 400 Invalid body (including a non-JSON/empty body) + * - 401 Missing/invalid bearer token + * - 403 Caller lacks `qr:update` (non-admin/owner), the code is already + * claimed by an organization (surfaced by `claimQrCode`), or the code is + * linked to an asset/kit and therefore not available for claiming + * (mirrors the web claim loader's `assetId: null, kitId: null` guard) + * - 404 QR code not found + */ +export async function action({ request }: ActionFunctionArgs) { + let userId: string | undefined; + + try { + const { user } = await requireMobileAuth(request); + userId = user.id; + const organizationId = await requireOrganizationAccess(request, user.id); + + // Same effective gate as the web claim route: qr/update is admin+owner + // only (see file-level JSDoc). + await requireMobilePermission({ + userId: user.id, + organizationId, + entity: PermissionEntity.qr, + action: PermissionAction.update, + }); + + // why: raw `.parse` surfaces a ZodError as a 500 through makeShelfError's + // unknown-error branch, and `request.json()` itself throws a SyntaxError + // (also a 500) on a non-JSON/empty body — `.catch(() => null)` funnels + // that into the same 400, since safeParse(null) fails cleanly. + const parsed = ClaimQrSchema.safeParse( + await request.json().catch(() => null) + ); + if (!parsed.success) { + throw new ShelfError({ + cause: parsed.error, + message: "Invalid request body", + additionalData: { validationErrors: parsed.error.flatten() }, + label: "QR", + status: 400, + }); + } + const { qrId } = parsed.data; + + // Mirror the web claim loader's guard: only a code with NO asset/kit link + // is available for claiming (`assetId: null, kitId: null` in + // $qrId_.claim.tsx). `claimQrCode` only checks `organizationId`, so an + // orgless-but-linked row (corrupted state) would otherwise be claimable + // here while web refuses it. + const existingQr = await db.qr.findUnique({ + // eslint-disable-next-line local-rules/require-org-scope-on-id-queries -- idor-safe: an unclaimed code has no organizationId to scope by; this read only gates claimability (linked / missing) and `claimQrCode` re-checks the claimed state before writing + where: { id: qrId }, + select: { id: true, assetId: true, kitId: true }, + }); + + if (!existingQr) { + throw new ShelfError({ + cause: null, + message: "QR code not found", + additionalData: { qrId }, + label: "QR", + status: 404, + shouldBeCaptured: false, + }); + } + + if (existingQr.assetId || existingQr.kitId) { + throw new ShelfError({ + cause: null, + message: "This QR code is not available for claiming.", + additionalData: { qrId }, + label: "QR", + status: 403, + shouldBeCaptured: false, + }); + } + + // Claim into the caller's CURRENT org (never a body-supplied org id) — + // `requireOrganizationAccess` already proved membership. `claimQrCode` + // itself rejects codes that already belong to an organization (403). + const qr = await claimQrCode({ + id: qrId, + organizationId, + userId: user.id, + }); + + return data({ + qr: { + id: qr.id, + organizationId: qr.organizationId, + assetId: qr.assetId, + kitId: qr.kitId, + }, + }); + } catch (cause) { + const reason = makeShelfError(cause, { userId }); + return data( + { error: { message: reason.message } }, + { status: reason.status } + ); + } +} diff --git a/apps/webapp/app/routes/api+/mobile+/qr.link-asset.test.ts b/apps/webapp/app/routes/api+/mobile+/qr.link-asset.test.ts new file mode 100644 index 000000000..3c95fcd2e --- /dev/null +++ b/apps/webapp/app/routes/api+/mobile+/qr.link-asset.test.ts @@ -0,0 +1,279 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// why: the route reads the QR state via `db.qr.findUnique` and the real +// `assertAssetsBelongToOrg` guard reads `db.asset.findMany`; stubbing just +// those avoids the real Prisma client (no DB in unit tests) while keeping +// the org-ownership guard's actual comparison logic in play. +vi.mock("~/database/db.server", () => ({ + db: { + qr: { findUnique: vi.fn() }, + asset: { findMany: vi.fn() }, + }, +})); + +// why: `mobile-auth.server` transitively loads the Supabase admin client +// (needs env + network wiring we don't have in unit tests). The route only +// calls these three gate functions. +vi.mock("~/modules/api/mobile-auth.server", () => ({ + requireMobileAuth: vi.fn(), + requireOrganizationAccess: vi.fn(), + requireMobilePermission: vi.fn(), +})); + +// why: `updateAssetQrCode` is the web-parity service the web link route uses; +// the module it lives in is huge and drags in storage/email integrations at +// import time. The route's observable job is gating + state guards, so the +// write itself is stubbed. +vi.mock("~/modules/asset/service.server", () => ({ + updateAssetQrCode: vi.fn(), +})); + +import { db } from "~/database/db.server"; +import { + requireMobileAuth, + requireMobilePermission, + requireOrganizationAccess, +} from "~/modules/api/mobile-auth.server"; +import { updateAssetQrCode } from "~/modules/asset/service.server"; +import { ShelfError } from "~/utils/error"; +import { action } from "./qr.link-asset"; + +/** + * Tests for POST /api/mobile/qr/link-asset — the native takeover of the web + * link-existing flow. Asserts observable branching: the QR state guards + * (not found / unclaimed / other org / already linked), the org-scoping of + * the user-supplied assetId, and the success envelope. + * + * @see {@link file://./qr.link-asset.ts} + */ + +/** Shape of the `data()` result the route action returns. */ +type DataResult = { data: T; init: ResponseInit | null }; + +/** Runs the action with a JSON body and unwraps the data() envelope. */ +async function callAction(body: unknown) { + const request = new Request( + "http://localhost/api/mobile/qr/link-asset?orgId=org-1", + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + } + ); + const result = await action({ request, params: {}, context: {} } as never); + const { data, init } = result as unknown as DataResult<{ + qr?: { id: string; organizationId: string | null; assetId: string | null }; + error?: { message: string; reason?: string; qrId?: string }; + }>; + return { body: data, status: init?.status ?? 200 }; +} + +/** Sets up the QR row the route will read. */ +function mockQr( + qr: { + id: string; + organizationId: string | null; + assetId: string | null; + kitId: string | null; + } | null +) { + // why: cast — the route selects a narrow shape, not the full Qr row. + vi.mocked(db.qr.findUnique).mockResolvedValue(qr as any); +} + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(requireMobileAuth).mockResolvedValue({ + user: { id: "user-1" }, + } as any); + vi.mocked(requireOrganizationAccess).mockResolvedValue("org-1"); + vi.mocked(requireMobilePermission).mockResolvedValue(undefined); + // why: assertAssetsBelongToOrg compares found rows against the requested + // ids; echoing them back makes the guard pass by default so each denial + // test overrides only the branch it exercises. + vi.mocked(db.asset.findMany).mockImplementation( + (args: any) => + Promise.resolve( + (args?.where?.id?.in ?? []).map((id: string) => ({ id })) + ) as any + ); + vi.mocked(updateAssetQrCode).mockResolvedValue({} as any); +}); + +describe("POST /api/mobile/qr/link-asset", () => { + it("links a claimed-but-unlinked QR to an org asset and returns the qr summary", async () => { + mockQr({ + id: "qr-1", + organizationId: "org-1", + assetId: null, + kitId: null, + }); + + const { body, status } = await callAction({ + qrId: "qr-1", + assetId: "asset-1", + }); + + expect(status).toBe(200); + expect(body).toEqual({ + qr: { + id: "qr-1", + organizationId: "org-1", + assetId: "asset-1", + kitId: null, + }, + }); + // The write is org-scoped to the caller's resolved org (same service + // call as the web link route). + expect(updateAssetQrCode).toHaveBeenCalledWith({ + newQrId: "qr-1", + assetId: "asset-1", + organizationId: "org-1", + }); + }); + + it("returns 404 when the QR does not exist", async () => { + mockQr(null); + + const { body, status } = await callAction({ + qrId: "qr-missing", + assetId: "asset-1", + }); + + expect(status).toBe(404); + expect(body.error?.message).toBe("QR code not found"); + expect(updateAssetQrCode).not.toHaveBeenCalled(); + }); + + it("returns 400 with reason 'unclaimed' when the QR has no organization yet", async () => { + mockQr({ id: "qr-1", organizationId: null, assetId: null, kitId: null }); + + const { body, status } = await callAction({ + qrId: "qr-1", + assetId: "asset-1", + }); + + expect(status).toBe(400); + expect(body.error).toEqual({ + message: "This QR code is not claimed yet. Claim it before linking.", + reason: "unclaimed", + qrId: "qr-1", + }); + expect(updateAssetQrCode).not.toHaveBeenCalled(); + }); + + it("returns 403 when the QR belongs to a different organization", async () => { + mockQr({ + id: "qr-1", + organizationId: "org-other", + assetId: null, + kitId: null, + }); + + const { body, status } = await callAction({ + qrId: "qr-1", + assetId: "asset-1", + }); + + expect(status).toBe(403); + expect(body.error?.message).toBe( + "This QR code doesn't belong to your current organization." + ); + expect(updateAssetQrCode).not.toHaveBeenCalled(); + }); + + it("returns 403 when the QR is already linked to an asset or kit", async () => { + mockQr({ + id: "qr-1", + organizationId: "org-1", + assetId: "asset-linked", + kitId: null, + }); + + const { body, status } = await callAction({ + qrId: "qr-1", + assetId: "asset-1", + }); + + expect(status).toBe(403); + expect(body.error?.message).toBe( + "This QR code is already linked to an asset or a kit." + ); + expect(updateAssetQrCode).not.toHaveBeenCalled(); + }); + + it("rejects an assetId that is not in the caller's org (cross-org IDOR)", async () => { + mockQr({ + id: "qr-1", + organizationId: "org-1", + assetId: null, + kitId: null, + }); + // why: simulate the asset living in another org — the org-scoped lookup + // finds nothing, so the shared guard must reject before any write. + vi.mocked(db.asset.findMany).mockResolvedValue([] as any); + + const { body, status } = await callAction({ + qrId: "qr-1", + assetId: "asset-of-org-b", + }); + + expect(status).toBe(400); + expect(body.error?.message).toContain( + "Some of the selected assets do not exist in your workspace" + ); + expect(updateAssetQrCode).not.toHaveBeenCalled(); + }); + + it("returns 403 and never links when the caller lacks qr:update (non-admin/owner)", async () => { + vi.mocked(requireMobilePermission).mockRejectedValue( + new ShelfError({ + cause: null, + message: "You have no permission to perform this action", + label: "Permission", + status: 403, + }) + ); + + const { body, status } = await callAction({ + qrId: "qr-1", + assetId: "asset-1", + }); + + expect(status).toBe(403); + expect(updateAssetQrCode).not.toHaveBeenCalled(); + expect(body.error?.message).toBe( + "You have no permission to perform this action" + ); + }); + + it("returns 400 for a body missing assetId", async () => { + const { body, status } = await callAction({ qrId: "qr-1" }); + + expect(status).toBe(400); + expect(body.error?.message).toBe("Invalid request body"); + expect(updateAssetQrCode).not.toHaveBeenCalled(); + }); + + it("returns 400 (not 500) for a non-JSON body", async () => { + // why: raw Request — `request.json()` must throw a SyntaxError here, and + // the route has to funnel it into the same 400 as a Zod failure instead + // of leaking a 500 through makeShelfError's unknown-error branch. + const request = new Request( + "http://localhost/api/mobile/qr/link-asset?orgId=org-1", + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: "not-json", + } + ); + const result = await action({ request, params: {}, context: {} } as never); + const { data, init } = result as unknown as DataResult<{ + error?: { message: string }; + }>; + + expect(init?.status).toBe(400); + expect(data.error?.message).toBe("Invalid request body"); + expect(updateAssetQrCode).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/webapp/app/routes/api+/mobile+/qr.link-asset.ts b/apps/webapp/app/routes/api+/mobile+/qr.link-asset.ts new file mode 100644 index 000000000..bb24dfbde --- /dev/null +++ b/apps/webapp/app/routes/api+/mobile+/qr.link-asset.ts @@ -0,0 +1,207 @@ +/** + * Mobile QR Link-Existing-Asset API + * + * Endpoint backing the companion's "Link Existing Asset" step after a QR code + * has been claimed (or when scanning a code the caller's org already claimed + * but never linked). The mobile twin of the web link-existing route + * (`qr+/_private+/$qrId_.link.asset.tsx`): same permission gate, same QR + * state guards (claimed by the caller's org, not yet linked to an asset or + * kit), and the exact same service call (`updateAssetQrCode`) so linking + * semantics stay in parity with web for free. + * + * Permission gate: `qr` / `update` — Role2PermissionMap only grants BASE and + * SELF_SERVICE `qr:read`, and ADMIN/OWNER short-circuit to allow-all in + * `hasPermission`, so this is effectively ADMIN/OWNER only, exactly like web. + * + * Security notes: + * - `assetId` comes from the request body (attacker-controlled) and is + * asserted to belong to the caller's organization via + * `assertAssetsBelongToOrg` before any write (org-scope-user-supplied-ids). + * - The QR is read unscoped only to distinguish unclaimed / other-org / ours; + * ownership is enforced immediately after the read. + * + * @see {@link file://./../../qr+/_private+/$qrId_.link.asset.tsx} — the mirrored web route + * @see {@link file://./../../../modules/asset/service.server.ts} — updateAssetQrCode + * @see {@link file://./qr.claim.ts} — the preceding claim step + */ + +import { data, type ActionFunctionArgs } from "react-router"; +import { z } from "zod"; +import { db } from "~/database/db.server"; +import { + requireMobileAuth, + requireMobilePermission, + requireOrganizationAccess, +} from "~/modules/api/mobile-auth.server"; +import { updateAssetQrCode } from "~/modules/asset/service.server"; +import { makeShelfError, ShelfError } from "~/utils/error"; +import { assertAssetsBelongToOrg } from "~/utils/org-validation.server"; +import { + PermissionAction, + PermissionEntity, +} from "~/utils/permissions/permission.data"; + +/** Zod schema for the link-existing-asset JSON body. */ +const LinkQrAssetSchema = z.object({ + qrId: z.string().min(1, "QR ID is required"), + assetId: z.string().min(1, "Asset ID is required"), +}); + +/** + * POST /api/mobile/qr/link-asset + * + * Links a claimed-but-unlinked QR code to an existing asset in the caller's + * current organization. The asset's previous QR codes are disconnected by the + * service (web parity: linking replaces the asset's code, and the old code + * can always be re-linked later). + * + * Body: `{ qrId: string, assetId: string }` + * Org: `?orgId=` query param or `x-shelf-organization` header. + * + * Success envelope: `{ qr: { id, organizationId, assetId, kitId } }` — the + * linked code summary, so the app can navigate straight to the asset detail. + * + * @param args - React Router action args (carrying the incoming request). + * @returns A JSON response with the linked QR summary on success, or + * `{ error: { message, reason? } }` with an appropriate HTTP status: + * - 400 Invalid body (including a non-JSON/empty body) / QR not claimed + * yet (`reason: "unclaimed"`) / asset not found in the caller's workspace + * - 401 Missing/invalid bearer token + * - 403 Caller lacks `qr:update` (non-admin/owner), the QR belongs to a + * different organization, or it is already linked to an asset or kit + * - 404 QR code not found + */ +export async function action({ request }: ActionFunctionArgs) { + let userId: string | undefined; + + try { + const { user } = await requireMobileAuth(request); + userId = user.id; + const organizationId = await requireOrganizationAccess(request, user.id); + + // Same effective gate as the web link flow: qr/update is admin+owner + // only (see file-level JSDoc). + await requireMobilePermission({ + userId: user.id, + organizationId, + entity: PermissionEntity.qr, + action: PermissionAction.update, + }); + + // why: raw `.parse` surfaces a ZodError as a 500 through makeShelfError's + // unknown-error branch, and `request.json()` itself throws a SyntaxError + // (also a 500) on a non-JSON/empty body — `.catch(() => null)` funnels + // that into the same 400, since safeParse(null) fails cleanly. + const parsed = LinkQrAssetSchema.safeParse( + await request.json().catch(() => null) + ); + if (!parsed.success) { + throw new ShelfError({ + cause: parsed.error, + message: "Invalid request body", + additionalData: { validationErrors: parsed.error.flatten() }, + label: "QR", + status: 400, + }); + } + const { qrId, assetId } = parsed.data; + + // Mirror the web link routes' state guards ($qrId_.link.tsx loader + + // $qrId_.link.asset.tsx loader) — mobile has no loader step, so the + // action enforces them itself before writing anything. + // + // Accepted residual: these guards are read-then-write, not atomic — + // updateAssetQrCode's `connect` is unconstrained on the QR's state, so + // two concurrent links of the same code both pass and the last write + // wins (first caller's 200 points at a link that no longer exists). + // Deliberate: the web action has NO state guards at all (mobile is + // strictly stronger), the race needs two admins linking the same + // physical label within milliseconds, and hardening would mean forking + // the shared service. If it ever bites, fix it in updateAssetQrCode + // (QR-side atomic update WHERE assetId/kitId null, like claimQrCode) + // for web AND mobile together — sibling-first. + const qr = await db.qr.findUnique({ + // eslint-disable-next-line local-rules/require-org-scope-on-id-queries -- idor-safe: intentionally unscoped so the route can distinguish three cases — unclaimed code (400 + reason "unclaimed" below), code owned by another org (403 below), and code owned by the caller's org (proceed). Ownership IS enforced immediately below before any write; scoping would collapse the unclaimed case + where: { id: qrId }, + select: { id: true, organizationId: true, assetId: true, kitId: true }, + }); + + if (!qr) { + throw new ShelfError({ + cause: null, + message: "QR code not found", + additionalData: { qrId }, + label: "QR", + status: 404, + shouldBeCaptured: false, + }); + } + + // Unclaimed codes must go through the claim step first (web redirects + // `/qr/:qrId/link` → `/qr/:qrId/claim` for this case). Returned directly + // (not thrown) so the structured `reason` reaches the client and the + // companion can recover by claiming. + if (!qr.organizationId) { + return data( + { + error: { + message: + "This QR code is not claimed yet. Claim it before linking.", + reason: "unclaimed" as const, + qrId: qr.id, + }, + }, + { status: 400 } + ); + } + + if (qr.organizationId !== organizationId) { + throw new ShelfError({ + cause: null, + message: "This QR code doesn't belong to your current organization.", + additionalData: { qrId, organizationId }, + label: "QR", + status: 403, + shouldBeCaptured: false, + }); + } + + if (qr.assetId || qr.kitId) { + throw new ShelfError({ + cause: null, + message: "This QR code is already linked to an asset or a kit.", + additionalData: { qrId, organizationId }, + label: "QR", + status: 403, + shouldBeCaptured: false, + }); + } + + // why: assetId is user-supplied — prove it belongs to the caller's org + // before connecting (org-scope-user-supplied-ids). The service also + // inline-scopes its updates, but the shared guard gives a clean 400 + // instead of a wrapped Prisma P2025. + await assertAssetsBelongToOrg({ assetIds: [assetId], organizationId }); + + await updateAssetQrCode({ + newQrId: qrId, + assetId, + organizationId, + }); + + return data({ + qr: { + id: qr.id, + organizationId: qr.organizationId, + assetId, + kitId: null, + }, + }); + } catch (cause) { + const reason = makeShelfError(cause, { userId }); + return data( + { error: { message: reason.message } }, + { status: reason.status } + ); + } +}