fix(settings): Fix avatar deletion failing

Because:
* After an avatar upload, if a user refreshes or just goes back to Settings at a different point in time, we are hitting /v/profile/default to delete the avatar instead of sending the avatar ID, causing a 400 but giving the usual visual feedback that the avatar has been removed

This commit:
* Fetches v1/avatar alongside v1/profile to get the current avatar ID
* Falls back to null instead of 'default' when no avatar ID is available
* Adjusts the functional test to account for this

closes FXA-13300

Author: LZoog

packages/functional-tests/tests/settings/avatar.spec.ts

@@ -73,6 +73,12 @@ test.describe('severity-1 #smoke', () => {
       /avatar/
     );
 
+    // Refresh to simulate returning to the page later, which causes the
+    // avatar ID to be lost (the /v1/profile endpoint doesn't return it)
+    await page.reload();
+    await expect(settings.settingsHeading).toBeVisible();
+    await expect(settings.avatar.photo).not.toHaveAttribute('src', /avatar/);
+
     await settings.avatar.changeButton.click();
 
     await expect(avatar.profilePictureHeading).toBeVisible();
@@ -86,6 +92,15 @@ test.describe('severity-1 #smoke', () => {
       'src',
       /avatar/
     );
+
+    // Verify removal persists after refresh
+    await page.reload();
+    await expect(settings.settingsHeading).toBeVisible();
+    await expect(settings.avatar.photo).toHaveAttribute('src', /avatar/);
+    await expect(settings.avatarDropDownMenuPhoto).toHaveAttribute(
+      'src',
+      /avatar/
+    );
   });
 });
 

packages/fxa-settings/src/lib/hooks/useAccountData.ts

@@ -11,7 +11,12 @@ import {
   RecoveryPhoneStatus,
   useAccountState,
 } from '../../models/contexts/AccountStateContext';
-import { Email, LinkedAccount, SecurityEvent, mapAttachedClient } from '../../models/Account';
+import {
+  Email,
+  LinkedAccount,
+  SecurityEvent,
+  mapAttachedClient,
+} from '../../models/Account';
 import { AccountTotp, AccountBackupCodes, AccountAvatar } from '../interfaces';
 import config from '../config';
 import { ERRNO } from '@fxa/accounts/errors';
@@ -60,21 +65,40 @@ interface AccountDataResult {
 /** Shape returned by the consolidated /account auth-server endpoint. */
 interface AccountResponse {
   emails?: Array<{ email: string; isPrimary: boolean; verified: boolean }>;
-  linkedAccounts?: Array<{ providerId: number; authAt: number; enabled: boolean }>;
-  subscriptions?: Array<{ created?: number; createdAt?: number; productName?: string; product_name?: string }>;
+  linkedAccounts?: Array<{
+    providerId: number;
+    authAt: number;
+    enabled: boolean;
+  }>;
+  subscriptions?: Array<{
+    created?: number;
+    createdAt?: number;
+    productName?: string;
+    product_name?: string;
+  }>;
   totp?: { exists?: boolean; verified?: boolean };
   backupCodes?: { hasBackupCodes?: boolean; count?: number };
   recoveryKey?: { exists?: boolean; estimatedSyncDeviceCount?: number };
-  recoveryPhone?: { exists?: boolean; phoneNumber?: string; available?: boolean };
-  securityEvents?: Array<{ name: string; createdAt: number; verified?: boolean }>;
+  recoveryPhone?: {
+    exists?: boolean;
+    phoneNumber?: string;
+    available?: boolean;
+  };
+  securityEvents?: Array<{
+    name: string;
+    createdAt: number;
+    verified?: boolean;
+  }>;
   metricsOptOutAt?: number | null;
   createdAt?: number;
   passwordCreatedAt?: number;
   hasPassword?: boolean;
 }
 
 /** Transform the consolidated /account endpoint response to AccountState format. */
-function transformAccountResponse(response: AccountResponse): Partial<AccountState> {
+function transformAccountResponse(
+  response: AccountResponse
+): Partial<AccountState> {
   const emails: Email[] = (response.emails || []).map((e) => ({
     email: e.email,
     isPrimary: e.isPrimary,
@@ -152,19 +176,22 @@ async function fetchProfileData(
       { scope: 'profile', ttl: PROFILE_OAUTH_TOKEN_TTL_SECONDS }
     );
 
-    const response = await fetch(`${config.servers.profile.url}/v1/profile`, {
-      headers: { Authorization: `Bearer ${access_token}` },
-    });
+    const headers = { Authorization: `Bearer ${access_token}` };
+    const [profileResponse, avatarResponse] = await Promise.all([
+      fetch(`${config.servers.profile.url}/v1/profile`, { headers }),
+      fetch(`${config.servers.profile.url}/v1/avatar`, { headers }),
+    ]);
 
-    if (!response.ok) {
+    if (!profileResponse.ok) {
       return { displayName: null, avatar: null };
     }
 
-    const profile = await response.json();
+    const profile = await profileResponse.json();
+    const avatarData = avatarResponse.ok ? await avatarResponse.json() : null;
     return {
       displayName: profile.displayName || null,
       avatar: profile.avatar
-        ? { id: profile.avatarId || 'default', url: profile.avatar }
+        ? { id: avatarData?.id || null, url: profile.avatar }
         : null,
     };
   } catch (error) {
@@ -192,10 +219,7 @@ export function useAccountData({
   authClientRef.current = authClient;
 
   const accountState = useAccountState();
-  const {
-    setAccountData,
-    ...stateData
-  } = accountState;
+  const { setAccountData, ...stateData } = accountState;
 
   const [localLoading, setLocalLoading] = useState(true);
   const [localError, setLocalError] = useState<Error | null>(null);
@@ -222,32 +246,45 @@ export function useAccountData({
         ]);
 
       // Check for invalid token errors - user needs to sign in again
-      const rejectedResults = [accountResult, profileResult, attachedClientsResult]
-        .filter((r): r is PromiseRejectedResult => r.status === 'rejected');
+      const rejectedResults = [
+        accountResult,
+        profileResult,
+        attachedClientsResult,
+      ].filter((r): r is PromiseRejectedResult => r.status === 'rejected');
       if (rejectedResults.some((r) => isInvalidTokenError(r.reason))) {
         throw new InvalidTokenError();
       }
 
       let accountData: Partial<AccountState> = {};
 
       if (accountResult.status === 'fulfilled') {
-        accountData = { ...accountData, ...transformAccountResponse(accountResult.value) };
+        accountData = {
+          ...accountData,
+          ...transformAccountResponse(accountResult.value),
+        };
       } else {
-        Sentry.captureMessage(`Failed to fetch account: ${accountResult.reason}`);
+        Sentry.captureMessage(
+          `Failed to fetch account: ${accountResult.reason}`
+        );
       }
 
       if (profileResult.status === 'fulfilled') {
         const { displayName, avatar } = profileResult.value;
         if (displayName !== null) accountData.displayName = displayName;
         if (avatar !== null) accountData.avatar = avatar;
       } else {
-        Sentry.captureMessage(`Failed to fetch profile: ${profileResult.reason}`);
+        Sentry.captureMessage(
+          `Failed to fetch profile: ${profileResult.reason}`
+        );
       }
 
       if (attachedClientsResult.status === 'fulfilled') {
-        accountData.attachedClients = attachedClientsResult.value.map(mapAttachedClient);
+        accountData.attachedClients =
+          attachedClientsResult.value.map(mapAttachedClient);
       } else {
-        Sentry.captureMessage(`Failed to fetch attached clients: ${attachedClientsResult.reason}`);
+        Sentry.captureMessage(
+          `Failed to fetch attached clients: ${attachedClientsResult.reason}`
+        );
         accountData.attachedClients = [];
       }
 
@@ -278,7 +315,10 @@ export function useAccountData({
           }
           case 'displayName':
           case 'avatar': {
-            const { displayName, avatar } = await fetchProfileData(client, token);
+            const { displayName, avatar } = await fetchProfileData(
+              client,
+              token
+            );
             if (displayName !== null) fieldData.displayName = displayName;
             if (avatar !== null) fieldData.avatar = avatar;
             break;
@@ -303,7 +343,11 @@ export function useAccountData({
   }, [fetchAccountData]);
 
   return {
-    data: { ...stateData, isLoading: localLoading, error: localError } as AccountState,
+    data: {
+      ...stateData,
+      isLoading: localLoading,
+      error: localError,
+    } as AccountState,
     isLoading: localLoading,
     error: localError,
     refetch: fetchAccountData,