Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.34k stars 2.77k forks source link

[$250] All report submissions display "submitted $0.00," regardless of the actual report amount. #48107

Open m-natarajan opened 3 weeks ago

m-natarajan commented 3 weeks ago

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: Reproducible in staging?: Needs Reproduction Reproducible in production?: Needs Reproduction 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: @puneetlath Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1724678229911119

Action Performed:

Prerequisite: Control workspace with scheduled submit enabled and set to anything but Instant/Manual

  1. Create expense as a submitter, ensure it doesn't have any violations
  2. Either wait for scheduled submit to run or trigger with this SO

Expected Result:

The comment for the submitted amount should match the actual report amount.

Actual Result:

All report submissions display "submitted $0.00," regardless of the actual report amount.

Workaround:

None

Platforms:

All

Screenshots/Videos

image (2)

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b5aba96981a28449
  • Upwork Job ID: 1829442437097820564
  • Last Price Increase: 2024-08-30
Issue OwnerCurrent Issue Owner: @suneox
melvin-bot[bot] commented 3 weeks ago

Triggered auto assignment to @garrettmknight (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

MelvinBot commented 3 weeks ago

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

puneetlath commented 3 weeks ago

FYI @garrettmknight this seems to only happen when the report is auto-submitted by harvesting. More context in the slack thread.

garrettmknight commented 2 weeks ago

Testing with https://staging.expensify.com/report?param={%22pageReportID%22:%221365542954725485%22,%22keepCollection%22:%22true%22} on my gknight+submittera@expensifail.com account. Will be able to confirm tomo.

garrettmknight commented 2 weeks ago

Confirmed

Screenshot 2024-08-30 at 9 51 48 AM
melvin-bot[bot] commented 2 weeks ago

Job added to Upwork: https://www.upwork.com/jobs/~01b5aba96981a28449

melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @suneox (External)

melvin-bot[bot] commented 2 weeks ago

@garrettmknight, @suneox Whoops! This issue is 2 days overdue. Let's get this updated quick!

suneox commented 2 weeks ago

Still waiting on a proposal. I just re-tested on the latest staging and will wait for the auto-submission tomorrow. Then I’ll double-check the response data to ensure whether this issue can be handled on the client side or if it’s a BE issue.

suneox commented 1 week ago

The reportAction from auto-submission flow is missing amount field in originalMessage response, so I think we need BE check the response for reportAction.actionName = SUBMITTED & automatic: true

Screenshot 2024-09-04 at 02 35 10

melvin-bot[bot] commented 1 week ago

@garrettmknight, @youssef-lr, @suneox Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Beamanator commented 1 week ago

cc @marcochavezf - i wonder if this came from https://github.com/Expensify/App/pull/45020/files#r1750911757?

Beamanator commented 1 week ago

Because it looks like BEFORE that PR, we didn't look for a submitted report action to have originalAmount.amount, instead i believe we went off of report.total

Beamanator commented 1 week ago

So i think we CAN fix this externally, OR if we WANT there to be a amount in the report action, then ya this should probably fixed internally

youssef-lr commented 1 week ago

I think there isn't much we can do for old submitted actions where we never stored the amount. We can display the report's total instead, but what if the user added expenses to the report after it was submitted? another option is to just display "submitted this report" if we don't have the amount stored in the action

Beamanator commented 1 week ago

Very fair - I like the another option is to just display "submitted this report" if we don't have the amount stored in the action option best probably

puneetlath commented 1 week ago

I kind of question the value of putting the amount in this message. The report total is already at the top of the report, why duplicate the info?

Beamanator commented 1 week ago

My understanding of why "using the report total" isn't perfect is this situation, for example:

  1. User A creates a report in OldDot & adds an expense for $1.00 & submits
  2. User A clicks "Undo Submit" (only available in OldDot atm)
  3. User A adds another expense for $2.00, then submits again
Screenshot 2024-09-10 at 9 55 04 AM

In NewDot, would we prefer:

  1. BOTH report actions showing submitted $3.00? Or
  2. The first report action showing submitted $1.00, and the second report action showing submitted $3.00?

I think I'd prefer seeing 2️⃣

puneetlath commented 1 week ago

I don't think the report total is that useful in either scenario. I feel like it just makes things more complicated. I think in both cases just seeing Submitted the report would be plenty.

Beamanator commented 1 week ago

aah i think that's 1000% fair 👍

JmillsExpensify commented 1 day ago

Doesn't that break our pattern though? e.g.

JmillsExpensify commented 1 day ago

It could work still in order to remove this issue, as I agree that we keep getting hit by $0.00 system messages. Maybe though if we went this direction we don't used report and simply say Submitted if we want to simplify the logic as a starting point.

puneetlath commented 5 hours ago

Doesn't that break our pattern though? e.g. Submitted [total] Approved [total] Paid [total]

Yeah, I'm questioning whether the [total] pattern has any value, given that the report total is already its own field on the report. Just thinking that if we remove this [total] pattern then that's one less feature to maintain.

trjExpensify commented 5 hours ago

Expense amounts can be changed though after submission prior to approval, can't they? So the total acts as a snapshot at submission for the history.

Equally though, I think in cases like paid or approved, a notification that reads "paid $X with Expensify" is much better to include the amount you were paid back as validation, over a generic "paid this report" notification.

Then I guess the other consideration is that one-expense reports aren't "reports" in the UI since we did away with them, so using "this report" terminology doesn't quite fit when it looks and feels like just an expense for them.

puneetlath commented 5 hours ago

Ok, fair enough. It feels to me like a feature we built over and above what we had in OldDot, even though I don't recall customers bringing this up as a problem with OldDot. But if people feel strongly that this is a good and important feature, then I'm fine fixing it and maintaining it instead of removing it.

trjExpensify commented 5 hours ago

Yeah, OldDot had a totally independent notification system though with standalone copy and what not to notify for these actions. In NewDot, we don't have that, so the system message is how you get told you were paid $x.

melvin-bot[bot] commented 2 hours ago

@garrettmknight, @youssef-lr, @suneox Whoops! This issue is 2 days overdue. Let's get this updated quick!