Skip to content

ignore periods on search #8375

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.

Sign up for GitHub

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

Merged
stitesExpensify merged 5 commits into from
Apr 13, 2022
Merged

ignore periods on search #8375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b6610ba
ignore periods on search
mateusbra Mar 29, 2022
bd09df6
removed unnecessary push to searchTerms and solved edge case
mateusbra Mar 31, 2022
1b2e519
added test unit in order to cover the search with results containing …
mateusbra Apr 7, 2022
2d5e311
added tests for userToInvite
mateusbra Apr 11, 2022
cd963aa
refactor noOptions, noOptionsMatchExactly
mateusbra Apr 13, 2022
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
9 changes: 5 additions & 4 deletions src/libs/OptionsListUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ function getSearchText(report, personalDetailList, isChatRoomOrPolicyExpenseChat
if (!isChatRoomOrPolicyExpenseChat) {
_.each(personalDetailList, (personalDetail) => {
searchTerms.push(personalDetail.displayName);
searchTerms.push(personalDetail.login);
searchTerms.push(personalDetail.login.replace(/\./g, ''));
});
}
if (report) {
Expand Down Expand Up @@ -333,6 +333,7 @@ function createOption(personalDetailList, report, {
function isSearchStringMatch(searchValue, searchText, participantNames = new Set(), isChatRoom = false) {
const searchWords = _.map(
searchValue
.replace(/\./g, '')
.replace(/,/g, ' ')
.split(' '),
word => word.trim(),
Expand Down Expand Up @@ -586,9 +587,9 @@ function getOptions(reports, personalDetails, activeReportID, {
}

let userToInvite = null;
if (searchValue
&& recentReportOptions.length === 0
&& personalDetailsOptions.length === 0
const noOptions = (recentReportOptions.length + personalDetailsOptions.length) === 0;
const noOptionsMatchExactly = !_.find(personalDetailsOptions.concat(recentReportOptions), option => option.login === searchValue.toLowerCase());
if (searchValue && (noOptions || noOptionsMatchExactly)
&& !isCurrentUser({login: searchValue})
&& _.every(selectedOptions, option => option.login !== searchValue)
&& ((Str.isValidEmail(searchValue) && !Str.isDomainEmail(searchValue)) || Str.isValidPhone(searchValue))
Expand Down
23 changes: 23 additions & 0 deletions tests/unit/OptionsListUtilsTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,15 @@ describe('OptionsListUtils', () => {
},
};

const PERSONAL_DETAILS_WITH_PERIODS = {
...PERSONAL_DETAILS,

'barry.allen@expensify.com': {
displayName: 'The Flash',
login: 'barry.allen@expensify.com',
},
};

// Set the currently logged in user, report data, and personal details
beforeAll(() => {
Onyx.init({
Expand Down Expand Up @@ -277,6 +286,13 @@ describe('OptionsListUtils', () => {
// Then we get both values with the pinned value still on top
expect(results.recentReports.length).toBe(2);
expect(results.recentReports[0].text).toBe('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');

// 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');

@marcaaron marcaaron Apr 8, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we add a test for the userToInvite case we are discussing above?

@mateusbra mateusbra Apr 11, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes

});

it('getNewChatOptions()', () => {
Expand Down Expand Up @@ -457,6 +473,13 @@ describe('OptionsListUtils', () => {
expect(results.personalDetails.length).toBe(0);
expect(results.userToInvite).not.toBe(null);

// When we add a search term for which exist options for it excluding its period.
results = OptionsListUtils.getNewChatOptions(REPORTS, PERSONAL_DETAILS, [], 'peter.parker@expensify.com');

// Then we will have an options at all and there should be a userToInvite too.
expect(results.recentReports.length).toBe(1);
expect(results.userToInvite).not.toBe(null);

// When we add a search term for which no options exist and the searchValue itself
// is a potential phone number without country code added
results = OptionsListUtils.getNewChatOptions(REPORTS, PERSONAL_DETAILS, [], '5005550006');
Expand Down