-
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-01-17] TU - Fix admin and approver editing request details on paid plan requests. #29914
Comments
Current assignee @youssef-lr is eligible for the Engineering assigner, not assigning anyone new. |
Current assignee @trjExpensify is eligible for the Bug assigner, not assigning anyone new. |
Job added to Upwork: https://www.upwork.com/jobs/~0104816b5fd6ba7fa8 |
Bug0 Triage Checklist (Main S/O)
|
Triggered auto assignment to Contributor Plus for review of internal employee PR - @mananjadhav ( |
@youssef-lr, you can link the PR here when you have it :) |
PR ready for review |
Ah, I think you linked the PR to the wrong issue? 😅 https://github.com/Expensify/Expensify/issues/326700 |
@mananjadhav, @youssef-lr, @trjExpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
Bump @youssef-lr on where this PR is at: #29914 (comment) |
Working on it today @trjExpensify |
Thanks for picking this up. |
@mananjadhav, @youssef-lr, @trjExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick! |
Will do @trjExpensify. The PRs are in progress. |
Ah nice, @youssef-lr! Are they linked somewhere else? |
Not yet, will have them ready today @trjExpensify, App PR is done, working on Auth now! |
@mananjadhav @youssef-lr @trjExpensify 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! |
Last PR is merged. |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.23-4 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-01-17. 🎊 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:
|
Payment Summary
BugZero Checklist (@trjExpensify)
|
👋 @c3024 checklist time! |
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:
The two tests (Expense reports (request made in a workspace) and Expense reports - Collect policy) in the Tests here in #33633 are regression tests.
👍 or 👎 |
Perfect. I've created an issue for Applause to discuss adding these. Payment summary:
|
Offer sent! |
Removed the ⭐ on Youssef, as this is over to me. |
Accepted. Thanks. |
Melvin is not being nice. Looking! |
Paid! |
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: v1.3.86-1
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: @RachCHopkins
Slack conversation: Internal ref
Action Performed:
Prerequisite: You need to complete the steps in this SO to have workspace chats created for a paid policy. On the policy set up an "advanced" approval workflow with employee submitting to a manager that is not an admin.
Expected Result:
When it comes to
date
andamount
fields on a request:open
reports, andprocessing
reports until they are approved by at least one person in the approval chain.open
andprocessing
reports.open
andprocessing
reports.For
merchant
anddescription
, the logic is as follows:open
reports, andprocessing
reports until they are approved by at least one person in the approval chain.open
andprocessing
reports.open
,processing
,approved
,reimbursed
reports.Other fields on the request are being introduced with other waves (i.e categories in wave6, tax in wave9), and the logic for editing on those has been determined and/or implemented accordingly in-line with the above for
merchant
anddescription
.CC: @heyjennahay @JmillsExpensify
Actual Result:
Admins and Managers can't edit any of the request details right now. The fields appear editable but the redirect bounces to "edit/amount"
Workaround:
Can the user still use Expensify without this being fixed? Have you informed them of the workaround?
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
0K94mmEHNW.mp4
MacOS: Desktop
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: