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

Add the Recents and Contacts headers to the single-user participants page used in the Request Money and New Chat flows #2818

Merged
merged 13 commits into from
May 24, 2021
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
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
2 changes: 2 additions & 0 deletions src/libs/OptionsListUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,8 @@ function getNewChatOptions(
return getOptions(reports, personalDetails, {}, 0, {
searchValue,
includePersonalDetails: true,
includeRecentReports: true,
maxRecentReportsToShow: 5,
excludeConcierge,
});
}
Expand Down
15 changes: 13 additions & 2 deletions src/pages/NewChatPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, {Component} from 'react';
import {View} from 'react-native';
import PropTypes from 'prop-types';
import {withOnyx} from 'react-native-onyx';
import _ from 'underscore';
import OptionsSelector from '../components/OptionsSelector';
import {getNewChatOptions, getHeaderMessage} from '../libs/OptionsListUtils';
import ONYXKEYS from '../ONYXKEYS';
Expand Down Expand Up @@ -54,6 +55,7 @@ class NewChatPage extends Component {
this.createNewChat = this.createNewChat.bind(this);

const {
recentReports,
personalDetails,
userToInvite,
} = getNewChatOptions(
Expand All @@ -64,6 +66,7 @@ class NewChatPage extends Component {

this.state = {
searchValue: '',
recentReports,
personalDetails,
userToInvite,
};
Expand All @@ -77,10 +80,17 @@ class NewChatPage extends Component {
getSections() {
const sections = [];

sections.push({
title: this.props.translate('iou.recents'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we're using this String for non-IOU pages, I think it should be refactored to a different group. Could you move the iou.contacts and iou.recents to common.contacts and common.recents please.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I moved

data: this.state.recentReports,
shouldShow: !_.isEmpty(this.state.recentReports),
indexOffset: sections.reduce((prev, {data}) => prev + data.length, 0),
});

sections.push({
title: this.props.translate('iou.contacts'),
data: this.state.personalDetails,
shouldShow: this.state.personalDetails.length > 0,
shouldShow: !_.isEmpty(this.state.personalDetails),
indexOffset: sections.reduce((prev, {data}) => prev + data.length, 0),
});

Expand Down Expand Up @@ -128,6 +138,7 @@ class NewChatPage extends Component {
onSelectRow={this.createNewChat}
onChangeText={(searchValue = '') => {
const {
recentReports,
personalDetails,
userToInvite,
} = getNewChatOptions(
Expand All @@ -137,12 +148,12 @@ class NewChatPage extends Component {
);
this.setState({
searchValue,
recentReports,
userToInvite,
personalDetails,
});
}}
headerMessage={headerMessage}
hideSectionHeaders
disableArrowKeysActions
hideAdditionalOptionStates
forceTextUnreadStyle
Expand Down
4 changes: 2 additions & 2 deletions src/pages/NewGroupPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,14 @@ class NewGroupPage extends Component {
sections.push({
title: this.props.translate('iou.recents'),
data: this.state.recentReports,
shouldShow: this.state.recentReports.length > 0,
shouldShow: !_.isEmpty(this.state.recentReports),
indexOffset: sections.reduce((prev, {data}) => prev + data.length, 0),
});

sections.push({
title: this.props.translate('iou.contacts'),
data: this.state.personalDetails,
shouldShow: this.state.personalDetails.length > 0,
shouldShow: !_.isEmpty(this.state.personalDetails),
indexOffset: sections.reduce((prev, {data}) => prev + data.length, 0),
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React, {Component} from 'react';
import PropTypes from 'prop-types';
import {withOnyx} from 'react-native-onyx';
import _ from 'underscore';
import {getNewChatOptions} from '../../../../libs/OptionsListUtils';
import OptionsSelector from '../../../../components/OptionsSelector';
import ONYXKEYS from '../../../../ONYXKEYS';
Expand Down Expand Up @@ -44,14 +45,19 @@ class IOUParticipantsRequest extends Component {

this.addSingleParticipant = this.addSingleParticipant.bind(this);

const {personalDetails, userToInvite} = getNewChatOptions(
const {
recentReports,
personalDetails,
userToInvite,
} = getNewChatOptions(
props.reports,
props.personalDetails,
'',
true,
);

this.state = {
recentReports,
personalDetails,
userToInvite,
searchValue: '',
Expand All @@ -65,10 +71,18 @@ class IOUParticipantsRequest extends Component {
*/
getSections() {
const sections = [];

sections.push({
title: this.props.translate('iou.recents'),
data: this.state.recentReports,
shouldShow: !_.isEmpty(this.state.recentReports),
indexOffset: sections.reduce((prev, {data}) => prev + data.length, 0),
});

sections.push({
title: this.props.translate('iou.contacts'),
data: this.state.personalDetails,
shouldShow: this.state.personalDetails.length > 0,
shouldShow: !_.isEmpty(this.state.personalDetails),
indexOffset: 0,
});

Expand Down Expand Up @@ -102,19 +116,23 @@ class IOUParticipantsRequest extends Component {
value={this.state.searchValue}
onSelectRow={this.addSingleParticipant}
onChangeText={(searchValue = '') => {
const {personalDetails, userToInvite} = getNewChatOptions(
const {
recentReports,
personalDetails,
userToInvite,
} = getNewChatOptions(
this.props.reports,
this.props.personalDetails,
searchValue,
true,
);
this.setState({
searchValue,
recentReports,
userToInvite,
personalDetails,
});
}}
hideSectionHeaders
disableArrowKeysActions
hideAdditionalOptionStates
forceTextUnreadStyle
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class IOUParticipantsSplit extends Component {
sections.push({
title: this.props.translate('iou.recents'),
data: this.state.recentReports,
shouldShow: this.state.recentReports.length > 0,
shouldShow: !_.isEmpty(this.state.recentReports),

// takes the sum off the length of all data
// (this.state.selectedOptions) in previous sections
Expand All @@ -120,7 +120,7 @@ class IOUParticipantsSplit extends Component {
sections.push({
title: this.props.translate('iou.contacts'),
data: this.state.personalDetails,
shouldShow: this.state.personalDetails.length > 0,
shouldShow: !_.isEmpty(this.state.personalDetails),

// takes the sum off the length of all data
// (this.state.selectedOptions, this.state.recentReports) in previous sections
Expand Down
44 changes: 26 additions & 18 deletions tests/unit/OptionsListUtilsTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,20 +193,25 @@ describe('OptionsListUtils', () => {
});

it('getNewChatOptions()', () => {
// maxRecentReportsToShow in src/libs/OptionsListUtils.js
const MAX_RECENT_REPORTS = 5;

// When we call getNewChatOptions() with no search value
let results = OptionsListUtils.getNewChatOptions(REPORTS, PERSONAL_DETAILS, '');

// Then no reports should be returned, only personalDetails and all the personalDetails should be returned
// minus the currently logged in user
expect(results.recentReports.length).toBe(0);
expect(results.personalDetails.length).toBe(_.size(PERSONAL_DETAILS) - 1);
// We should expect maximimum of 5 recent reports to be returned
expect(results.recentReports.length).toBe(MAX_RECENT_REPORTS);

// We should expect all personalDetails to be returned,
// minus the currently logged in user and recent reports count
expect(results.personalDetails.length).toBe(_.size(PERSONAL_DETAILS) - 1 - MAX_RECENT_REPORTS);

// Then the result which has an existing report should also have the reportID attached
const personalDetailWithExistingReport = _.find(
results.personalDetails,
personalDetail => personalDetail.login === 'reedrichards@expensify.com',
personalDetail => personalDetail.login === 'peterparker@expensify.com',
Julesssss marked this conversation as resolved.
Show resolved Hide resolved
);
expect(personalDetailWithExistingReport.reportID).toBe(3);
expect(personalDetailWithExistingReport.reportID).toBe(2);

// When we provide a search value that does not match any personal details
results = OptionsListUtils.getNewChatOptions(REPORTS, PERSONAL_DETAILS, 'magneto');
Expand All @@ -217,27 +222,30 @@ describe('OptionsListUtils', () => {
// When we provide a search value that matches an email
results = OptionsListUtils.getNewChatOptions(REPORTS, PERSONAL_DETAILS, 'peterparker@expensify.com');

// Then one option will be returned and it will be the correct option
expect(results.personalDetails.length).toBe(1);
expect(results.personalDetails[0].text).toBe('Spider-Man');
// 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.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');

// 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(3);
expect(results.personalDetails[0].text).toBe('Spider-Man');
expect(results.personalDetails[1].text).toBe('Invisible Woman');
expect(results.personalDetails[2].login).toBe('natasharomanoff@expensify.com');
expect(results.personalDetails.length).toBe(1);
expect(results.recentReports.length).toBe(2);
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');

// Test for Concierge's existence in chat options
results = OptionsListUtils.getNewChatOptions(REPORTS_WITH_CONCIERGE, PERSONAL_DETAILS_WITH_CONCIERGE);

// Concierge is included in the results by default and all the personalDetails should be returned
// minus the currently logged in user
expect(results.personalDetails.length).toBe(_.size(PERSONAL_DETAILS_WITH_CONCIERGE) - 1);
expect(results.personalDetails).toEqual(
// Concierge is included in the results by default. We should expect all the personalDetails to show
// (minus the 5 that are already showing and the currently logged in user)
expect(results.personalDetails.length).toBe(_.size(PERSONAL_DETAILS_WITH_CONCIERGE) - 1 - MAX_RECENT_REPORTS);
expect(results.recentReports).toEqual(
expect.arrayContaining([
expect.objectContaining({login: 'concierge@expensify.com'}),
]),
Expand All @@ -247,7 +255,7 @@ describe('OptionsListUtils', () => {
results = OptionsListUtils.getNewChatOptions(REPORTS_WITH_CONCIERGE, PERSONAL_DETAILS_WITH_CONCIERGE, '', true);

// All the personalDetails should be returned minus the currently logged in user and Concierge
expect(results.personalDetails.length).toBe(_.size(PERSONAL_DETAILS_WITH_CONCIERGE) - 2);
expect(results.personalDetails.length).toBe(_.size(PERSONAL_DETAILS_WITH_CONCIERGE) - 2 - MAX_RECENT_REPORTS);
expect(results.personalDetails).not.toEqual(
expect.arrayContaining([
expect.objectContaining({login: 'concierge@expensify.com'}),
Expand Down