refactor(mosaic): adopt usePendingAction in logo and enterprise accounts controllers (#10517)

Author: alexcarpenter

.changeset/mosaic-pending-action-adoption.md

@@ -0,0 +1,2 @@
+---
+---

packages/mosaic/src/features/organization-profile/organization-profile-profile-section/organization-profile-logo-row.view.tsx

@@ -43,7 +43,7 @@ export function OrganizationProfileLogoRowView({
   const errorText = useErrorText();
   const logo = useOrganizationProfileLogoController({ onChange, onRemove });
   const [rejection, setRejection] = useState<LocalizableError>();
-  const error = rejection ?? logo.error;
+  const error = rejection ? errorText(rejection) : logo.error;
   const remove = logo.onRemove;
   const handleRemove = remove
     ? () => {
@@ -101,7 +101,7 @@ export function OrganizationProfileLogoRowView({
           onRemove={handleRemove}
         />
       </Section.Item>
-      <Section.Error>{error ? errorText(error) : undefined}</Section.Error>
+      <Section.Error>{error}</Section.Error>
     </FileUpload.Root>
   );
 }
@@ -113,7 +113,7 @@ function LogoActions({
 }: {
   hasImage: boolean;
   canChange: boolean;
-  onRemove?: () => Promise<void>;
+  onRemove?: () => Promise<unknown>;
 }) {
   const m = useMessages('organizationProfileProfileSection');
   const { openFilePicker } = FileUpload.useFileUpload();

packages/mosaic/src/features/organization-profile/organization-profile-profile-section/organization-profile-logo.controller.ts

@@ -1,70 +1,51 @@
-import { useRef, useState } from 'react';
+import { useState } from 'react';
 
-import type { LocalizableError } from '../../../localization';
+import { usePendingAction } from '../../../hooks/use-pending-action';
 import { FileUpload } from '../../../primitives/file-upload';
-import { toGlobalError } from '../../../utils/errors';
 
 interface OrganizationProfileLogoControllerOptions {
   onChange?: (file: File) => Promise<void>;
   onRemove?: () => Promise<void>;
 }
 
 interface OrganizationProfileLogoController {
-  onChange?: (file: File) => Promise<void>;
-  onRemove?: () => Promise<void>;
+  onChange?: (file: File) => Promise<boolean>;
+  onRemove?: () => Promise<boolean>;
   isPending: boolean;
   previewUrl: string | undefined;
-  error: LocalizableError | undefined;
+  error: string | undefined;
 }
 
 export function useOrganizationProfileLogoController({
   onChange,
   onRemove,
 }: OrganizationProfileLogoControllerOptions): OrganizationProfileLogoController {
-  const [isPending, setIsPending] = useState(false);
-  const [error, setError] = useState<LocalizableError>();
+  const actions = usePendingAction<'change' | 'remove'>();
   const [preview, setPreview] = useState<File>();
   const previewUrl = FileUpload.useObjectUrl(preview);
-  const inFlight = useRef(false);
-
-  const run = async (action: () => Promise<void>, revert?: () => void) => {
-    if (inFlight.current) {
-      return;
-    }
-    inFlight.current = true;
-    setIsPending(true);
-    setError(undefined);
-    try {
-      await action();
-    } catch (cause) {
-      revert?.();
-      setError(toGlobalError(cause));
-    } finally {
-      inFlight.current = false;
-      setIsPending(false);
-    }
-  };
 
   return {
     onChange: onChange
-      ? file =>
-          run(
-            () => {
-              setPreview(file);
-              return onChange(file);
-            },
-            () => setPreview(undefined),
-          )
+      ? async file => {
+          const changed = await actions.run('change', () => {
+            setPreview(file);
+            return onChange(file);
+          });
+          if (!changed) {
+            setPreview(current => (current === file ? undefined : current));
+          }
+          return changed;
+        }
       : undefined,
     onRemove: onRemove
       ? () =>
-          run(async () => {
+          actions.run('remove', async () => {
             await onRemove();
             setPreview(undefined);
           })
       : undefined,
-    isPending,
+    isPending: actions.isPending,
     previewUrl,
-    error,
+    error: actions.error,
   };
 }

packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.controller.ts

@@ -9,6 +9,7 @@ import { useListRemovalFocus } from '../../../hooks/use-list-removal-focus';
 import { usePendingAction } from '../../../hooks/use-pending-action';
 import { useMessages } from '../../../localization';
 import type { ReverificationController } from '../../reverification';
+import { REDIRECT_HOLD_MS } from '../user-profile.utils';
 import type {
   ConnectedAccountActionResult,
   UserProfileConnectedAccount,
@@ -42,8 +43,6 @@ export interface UserProfileConnectedAccountsController {
   connectFinalFocus: DialogFocusTarget;
 }
 
-const OAUTH_REDIRECT_HOLD_MS = 2000;
-
 export function useUserProfileConnectedAccountsController({
   accounts,
   availableProviders,
@@ -74,7 +73,7 @@ export function useUserProfileConnectedAccountsController({
   const run = (id: string, action: (id: string) => Promise<ConnectedAccountActionResult>) =>
     void actions.run(id, async () => {
       if ((await action(id)) === 'redirecting') {
-        await new Promise(resolve => setTimeout(resolve, OAUTH_REDIRECT_HOLD_MS));
+        await new Promise(resolve => setTimeout(resolve, REDIRECT_HOLD_MS));
       }
     });
 

packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/__tests__/user-profile-enterprise-accounts.feature.test.tsx

@@ -354,6 +354,20 @@ describe('enterprise accounts', () => {
     retry.fail();
   });
 
+  it('clears the previous connection error when another connection starts', async () => {
+    await renderSection();
+    const user = userEvent.setup();
+    const request = holdRequests('post', '/v1/me/external_accounts');
+    await user.click(await screen.findByRole('button', { name: 'Connect Acme Okta' }));
+    request.fail('enterprise_error', 'Acme is unavailable.');
+    await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Acme is unavailable.'));
+    const other = holdRequests('post', '/v1/me/external_accounts');
+    await user.click(screen.getByRole('button', { name: 'Connect Custom SAML' }));
+    await waitFor(() => expect(other.requests).toHaveLength(1));
+    expect(screen.queryByText('Acme is unavailable.')).toBeNull();
+    other.fail();
+  });
+
   it('releases pending after the redirect grace period', async () => {
     const { clerk } = await renderSection();
     const navigate = vi.spyOn(clerk, '__internal_windowNavigate').mockImplementation(() => {});

packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.controller.ts

@@ -1,10 +1,6 @@
-import { isReverificationCancelledError } from '@clerk/shared/error';
-import { useRef, useState } from 'react';
-
-import type { ErrorDescription } from '../../../localization';
-import { useErrorText } from '../../../localization';
-import { toLocalizableError } from '../../../utils/errors';
+import { usePendingAction } from '../../../hooks/use-pending-action';
 import type { ReverificationController } from '../../reverification';
+import { REDIRECT_HOLD_MS } from '../user-profile.utils';
 import type {
   EnterpriseAccountActionResult,
   UserProfileEnterpriseAccount,
@@ -34,42 +30,20 @@ export function useUserProfileEnterpriseAccountsController({
   connectReverification,
   errorFallback,
 }: UserProfileEnterpriseAccountsControllerOptions): UserProfileEnterpriseAccountsController {
-  const [pendingId, setPendingId] = useState<string>();
-  const errorText = useErrorText();
-  const [connectErrors, setConnectErrors] = useState<Record<string, ErrorDescription>>({});
-  const connecting = useRef(false);
-
-  const run = async (id: string) => {
-    if (connecting.current) {
-      return;
-    }
-    connecting.current = true;
-    setPendingId(id);
-    setConnectErrors(({ [id]: _cleared, ...rest }) => rest);
-
-    try {
-      if ((await onConnect(id)) === 'redirecting') {
-        await new Promise(resolve => setTimeout(resolve, 2000));
-      }
-    } catch (error) {
-      if (isReverificationCancelledError(error)) {
-        return;
-      }
-      setConnectErrors(current => ({ ...current, [id]: toLocalizableError(error) }));
-    } finally {
-      connecting.current = false;
-      setPendingId(undefined);
-    }
-  };
+  const actions = usePendingAction({ errorFallback });
 
   return {
     accounts,
-    connections: connections.map(connection => {
-      const error = connectErrors[connection.id];
-      return error ? { ...connection, connectError: errorText(error, errorFallback) } : connection;
-    }),
-    pendingId,
-    onConnect: id => void run(id),
+    connections: connections.map(connection =>
+      actions.errorKey === connection.id ? { ...connection, connectError: actions.error } : connection,
+    ),
+    pendingId: actions.pendingKey,
+    onConnect: id =>
+      void actions.run(id, async () => {
+        if ((await onConnect(id)) === 'redirecting') {
+          await new Promise(resolve => setTimeout(resolve, REDIRECT_HOLD_MS));
+        }
+      }),
     connectReverification,
   };
 }

packages/mosaic/src/features/user-profile/user-profile.utils.ts

@@ -3,3 +3,5 @@ import type { AttributeData } from '@clerk/shared/types';
 export function isAttributeAvailable(attribute: AttributeData | undefined): boolean {
   return Boolean(attribute?.enabled || attribute?.used_for_first_factor || attribute?.used_for_second_factor);
 }
+
+export const REDIRECT_HOLD_MS = 2000;