diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 2151679dcd83..da16be11ca63 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -370,12 +370,13 @@ function getOptions(reports, personalDetails, activeReportID, { // When sortByReportTypeInSearch flag is true, recentReports will include the personalDetails options as well. sortByReportTypeInSearch = false, - sortByLastMessageTimestamp = false, + sortByLastMessageTimestamp = true, searchValue = '', showChatPreviewLine = false, showReportsWithNoComments = false, hideReadReports = false, sortByAlphaAsc = false, + sortPersonalDetailsByAlphaAsc = true, forcePolicyNamePreview = false, prioritizeIOUDebts = false, prioritizeReportsWithDraftComments = false, @@ -464,13 +465,18 @@ function getOptions(reports, personalDetails, activeReportID, { })); }); - const allPersonalDetailsOptions = _.map(personalDetails, personalDetail => ( + let allPersonalDetailsOptions = _.map(personalDetails, personalDetail => ( createOption([personalDetail.login], personalDetails, reportMapForLogins[personalDetail.login], { showChatPreviewLine, forcePolicyNamePreview, }) )); + if (sortPersonalDetailsByAlphaAsc) { + // PersonalDetails should be ordered Alphabetically by default - https://github.com/Expensify/App/issues/8220#issuecomment-1104009435 + allPersonalDetailsOptions = lodashOrderBy(allPersonalDetailsOptions, [personalDetail => personalDetail.text.toLowerCase()], 'asc'); + } + // Always exclude already selected options and the currently logged in user const loginOptionsToExclude = [...selectedOptions, {login: currentUserLogin}]; @@ -642,7 +648,6 @@ function getSearchOptions( showChatPreviewLine: true, showReportsWithNoComments: true, includePersonalDetails: true, - sortByLastMessageTimestamp: false, forcePolicyNamePreview: true, prioritizeIOUDebts: false, }); @@ -711,6 +716,31 @@ function getNewChatOptions( }); } +/** + * Build the options for the Workspace Member Invite view + * + * @param {Object} personalDetails + * @param {Array} betas + * @param {String} searchValue + * @param {Array} excludeLogins + * @returns {Object} + */ +function getMemberInviteOptions( + personalDetails, + betas = [], + searchValue = '', + excludeLogins = [], +) { + return getOptions([], personalDetails, 0, { + betas, + searchValue: searchValue.trim(), + excludeDefaultRooms: true, + includePersonalDetails: true, + excludeLogins, + sortPersonalDetailsByAlphaAsc: false, + }); +} + /** * Build the options for the Sidebar a.k.a. LHN * @@ -744,7 +774,6 @@ function getSidebarOptions( includeRecentReports: true, includeMultipleParticipantReports: true, maxRecentReportsToShow: 0, // Unlimited - sortByLastMessageTimestamp: true, showChatPreviewLine: true, prioritizePinnedReports: true, ...sideBarOptions, @@ -804,6 +833,7 @@ export { isCurrentUser, getSearchOptions, getNewChatOptions, + getMemberInviteOptions, getSidebarOptions, getHeaderMessage, getPersonalDetailsForLogins, diff --git a/src/pages/workspace/WorkspaceInvitePage.js b/src/pages/workspace/WorkspaceInvitePage.js index 0aa6dbcd079c..8976f10d934c 100644 --- a/src/pages/workspace/WorkspaceInvitePage.js +++ b/src/pages/workspace/WorkspaceInvitePage.js @@ -68,12 +68,10 @@ class WorkspaceInvitePage extends React.Component { const { personalDetails, userToInvite, - } = OptionsListUtils.getNewChatOptions( - [], + } = OptionsListUtils.getMemberInviteOptions( props.personalDetails, props.betas, '', - [], this.getExcludedUsers(), ); this.state = { @@ -192,12 +190,10 @@ class WorkspaceInvitePage extends React.Component { const { personalDetails, userToInvite, - } = OptionsListUtils.getNewChatOptions( - [], + } = OptionsListUtils.getMemberInviteOptions( this.props.personalDetails, this.props.betas, prevState.searchValue, - [], this.getExcludedUsers(), ); @@ -275,12 +271,10 @@ class WorkspaceInvitePage extends React.Component { const { personalDetails, userToInvite, - } = OptionsListUtils.getNewChatOptions( - [], + } = OptionsListUtils.getMemberInviteOptions( this.props.personalDetails, this.props.betas, searchValue, - [], this.getExcludedUsers(), ); this.setState({ diff --git a/tests/unit/OptionsListUtilsTest.js b/tests/unit/OptionsListUtilsTest.js index 4975fa2b0025..3a174a50cf1f 100644 --- a/tests/unit/OptionsListUtilsTest.js +++ b/tests/unit/OptionsListUtilsTest.js @@ -11,7 +11,7 @@ describe('OptionsListUtils', () => { const REPORTS = { 1: { lastVisitedTimestamp: 1610666739295, - lastMessageTimestamp: 1, + lastMessageTimestamp: 15, isPinned: false, reportID: 1, participants: ['tonystark@expensify.com', 'reedrichards@expensify.com'], @@ -20,7 +20,7 @@ describe('OptionsListUtils', () => { }, 2: { lastVisitedTimestamp: 1610666739296, - lastMessageTimestamp: 1, + lastMessageTimestamp: 16, isPinned: false, reportID: 2, participants: ['peterparker@expensify.com'], @@ -31,7 +31,7 @@ describe('OptionsListUtils', () => { // This is the only report we are pinning in this test 3: { lastVisitedTimestamp: 1610666739297, - lastMessageTimestamp: 1, + lastMessageTimestamp: 170, isPinned: true, reportID: 3, participants: ['reedrichards@expensify.com'], @@ -40,7 +40,7 @@ describe('OptionsListUtils', () => { }, 4: { lastVisitedTimestamp: 1610666739298, - lastMessageTimestamp: 1, + lastMessageTimestamp: 180, isPinned: false, reportID: 4, participants: ['tchalla@expensify.com'], @@ -49,7 +49,7 @@ describe('OptionsListUtils', () => { }, 5: { lastVisitedTimestamp: 1610666739299, - lastMessageTimestamp: 1, + lastMessageTimestamp: 19, isPinned: false, reportID: 5, participants: ['suestorm@expensify.com'], @@ -58,7 +58,7 @@ describe('OptionsListUtils', () => { }, 6: { lastVisitedTimestamp: 1610666739300, - lastMessageTimestamp: 1, + lastMessageTimestamp: 20, isPinned: false, reportID: 6, participants: ['thor@expensify.com'], @@ -167,7 +167,7 @@ describe('OptionsListUtils', () => { 11: { lastVisitedTimestamp: 1610666739302, - lastMessageTimestamp: 1, + lastMessageTimestamp: 22, isPinned: false, reportID: 11, participants: ['concierge@expensify.com'], @@ -180,7 +180,7 @@ describe('OptionsListUtils', () => { ...REPORTS, 12: { lastVisitedTimestamp: 1610666739302, - lastMessageTimestamp: 1, + lastMessageTimestamp: 22, isPinned: false, reportID: 12, participants: ['chronos@expensify.com'], @@ -193,7 +193,7 @@ describe('OptionsListUtils', () => { ...REPORTS, 13: { lastVisitedTimestamp: 1610666739302, - lastMessageTimestamp: 1, + lastMessageTimestamp: 22, isPinned: false, reportID: 13, participants: ['receipts@expensify.com'], @@ -206,7 +206,7 @@ describe('OptionsListUtils', () => { ...REPORTS, 14: { lastVisitedTimestamp: 1610666739302, - lastMessageTimestamp: 1, + lastMessageTimestamp: 22, isPinned: true, reportID: 14, participants: ['d_email@email.com'], @@ -215,7 +215,7 @@ describe('OptionsListUtils', () => { }, 15: { lastVisitedTimestamp: 1610666732302, - lastMessageTimestamp: 1, + lastMessageTimestamp: 22, isPinned: true, reportID: 15, participants: ['z_email@email.com'], @@ -302,9 +302,10 @@ describe('OptionsListUtils', () => { // When we filter again but provide a searchValue that should match multiple times results = OptionsListUtils.getSearchOptions(REPORTS, PERSONAL_DETAILS, 'fantastic'); - // Then we get both values with the pinned value still on top + // Value with latest lastMessageTimestamp 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('Iron Man, Mister Fantastic'); // When we filter again but provide a searchValue that should match with periods results = OptionsListUtils.getSearchOptions(REPORTS, PERSONAL_DETAILS_WITH_PERIODS, 'barryallen@expensify.com'); @@ -328,6 +329,12 @@ describe('OptionsListUtils', () => { // minus the currently logged in user and recent reports count 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'); + // Then the result which has an existing report should also have the reportID attached const personalDetailWithExistingReport = _.find( results.personalDetails, @@ -335,6 +342,15 @@ describe('OptionsListUtils', () => { ); expect(personalDetailWithExistingReport.reportID).toBe(2); + // When we only pass personal details + 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'); + // When we provide a search value that does not match any personal details results = OptionsListUtils.getNewChatOptions(REPORTS, PERSONAL_DETAILS, [], 'magneto'); @@ -351,15 +367,17 @@ describe('OptionsListUtils', () => { expect(results.personalDetails.length).toBe(0); // When we provide a search value that matches a partial display name or email - results = OptionsListUtils.getNewChatOptions(REPORTS, PERSONAL_DETAILS, [], 'man'); + results = OptionsListUtils.getNewChatOptions(REPORTS, PERSONAL_DETAILS, [], '.com'); // Then several options will be returned and they will be each have the search string in their email or name // even though the currently logged in user matches they should not show. - expect(results.personalDetails.length).toBe(1); - expect(results.recentReports.length).toBe(2); + // Should be ordered by lastMessageTimestamp values. + 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('Invisible Woman'); - expect(results.recentReports[1].text).toBe('Spider-Man'); + 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); @@ -424,6 +442,12 @@ describe('OptionsListUtils', () => { // showing and the currently logged in user) 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'); + // And none of our personalDetails should include any of the users with recent reports const reportLogins = _.map(results.recentReports, reportOption => reportOption.login); const personalDetailsOverlapWithReports = _.every(results.personalDetails, ( @@ -442,15 +466,14 @@ describe('OptionsListUtils', () => { expect(results.personalDetails[0].login).toBe('brucebanner@expensify.com'); // When we search for an option that matches things in both personalDetails and reports - results = OptionsListUtils.getNewChatOptions(REPORTS, PERSONAL_DETAILS, [], 'man'); + results = OptionsListUtils.getNewChatOptions(REPORTS, PERSONAL_DETAILS, [], '.com'); - // Then all single participant reports that match will show up in the recentReports array - expect(results.recentReports.length).toBe(2); - expect(results.recentReports[0].text).toBe('Invisible Woman'); - expect(results.recentReports[1].text).toBe('Spider-Man'); + // 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'); // And logins with no single participant reports will show up in personalDetails - expect(results.personalDetails.length).toBe(1); + expect(results.personalDetails.length).toBe(4); expect(results.personalDetails[0].login).toBe('natasharomanoff@expensify.com'); // When we provide no selected options to getNewChatOptions() @@ -606,6 +629,30 @@ describe('OptionsListUtils', () => { ); }); + it('getMemberInviteOptions()', () => { + // When we only pass personal details + 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'); + + // When we provide a search value that does not match any personal details + results = OptionsListUtils.getMemberInviteOptions(PERSONAL_DETAILS, [], 'magneto'); + + // Then no options will be returned + expect(results.personalDetails.length).toBe(0); + + // When we provide a search value that matches an email + results = OptionsListUtils.getMemberInviteOptions(PERSONAL_DETAILS, [], 'peterparker@expensify.com'); + + // Then one personal should be in personalDetails list + expect(results.personalDetails.length).toBe(1); + expect(results.personalDetails[0].text).toBe('Spider-Man'); + }); + it('getSidebarOptions() with default priority mode', () => { const reportsWithAddedPinnedMessagelessReport = { ...REPORTS,