Просмотр исходного кода

Improve UX for entering multi-digit quantities in Buy More Licenses feature

GitOrigin-RevId: c51e2146dbb53144e6951a16f7162ba2d10c5c4e
Simon Gardner 10 месяцев назад
Родитель
Сommit
14356f2675

+ 19 - 11
services/web/frontend/js/features/group-management/components/add-seats/add-seats.tsx

@@ -20,6 +20,7 @@ import PoNumber from '@/features/group-management/components/add-seats/po-number
 import CostSummary from '@/features/group-management/components/add-seats/cost-summary'
 import RequestStatus from '@/features/group-management/components/request-status'
 import useAsync from '@/shared/hooks/use-async'
+import useAsyncWithCancel from '@/shared/hooks/use-async-with-cancel'
 import getMeta from '@/utils/meta'
 import { FetchError, postJSON } from '@/infrastructure/fetch-json'
 import { debugConsole } from '@/utils/debugging'
@@ -50,7 +51,6 @@ function AddSeats() {
   const [addSeatsInputError, setAddSeatsInputError] = useState<string>()
   const [poNumberInputError, setPoNumberInputError] = useState<string>()
   const [shouldContactSales, setShouldContactSales] = useState(false)
-  const controller = useAbortController()
   const { signal: addSeatsSignal } = useAbortController()
   const { signal: contactSalesSignal } = useAbortController()
   const {
@@ -60,7 +60,8 @@ function AddSeats() {
     data: costSummaryData,
     reset: resetCostSummaryData,
     error: errorCostSummary,
-  } = useAsync<CostSummaryData, FetchError>()
+    cancelAll: cancelCostSummaryRequest,
+  } = useAsyncWithCancel<CostSummaryData, FetchError>()
   const [isAddingSeats, setIsAddingSeats] = useState(false)
   const [isErrorAddingSeats, setIsErrorAddingSeats] = useState(false)
   const [isSuccessAddingSeats, setIsSuccessAddingSeats] = useState(false)
@@ -85,14 +86,21 @@ function AddSeats() {
 
   const debouncedCostSummaryRequest = useMemo(
     () =>
-      debounce((value: number, signal: AbortSignal) => {
-        const post = postJSON('/user/subscription/group/add-users/preview', {
-          signal,
-          body: { adding: value },
+      debounce((value: number) => {
+        cancelCostSummaryRequest()
+        const post = (signal: AbortSignal) =>
+          postJSON('/user/subscription/group/add-users/preview', {
+            body: { adding: value },
+            signal,
+          })
+
+        runAsyncCostSummary(post).catch(error => {
+          if (error.name !== 'AbortError') {
+            debugConsole.error(error)
+          }
         })
-        runAsyncCostSummary(post).catch(debugConsole.error)
       }, 500),
-    [runAsyncCostSummary]
+    [runAsyncCostSummary, cancelCostSummaryRequest]
   )
 
   const debouncedTrackUserEnterSeatNumberEvent = useMemo(
@@ -168,14 +176,15 @@ function AddSeats() {
         debouncedCostSummaryRequest.cancel()
         shouldContactSales = true
       } else {
-        debouncedCostSummaryRequest(seats, controller.signal)
+        debouncedCostSummaryRequest(seats)
       }
     } else {
       debouncedTrackUserEnterSeatNumberEvent.cancel()
       debouncedCostSummaryRequest.cancel()
+      cancelCostSummaryRequest()
+      resetCostSummaryData()
     }
 
-    resetCostSummaryData()
     setShouldContactSales(shouldContactSales)
   }
 
@@ -374,7 +383,6 @@ function AddSeats() {
                       required
                       className="w-25"
                       name="seats"
-                      disabled={isLoadingCostSummary}
                       onChange={handleSeatsChange}
                       isInvalid={Boolean(addSeatsInputError)}
                     />

+ 1 - 1
services/web/frontend/js/features/group-management/components/add-seats/cost-summary.tsx

@@ -28,7 +28,7 @@ function CostSummary({ subscriptionChange, totalLicenses }: CostSummaryProps) {
       data-testid="cost-summary"
     >
       <Card.Body className="d-grid gap-2 p-3">
-        <div>
+        <div data-testid="adding-licenses-summary">
           <div className="fw-bold">{t('cost_summary')}</div>
           {subscriptionChange ? (
             <Trans

+ 125 - 0
services/web/frontend/js/shared/hooks/use-async-with-cancel.ts

@@ -0,0 +1,125 @@
+import * as React from 'react'
+import useSafeDispatch from './use-safe-dispatch'
+import { Nullable } from '../../../../types/utils'
+import { FetchError } from '../../infrastructure/fetch-json'
+
+type State<T, E> = {
+  status: 'idle' | 'pending' | 'resolved' | 'rejected'
+  data: Nullable<T>
+  error: Nullable<E>
+}
+type Action<T, E> = Partial<State<T, E>>
+type AsyncRunner<T> = (signal: AbortSignal) => Promise<T>
+
+const defaultInitialState: State<null, null> = {
+  status: 'idle',
+  data: null,
+  error: null,
+}
+
+const abortError = new Error('Aborted by the caller')
+abortError.name = 'AbortError'
+
+function useAsync<T = any, E extends Error | FetchError = Error>(
+  initialState?: Partial<State<T, E>>
+) {
+  const initialStateRef = React.useRef({
+    ...defaultInitialState,
+    ...initialState,
+  })
+
+  // Use a Set to track all active AbortController instances
+  const abortControllerSetRef = React.useRef<Set<AbortController>>(new Set())
+
+  const [{ status, data, error }, setState] = React.useReducer(
+    (state: State<T, E>, action: Action<T, E>) => ({ ...state, ...action }),
+    initialStateRef.current
+  )
+
+  const safeSetState = useSafeDispatch(setState)
+
+  const setData = React.useCallback(
+    (data: Nullable<T>) => safeSetState({ data, status: 'resolved' }),
+    [safeSetState]
+  )
+
+  const setError = React.useCallback(
+    (error: Nullable<E>) => safeSetState({ error, status: 'rejected' }),
+    [safeSetState]
+  )
+
+  const reset = React.useCallback(
+    () => safeSetState(initialStateRef.current),
+    [safeSetState]
+  )
+
+  const cancelAll = React.useCallback(() => {
+    // Abort all controllers in the set and clear it
+    abortControllerSetRef.current.forEach(controller => controller.abort())
+    abortControllerSetRef.current.clear()
+  }, [])
+
+  const runAsync = React.useCallback(
+    (asyncRunner: AsyncRunner<T>) => {
+      safeSetState({ status: 'pending' })
+
+      const controller = new AbortController()
+      abortControllerSetRef.current.add(controller)
+
+      // The original promise is now created using the provided factory function,
+      // which receives the signal for cancellation.
+      const promise = asyncRunner(controller.signal)
+
+      const abortPromise = new Promise<never>((_resolve, reject) => {
+        controller.signal.addEventListener('abort', () => {
+          reject(abortError)
+        })
+      })
+
+      return Promise.race([promise, abortPromise])
+        .then(
+          data => {
+            setData(data)
+            return data
+          },
+          error => {
+            if (error !== abortError) {
+              setError(error)
+            }
+            return Promise.reject(error)
+          }
+        )
+        .finally(() => {
+          // Remove the controller from the set, whether it succeeded or failed
+          abortControllerSetRef.current.delete(controller)
+        })
+    },
+    [safeSetState, setData, setError]
+  )
+
+  // Abort all requests when the component unmounts to prevent memory leaks
+  React.useEffect(() => {
+    return () => {
+      cancelAll()
+    }
+  }, [cancelAll])
+
+  return {
+    isIdle: status === 'idle',
+    isLoading: status === 'pending',
+    isError: status === 'rejected',
+    isSuccess: status === 'resolved',
+    setData,
+    setError,
+    error,
+    status,
+    data,
+    runAsync,
+    reset,
+    cancelAll,
+  }
+}
+
+export default useAsync
+export type UseAsyncReturnType = ReturnType<typeof useAsync>
+export { useAsync, abortError }

+ 35 - 0
services/web/test/frontend/features/group-management/components/add-seats.spec.tsx

@@ -3,6 +3,7 @@ import AddSeats, {
   MAX_NUMBER_OF_PO_NUMBER_CHARACTERS,
 } from '@/features/group-management/components/add-seats/add-seats'
 import { SplitTestProvider } from '@/shared/context/split-test-context'
+import { cloneDeep } from 'lodash'
 
 describe('<AddSeats />', function () {
   beforeEach(function () {
@@ -409,6 +410,40 @@ describe('<AddSeats />', function () {
         })
       })
 
+      it('handles double digit numbers of licenses gracefully', function () {
+        const { promise, resolve } = Promise.withResolvers<void>()
+        const body = cloneDeep(this.body)
+        cy.intercept(
+          'POST',
+          '/user/subscription/group/add-users/preview',
+          async req => {
+            await promise
+            // make the response reflect back whatever quantity was sent in the request
+            // we don't really care about the rest of the body for this test
+            const { adding } = req.body
+            body.change.addOn.quantity = body.change.addOn.prevQuantity + adding
+            req.reply({
+              statusCode: 200,
+              body,
+            })
+          }
+        ).as('addUsersRequest')
+
+        cy.get('@input').type('1')
+        cy.get('@input').type('2')
+        resolve()
+
+        cy.findByTestId('adding-licenses-summary').within(() => {
+          cy.findByText((_, el) =>
+            Boolean(
+              el?.textContent?.includes(
+                'You’re adding 12 licenses to your plan giving you a total of 17 licenses'
+              )
+            )
+          )
+        })
+      })
+
       describe('request', function () {
         afterEach(function () {
           cy.findByRole('link', { name: /go to subscriptions/i }).should(