fix: prevent stale values when switching edit drawer records (#7989)

web/src/features/keys/components/__tests__/api-keys-mutate-drawer.test.tsx

@@ -16,8 +16,10 @@ along with this program. If not, see <https://www.gnu.org/licenses/>.
 
 For commercial licensing, please contact support@quantumnous.com
 */
-import { fireEvent, render, screen, waitFor } from '@testing-library/react'
-import { afterEach, describe, expect, test } from 'vitest'
+import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'
+import { afterEach, describe, expect, test, vi } from 'vitest'
+
+import type { ApiKey } from '../../types'
 
 const { createInstance } = await import('i18next')
 const { I18nextProvider, initReactI18next } = await import('react-i18next')
@@ -279,6 +281,129 @@ describe('API keys mutate drawer Auto group integration', () => {
   })
 })
 
+describe('API key expiry across editing sessions', () => {
+  test.each(['close', 'save'] as const)(
+    'keeps an uncached key non-expiring after %s of a key with an expiry',
+    async (finish) => {
+      installApiFixtures([])
+      const first: ApiKey = {
+        id: 1,
+        name: 'expiring',
+        key: '',
+        status: 1,
+        remain_quota: 0,
+        used_quota: 0,
+        unlimited_quota: true,
+        expired_time: 2000000000,
+        created_time: 1,
+        accessed_time: 0,
+        group: 'default',
+        auto_groups: [],
+        cross_group_retry: false,
+        model_limits_enabled: false,
+        model_limits: '',
+        allow_ips: '',
+        allow_user_quota_query: true,
+      }
+      const second = { ...first, id: 2, name: 'non-expiring', expired_time: -1 }
+      let resolveDetail!: (value: { data: unknown }) => void
+      const detail = new Promise<{ data: unknown }>((resolve) => {
+        resolveDetail = resolve
+      })
+      const getFixture = apiClient.get
+      const get = vi.spyOn(apiClient, 'get').mockImplementation((url) => {
+        if (url === '/api/token/1') {
+          return Promise.resolve({ data: { success: true, data: first } })
+        }
+        if (url === '/api/token/2') return detail
+        return getFixture(url)
+      })
+      const put = vi
+        .spyOn(api, 'put')
+        .mockResolvedValue({ data: { success: true } })
+      const queryClient = new QueryClient({
+        defaultOptions: { queries: { retry: false } },
+      })
+      renderedDrawer = { queryClient }
+      const close = vi.fn()
+      const tree = (currentRow: ApiKey, open: boolean) => (
+        <QueryClientProvider client={queryClient}>
+          <I18nextProvider i18n={i18n}>
+            <ApiKeysProvider>
+              <ApiKeysMutateDrawer
+                open={open}
+                currentRow={currentRow}
+                onOpenChange={close}
+              />
+            </ApiKeysProvider>
+          </I18nextProvider>
+        </QueryClientProvider>
+      )
+      const view = render(tree(first, true))
+      await waitFor(() =>
+        expect(findButton('Save changes', true)).toBeEnabled()
+      )
+      await waitFor(() => expect(screen.getByLabelText('Time')).toBeEnabled())
+
+      if (finish === 'save') {
+        fireEvent.click(findButton('Save changes', true))
+        await waitFor(() => expect(put).toHaveBeenCalled())
+        expect(put.mock.calls[0][1]).toMatchObject({
+          id: 1,
+          expired_time: 2000000000,
+        })
+      } else {
+        fireEvent.click(screen.getAllByRole('button', { name: 'Close' })[0])
+      }
+      await waitFor(() => expect(close.mock.calls[0]?.[0]).toBe(false))
+      view.rerender(tree(first, false))
+      await waitFor(() =>
+        expect(screen.queryByRole('dialog')).not.toBeInTheDocument()
+      )
+      view.rerender(tree(second, true))
+      await waitFor(() => expect(get).toHaveBeenCalledWith('/api/token/2'))
+      expect(findButton('Save changes', true)).toBeDisabled()
+      await act(async () =>
+        resolveDetail({ data: { success: true, data: second } })
+      )
+      await waitFor(() =>
+        expect(findButton('Save changes', true)).toBeEnabled()
+      )
+      expect(
+        screen.getByRole('button', { name: 'Never expires' })
+      ).toBeVisible()
+      expect(screen.getByLabelText('Time')).toBeDisabled()
+      fireEvent.click(findButton('Save changes', true))
+      await waitFor(() =>
+        expect(put).toHaveBeenLastCalledWith(
+          '/api/token/',
+          expect.objectContaining({ id: 2, expired_time: -1 })
+        )
+      )
+    }
+  )
+
+  test.each(['Never', 'Clear'])(
+    'submits a non-expiring key when its date is cleared with %s',
+    async (action) => {
+      const payloads: Array<Record<string, unknown>> = []
+      installApiFixtures(payloads)
+      await renderCreateDrawer()
+      changeInput(getControlByLabel('Name'), 'no-expiry')
+      fireEvent.click(screen.getByRole('button', { name: '1 Month' }))
+      expect(screen.getByLabelText('Time')).toBeEnabled()
+      fireEvent.click(screen.getByRole('button', { name: action }))
+      expect(
+        screen.getByRole('button', { name: 'Never expires' })
+      ).toBeVisible()
+      expect(screen.getByLabelText('Time')).toBeDisabled()
+      fireEvent.click(findButton('Save changes', true))
+      await waitFor(() => expect(payloads).toHaveLength(1))
+      expect(payloads[0]?.expired_time).toBe(-1)
+    }
+  )
+})
+
 describe('API keys mutate drawer user quota query switch', () => {
   test('sends false for a new key after the switch, which starts on, is turned off', async () => {
     const createdPayloads: Array<Record<string, unknown>> = []

web/src/features/keys/components/api-keys-mutate-drawer.tsx

@@ -208,6 +208,9 @@ export function ApiKeysMutateDrawer({
   // Load existing data when updating
   useEffect(() => {
     if (!open) {
+      form.reset(
+        getApiKeyFormDefaultValues(defaultUseAutoGroup && backendHasAuto)
+      )
       setInitializedTarget(null)
       return
     }
@@ -345,7 +348,7 @@ export function ApiKeysMutateDrawer({
 
   const handleSetExpiry = (months: number, days: number, hours: number) => {
     if (months === 0 && days === 0 && hours === 0) {
-      form.setValue('expired_time', undefined)
+      form.setValue('expired_time', null)
       return
     }
 
@@ -368,15 +371,7 @@ export function ApiKeysMutateDrawer({
   const unlimitedQuota = form.watch('unlimited_quota')
 
   return (
-    <Sheet
-      open={open}
-      onOpenChange={(v) => {
-        onOpenChange(v)
-        if (!v) {
-          form.reset()
-        }
-      }}
-    >
+    <Sheet open={open} onOpenChange={onOpenChange}>
       <SheetContent
         className={sideDrawerContentClassName('max-w-none sm:!max-w-[620px]')}
       >
@@ -527,8 +522,8 @@ export function ApiKeysMutateDrawer({
                     <div className='grid gap-2 sm:grid-cols-[minmax(0,1fr)_auto] sm:items-center'>
                       <FormControl>
                         <DateTimePicker
-                          value={field.value}
-                          onChange={field.onChange}
+                          value={field.value ?? undefined}
+                          onChange={(date) => field.onChange(date ?? null)}
                           placeholder={t('Never expires')}
                           className='min-w-0 [&_input[type=time]]:w-24 sm:[&_input[type=time]]:w-32'
                         />

web/src/features/keys/lib/api-key-form.ts

@@ -36,7 +36,7 @@ export function getApiKeyFormSchema(t: TFunction, maxAutoGroups = 5) {
     .object({
       name: z.string().min(1, t('Please enter a name')),
       remain_quota_dollars: z.number().optional(),
-      expired_time: z.date().optional(),
+      expired_time: z.date().nullable(),
       unlimited_quota: z.boolean(),
       model_limits: z.array(z.string()),
       allow_ips: z.string().optional(),
@@ -107,7 +107,7 @@ export type ApiKeyFormValues = z.infer<ReturnType<typeof getApiKeyFormSchema>>
 export const API_KEY_FORM_DEFAULT_VALUES: ApiKeyFormValues = {
   name: '',
   remain_quota_dollars: 10,
-  expired_time: undefined,
+  expired_time: null,
   unlimited_quota: true,
   model_limits: [],
   allow_ips: '',
@@ -184,9 +184,7 @@ export function transformApiKeyToFormDefaults(
       ? 0
       : quotaUnitsToDollars(apiKey.remain_quota),
     expired_time:
-      apiKey.expired_time > 0
-        ? new Date(apiKey.expired_time * 1000)
-        : undefined,
+      apiKey.expired_time > 0 ? new Date(apiKey.expired_time * 1000) : null,
     unlimited_quota: apiKey.unlimited_quota,
     model_limits: apiKey.model_limits
       ? apiKey.model_limits.split(',').filter(Boolean)

web/src/features/models/__tests__/metadata-editing.test.tsx

@@ -18,7 +18,14 @@ For commercial licensing, please contact support@quantumnous.com
 */
 import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
 import type { Row } from '@tanstack/react-table'
-import { render, screen, waitFor, cleanup, act } from '@testing-library/react'
+import {
+  render,
+  screen,
+  waitFor,
+  cleanup,
+  act,
+  fireEvent,
+} from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 import { AxiosError } from 'axios'
 import { afterEach, describe, expect, it, vi } from 'vitest'
@@ -361,6 +368,96 @@ describe('metadata editing', () => {
     }
   )
 
+  it.each([undefined, 0])(
+    'does not copy a vendor to the next model when its vendor_id is %s',
+    async (vendorId) => {
+      const second = {
+        ...model,
+        id: 8,
+        model_name: 'unassigned-model',
+        vendor_id: vendorId,
+      }
+      let resolveDetail!: (value: Awaited<ReturnType<typeof api.get>>) => void
+      const detail = new Promise<Awaited<ReturnType<typeof api.get>>>(
+        (resolve) => {
+          resolveDetail = resolve
+        }
+      )
+      const get = vi.spyOn(api, 'get').mockImplementation(async (url) => {
+        if (url === '/api/models/7') {
+          return { data: { success: true, data: model } }
+        }
+        if (url === '/api/models/8') return detail
+        if (url === '/api/vendors/') {
+          return {
+            data: {
+              success: true,
+              data: {
+                items: [{ id: 3, name: 'Existing vendor' }],
+              },
+            },
+          }
+        }
+        throw new Error(`Unexpected GET ${url}`)
+      })
+      const put = vi
+        .spyOn(api, 'put')
+        .mockResolvedValue({ data: { success: true } })
+      const client = new QueryClient({
+        defaultOptions: {
+          queries: { retry: false },
+          mutations: { retry: false },
+        },
+      })
+      const close = vi.fn()
+      const tree = (currentRow: Model, open: boolean) => (
+        <QueryClientProvider client={client}>
+          <ModelMutateDrawer
+            open={open}
+            currentRow={currentRow}
+            onOpenChange={close}
+          />
+        </QueryClientProvider>
+      )
+      const view = render(tree(model, true))
+      await waitFor(() =>
+        expect(screen.getByRole('combobox', { name: 'Vendor' })).toHaveValue(
+          'Existing vendor'
+        )
+      )
+      fireEvent.click(screen.getAllByRole('button', { name: 'Close' })[0])
+      expect(close).toHaveBeenCalledWith(false)
+      view.rerender(tree(model, false))
+      await waitFor(() =>
+        expect(screen.queryByRole('dialog')).not.toBeInTheDocument()
+      )
+      view.rerender(tree(second, true))
+      await waitFor(() => expect(get).toHaveBeenCalledWith('/api/models/8'))
+      expect(
+        screen.getByRole('button', { name: 'Save metadata' })
+      ).toBeDisabled()
+      await act(async () =>
+        resolveDetail({ data: { success: true, data: second } })
+      )
+      await waitFor(() =>
+        expect(screen.getByLabelText('Model Name *')).toHaveValue(
+          'unassigned-model'
+        )
+      )
+      expect(screen.getByRole('combobox', { name: 'Vendor' })).toHaveValue('')
+      fireEvent.click(screen.getByRole('button', { name: 'Save metadata' }))
+      await waitFor(() =>
+        expect(put).toHaveBeenCalledWith(
+          '/api/models/',
+          expect.objectContaining({ id: 8, vendor_id: 0 }),
+          { skipBusinessError: true, skipErrorHandler: true }
+        )
+      )
+      view.unmount()
+      client.clear()
+    }
+  )
+
   it('allows an administrator to save metadata without loading or changing system pricing', async () => {
     useAuthStore.getState().auth.setUser({ id: 2, username: 'admin', role: 10 })
     const get = vi.spyOn(api, 'get').mockImplementation(async (url) => {

web/src/features/models/components/drawers/model-mutate-drawer.tsx

@@ -75,6 +75,7 @@ import { getNameRuleOptions, ENDPOINT_TEMPLATES } from '../../constants'
 import { modelsQueryKeys, vendorsQueryKeys } from '../../lib'
 import {
   modelFormSchema,
+  MODEL_FORM_DEFAULT_VALUES,
   transformModelToFormDefaults,
   transformFormDataToModelPayload,
   type ModelFormValues,
@@ -115,12 +116,7 @@ export function ModelMutateDrawer(props: {
   const loadedKey = useRef('')
   const form = useForm({
     resolver: zodResolver(modelFormSchema),
-    defaultValues: transformModelToFormDefaults({
-      model_name: '',
-      status: 1,
-      sync_official: 1,
-      name_rule: 0,
-    } as Model),
+    defaultValues: MODEL_FORM_DEFAULT_VALUES,
   })
   const vendorsQuery = useQuery({
     queryKey: vendorsQueryKeys.list(),
@@ -163,6 +159,7 @@ export function ModelMutateDrawer(props: {
 
   useEffect(() => {
     if (!props.open) {
+      form.reset(MODEL_FORM_DEFAULT_VALUES)
       setCreatedModel(null)
       loadedKey.current = ''
       return
@@ -172,16 +169,12 @@ export function ModelMutateDrawer(props: {
       : `channel:${currentRow?.model_name ?? ''}`
     if (loadedKey.current === key || (isEditing && !modelQuery.data)) return
     form.reset(
-      transformModelToFormDefaults(
-        (isEditing
-          ? modelQuery.data
-          : {
-              model_name: currentRow?.model_name ?? '',
-              status: 1,
-              sync_official: 1,
-              name_rule: 0,
-            }) as Model
-      )
+      isEditing && modelQuery.data
+        ? transformModelToFormDefaults(modelQuery.data)
+        : {
+            ...MODEL_FORM_DEFAULT_VALUES,
+            model_name: currentRow?.model_name ?? '',
+          }
     )
     loadedKey.current = key
   }, [props.open, currentRow, isEditing, modelQuery.data, form])
@@ -401,7 +394,7 @@ export function ModelMutateDrawer(props: {
                                 }))}
                                 onValueChange={(value) =>
                                   field.onChange(
-                                    value ? Number.parseInt(value) : undefined
+                                    value ? Number.parseInt(value) : 0
                                   )
                                 }
                                 value={field.value ? String(field.value) : null}

web/src/features/models/lib/model-form.ts

@@ -34,7 +34,7 @@ export const modelFormSchema = z.object({
   description: z.string().default(''),
   icon: z.string().default(''),
   tags: z.array(z.string()).default([]),
-  vendor_id: z.number().optional(),
+  vendor_id: z.number(),
   endpoints: z.string().default(''),
   name_rule: z.number().min(0).max(3).default(0),
   status: z.boolean().default(true),
@@ -45,6 +45,20 @@ export const modelFormSchema = z.object({
 
 export type ModelFormValues = z.infer<typeof modelFormSchema>
 
+export const MODEL_FORM_DEFAULT_VALUES: ModelFormValues = {
+  model_name: '',
+  description: '',
+  icon: '',
+  tags: [],
+  vendor_id: 0,
+  endpoints: '',
+  name_rule: 0,
+  status: true,
+  sync_official: true,
+  enable_groups: [],
+  quota_types: [],
+}
+
 // ============================================================================
 // Vendor Form Schema
 // ============================================================================
@@ -76,7 +90,7 @@ export function transformModelToFormDefaults(model: Model): ModelFormValues {
     description: model.description || '',
     icon: model.icon || '',
     tags: parseTagsFromUtils(model.tags),
-    vendor_id: model.vendor_id,
+    vendor_id: model.vendor_id ?? 0,
     endpoints: model.endpoints || '',
     name_rule: model.name_rule || 0,
     status: model.status === 1,

web/src/features/redemption-codes/components/__tests__/redemptions-mutate-drawer.test.tsx

@@ -93,12 +93,12 @@ function deferred<T>() {
   return { promise, reject, resolve }
 }
 
-function drawerTree(currentRow: Redemption) {
+function drawerTree(currentRow: Redemption, open = true) {
   return (
     <I18nextProvider i18n={i18n}>
       <RedemptionsProvider>
         <RedemptionsMutateDrawer
-          open
+          open={open}
           currentRow={currentRow}
           onOpenChange={() => undefined}
         />
@@ -129,11 +129,14 @@ async function renderDrawer(
   renderedDrawer = { result: render(drawerTree(currentRow)) }
 }
 
-async function rerenderDrawer(currentRow: Redemption): Promise<void> {
+async function rerenderDrawer(
+  currentRow: Redemption,
+  open = true
+): Promise<void> {
   if (!renderedDrawer) {
     throw new Error('Expected a rendered redemption drawer')
   }
-  renderedDrawer.result.rerender(drawerTree(currentRow))
+  renderedDrawer.result.rerender(drawerTree(currentRow, open))
 }
 
 function getSaveButton(): HTMLButtonElement {
@@ -280,6 +283,68 @@ describe('redemption drawer', () => {
     expect(updates[0]?.quota).toBe(1000000)
   })
 
+  test('keeps a non-expiring code unchanged after closing an expiring code', async () => {
+    const first = { ...redemption(1), expired_time: 2000000000 }
+    const second = redemption(2)
+    const detail = deferred<{ data: unknown }>()
+    const updates: unknown[] = []
+    apiClient.get = (url) =>
+      url === '/api/redemption/1'
+        ? Promise.resolve({ data: { success: true, data: first } })
+        : detail.promise
+    apiClient.put = async (_url, data) => {
+      updates.push(data)
+      return { data: { success: true } }
+    }
+    await renderDrawer(first)
+    await waitForLoadedForm()
+    await waitFor(() => expect(screen.getByLabelText('Time')).toBeEnabled())
+    fireEvent.click(screen.getAllByRole('button', { name: 'Close' })[0])
+    await rerenderDrawer(first, false)
+    await waitFor(() =>
+      expect(screen.queryByRole('dialog')).not.toBeInTheDocument()
+    )
+    await rerenderDrawer(second)
+    expect(screen.getByRole('button', { name: 'Loading...' })).toBeDisabled()
+    detail.resolve({ data: { success: true, data: second } })
+    await waitForLoadedForm()
+    expect(screen.getByRole('button', { name: 'Never expires' })).toBeVisible()
+    expect(screen.getByLabelText('Time')).toBeDisabled()
+    fireEvent.click(getSaveButton())
+    await waitFor(() =>
+      expect(updates).toEqual([
+        expect.objectContaining({ id: 2, expired_time: 0 }),
+      ])
+    )
+  })
+
+  test.each(['Never', 'Clear'])(
+    'submits a non-expiring code when its date is cleared with %s',
+    async (action) => {
+      const original = { ...redemption(1), expired_time: 2000000000 }
+      const updates: unknown[] = []
+      apiClient.get = async () => ({ data: { success: true, data: original } })
+      apiClient.put = async (_url, data) => {
+        updates.push(data)
+        return { data: { success: true } }
+      }
+      await renderDrawer(original)
+      await waitForLoadedForm()
+      await waitFor(() => expect(screen.getByLabelText('Time')).toBeEnabled())
+      fireEvent.click(screen.getByRole('button', { name: action }))
+      expect(
+        screen.getByRole('button', { name: 'Never expires' })
+      ).toBeVisible()
+      expect(screen.getByLabelText('Time')).toBeDisabled()
+      fireEvent.click(getSaveButton())
+      await waitFor(() =>
+        expect(updates).toEqual([
+          expect.objectContaining({ id: 1, expired_time: 0 }),
+        ])
+      )
+    }
+  )
+
   test('ignores an older response after switching records', async () => {
     const first = redemption(1, 500001)
     const second = redemption(2, 1000001)

web/src/features/redemption-codes/components/redemptions-mutate-drawer.tsx

@@ -113,6 +113,7 @@ export function RedemptionsMutateDrawer({
   // Load existing data when updating
   useEffect(() => {
     if (!open) {
+      form.reset(REDEMPTION_FORM_DEFAULT_VALUES)
       setRedemptionLoadState('idle')
       setLoadedRedemption(null)
       return
@@ -239,7 +240,7 @@ export function RedemptionsMutateDrawer({
 
   const handleSetExpiry = (months: number, days: number, hours: number) => {
     const newDate = addTimeToDate(months, days, hours)
-    form.setValue('expired_time', newDate)
+    form.setValue('expired_time', newDate ?? null)
   }
 
   const { meta: currencyMeta } = getCurrencyDisplay()
@@ -259,15 +260,7 @@ export function RedemptionsMutateDrawer({
 
   return (
     <>
-      <Sheet
-        open={open}
-        onOpenChange={(v) => {
-          onOpenChange(v)
-          if (!v) {
-            form.reset()
-          }
-        }}
-      >
+      <Sheet open={open} onOpenChange={onOpenChange}>
         <SheetContent
           className={sideDrawerContentClassName('sm:max-w-[600px]')}
         >
@@ -355,8 +348,8 @@ export function RedemptionsMutateDrawer({
                         <div className='flex flex-col gap-2'>
                           <FormControl>
                             <DateTimePicker
-                              value={field.value}
-                              onChange={field.onChange}
+                              value={field.value ?? undefined}
+                              onChange={(date) => field.onChange(date ?? null)}
                               placeholder={t('Never expires')}
                             />
                           </FormControl>

web/src/features/redemption-codes/lib/redemption-form.ts

@@ -19,10 +19,7 @@ For commercial licensing, please contact support@quantumnous.com
 import type { TFunction } from 'i18next'
 import { z } from 'zod'
 
-import {
-  parseQuotaFromDollars,
-  quotaUnitsToEditableAmount,
-} from '@/lib/format'
+import { parseQuotaFromDollars, quotaUnitsToEditableAmount } from '@/lib/format'
 
 import {
   REDEMPTION_VALIDATION,
@@ -42,7 +39,7 @@ export function getRedemptionFormSchema(t: TFunction) {
       .min(REDEMPTION_VALIDATION.NAME_MIN_LENGTH, msg.NAME_LENGTH_INVALID)
       .max(REDEMPTION_VALIDATION.NAME_MAX_LENGTH, msg.NAME_LENGTH_INVALID),
     quota_dollars: z.number().min(0, t('Quota must be a positive number')),
-    expired_time: z.date().optional(),
+    expired_time: z.date().nullable(),
     count: z
       .number()
       .min(REDEMPTION_VALIDATION.COUNT_MIN, msg.COUNT_INVALID)
@@ -54,7 +51,7 @@ export function getRedemptionFormSchema(t: TFunction) {
 export type RedemptionFormValues = {
   name: string
   quota_dollars: number
-  expired_time?: Date
+  expired_time: Date | null
   count?: number
 }
 
@@ -65,7 +62,7 @@ export type RedemptionFormValues = {
 export const REDEMPTION_FORM_DEFAULT_VALUES: RedemptionFormValues = {
   name: '',
   quota_dollars: 10,
-  expired_time: undefined,
+  expired_time: null,
   count: 1,
 }
 
@@ -101,7 +98,7 @@ export function transformRedemptionToFormDefaults(
     expired_time:
       redemption.expired_time > 0
         ? new Date(redemption.expired_time * 1000)
-        : undefined,
+        : null,
     count: 1,
   }
 }