From d211ac545a6c3f35cb9ff954c2b8a06249d18a40 Mon Sep 17 00:00:00 2001 From: Youssef Lourayad Date: Thu, 13 Apr 2023 22:38:43 +0000 Subject: [PATCH 1/7] Sort participants in report participants page --- src/pages/ReportParticipantsPage.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/ReportParticipantsPage.js b/src/pages/ReportParticipantsPage.js index 34189f347ef3..8dc40db0398f 100755 --- a/src/pages/ReportParticipantsPage.js +++ b/src/pages/ReportParticipantsPage.js @@ -57,7 +57,7 @@ const defaultProps = { const getAllParticipants = (report, personalDetails) => { const {participants} = report; - return _.map(participants, (login) => { + return _.chain(participants).sort().map((login) => { const userLogin = Str.removeSMSDomain(login); const userPersonalDetail = lodashGet(personalDetails, login, {displayName: userLogin, avatar: ''}); @@ -74,7 +74,7 @@ const getAllParticipants = (report, personalDetails) => { text: userPersonalDetail.displayName, tooltipText: userLogin, participantsList: [{login, displayName: userPersonalDetail.displayName}], - }); + }).value(); }); }; From c58bda76d19c827f253a2c6871c021c5be57f05b Mon Sep 17 00:00:00 2001 From: Youssef Lourayad Date: Fri, 28 Apr 2023 01:05:01 +0100 Subject: [PATCH 2/7] Bug fix --- src/pages/ReportParticipantsPage.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/ReportParticipantsPage.js b/src/pages/ReportParticipantsPage.js index 8dc40db0398f..2863ec7d1754 100755 --- a/src/pages/ReportParticipantsPage.js +++ b/src/pages/ReportParticipantsPage.js @@ -74,8 +74,8 @@ const getAllParticipants = (report, personalDetails) => { text: userPersonalDetail.displayName, tooltipText: userLogin, participantsList: [{login, displayName: userPersonalDetail.displayName}], - }).value(); - }); + }); + }).value(); }; const ReportParticipantsPage = (props) => { From 4183a36aec159a6ebf670aa957ff0c9dd8ab9a19 Mon Sep 17 00:00:00 2001 From: Youssef Lourayad Date: Thu, 11 May 2023 10:11:37 +0100 Subject: [PATCH 3/7] Sort participants in workspace invite page alphabetically --- src/libs/OptionsListUtils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 66b38ab24e90..52fe8d25532f 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -618,7 +618,7 @@ function getOptions( if (sortPersonalDetailsByAlphaAsc) { // PersonalDetails should be ordered Alphabetically by default - https://github.com/Expensify/App/issues/8220#issuecomment-1104009435 - allPersonalDetailsOptions = lodashOrderBy(allPersonalDetailsOptions, [(personalDetail) => personalDetail.text && personalDetail.text.toLowerCase()], 'asc'); + allPersonalDetailsOptions = lodashOrderBy(allPersonalDetailsOptions, [(personalDetail) => personalDetail.displayName && personalDetail.displayName.toLowerCase()], 'asc'); } // Always exclude already selected options and the currently logged in user @@ -870,7 +870,7 @@ function getMemberInviteOptions(personalDetails, betas = [], searchValue = '', e searchInputValue: searchValue.trim(), includePersonalDetails: true, excludeLogins, - sortPersonalDetailsByAlphaAsc: false, + sortPersonalDetailsByAlphaAsc: true, }); } From 02cee7f42c953e7b74ab9b121d4ccf2a7ec7cae1 Mon Sep 17 00:00:00 2001 From: Youssef Lourayad Date: Thu, 11 May 2023 10:23:49 +0100 Subject: [PATCH 4/7] Fix test --- tests/unit/OptionsListUtilsTest.js | 50 +++++++++++++++--------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/tests/unit/OptionsListUtilsTest.js b/tests/unit/OptionsListUtilsTest.js index 2658115c81fa..0dbdf7cd9e3c 100644 --- a/tests/unit/OptionsListUtilsTest.js +++ b/tests/unit/OptionsListUtilsTest.js @@ -286,8 +286,8 @@ describe('OptionsListUtils', () => { // Value with latest lastVisibleActionCreated should be at the top. expect(results.recentReports.length).toBe(2); - expect(results.recentReports[0].text).toBe('Mister Fantastic'); - expect(results.recentReports[1].text).toBe('Mister Fantastic'); + expect(results.recentReports[0].displayName).toBe('Mister Fantastic'); + expect(results.recentReports[1].displayName).toBe('Mister Fantastic'); return waitForPromisesToResolve() .then(() => Onyx.set(ONYXKEYS.PERSONAL_DETAILS, PERSONAL_DETAILS_WITH_PERIODS)) @@ -297,7 +297,7 @@ describe('OptionsListUtils', () => { // Then we expect to have the personal detail with period filtered expect(results.recentReports.length).toBe(1); - expect(results.recentReports[0].text).toBe('The Flash'); + expect(results.recentReports[0].displayName).toBe('The Flash'); }); }); @@ -316,10 +316,10 @@ describe('OptionsListUtils', () => { expect(results.personalDetails.length).toBe(_.size(PERSONAL_DETAILS) - 1 - MAX_RECENT_REPORTS); // We should expect personal details sorted alphabetically - expect(results.personalDetails[0].text).toBe('Black Widow'); - expect(results.personalDetails[1].text).toBe('Invisible Woman'); - expect(results.personalDetails[2].text).toBe('Spider-Man'); - expect(results.personalDetails[3].text).toBe('The Incredible Hulk'); + expect(results.personalDetails[0].displayName).toBe('Black Widow'); + expect(results.personalDetails[1].displayName).toBe('Invisible Woman'); + expect(results.personalDetails[2].displayName).toBe('Spider-Man'); + expect(results.personalDetails[3].displayName).toBe('The Incredible Hulk'); // Then the result which has an existing report should also have the reportID attached const personalDetailWithExistingReport = _.find(results.personalDetails, (personalDetail) => personalDetail.login === 'peterparker@expensify.com'); @@ -329,10 +329,10 @@ describe('OptionsListUtils', () => { results = OptionsListUtils.getNewChatOptions([], PERSONAL_DETAILS, [], ''); // We should expect personal details sorted alphabetically - expect(results.personalDetails[0].text).toBe('Black Panther'); - expect(results.personalDetails[1].text).toBe('Black Widow'); - expect(results.personalDetails[2].text).toBe('Captain America'); - expect(results.personalDetails[3].text).toBe('Invisible Woman'); + expect(results.personalDetails[0].displayName).toBe('Black Panther'); + expect(results.personalDetails[1].displayName).toBe('Black Widow'); + expect(results.personalDetails[2].displayName).toBe('Captain America'); + expect(results.personalDetails[3].displayName).toBe('Invisible Woman'); // When we provide a search value that does not match any personal details results = OptionsListUtils.getNewChatOptions(REPORTS, PERSONAL_DETAILS, [], 'magneto'); @@ -346,7 +346,7 @@ describe('OptionsListUtils', () => { // Then one recentReports will be returned and it will be the correct option // personalDetails should be empty array expect(results.recentReports.length).toBe(1); - expect(results.recentReports[0].text).toBe('Spider-Man'); + expect(results.recentReports[0].displayName).toBe('Spider-Man'); expect(results.personalDetails.length).toBe(0); // When we provide a search value that matches a partial display name or email @@ -358,9 +358,9 @@ describe('OptionsListUtils', () => { expect(results.personalDetails.length).toBe(4); expect(results.recentReports.length).toBe(5); expect(results.personalDetails[0].login).toBe('natasharomanoff@expensify.com'); - expect(results.recentReports[0].text).toBe('Captain America'); - expect(results.recentReports[1].text).toBe('Mr Sinister'); - expect(results.recentReports[2].text).toBe('Black Panther'); + expect(results.recentReports[0].displayName).toBe('Captain America'); + expect(results.recentReports[1].displayName).toBe('Mr Sinister'); + expect(results.recentReports[2].displayName).toBe('Black Panther'); // Test for Concierge's existence in chat options results = OptionsListUtils.getNewChatOptions(REPORTS_WITH_CONCIERGE, PERSONAL_DETAILS_WITH_CONCIERGE); @@ -404,10 +404,10 @@ describe('OptionsListUtils', () => { expect(results.personalDetails.length).toBe(_.size(PERSONAL_DETAILS) - 6); // We should expect personal details sorted alphabetically - expect(results.personalDetails[0].text).toBe('Black Widow'); - expect(results.personalDetails[1].text).toBe('Invisible Woman'); - expect(results.personalDetails[2].text).toBe('Spider-Man'); - expect(results.personalDetails[3].text).toBe('The Incredible Hulk'); + expect(results.personalDetails[0].displayName).toBe('Black Widow'); + expect(results.personalDetails[1].displayName).toBe('Invisible Woman'); + expect(results.personalDetails[2].displayName).toBe('Spider-Man'); + expect(results.personalDetails[3].displayName).toBe('The Incredible Hulk'); // And none of our personalDetails should include any of the users with recent reports const reportLogins = _.map(results.recentReports, (reportOption) => reportOption.login); @@ -429,7 +429,7 @@ describe('OptionsListUtils', () => { // Then all single participant reports that match will show up in the recentReports array, Recently used contact should be at the top expect(results.recentReports.length).toBe(5); - expect(results.recentReports[0].text).toBe('Captain America'); + expect(results.recentReports[0].displayName).toBe('Captain America'); // And logins with no single participant reports will show up in personalDetails expect(results.personalDetails.length).toBe(4); @@ -589,10 +589,10 @@ describe('OptionsListUtils', () => { let results = OptionsListUtils.getMemberInviteOptions(PERSONAL_DETAILS, [], ''); // We should expect personal details PERSONAL_DETAILS order - expect(results.personalDetails[0].text).toBe('Mister Fantastic'); - expect(results.personalDetails[1].text).toBe('Spider-Man'); - expect(results.personalDetails[2].text).toBe('Black Panther'); - expect(results.personalDetails[3].text).toBe('Invisible Woman'); + expect(results.personalDetails[0].displayName).toBe('Mister Fantastic'); + expect(results.personalDetails[1].displayName).toBe('Spider-Man'); + expect(results.personalDetails[2].displayName).toBe('Black Panther'); + expect(results.personalDetails[3].displayName).toBe('Invisible Woman'); // When we provide a search value that does not match any personal details results = OptionsListUtils.getMemberInviteOptions(PERSONAL_DETAILS, [], 'magneto'); @@ -605,6 +605,6 @@ describe('OptionsListUtils', () => { // Then one personal should be in personalDetails list expect(results.personalDetails.length).toBe(1); - expect(results.personalDetails[0].text).toBe('Spider-Man'); + expect(results.personalDetails[0].displayName).toBe('Spider-Man'); }); }); From b25da6d6f96780b19af47a79f7597e4bbfc8d802 Mon Sep 17 00:00:00 2001 From: Youssef Lourayad Date: Tue, 16 May 2023 13:52:25 +0100 Subject: [PATCH 5/7] Fix test --- tests/unit/OptionsListUtilsTest.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/unit/OptionsListUtilsTest.js b/tests/unit/OptionsListUtilsTest.js index 0dbdf7cd9e3c..d4090a35f1dd 100644 --- a/tests/unit/OptionsListUtilsTest.js +++ b/tests/unit/OptionsListUtilsTest.js @@ -588,11 +588,11 @@ describe('OptionsListUtils', () => { // When we only pass personal details let results = OptionsListUtils.getMemberInviteOptions(PERSONAL_DETAILS, [], ''); - // We should expect personal details PERSONAL_DETAILS order - expect(results.personalDetails[0].displayName).toBe('Mister Fantastic'); - expect(results.personalDetails[1].displayName).toBe('Spider-Man'); - expect(results.personalDetails[2].displayName).toBe('Black Panther'); - expect(results.personalDetails[3].displayName).toBe('Invisible Woman'); + // We should expect personal details to be sorted alphabetically + expect(results.personalDetails[0].text).toBe('Black Panther'); + expect(results.personalDetails[1].text).toBe('Black Widow'); + expect(results.personalDetails[2].text).toBe('Captain America'); + expect(results.personalDetails[3].text).toBe('Invisible Woman'); // When we provide a search value that does not match any personal details results = OptionsListUtils.getMemberInviteOptions(PERSONAL_DETAILS, [], 'magneto'); @@ -605,6 +605,6 @@ describe('OptionsListUtils', () => { // Then one personal should be in personalDetails list expect(results.personalDetails.length).toBe(1); - expect(results.personalDetails[0].displayName).toBe('Spider-Man'); + expect(results.personalDetails[0].text).toBe('Spider-Man'); }); }); From 63a4463ce83f06b4657038aec8ddd0cd1769d282 Mon Sep 17 00:00:00 2001 From: Youssef Lourayad Date: Tue, 16 May 2023 14:21:33 +0100 Subject: [PATCH 6/7] Fix test --- src/libs/OptionsListUtils.js | 2 +- tests/unit/OptionsListUtilsTest.js | 40 +++++++++++++++--------------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 81822fdd6661..489bb4f4b120 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -630,7 +630,7 @@ function getOptions( if (sortPersonalDetailsByAlphaAsc) { // PersonalDetails should be ordered Alphabetically by default - https://github.com/Expensify/App/issues/8220#issuecomment-1104009435 - allPersonalDetailsOptions = lodashOrderBy(allPersonalDetailsOptions, [(personalDetail) => personalDetail.displayName && personalDetail.displayName.toLowerCase()], 'asc'); + allPersonalDetailsOptions = lodashOrderBy(allPersonalDetailsOptions, [(personalDetail) => personalDetail.text && personalDetail.text.toLowerCase()], 'asc'); } // Always exclude already selected options and the currently logged in user diff --git a/tests/unit/OptionsListUtilsTest.js b/tests/unit/OptionsListUtilsTest.js index d4090a35f1dd..7e6fb4146ca6 100644 --- a/tests/unit/OptionsListUtilsTest.js +++ b/tests/unit/OptionsListUtilsTest.js @@ -286,8 +286,8 @@ describe('OptionsListUtils', () => { // Value with latest lastVisibleActionCreated should be at the top. expect(results.recentReports.length).toBe(2); - expect(results.recentReports[0].displayName).toBe('Mister Fantastic'); - expect(results.recentReports[1].displayName).toBe('Mister Fantastic'); + expect(results.recentReports[0].text).toBe('Mister Fantastic'); + expect(results.recentReports[1].text).toBe('Mister Fantastic'); return waitForPromisesToResolve() .then(() => Onyx.set(ONYXKEYS.PERSONAL_DETAILS, PERSONAL_DETAILS_WITH_PERIODS)) @@ -297,7 +297,7 @@ describe('OptionsListUtils', () => { // Then we expect to have the personal detail with period filtered expect(results.recentReports.length).toBe(1); - expect(results.recentReports[0].displayName).toBe('The Flash'); + expect(results.recentReports[0].text).toBe('The Flash'); }); }); @@ -316,10 +316,10 @@ describe('OptionsListUtils', () => { expect(results.personalDetails.length).toBe(_.size(PERSONAL_DETAILS) - 1 - MAX_RECENT_REPORTS); // We should expect personal details sorted alphabetically - expect(results.personalDetails[0].displayName).toBe('Black Widow'); - expect(results.personalDetails[1].displayName).toBe('Invisible Woman'); - expect(results.personalDetails[2].displayName).toBe('Spider-Man'); - expect(results.personalDetails[3].displayName).toBe('The Incredible Hulk'); + expect(results.personalDetails[0].text).toBe('Black Widow'); + expect(results.personalDetails[1].text).toBe('Invisible Woman'); + expect(results.personalDetails[2].text).toBe('Spider-Man'); + expect(results.personalDetails[3].text).toBe('The Incredible Hulk'); // Then the result which has an existing report should also have the reportID attached const personalDetailWithExistingReport = _.find(results.personalDetails, (personalDetail) => personalDetail.login === 'peterparker@expensify.com'); @@ -329,10 +329,10 @@ describe('OptionsListUtils', () => { results = OptionsListUtils.getNewChatOptions([], PERSONAL_DETAILS, [], ''); // We should expect personal details sorted alphabetically - expect(results.personalDetails[0].displayName).toBe('Black Panther'); - expect(results.personalDetails[1].displayName).toBe('Black Widow'); - expect(results.personalDetails[2].displayName).toBe('Captain America'); - expect(results.personalDetails[3].displayName).toBe('Invisible Woman'); + expect(results.personalDetails[0].text).toBe('Black Panther'); + expect(results.personalDetails[1].text).toBe('Black Widow'); + expect(results.personalDetails[2].text).toBe('Captain America'); + expect(results.personalDetails[3].text).toBe('Invisible Woman'); // When we provide a search value that does not match any personal details results = OptionsListUtils.getNewChatOptions(REPORTS, PERSONAL_DETAILS, [], 'magneto'); @@ -346,7 +346,7 @@ describe('OptionsListUtils', () => { // Then one recentReports will be returned and it will be the correct option // personalDetails should be empty array expect(results.recentReports.length).toBe(1); - expect(results.recentReports[0].displayName).toBe('Spider-Man'); + expect(results.recentReports[0].text).toBe('Spider-Man'); expect(results.personalDetails.length).toBe(0); // When we provide a search value that matches a partial display name or email @@ -358,9 +358,9 @@ describe('OptionsListUtils', () => { expect(results.personalDetails.length).toBe(4); expect(results.recentReports.length).toBe(5); expect(results.personalDetails[0].login).toBe('natasharomanoff@expensify.com'); - expect(results.recentReports[0].displayName).toBe('Captain America'); - expect(results.recentReports[1].displayName).toBe('Mr Sinister'); - expect(results.recentReports[2].displayName).toBe('Black Panther'); + expect(results.recentReports[0].text).toBe('Captain America'); + expect(results.recentReports[1].text).toBe('Mr Sinister'); + expect(results.recentReports[2].text).toBe('Black Panther'); // Test for Concierge's existence in chat options results = OptionsListUtils.getNewChatOptions(REPORTS_WITH_CONCIERGE, PERSONAL_DETAILS_WITH_CONCIERGE); @@ -404,10 +404,10 @@ describe('OptionsListUtils', () => { expect(results.personalDetails.length).toBe(_.size(PERSONAL_DETAILS) - 6); // We should expect personal details sorted alphabetically - expect(results.personalDetails[0].displayName).toBe('Black Widow'); - expect(results.personalDetails[1].displayName).toBe('Invisible Woman'); - expect(results.personalDetails[2].displayName).toBe('Spider-Man'); - expect(results.personalDetails[3].displayName).toBe('The Incredible Hulk'); + expect(results.personalDetails[0].text).toBe('Black Widow'); + expect(results.personalDetails[1].text).toBe('Invisible Woman'); + expect(results.personalDetails[2].text).toBe('Spider-Man'); + expect(results.personalDetails[3].text).toBe('The Incredible Hulk'); // And none of our personalDetails should include any of the users with recent reports const reportLogins = _.map(results.recentReports, (reportOption) => reportOption.login); @@ -429,7 +429,7 @@ describe('OptionsListUtils', () => { // Then all single participant reports that match will show up in the recentReports array, Recently used contact should be at the top expect(results.recentReports.length).toBe(5); - expect(results.recentReports[0].displayName).toBe('Captain America'); + expect(results.recentReports[0].text).toBe('Captain America'); // And logins with no single participant reports will show up in personalDetails expect(results.personalDetails.length).toBe(4); From 314e24dd794dce6d66c5aa7f8853c9137a591fa1 Mon Sep 17 00:00:00 2001 From: Youssef Lourayad Date: Tue, 16 May 2023 14:29:44 +0100 Subject: [PATCH 7/7] Prettier --- src/pages/ReportParticipantsPage.js | 43 +++++++++++++++-------------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/src/pages/ReportParticipantsPage.js b/src/pages/ReportParticipantsPage.js index 794a4a5e9725..2e7e703a4655 100755 --- a/src/pages/ReportParticipantsPage.js +++ b/src/pages/ReportParticipantsPage.js @@ -55,27 +55,30 @@ const defaultProps = { const getAllParticipants = (report, personalDetails) => { const {participants} = report; - return _.chain(participants).map((login) => { - const userLogin = Str.removeSMSDomain(login); - const userPersonalDetail = lodashGet(personalDetails, login, {displayName: userLogin, avatar: ''}); + return _.chain(participants) + .map((login) => { + const userLogin = Str.removeSMSDomain(login); + const userPersonalDetail = lodashGet(personalDetails, login, {displayName: userLogin, avatar: ''}); - return { - alternateText: userLogin, - displayName: userPersonalDetail.displayName, - icons: [ - { - source: ReportUtils.getAvatar(userPersonalDetail.avatar, login), - name: login, - type: CONST.ICON_TYPE_AVATAR, - }, - ], - keyForList: userLogin, - login, - text: userPersonalDetail.displayName, - tooltipText: userLogin, - participantsList: [{login, displayName: userPersonalDetail.displayName}], - } - }).sortBy(participant => participant.displayName.toLowerCase()).value(); + return { + alternateText: userLogin, + displayName: userPersonalDetail.displayName, + icons: [ + { + source: ReportUtils.getAvatar(userPersonalDetail.avatar, login), + name: login, + type: CONST.ICON_TYPE_AVATAR, + }, + ], + keyForList: userLogin, + login, + text: userPersonalDetail.displayName, + tooltipText: userLogin, + participantsList: [{login, displayName: userPersonalDetail.displayName}], + }; + }) + .sortBy((participant) => participant.displayName.toLowerCase()) + .value(); }; const ReportParticipantsPage = (props) => {