-
Notifications
You must be signed in to change notification settings - Fork 3k
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 2024-07-17] [$250] Troubleshoot - Troubleshoot modal opened from shortcut is missing Testing preferences #44178
Comments
Triggered auto assignment to @anmurali ( |
@anmurali FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors |
ProposalPlease re-state the problem that we are trying to solve in this issue.Troubleshoot modal opened from shortcut is missing Testing preferences What is the root cause of that problem?The Testing preference on troubleshoot modal is only displayed if the environment is Development App/src/components/TestToolsModal.tsx Line 47 in b021195
What changes do you think we should make in order to solve the problem?Make the Testing preference on troubleshoot displayed if the environment is not Production, so it will be displayed on Development and Staging environment What alternative solutions did you explore? (Optional)Create a reusable component to display a list of troubleshooting configurations. This component will ensure that both the Troubleshoot modal and the Troubleshoot page consistently display the same configurations. It will also simplify the process of adding or modifying configurations in the future if needed. Result: |
ProposalPlease re-state the problem that we are trying to solve in this issue.Troubleshoot modal opened from shortcut is missing Testing preferences What is the root cause of that problem?We do not allow the App/src/components/TestToolsModal.tsx Line 47 in b021195
I also found one more bug, the order of options is not right, we should first display the Logs options and then the TestToolMenu (We do the same with the troubleshoot page too):![]() As you can see the first option is of client side logging then the testing preference, we should maintain this order in shortcut too. What changes do you think we should make in order to solve the problem?
App/src/components/TestToolsModal.tsx Lines 66 to 67 in b021195
</TestToolRow>
)}
{!isProduction && <TestToolMenu />} What alternative solutions did you explore? (Optional) |
@anmurali Whoops! This issue is 2 days overdue. Let's get this updated quick! |
Job added to Upwork: https://www.upwork.com/jobs/~01a41f20c0677b7543 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @allgandalf ( |
Trying to figure out where this issue goes and I don't think any of the roadmap projects make sense. It is about testing, and so quality perhaps? |
Updating proposal , adding alternative solution |
thanks for your proposals everyone! Both contributors have correct RCA. Lets go with @aeioual's proposal here, that bug that they found out is important here considering the consistency between options. 🎀👀🎀 C+ reviewed Also want to confirm the @Expensify/design opinion once, the
After this PR, we should follow the same order in the troubleshoot shortcut modal right? |
Triggered auto assignment to @bondydaa, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
That sounds correct to me? I'm not too sure but it does seem to make sense that the page and the modal would be consistent. |
The idea behind the modal was to mirror the "four finger tap" options that you have on mobile - adding more options to the modal would be a new feature, not a bug. #43256 |
Current assignee @anmurali is eligible for the NewFeature assigner, not assigning anyone new. |
Automation Failed |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.5-13 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 2024-07-17. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.6-8 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 2024-07-22. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
Regression Test ProposalNote: Can only test on Desktop/MacOS Web:
Verify that the Troubleshoot modal opens up Do we agree 👍 or 👎 |
Waiting for payment! |
@bondydaa, @anmurali, @allgandalf, @aeioual Whoops! This issue is 2 days overdue. Let's get this updated quick! |
This issue can be closed, payment has been completed |
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.0.0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause -Internal Team
Issue found when executing PR #43543
Action Performed:
Expected Result:
Troubleshoot modal opened from shortcut will be complete
Actual Result:
Troubleshoot modal opened from shortcut is missing Testing preferences
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6520668_1718987164941.20240622_002210.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @anmuraliThe text was updated successfully, but these errors were encountered: