-
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-10-17] [HOLD for payment 2024-10-10] [Simple AA in NewDot] [HIGH] Improve how we handle auto-submission in NewDot #35091
Comments
Triggered auto assignment to @trjExpensify ( |
Tyler couldn't reproduce this issue. He did run into a different bug with not being able to delete an expense off a processing report, but that's totally different and being figured out over here. |
I think the issue involves auto-submission. I've noticed that when the report has already been auto-submitted and then I try to submit again. @tylerkaraszewski, do you see any system messages when clicking into the report preview? |
Discussion for ref. |
Spruced this up in-line with the thread discussion. @mountiny are you taking this one? |
Yes, taking this! Seems like we do send the updates when this comes from harvesting too, will have to dig more |
The plot thickens.. Thanks! |
focusing on wave8 critical issue |
still focusing on ideal nav |
Still focusing on the ideal nav over this one, hopefully I can make progress on this one this week |
@trjExpensify @mountiny 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! |
Still focusing on managing issues in wave8 and fire today |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.47-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-10-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)
|
What's the dealio with this project and payments? |
Pshhhh you don't wanna search through all 168 hidden messages to find linked PRs & who's owed what? 😅 I'll help look tomorrow 😳 🙈 |
Lol, I more so meant is there 1) some kind of price agreed with the C+ on this for the wider project or is it standard and 2) are regression tests being added centrally for this project. |
I'll certainly take it though! |
Shit well I didn't have time to help today, sry :/ but yeah good question - so far no wider price agreement elsewhere so I thinkkkk it should be paid standard per PR reviewed |
Great, so then of the gazillion and one PRs for this issue... narrowing it down to the App PRs only, are any of these additional App PRs considered regressions? |
Personally I wouldn't consider any regressions, I just split up the work into many PRs 😅 |
Okay, so confirming payments will be as follows:
So in total that's $750 to @mollfpr for the C+ reviews here. With that cleared up, let's talk about the checklist. At the end of the day, what we ended up doing here was an improvement rather than a regression. That being to improve the auto-submit/approver/pay reportActions copy after introducing automation. So I don't think we have a RCA to chase. We should add/update regression tests for these cases, so @mollfpr can you suggest them for:
Thanks! |
auto-submitted
auto-approval
auto-approval and auto-forwarding
auto-payment
|
$750 approved for @mollfpr based on this summary |
@mollfpr I'm not following this test "auto-approval and auto-forwarding"? If auto-approval is enabled in that case, why is the report action |
@trjExpensify I believe Employee C can't auto-approve the expense and the report action is expected to |
Huh, why not? 🤔 If auto-approval is enabled and the report isn't flagged for review because of the amount and/or random audit percentage, it should be final approved. |
Maybe @Beamanator can help to answer it 😅 From your above comment, I have the same thought now. |
Huh, Idk if I wrote that initially but I'd definitely expect that to say " Code ref: App/src/libs/OptionsListUtils.ts Line 638 in 7b9a0cd
|
Okay, @mollfpr can we modify the tests then? Do we scrap that second one completely or what else are we including it for? |
@trjExpensify I have updated step 5 on |
Sorry, I still don't get it. When auto-approval is enabled, the report is auto-approved at every step in the approval chain, not just the first one. So how is this happening and/or an accurate test?
|
oh uhhhhh we might not have built multi-auto-approvals yet 🙃 https://github.com/Expensify/Expensify/issues/434467 |
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.4.33-4
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: @tylerkaraszewski
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1706118036323349
Action Performed:
Prerequisites:
use a collect policy
has scheduled submit enabled
has access to the isPolicyExpenseChatEnabled beta.
Daily
)Submit
button is still present on the report preview for the submitter, despite it being submitted.Expected Result:
Actual Result:
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?
All
Screenshots/Videos
Add any screenshot/video evidence
Submit
button still visible:https://github.com/Expensify/App/assets/43996225/f09c69e2-434c-4205-9949-1457cfee28fa
OldDot reportAction appearing not the NewDot one
Chat not unread, no GBR on the chat row in the LHN
To be added
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @trjExpensifyThe text was updated successfully, but these errors were encountered: