-
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
[$250] Tag - Disabled "Region" not shown in confirmation page but displayed after creating expense #45889
Comments
Triggered auto assignment to @JmillsExpensify ( |
We think this issue might be related to the #vip-vsb |
ProposalPlease re-state the problem that we are trying to solve in this issue.Tag - Disabled "Region" not shown in confirmation page but displayed after creating expense What is the root cause of that problem?We are not checking if there are enabled tags before displaying in App/src/components/MoneyRequestConfirmationListFooter.tsx Lines 448 to 449 in b8b99d0
What changes do you think we should make in order to solve the problem?We need to include a check we use in confirmation page like here App/src/components/MoneyRequestConfirmationListFooter.tsx Lines 448 to 449 in b8b99d0
We should first determine shouldShow tag by changingApp/src/components/ReportActionItem/MoneyRequestView.tsx Lines 363 to 364 in b8b99d0
and only display the specific tag menu when the tag has an enabled tags otherwise return But if the transaction had a tag selected with the disabled multi-level tag type we might need to display the menu but disable the interactiveness, in that case we can disable interactiveness when
What alternative solutions did you explore? (Optional) |
@JmillsExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick! |
@JmillsExpensify Still overdue 6 days?! Let's take care of this! |
@JmillsExpensify 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it! |
@JmillsExpensify 10 days overdue. Is anyone even seeing these? Hello? |
@JmillsExpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks! |
Job added to Upwork: https://www.upwork.com/jobs/~01a4446baaccd96a5f |
Added to wave-collect as polish and opening up for reviews. |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @paultsimura ( |
The proposal by @FitseTLT looks good to me. 🎀👀🎀 C+ reviewed With only one suggestion:
I don't think we should make the field non-interactive in this case. We may want to allow the user to remove the tag value one last time: 2024-08-07.-.18.16.-.Screen.Recording.2024-08-07.at.18.16.40.mp4WDYT about this @JmillsExpensify? |
Triggered auto assignment to @stitesExpensify, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
Looking at the OP, this issue confused me initially. The tester has configured dependent multi-tags on OldDot. We don't officially support that or manual configuration of multi-tags on NewDot collect. I was worried we were just going to hack solutions to fix bugs for dependent multi-tags and this should be in a #wave-control doc down the line to properly support them on that plan or something. A more appropriate test for what's supported on collect in NewDot would be to:
Seems like we still have this bug, so it's not limited to handling dependent multi-tags, so that's good: 2024-08-08_16-15-23.mp4
I agree with this. If there's a value selected, we shouldn't disable it or not show it. If they do deselect the selected value, then we can not show it. 👍 |
📣 @paultsimura 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @FitseTLT 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
Not overdue – just got the Contributor assigned. |
The PR has been reviewed, but we need to hold for #47687 for it to be tested on one more case. |
This was deployed to production in v9.0.26-6. |
Regression Test ProposalPre-condition:
Test:
Do we agree 👍 or 👎 |
FYI @JmillsExpensify – the payment is due tomorrow. |
Payment summary:
|
This issue has not been updated in over 15 days. @JmillsExpensify, @paultsimura, @FitseTLT, @stitesExpensify eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
@JmillsExpensify can this be closed? |
@JmillsExpensify I think we can close this issue. 👍 |
Thanks, closing this one out. |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 9.0.10
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4751882
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team
Action Performed:
Pre-condition: Admin -Have a workplace with tags enabled. Have a member invited as employee.
In old dot, upload dependant tags. ( Make sure to enable multiple level tags and uptick independent tags)
Expected Result:
Disabled "Region" tag is not shown in confirmation page and so must not be displayed after creating distance.
Actual Result:
Disabled "Region" tag is not shown in confirmation page but displayed after creating distance.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Bug6549075_1721631628613.gt.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @paultsimuraThe text was updated successfully, but these errors were encountered: