-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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-04-09] [$500] Workspace - New Workspace button doesn't create WS and shows no default name #38420
Comments
Triggered auto assignment to @kadiealexander ( |
@kadiealexander 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 |
We think that this bug might be related to #wave-collect - Release 1 |
This comment was marked as resolved.
This comment was marked as resolved.
ProposalPlease re-state the problem that we are trying to solve in this issue.Workspace lists page opens up when trying to update workspace name, currency etc after creating a new workspace. What is the root cause of that problem?We shouldn't be seeing the workspaces list when editing the newly created workspace name. This logic is unnecessarily pushing the workspaces list page in the central pane. App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts Lines 185 to 189 in e40fba1
What changes do you think we should make in order to solve the problem?There's no need of the above logic, we can simply remove it and it will solve the navigation issue. Same issue can be seen at multiple other places which will also get resolved. This is same as my proposal here: #38436 (comment) |
ProposalPlease re-state the problem that we are trying to solve in this issue.What is the root cause of that problem?We have a logic added from #37421 Ideal Nav PR which sets the Central Pane to Workspace settings page App/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts Lines 185 to 189 in e40fba1
This was done to address the condition where RHP was opened from FullScreenNavigator , to adjust to the correct page of central pane.
But over here we only consider the condition when What changes do you think we should make in order to solve the problem?We need to consider an additional Condition to check if the - if (matchingRootRoute.name === NAVIGATORS.FULL_SCREEN_NAVIGATOR) {
+ if ((matchingRootRoute.name === NAVIGATORS.FULL_SCREEN_NAVIGATOR) && isRHPScreenOpenedFromLHN) {
routes.push(createCentralPaneNavigator({name: SCREENS.SETTINGS.WORKSPACES}));
} What alternative solutions did you explore? (Optional)N/A |
Job added to Upwork: https://www.upwork.com/jobs/~0171b0f044271b7cff |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @alitoshmatov ( |
@GandalfGwaihir Do you have any example of a use-case where this would be needed? |
Hi there! I've carefully reviewed the issue you've reported regarding the default workspace creation problem in the Expensify app. I understand the importance of maintaining a reliable and seamless user experience, especially when it comes to financial transactions. To address this issue, I propose the following technical solution: Investigation and Analysis: I'll start by examining the codebase related to workspace creation in the Expensify app. If necessary, I'll refactor the relevant code to ensure that the default workspace name is properly displayed. Once the changes are implemented, I'll thoroughly test the workspace creation functionality across different environments, including staging and production. Throughout the process, I'll maintain detailed documentation of the steps taken and the changes made. I'll maintain open communication with the Expensify team, providing regular updates on the progress of the resolution process. I look forward to the opportunity to contribute to the success of the Expensify project. If you have any questions or need further clarification, please don't hesitate to reach out. Best regards, P.S. I'm excited about the opportunity to work with the Expensify team and contribute to the ongoing improvement of the app! 😊 |
📣 @iammudassirali! 📣
|
Contributor details |
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
Hi there! I've carefully reviewed the issue you've reported regarding the default workspace creation problem in the Expensify app. I understand the importance of maintaining a reliable and seamless user experience, especially when it comes to financial transactions. To address this issue, I propose the following technical solution: Investigation and Analysis: I'll start by examining the codebase related to workspace creation in the Expensify app. If necessary, I'll refactor the relevant code to ensure that the default workspace name is properly displayed. Once the changes are implemented, I'll thoroughly test the workspace creation functionality across different environments, including staging and production. Throughout the process, I'll maintain detailed documentation of the steps taken and the changes made. I'll maintain open communication with the Expensify team, providing regular updates on the progress of the resolution process. I look forward to the opportunity to contribute to the success of the Expensify project. If you have any questions or need further clarification, please don't hesitate to reach out. Best regards, P.S. I'm excited about the opportunity to work with the Expensify team and contribute to the ongoing improvement of the app! 😊 |
Welcome to Expensify open source @iammudassirali 👋 , here's the proposal template to follow while posting a proposal, also refer to contribution guidelines , thanks |
ProposalPlease re-state the problem that we are trying to solve in this issue.
What is the root cause of that problem?Workspace is being created from a policy draft rather than a policy. The current logic does not handle policy drafts and the name is undefined in WorkspaceProfilePage does not accept policyDraft, so policy is undefined and the policyDraft is ignored.
The policy draft is also never saved, so What changes do you think we should make in order to solve the problem?Policy needs to be inherited by
App/src/pages/workspace/WorkspaceInitialPage.tsx Lines 76 to 86 in e737a63
Proposed solution in
What alternative solutions did you explore? (Optional)Changing the props directly in the HOC |
📣 @gieoon! 📣
|
@WojtekBoman Do you have any feedback for my proposal? |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.58-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-04-09. 🎊 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:
|
$500 to @alitoshmatov |
@alitoshmatov please don't forget about the checklist! |
Bumped @alitoshmatov here. |
|
Payouts due:
Upwork job is here. |
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: 1.4.53-2
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: Exploratory around https://expensify.testrail.io/index.php?/tests/view/4428235
Issue reported by: Applause - Internal Team
Action Performed:
Expected Result:
New WS created, default name should be displayed
Actual Result:
Default name is not displayed, and WS is not created
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6415317_1710529817272.video_32.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @alitoshmatovThe text was updated successfully, but these errors were encountered: