-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[HOLD for payment 2023-11-22] [$500] [Wave 6: Tags] IOU-No result found message is not shown in tag search in Workspace #30166
Comments
Job added to Upwork: https://www.upwork.com/jobs/~01b19b081253474e07 |
Triggered auto assignment to @mallenexpensify ( |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @abdulrahuman5196 ( |
Bug0 Triage Checklist (Main S/O)
|
ProposalPlease re-state the problem that we are trying to solve in this issue.No header message in tag search when there are no tag results for a search term What is the root cause of that problem?Here in the App/src/components/TagPicker/index.js Line 56 in 7bbcda6
we are not passing the searchValue as the second param.We are passing empty string as searchValue and this here App/src/libs/OptionsListUtils.js Lines 1572 to 1576 in 7bbcda6
returns empty string. What changes do you think we should make in order to solve the problem?We should change the above line to const headerMessage = OptionsListUtils.getHeaderMessageForNonUserList(lodashGet(sections, '[0].data.length', 0) > 0, searchValue); The other proposal suggests this scenario: we search with a term returning no results -> then the admin deletes some tags from another account so that the number of tags go below threshold -> pusher sends these reduced number of tags. -> search input disappears. If we are to face such issue, it is better to solve it by removing the threshold altogether and show the search always instead of adding pieces of code unnecessarily complicating the component to address such unlikely cases. What alternative solutions did you explore? (Optional)At many places we pass the trimmed search value to the function getting header message. We can pass the trimmed search value to the function above as well. |
Proposalfrom: @cooldev900 Please re-state the problem that we are trying to solve in this issue.In tag search, if there is no result for the term we entered, it's not showing "no results found" or any message. What is the root cause of that problem?The root cause is that headerMessage is always empty because we always send empty search value here. App/src/components/TagPicker/index.js Line 56 in cb5aeb1
App/src/libs/OptionsListUtils.js Lines 1572 to 1577 in cb5aeb1
What changes do you think we should make in order to solve the problem?We need to send search value so that it return correct error message.
What alternative solutions did you explore? (Optional)N/A |
ProposalPlease re-state the problem that we are trying to solve in this issue.In tag search, if there is no result for the term we entered, it's not showing "no results found" or any message. What is the root cause of that problem?We're not setting the What changes do you think we should make in order to solve the problem?We need to pass
After this, we have another problem, if our search result returns not found, and the policy tags list is updated via pusher such that the tag count goes below the threshold (see the logic here), we'll be stuck in the screen with our newly added To fix this, we need to reset the
What alternative solutions did you explore? (Optional)NA |
Not sure if we're working on tag-related issues/bugs |
@mallenexpensify is checking internally |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
We are working on tags, it's in beta and contributors should be able to set them locally. @abdulrahuman5196 please review the proposals above. Thx |
Thank you. Taking a look |
Requesting for tags beta, since i am unable to see tags itself - https://expensify.slack.com/archives/C01GTK53T8Q/p1698841253711199 |
@abdulrahuman5196 just got access to the tags beta, if anyone else needs, please share your email address here or in the Slack thread above |
@mallenexpensify @abdulrahuman5196 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks! |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
Triggered auto assignment to @nkuoch, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
📣 @abdulrahuman5196 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @c3024 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
@abdulrahuman5196 I'm sure it's not something obvious that can be discovered during PR process, could you kindly evaluate again? It's a very hard to reproduce case that takes a lot of effort to get right. It's the only proposal that fixes all case of the issues so I don't think we can say "all other proposals are same" here cc @nkuoch |
@nkuoch @mallenexpensify @abdulrahuman5196 @c3024 this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks! |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.99-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2023-11-22. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
@nkuoch, @mallenexpensify, @abdulrahuman5196, @c3024 Huh... This is 4 days overdue. Who can take care of this? |
Contributor: @c3024 paid $500 via Upwork @abdulrahuman5196 can you please fill out the checklist above? Thx |
Not a regression. Seems to be implementation miss.
Yes.
|
Thanks @abdulrahuman5196 , TestRail update GH created |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 1.3.88-3
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause-Internal Team
Slack conversation: @
Action Performed:
Pre-Condition: 1 Enable tags in admin account in old dot by visiting policy settings----tags
2.Give a custom name for tag eg shahrukh khan
3.Create few tags
Expected Result:
In tag search, if there is no result for the term we entered, it must show "no results found" message.
Actual Result:
In tag search, if there is no result for the term we entered, it's not showing "no results found" or any message.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Android: Native
Bug6247197_1698057889937.no_result.mp4
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: