Repository navigation
[HOLD for payment 2022-12-20] [HOLD for payment 2022-12-15] Request money - The list of currency is empty until you enter any letter in search field #13286
Description
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
- Go to URL https://staging.new.expensify.com/
- Login with any account
- Click on FAB button -> Request money
- Click on the currency icon
Expected Result:
The currency list must be filled with a list of currencies
Actual Result:
The list of currency is empty until you enter any letter in search field
Workaround:
unknown
Platform:
Where is this issue occurring?
- Web
- iOS
- Android
- Desktop App
- Mobile Web
Version Number: 1.2.36-0
Reproducible in staging?: y
Reproducible in production?: n
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:
Bug5845727_Recording__3165.mp4
Expensify/Expensify Issue URL:
Issue reported by: Applause internal team
Slack conversation:
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~01cfd75a5dd3d584dd
- Upwork Job ID: 1606450618598191104
- Last Price Increase: 2022-12-24
Activity
OSBotify commentedon Dec 2, 2022
👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open
StagingDeployCashdeploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:melvin-bot commentedon Dec 2, 2022
Triggered auto assignment to @tgolen (
Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.0xmiroslav commentedon Dec 2, 2022
This is a regression from #13186 which fixes #12026
s77rt commentedon Dec 2, 2022
Proposal
Details
If the
searchValueis empty return all the currency options@0xmiroslav I don't think this is a regression, it's more like a hidden bug that has been revealed. If the search value is empty we should return nothing instead of everything unless explicitly specified. Also this will avoid the unnecessary
_filtercall and squeeze more performance.tgolen commentedon Dec 2, 2022
The proposal looks good to me 👍 I'm a bit conflicted on it being a regression or not.
I think it's technically a regression if it worked properly before and now it doesn't. I think this is just the nature of some regressions and it's why we have the checklist item:
Since
isSearchStringMatchwas modified, then all the references to it should have been tested and this issue could have been caught.tgolen commentedon Dec 2, 2022
cc @cead22 for your thoughts on it since you reviewed the original PR
0xmiroslav commentedon Dec 2, 2022
This happens on staging, not on production
That's why @kavimuru marked this as deploy blocker
@s77rt you can test yourself by reverting code changes in #13186
s77rt commentedon Dec 2, 2022
@0xmiroslav Yes I did, I agree with you, I'm just saying that we have found a hidden bug and posted a solution above.
situchan commentedon Dec 2, 2022
Proposal:
https://github.com/Expensify/App/blob/main/src/libs/OptionsListUtils.js#L391-L401
function isSearchStringMatch(searchValue, searchText, participantNames = new Set(), isChatRoom = false) { + if (!searchValue.trim()) { + return true; + } const searchWords = _.compact(uniqFast([ searchValue, ..._.map(searchValue.replace(/,/g, ' ').split(' '), word => word.trim()), ])); const valueToSearch = searchText && searchText.replace(new RegExp(/ /g), ''); return _.some(searchWords, (word) => { const matchRegex = new RegExp(Str.escapeForRegExp(word), 'i'); return matchRegex.test(valueToSearch) || (!isChatRoom && participantNames.has(word)); }); }This allows no filter with endless blank text like all other popular search engines do
Or if it should be filtered by blank space, just this condition would be fine:
if (!searchValue)49 remaining items
[-][HOLD for payment 2022-12-15] Request money - The list of currency is empty until you enter any letter in search field[/-][+][HOLD for payment 2022-12-20] [HOLD for payment 2022-12-15] Request money - The list of currency is empty until you enter any letter in search field[/+]melvin-bot commentedon Dec 21, 2022
@tgolen Whoops! This issue is 2 days overdue. Let's get this updated quick!
melvin-bot commentedon Dec 23, 2022
@tgolen Huh... This is 4 days overdue. Who can take care of this?
melvin-bot commentedon Dec 24, 2022
Job added to Upwork: https://www.upwork.com/jobs/~01cfd75a5dd3d584dd
melvin-bot commentedon Dec 24, 2022
Current assignee @aimane-chnaif is eligible for the Internal assigner, not assigning anyone new.
mallenexpensify commentedon Dec 24, 2022
@aimane-chnaif can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~01cfd75a5dd3d584dd
aimane-chnaif commentedon Dec 24, 2022
@mallenexpensify I didn't fill checklist because PR was merged quickly as it fixes deploy blocker and labeled
CP Staging, though I started reviewing.So I think I am not eligible for full compensation.
But I see you already hired me on UP so instead can we use this job to compensate this issue (BZ missing here)?
aimane-chnaif commentedon Dec 26, 2022
@mallenexpensify I accepted your offer for #12235 so you can cancel this job since I am not eligible
mallenexpensify commentedon Dec 27, 2022
OK, I think this can be closed then. I'm leaving the duplicate 'hold for payment' notifications in the title so we can see why it's happening, I'll create a GH if a fix is needed. Discussing here.