-
Notifications
You must be signed in to change notification settings - Fork 237
[comp] Production Deploy #1996
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[comp] Production Deploy #1996
Changes from all commits
0469943
6272146
e54177a
4d554a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,219 @@ | ||
| 'use client'; | ||
|
|
||
| import { useDebouncedCallback } from '@/hooks/use-debounced-callback'; | ||
| import { FormControl, FormField, FormItem, FormLabel, FormMessage } from '@comp/ui/form'; | ||
| import { Input } from '@comp/ui/input'; | ||
| import type { GlobalVendors } from '@db'; | ||
| import { useAction } from 'next-safe-action/hooks'; | ||
| import { useEffect, useMemo, useRef, useState } from 'react'; | ||
| import type { UseFormReturn } from 'react-hook-form'; | ||
| import { searchGlobalVendorsAction } from '../actions/search-global-vendors-action'; | ||
| import type { CreateVendorFormValues } from './create-vendor-form-schema'; | ||
|
|
||
| const getVendorDisplayName = (vendor: GlobalVendors): string => { | ||
| return vendor.company_name ?? vendor.legal_name ?? vendor.website ?? ''; | ||
| }; | ||
|
|
||
| const normalizeVendorName = (name: string): string => { | ||
| return name.toLowerCase().trim(); | ||
| }; | ||
|
|
||
| const getVendorKey = (vendor: GlobalVendors): string => { | ||
| // `website` is the primary key and should always be present. | ||
| if (vendor.website) return vendor.website; | ||
|
|
||
| const name = vendor.company_name || vendor.legal_name || 'unknown'; | ||
| const timestamp = vendor.createdAt?.getTime() ?? 0; | ||
| return `${name}-${timestamp}`; | ||
| }; | ||
|
|
||
| type Props = { | ||
| form: UseFormReturn<CreateVendorFormValues>; | ||
| isSheetOpen: boolean; | ||
| }; | ||
|
|
||
| export function VendorNameAutocompleteField({ form, isSheetOpen }: Props) { | ||
| const [searchQuery, setSearchQuery] = useState(''); | ||
| const [searchResults, setSearchResults] = useState<GlobalVendors[]>([]); | ||
| const [isSearching, setIsSearching] = useState(false); | ||
| const [popoverOpen, setPopoverOpen] = useState(false); | ||
|
|
||
| // Used to avoid resetting on initial mount. | ||
| const hasOpenedOnceRef = useRef(false); | ||
|
|
||
| const searchVendors = useAction(searchGlobalVendorsAction, { | ||
| onExecute: () => setIsSearching(true), | ||
| onSuccess: (result) => { | ||
| if (result.data?.success && result.data.data?.vendors) { | ||
| setSearchResults(result.data.data.vendors); | ||
| } else { | ||
| setSearchResults([]); | ||
| } | ||
| setIsSearching(false); | ||
| }, | ||
| onError: () => { | ||
| setSearchResults([]); | ||
| setIsSearching(false); | ||
| }, | ||
| }); | ||
|
|
||
| const debouncedSearch = useDebouncedCallback((query: string) => { | ||
| if (query.trim().length > 1) { | ||
| searchVendors.execute({ name: query }); | ||
| } else { | ||
| setSearchResults([]); | ||
| } | ||
| }, 300); | ||
|
|
||
| // Reset autocomplete state when the sheet closes. | ||
| useEffect(() => { | ||
| if (isSheetOpen) { | ||
| hasOpenedOnceRef.current = true; | ||
| return; | ||
| } | ||
|
|
||
| if (!hasOpenedOnceRef.current) return; | ||
|
|
||
| setSearchQuery(''); | ||
| setSearchResults([]); | ||
| setIsSearching(false); | ||
| setPopoverOpen(false); | ||
| }, [isSheetOpen]); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Stale search results persist after sheet reopensLow Severity When a user closes the sheet while a vendor search is in flight, the cleanup effect at lines 77-80 clears Additional Locations (1) |
||
|
|
||
| const deduplicatedSearchResults = useMemo(() => { | ||
| if (searchResults.length === 0) return []; | ||
|
|
||
| const seen = new Map<string, GlobalVendors>(); | ||
|
|
||
| for (const vendor of searchResults) { | ||
| const displayName = getVendorDisplayName(vendor); | ||
| const normalizedName = normalizeVendorName(displayName); | ||
| const existing = seen.get(normalizedName); | ||
|
|
||
| if (!existing) { | ||
| seen.set(normalizedName, vendor); | ||
| continue; | ||
| } | ||
|
|
||
| // Prefer vendor with more complete data. | ||
| const existingHasCompanyName = !!existing.company_name; | ||
| const currentHasCompanyName = !!vendor.company_name; | ||
|
|
||
| if (!existingHasCompanyName && currentHasCompanyName) { | ||
| seen.set(normalizedName, vendor); | ||
| continue; | ||
| } | ||
|
|
||
| if (existingHasCompanyName === currentHasCompanyName) { | ||
| if (!existing.website && vendor.website) { | ||
| seen.set(normalizedName, vendor); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return Array.from(seen.values()); | ||
| }, [searchResults]); | ||
|
|
||
| const handleSelectVendor = (vendor: GlobalVendors) => { | ||
| // Use same fallback logic as getVendorDisplayName for consistency | ||
| const name = getVendorDisplayName(vendor); | ||
|
|
||
| form.setValue('name', name, { shouldDirty: true, shouldValidate: true }); | ||
| form.setValue('website', vendor.website ?? '', { shouldDirty: true, shouldValidate: true }); | ||
| form.setValue('description', vendor.company_description ?? '', { | ||
| shouldDirty: true, | ||
| shouldValidate: true, | ||
| }); | ||
|
|
||
| setSearchQuery(name); | ||
| setSearchResults([]); | ||
| setPopoverOpen(false); | ||
| }; | ||
|
|
||
| return ( | ||
| <FormField | ||
| control={form.control} | ||
| name="name" | ||
| render={({ field }) => ( | ||
| <FormItem className="relative flex flex-col"> | ||
| <FormLabel>{'Vendor Name'}</FormLabel> | ||
| <FormControl> | ||
| <div className="relative"> | ||
| <Input | ||
| placeholder={'Search or enter vendor name...'} | ||
| value={searchQuery} | ||
| onChange={(e) => { | ||
| const val = e.target.value; | ||
| setSearchQuery(val); | ||
| field.onChange(val); | ||
| debouncedSearch(val); | ||
|
|
||
| if (val.trim().length > 1) { | ||
| setPopoverOpen(true); | ||
| } else { | ||
| setPopoverOpen(false); | ||
| setSearchResults([]); | ||
| } | ||
| }} | ||
| onBlur={() => { | ||
| setTimeout(() => setPopoverOpen(false), 150); | ||
| }} | ||
| onFocus={() => { | ||
| // Prevent flicker on initial focus: only show if we have results or an active search. | ||
| if (searchQuery.trim().length > 1 && (isSearching || searchResults.length > 0)) { | ||
| setPopoverOpen(true); | ||
| } | ||
| }} | ||
| autoFocus | ||
| /> | ||
|
|
||
| {popoverOpen && ( | ||
| <div className="bg-background absolute top-full z-10 mt-1 w-full rounded-md border shadow-lg"> | ||
| <div className="max-h-[300px] overflow-y-auto p-1"> | ||
| {isSearching && ( | ||
| <div className="text-muted-foreground p-2 text-sm">Loading...</div> | ||
| )} | ||
|
|
||
| {!isSearching && deduplicatedSearchResults.length > 0 && ( | ||
| <> | ||
| <p className="text-muted-foreground px-2 py-1.5 text-xs font-medium"> | ||
| {'Suggestions'} | ||
| </p> | ||
| {deduplicatedSearchResults.map((vendor) => ( | ||
| <div | ||
| key={getVendorKey(vendor)} | ||
| className="hover:bg-accent cursor-pointer rounded-sm p-2 text-sm" | ||
| onMouseDown={() => handleSelectVendor(vendor)} | ||
| > | ||
| {getVendorDisplayName(vendor)} | ||
| </div> | ||
| ))} | ||
| </> | ||
| )} | ||
|
|
||
| {!isSearching && | ||
| searchQuery.trim().length > 1 && | ||
| deduplicatedSearchResults.length === 0 && ( | ||
| <div | ||
| className="hover:bg-accent cursor-pointer rounded-sm p-2 text-sm italic" | ||
| onMouseDown={() => { | ||
| field.onChange(searchQuery); | ||
| setSearchResults([]); | ||
| setPopoverOpen(false); | ||
| }} | ||
| > | ||
| {`Create "${searchQuery}"`} | ||
| </div> | ||
| )} | ||
| </div> | ||
| </div> | ||
| )} | ||
| </div> | ||
| </FormControl> | ||
| <FormMessage /> | ||
| </FormItem> | ||
| )} | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| import { VendorCategory, VendorStatus } from '@db'; | ||
| import { z } from 'zod'; | ||
|
|
||
| export const createVendorSchema = z.object({ | ||
| name: z.string().trim().min(1, 'Name is required'), | ||
| // Allow empty string in the input and treat it as "not provided" | ||
| website: z | ||
| .union([z.string().url('URL must be valid and start with https://'), z.literal('')]) | ||
| .transform((value) => (value === '' ? undefined : value)) | ||
| .optional(), | ||
| description: z.string().optional(), | ||
| category: z.nativeEnum(VendorCategory), | ||
| status: z.nativeEnum(VendorStatus), | ||
| assigneeId: z.string().optional(), | ||
| }); | ||
|
|
||
| export type CreateVendorFormValues = z.infer<typeof createVendorSchema>; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Race condition allows duplicate vendor names
Medium Severity
The duplicate vendor name check and vendor creation are separate database operations without transaction protection. Two concurrent requests can both pass the
findFirstcheck before either creates the vendor, resulting in duplicate vendors with the same name in the organization. The Vendor model lacks a@@unique([organizationId, name])constraint that would provide database-level protection against this race condition.