[$250] Expense report -"Required" Error Not Triggered for Auto-Filled Report Field on Second Expense Submission
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: v9.0.19-0 Reproducible in staging?: Y Reproducible in production?: N Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause Internal Team
Action Performed:
- Go to FAB > Workspace.
- Navigate to Workspace Settings > More Features > Enable Report Field.
- In Report Field, add a field, name it, choose a type, and save.
- Go to Workspace Chat > Click the plus sign > Submit Expense > Add amount, merchant, and submit.
- In Expense report, fill in the Report Field > Notice that the error triggers > Go back to the workspace chat, click Submit, and select Pay Elsewhere.
- Submit a second expense by repeating Step 4 > Go to Expense report > Notice that the Report Field is auto-filled, but the error does trigger.
Expected Result:
The "Required" error message should not appear if the Report Field in the second expense is auto-filled with information from the first expense.
Actual Result:
The "Required" error message does not trigger for the Report Field on the second submitted expense when auto-filled.
Workaround:
Unknown
Platforms:
- [x] Android: Native
- [x] Android: mWeb Chrome
- [ ] iOS: Native
- [x] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [x] MacOS: Desktop
Screenshots/Videos
https://github.com/user-attachments/assets/69cc30dc-7e05-47f5-a160-6bc73c415c73
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~0185bccf1e3226865f
- Upwork Job ID: 1823114245300473767
- Last Price Increase: 2024-09-02
- Automatic offers:
- dukenv0307 | Reviewer | 103834642
- nkdengineer | Contributor | 103834644
Issue Owner
Current Issue Owner: @dukenv0307
Triggered auto assignment to @puneetlath (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.
Triggered auto assignment to @VictoriaExpensify (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.
:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:
- Identify the pull request that introduced this issue and revert it.
- Find someone who can quickly fix the issue.
- Fix the issue yourself.
We think that this bug might be related to #vip-vsb
Agree this is an issue and we should fix it.
Job added to Upwork: https://www.upwork.com/jobs/~0185bccf1e3226865f
Triggered auto assignment to Contributor-plus team member for initial proposal review - @dukenv0307 (External)
Report Fields are part of Wave_Control, so I'm adding this issue to that project
Report Fields are behind a beta, so i'm demoting this one
Edited by proposal-police: This proposal was edited at 2024-08-13 08:42:12 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
The "Required" error message does not trigger for the Report Field on the second submitted expense when auto-filled.
What is the root cause of that problem?
When we create an expense report, the fieldList is assigned as the fieldList of the policy.
https://github.com/Expensify/App/blob/420c2a13cc176dca5adf8dcc8132ef5f8ff678f7/src/libs/ReportUtils.ts#L4285
Then because the field has default value is empty, a report violation is added here. https://github.com/Expensify/App/blob/420c2a13cc176dca5adf8dcc8132ef5f8ff678f7/src/libs/actions/IOU.ts#L480-L486
After BE returns the data, this field is auto-filled but the report violation isn't cleared then the error still appears
What changes do you think we should make in order to solve the problem?
When we create an expense report here, we should auto-fill the report field from the last paid expense report of this policy expense chat (we can confirm the logic from BE).
const reportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReportID}`] ?? {};
const lastSettledReportPreviewAction = Object.values(reportActions)
.filter((action) => ReportActionsUtils.isReportPreviewAction(action) && isSettled(action.childReportID))
.sort((a, b) => {
if (a.created < b.created) {
return -1;
}
return 1;
})
?.at(-1);
const lastSettedExpenseReport = getReportOrDraftReport(lastSettledReportPreviewAction?.childReportID);
expenseReport.fieldList = lastSettedExpenseReport?.fieldList ?? policy?.fieldList;
return expenseReport;
https://github.com/Expensify/App/blob/420c2a13cc176dca5adf8dcc8132ef5f8ff678f7/src/libs/ReportUtils.ts#L4285
Then the violation will not appear here since the value already exists https://github.com/Expensify/App/blob/420c2a13cc176dca5adf8dcc8132ef5f8ff678f7/src/libs/actions/IOU.ts#L480-L486
What alternative solutions did you explore? (Optional)
If the violation is fieldRequired and the value exists we should return an empty string here
https://github.com/Expensify/App/blob/420c2a13cc176dca5adf8dcc8132ef5f8ff678f7/src/libs/ReportUtils.ts#L7471
and should not display a red dot here https://github.com/Expensify/App/blob/420c2a13cc176dca5adf8dcc8132ef5f8ff678f7/src/components/ReportActionItem/MoneyReportView.tsx#L122
@nkdengineer Your solution makes sense to me. Can you pls share the detail implementation? Thanks
@dukenv0307 Update my proposal with implementation detail. We can confirm with the internal team about this logic and the BE logic.
We can confirm with the internal team about this logic and the BE logic.
@puneetlath can you help check the concern above? Thanks
It feels to me like this is working as expected. But confirming here: https://expensify.slack.com/archives/C06ML6X0W9L/p1723746405468429
@puneetlath The problem here is I want to know how BE auto-fill the report field for an expense report when we create an expense. So I can do this in optimistic data to prevent the incorrect report violation in optimistic data.
@puneetlath, @VictoriaExpensify, @dukenv0307 Whoops! This issue is 2 days overdue. Let's get this updated quick!
@nkdengineer we are still discussing this one. It seems like the report fields should be sticky between the reports but when I tried on OldDot, it was not sticky. Can you try as well what is the different behaviour between NewDot and OldDot in this flow? Thanks!
@mountiny I tested in OldDot, it was sticky
https://github.com/user-attachments/assets/e2d9e0a2-6415-476f-85b2-cfba197181f2
@nkdengineer what if you create a new workspace and report and set up the report fields in oldDot solely (sorry mobile today so cant test myself) - i think that was the flow where i got no stickiness
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
@puneetlath, @VictoriaExpensify, @dukenv0307 Still overdue 6 days?! Let's take care of this!
what if you create a new workspace and report and set up the report fields in oldDot solely (sorry mobile today so cant test myself) - i think that was the flow where i got no stickiness
@mountiny I tested this case and the new report is also auto-filled by previous paid expense report after we paid an expense report.
It sounds like everything is working correctly then, is that right?
@puneetlath Yes BE is working correctly but we need to update the optimistic data in front-end to prevent incorrect violation is displayed.
The problem here is I want to know how BE auto-fill the report field for an expense report when we create an expense. So I can do this in optimistic data to prevent the incorrect report violation in optimistic data.
Can you check my concern above?
cc @mountiny
@puneetlath, @VictoriaExpensify, @dukenv0307 Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!
@nkdengineer Yes you are right, we need to update the optimistic case, althouhg I wonder if we have all the data for this available when user signs out and signs back in, can you check that and make a proposal for that case?
Not overdue
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
@nkdengineer are you able to look into this one further after my latest comment?
I'm checking this by testing some cases with many report previews. Will give an update tomorrow.