Merge pull request #25416 from honnamkuan/23364

Expensify · Sep 22, 2023 · f2405de · f2405de
2 parents a240941 + 4d93923
commit f2405de
Show file tree

Hide file tree

Showing 2 changed files with 90 additions and 10 deletions.
diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js
@@ -947,23 +947,31 @@ function getIconsForParticipants(participants, personalDetails) {
     for (let i = 0; i < participantsList.length; i++) {
         const accountID = participantsList[i];
         const avatarSource = UserUtils.getAvatar(lodashGet(personalDetails, [accountID, 'avatar'], ''), accountID);
-        participantDetails.push([
-            accountID,
-            lodashGet(personalDetails, [accountID, 'displayName']) || lodashGet(personalDetails, [accountID, 'login'], ''),
-            lodashGet(personalDetails, [accountID, 'firstName'], ''),
-            avatarSource,
-        ]);
+        const displayNameLogin = lodashGet(personalDetails, [accountID, 'displayName']) || lodashGet(personalDetails, [accountID, 'login'], '');
+        participantDetails.push([accountID, displayNameLogin, avatarSource]);
     }
 
-    // Sort all logins by first name (which is the second element in the array)
-    const sortedParticipantDetails = participantDetails.sort((a, b) => a[2] - b[2]);
+    const sortedParticipantDetails = _.chain(participantDetails)
+        .sort((first, second) => {
+            // First sort by displayName/login
+            const displayNameLoginOrder = first[1].localeCompare(second[1]);
+            if (displayNameLoginOrder !== 0) {
+                return displayNameLoginOrder;
+            }
+
+            // Then fallback on accountID as the final sorting criteria.
+            // This will ensure that the order of avatars with same login/displayName
+            // stay consistent across all users and devices
+            return first[0] > second[0];
+        })
+        .value();
 
-    // Now that things are sorted, gather only the avatars (third element in the array) and return those
+    // Now that things are sorted, gather only the avatars (second element in the array) and return those
     const avatars = [];
     for (let i = 0; i < sortedParticipantDetails.length; i++) {
         const userIcon = {
             id: sortedParticipantDetails[i][0],
-            source: sortedParticipantDetails[i][3],
+            source: sortedParticipantDetails[i][2],
             type: CONST.ICON_TYPE_AVATAR,
             name: sortedParticipantDetails[i][1],
         };

diff --git a/tests/unit/ReportUtilsTest.js b/tests/unit/ReportUtilsTest.js
@@ -35,6 +35,13 @@ const participantsPersonalDetails = {
         login: '+18332403627@expensify.sms',
         displayName: '(833) 240-3627',
     },
+    5: {
+        accountID: 5,
+        displayName: 'Lagertha Lothbrok',
+        firstName: 'Lagertha',
+        login: 'lagertha2@vikings.net',
+        pronouns: 'She/her',
+    },
 };
 const policy = {
     policyID: 1,
@@ -55,6 +62,53 @@ describe('ReportUtils', () => {
     });
     beforeEach(() => Onyx.set(ONYXKEYS.NVP_PREFERRED_LOCALE, CONST.LOCALES.DEFAULT).then(waitForBatchedUpdates));
 
+    describe('getIconsForParticipants', () => {
+        it('returns sorted avatar source by name, then accountID', () => {
+            expect(ReportUtils.getIconsForParticipants([1, 2, 3, 4, 5], participantsPersonalDetails)).toEqual([
+                {
+                    id: 4,
+                    name: '(833) 240-3627',
+                    source: {
+                        testUri: '../../../assets/images/avatars/user/default-avatar_5.svg',
+                    },
+                    type: 'avatar',
+                },
+                {
+                    id: 2,
+                    name: 'floki@vikings.net',
+                    source: {
+                        testUri: '../../../assets/images/avatars/user/default-avatar_3.svg',
+                    },
+                    type: 'avatar',
+                },
+                {
+                    id: 3,
+                    name: 'Lagertha Lothbrok',
+                    source: {
+                        testUri: '../../../assets/images/avatars/user/default-avatar_4.svg',
+                    },
+                    type: 'avatar',
+                },
+                {
+                    id: 5,
+                    name: 'Lagertha Lothbrok',
+                    source: {
+                        testUri: '../../../assets/images/avatars/user/default-avatar_6.svg',
+                    },
+                    type: 'avatar',
+                },
+                {
+                    id: 1,
+                    name: 'Ragnar Lothbrok',
+                    source: {
+                        testUri: '../../../assets/images/avatars/user/default-avatar_2.svg',
+                    },
+                    type: 'avatar',
+                },
+            ]);
+        });
+    });
+
     describe('getDisplayNamesWithTooltips', () => {
         test('withSingleParticipantReport', () => {
             expect(ReportUtils.getDisplayNamesWithTooltips(participantsPersonalDetails, false)).toStrictEqual([
@@ -94,6 +148,15 @@ describe('ReportUtils', () => {
                     accountID: 4,
                     pronouns: undefined,
                 },
+                {
+                    displayName: 'Lagertha Lothbrok',
+                    avatar: {
+                        testUri: '../../../assets/images/avatars/user/default-avatar_6.svg',
+                    },
+                    login: 'lagertha2@vikings.net',
+                    accountID: 5,
+                    pronouns: 'She/her',
+                },
             ]);
         });
 
@@ -135,6 +198,15 @@ describe('ReportUtils', () => {
                     accountID: 4,
                     pronouns: undefined,
                 },
+                {
+                    displayName: 'Lagertha',
+                    avatar: {
+                        testUri: '../../../assets/images/avatars/user/default-avatar_6.svg',
+                    },
+                    login: 'lagertha2@vikings.net',
+                    accountID: 5,
+                    pronouns: 'She/her',
+                },
             ]);
         });
     });