Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Request money - Five most recent chats aren't shown correctly #8549

Merged
merged 16 commits into from
Jun 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 34 additions & 4 deletions src/libs/OptionsListUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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}];

Expand Down Expand Up @@ -642,7 +648,6 @@ function getSearchOptions(
showChatPreviewLine: true,
showReportsWithNoComments: true,
includePersonalDetails: true,
sortByLastMessageTimestamp: false,
forcePolicyNamePreview: true,
prioritizeIOUDebts: false,
});
Expand Down Expand Up @@ -711,6 +716,31 @@ function getNewChatOptions(
});
}

/**
* Build the options for the Workspace Member Invite view
*
* @param {Object} personalDetails
* @param {Array<String>} 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
*
Expand Down Expand Up @@ -744,7 +774,6 @@ function getSidebarOptions(
includeRecentReports: true,
includeMultipleParticipantReports: true,
maxRecentReportsToShow: 0, // Unlimited
sortByLastMessageTimestamp: true,
showChatPreviewLine: true,
prioritizePinnedReports: true,
...sideBarOptions,
Expand Down Expand Up @@ -804,6 +833,7 @@ export {
isCurrentUser,
getSearchOptions,
getNewChatOptions,
getMemberInviteOptions,
getSidebarOptions,
getHeaderMessage,
getPersonalDetailsForLogins,
Expand Down
12 changes: 3 additions & 9 deletions src/pages/workspace/WorkspaceInvitePage.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,10 @@ class WorkspaceInvitePage extends React.Component {
const {
personalDetails,
userToInvite,
} = OptionsListUtils.getNewChatOptions(
[],
} = OptionsListUtils.getMemberInviteOptions(
props.personalDetails,
props.betas,
'',
[],
this.getExcludedUsers(),
);
this.state = {
Expand Down Expand Up @@ -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(),
);

Expand Down Expand Up @@ -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({
Expand Down
93 changes: 70 additions & 23 deletions tests/unit/OptionsListUtilsTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand All @@ -20,7 +20,7 @@ describe('OptionsListUtils', () => {
},
2: {
lastVisitedTimestamp: 1610666739296,
lastMessageTimestamp: 1,
lastMessageTimestamp: 16,
isPinned: false,
reportID: 2,
participants: ['peterparker@expensify.com'],
Expand All @@ -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'],
Expand All @@ -40,7 +40,7 @@ describe('OptionsListUtils', () => {
},
4: {
lastVisitedTimestamp: 1610666739298,
lastMessageTimestamp: 1,
lastMessageTimestamp: 180,
isPinned: false,
reportID: 4,
participants: ['tchalla@expensify.com'],
Expand All @@ -49,7 +49,7 @@ describe('OptionsListUtils', () => {
},
5: {
lastVisitedTimestamp: 1610666739299,
lastMessageTimestamp: 1,
lastMessageTimestamp: 19,
isPinned: false,
reportID: 5,
participants: ['suestorm@expensify.com'],
Expand All @@ -58,7 +58,7 @@ describe('OptionsListUtils', () => {
},
6: {
lastVisitedTimestamp: 1610666739300,
lastMessageTimestamp: 1,
lastMessageTimestamp: 20,
isPinned: false,
reportID: 6,
participants: ['thor@expensify.com'],
Expand Down Expand Up @@ -167,7 +167,7 @@ describe('OptionsListUtils', () => {

11: {
lastVisitedTimestamp: 1610666739302,
lastMessageTimestamp: 1,
lastMessageTimestamp: 22,
isPinned: false,
reportID: 11,
participants: ['concierge@expensify.com'],
Expand All @@ -180,7 +180,7 @@ describe('OptionsListUtils', () => {
...REPORTS,
12: {
lastVisitedTimestamp: 1610666739302,
lastMessageTimestamp: 1,
lastMessageTimestamp: 22,
isPinned: false,
reportID: 12,
participants: ['chronos@expensify.com'],
Expand All @@ -193,7 +193,7 @@ describe('OptionsListUtils', () => {
...REPORTS,
13: {
lastVisitedTimestamp: 1610666739302,
lastMessageTimestamp: 1,
lastMessageTimestamp: 22,
isPinned: false,
reportID: 13,
participants: ['receipts@expensify.com'],
Expand All @@ -206,7 +206,7 @@ describe('OptionsListUtils', () => {
...REPORTS,
14: {
lastVisitedTimestamp: 1610666739302,
lastMessageTimestamp: 1,
lastMessageTimestamp: 22,
isPinned: true,
reportID: 14,
participants: ['d_email@email.com'],
Expand All @@ -215,7 +215,7 @@ describe('OptionsListUtils', () => {
},
15: {
lastVisitedTimestamp: 1610666732302,
lastMessageTimestamp: 1,
lastMessageTimestamp: 22,
isPinned: true,
reportID: 15,
participants: ['z_email@email.com'],
Expand Down Expand Up @@ -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');
Expand All @@ -328,13 +329,28 @@ 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,
personalDetail => personalDetail.login === 'peterparker@expensify.com',
);
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');

Expand All @@ -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);
Expand Down Expand Up @@ -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, (
Expand All @@ -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()
Expand Down Expand Up @@ -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,
Expand Down