-
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-10] [$250] Workspace's default currency isn't showing when submitting a request #42597
Comments
Triggered auto assignment to @puneetlath ( |
I'm happy to take over this issue as C+ since I'm the reporter for this bug and have more context in this bug. |
ProposalPlease re-state the problem that we are trying to solve in this issue.The default currency in the amount step isn't the WS currency What is the root cause of that problem?When we init money request, we always set the default currency as the local currency of the user Line 320 in 8d73db3
What changes do you think we should make in order to solve the problem?We should prioritize the policy's currency first instead of user's local currency
Line 320 in 8d73db3
What alternative solutions did you explore? (Optional)NA |
ProposalPlease re-state the problem that we are trying to solve in this issue.Policy currency is not being used when adding a money request and in teachers unite. Also, when workspace is made using iou report, then report currency is not being used. What is the root cause of that problem?Policy currency is not being used in Line 320 in 525ad6f
Also, policy currency is not being used in App/src/libs/actions/TeachersUnite.ts Line 98 in 525ad6f
Then, there's one more bug. When creating a workspace using report, the report's currency should be set as the default for the workspace but that is not happening as of now. App/src/libs/actions/Policy.ts Lines 2039 to 2040 in 525ad6f
Just the What changes do you think we should make in order to solve the problem?We should use
Update it in TeachersUnite.ts:
Pass currency via iouReport:
Now, it can be used in
|
@puneetlath Whoops! This issue is 2 days overdue. Let's get this updated quick! |
@puneetlath Huh... This is 4 days overdue. Who can take care of this? |
Job added to Upwork: https://www.upwork.com/jobs/~01ce85c0267e9bbc85 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @fedirjh ( |
@dukenv0307 Are you still interested in taking this over? or should I proceed with my review ? |
@fedirjh Pls go ahead, you already got assigned to this issue. Thank you |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@puneetlath, @fedirjh Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
What's the status of this issue? |
@puneetlath @fedirjh 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! |
Thank you @nkdengineer and @ShridharGoel for the proposals. It seems that @ShridharGoel's proposal effectively resolves all instances of this issue. Therefore, I suggest we proceed with it. Let's proceed with @ShridharGoel's proposal 🎀 👀 🎀 C+ reviewed |
Current assignee @puneetlath is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new. |
Two additional cases in the selected proposal aren't bugs. For both case |
@nkdengineer We are creating new policy chats, rather than new policies. Therefore, it is expected for the chat created within a policy to use the same currency. |
@fedirjh Let see this case App/src/libs/actions/TeachersUnite.ts Line 98 in f2b7460
and App/src/libs/actions/Policy/Policy.ts Line 2072 in f2b7460
Both cases we're generating a new policy and create new policy chats with the optimistic policy so the new policy chats always have the same currency with the policy. |
📣 @fedirjh 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @ShridharGoel 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.3-7 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-10. 🎊 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:
|
BugZero Checklist:
Regression Test Proposal
|
Payment summary:
Regression test: https://github.com/Expensify/Expensify/issues/412084 Thanks everyone! |
$250 approved for @fedirjh |
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.75-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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @dukenv0307
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1716540568640839
Action Performed:
Expected Result:
The default currency in the amount step should be the WS currency if we submit an expense in the WS chat.
Actual Result:
The default currency in the amount step isn't the WS currency
Workaround:
unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Recording.121.mp4
Screen.Recording.2024-05-24.at.15.51.19.mov
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @puneetlathThe text was updated successfully, but these errors were encountered: