fix(settings): update ConnectedServices test for current API

packages/fxa-settings/src/components/Settings/ConnectedServices/index.test.tsx

@@ -9,6 +9,7 @@ import {
   Account,
   AlertBarInfo,
   AppContext,
+  AttachedClient,
   OAuthNativeClients,
 } from '../../../models';
 import {
@@ -158,21 +159,55 @@ describe('Connected Services', () => {
       expect(result[result.length - 1]).toHaveTextContent('6 months ago');
     });
 
-    const { sortedAndUniqueClients, groupedByName } =
+    const { sortedAndUniqueClients } =
       sortAndFilterConnectedClients(MOCK_SERVICES);
 
-    expect(sortedAndUniqueClients.length).toEqual(14);
-
+    // Verify 'Mozilla Monitor' is grouped (since both have deviceId: null)
     expect(
       sortedAndUniqueClients.filter((item) => item.name === 'Mozilla Monitor')
         .length
     ).toEqual(1);
-    expect(
-      sortedAndUniqueClients.filter(
-        (item) => item.name === 'Mozilla Monitor'
-      )[0].lastAccessTime
-    ).toEqual(1570736983000);
-    expect(groupedByName['Mozilla Monitor'].length).toEqual(2);
+
+    // Test grouping by deviceId: two devices with same name but different deviceId
+    // should NOT be merged.
+    const sameNameDifferentDevice = [
+      {
+        ...MOCK_SERVICES[0],
+        name: 'Same Name',
+        deviceId: 'device-1',
+        lastAccessTime: 1000,
+      },
+      {
+        ...MOCK_SERVICES[0],
+        name: 'Same Name',
+        deviceId: 'device-2',
+        lastAccessTime: 2000,
+      },
+    ];
+    const { sortedAndUniqueClients: groupedByDeviceResult } =
+      sortAndFilterConnectedClients(sameNameDifferentDevice as any);
+    expect(groupedByDeviceResult.length).toEqual(2);
+
+    // Test grouping by name: two devices with same name and null deviceId
+    // SHOULD be merged.
+    const sameNameNoDevice = [
+      {
+        ...MOCK_SERVICES[0],
+        name: 'Same Name',
+        deviceId: null,
+        lastAccessTime: 1000,
+      },
+      {
+        ...MOCK_SERVICES[0],
+        name: 'Same Name',
+        deviceId: null,
+        lastAccessTime: 2000,
+      },
+    ];
+    const { sortedAndUniqueClients: groupedByNameResult } =
+      sortAndFilterConnectedClients(sameNameNoDevice as any);
+    expect(groupedByNameResult.length).toEqual(1);
+    expect(groupedByNameResult[0].lastAccessTime).toEqual(2000);
   });
 
   it('should show the monitor icon and link', async () => {
@@ -610,28 +645,14 @@ describe('Connected Services', () => {
   });
 
   describe('scope-based sub row', () => {
-    const baseMockClient = {
-      deviceId: null,
-      sessionTokenId: null,
-      refreshTokenId: 'abc123',
-      isCurrentSession: false,
-      deviceType: null,
-      createdTime: 1571412069000,
-      lastAccessTime: 1571412069000,
-      location: { city: null, country: null, state: null, stateCode: null },
-      userAgent: '',
-      os: null,
-      createdTimeFormatted: 'a month ago',
-      lastAccessTimeFormatted: 'a month ago',
-      approximateLastAccessTime: null,
-      approximateLastAccessTimeFormatted: null,
-    };
+    const baseMockClient = MOCK_SERVICES[0];
 
-    const renderWithClient = (client: Record<string, unknown>) => {
+    const renderWithClient = (client: AttachedClient) => {
       const account = {
         attachedClients: [client],
         disconnectClient: jest.fn().mockResolvedValue(true),
       } as unknown as Account;
+
       renderWithRouter(
         <AppContext.Provider value={mockAppContext({ account })}>
           <ConnectedServices />
@@ -661,7 +682,7 @@ describe('Connected Services', () => {
       expect(screen.queryByTestId('scope-service')).not.toBeInTheDocument();
     });
 
-    it('renders Relay scope sub-entry when scope includes relay and client ID is OAuthNative', async () => {
+    it('renders scope sub-entry when scope includes relay and client ID is OAuthNative', async () => {
       renderWithClient({
         ...baseMockClient,
         clientId: OAuthNativeClients.FirefoxIOS,

packages/fxa-settings/src/components/Settings/ConnectedServices/index.tsx

@@ -2,12 +2,14 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+import React from 'react';
+import { ApolloError } from '@apollo/client';
 import { Localized, useLocalization } from '@fluent/react';
 import { LinkExternal } from 'fxa-react/components/LinkExternal';
 import { useBooleanState } from 'fxa-react/lib/hooks';
 import groupBy from 'lodash.groupby';
 import { forwardRef, useCallback, useState } from 'react';
-import { clearSignedInAccountUid, setSigningOut } from '../../../lib/cache';
+import { clearSignedInAccountUid } from '../../../lib/cache';
 import { logViewEvent } from '../../../lib/metrics';
 import { isMobileDevice } from '../../../lib/utilities';
 import { AttachedClient, useAccount, useAlertBar } from '../../../models';
@@ -27,12 +29,14 @@ const DEVICES_SUPPORT_URL =
 export function sortAndFilterConnectedClients(
   attachedClients: Array<AttachedClient>
 ) {
-  const groupedByName = groupBy(attachedClients, 'name');
+  // Group clients by deviceId (for sync devices) or name (for others).
+  // This avoids merging distinct devices that happen to share the same name.
+  const groupedByDevice = groupBy(attachedClients, (c) => c.deviceId || c.name);
 
-  // get a unique (by name) list and sort by time last accessed
-  const sortedAndUniqueClients = Object.keys(groupedByName)
+  // get a unique (by device or name) list and sort by time last accessed
+  const sortedAndUniqueClients = Object.keys(groupedByDevice)
     .map((key) => {
-      return groupedByName[key].sort(
+      return groupedByDevice[key].sort(
         (a: AttachedClient, b: AttachedClient) =>
           b.lastAccessTime - a.lastAccessTime
       )[0];
@@ -47,14 +51,14 @@ export function sortAndFilterConnectedClients(
     }
   });
 
-  return { groupedByName, sortedAndUniqueClients };
+  return { groupedByDevice, sortedAndUniqueClients };
 }
 
 export const ConnectedServices = forwardRef<HTMLDivElement>((_, ref) => {
   const alertBar = useAlertBar();
   const account = useAccount();
   const attachedClients = account.attachedClients;
-  const { groupedByName, sortedAndUniqueClients } =
+  const { groupedByDevice, sortedAndUniqueClients } =
     sortAndFilterConnectedClients([...attachedClients]);
 
   const showMobilePromo = !sortedAndUniqueClients.filter(isMobileDevice).length;
@@ -82,7 +86,7 @@ export const ConnectedServices = forwardRef<HTMLDivElement>((_, ref) => {
   const [isRefreshingClients, setIsRefreshingClients] = useState(false);
 
   const clearDisconnectingState = useCallback(
-    (errorMessage?: string, error?: Error) => {
+    (errorMessage?: string, error?: ApolloError | Error) => {
       hideConfirmDisconnectModal();
       setSelectedClient(null);
       setReason('');
@@ -102,17 +106,15 @@ export const ConnectedServices = forwardRef<HTMLDivElement>((_, ref) => {
           event: { reason: reasonValue },
         });
 
-        // disconnect all clients/sessions with this name since only unique names
-        // are displayed to the user. This is batched into one network request
-        // via BatchHttpLink
-        const groupByKey = client.name ?? 'undefined';
-        const clientsWithMatchingName = groupedByName[groupByKey];
-        const hasMultipleSessions = clientsWithMatchingName.length > 1;
+        // Disconnect all clients/sessions in the group (same deviceId or name).
+        // Since we only display one entry per group, signing out of that entry
+        // should revoke all associated sessions. This is batched via BatchHttpLink.
+        const groupByKey = (client.deviceId || client.name) ?? 'undefined';
+        const sessionsInGroup = groupedByDevice[groupByKey];
+        const hasMultipleSessions = sessionsInGroup.length > 1;
         if (hasMultipleSessions) {
           await Promise.all(
-            clientsWithMatchingName.map(
-              async (c) => await account.disconnectClient(c)
-            )
+            sessionsInGroup.map(async (c) => await account.disconnectClient(c))
           );
         } else {
           await account.disconnectClient(client);
@@ -123,9 +125,8 @@ export const ConnectedServices = forwardRef<HTMLDivElement>((_, ref) => {
         if (
           client.isCurrentSession ||
           (hasMultipleSessions &&
-            clientsWithMatchingName.find((c) => c.isCurrentSession))
+            sessionsInGroup.find((c) => c.isCurrentSession))
         ) {
-          setSigningOut(true);
           clearSignedInAccountUid();
           window.location.assign(`${window.location.origin}/signin`);
         } else if (reason === 'suspicious' || reason === 'lost') {
@@ -150,7 +151,7 @@ export const ConnectedServices = forwardRef<HTMLDivElement>((_, ref) => {
     [
       account,
       hideConfirmDisconnectModal,
-      groupedByName,
+      groupedByDevice,
       revealAdviceModal,
       alertBar,
       l10n,