[$250] Expense report -"Required" Error Not Triggered for Auto-Filled Report Field on Second Expense Submission

[IuliiaHerets]

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: v9.0.19-0 Reproducible in staging?: Y Reproducible in production?: N Email or phone of affected tester (no customers): abebemiherat@gmail.com Issue reported by: Applause Internal Team

Action Performed:

1. Go to FAB > Workspace.
2. Navigate to Workspace Settings > More Features > Enable Report Field.
3. In Report Field, add a field, name it, choose a type, and save.
4. Go to Workspace Chat > Click the plus sign > Submit Expense > Add amount, merchant, and submit.
5. 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.
6. 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

View all open jobs on GitHub

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

[melvin-bot[bot]]

Triggered auto assignment to @puneetlath (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot[bot]]

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.

[github-actions[bot]]

: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:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[IuliiaHerets]

We think that this bug might be related to #vip-vsb

[VictoriaExpensify]

Agree this is an issue and we should fix it.

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~0185bccf1e3226865f

[melvin-bot[bot]]

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

[VictoriaExpensify]

Report Fields are part of Wave_Control, so I'm adding this issue to that project

[Beamanator]

Report Fields are behind a beta, so i'm demoting this one

[nkdengineer]

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 isfieldRequired 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

[dukenv0307]

@nkdengineer Your solution makes sense to me. Can you pls share the detail implementation? Thanks

[nkdengineer]

@dukenv0307 Update my proposal with implementation detail. We can confirm with the internal team about this logic and the BE logic.

[dukenv0307]

We can confirm with the internal team about this logic and the BE logic.

@puneetlath can you help check the concern above? Thanks

[puneetlath]

It feels to me like this is working as expected. But confirming here: https://expensify.slack.com/archives/C06ML6X0W9L/p1723746405468429

[nkdengineer]

@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.

[melvin-bot[bot]]

@puneetlath, @VictoriaExpensify, @dukenv0307 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[mountiny]

@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!

[nkdengineer]

@mountiny I tested in OldDot, it was sticky

https://github.com/user-attachments/assets/e2d9e0a2-6415-476f-85b2-cfba197181f2

[mountiny]

@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

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot[bot]]

@puneetlath, @VictoriaExpensify, @dukenv0307 Still overdue 6 days?! Let's take care of this!

[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

@mountiny I tested this case and the new report is also auto-filled by previous paid expense report after we paid an expense report.

[puneetlath]

It sounds like everything is working correctly then, is that right?

[nkdengineer]

@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

[melvin-bot[bot]]

@puneetlath, @VictoriaExpensify, @dukenv0307 Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

[mountiny]

@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?

[mountiny]

Not overdue

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[mountiny]

@nkdengineer are you able to look into this one further after my latest comment?

[nkdengineer]

I'm checking this by testing some cases with many report previews. Will give an update tomorrow.

[melvin-bot[bot]]

@puneetlath @mountiny @VictoriaExpensify @dukenv0307 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!

[nkdengineer]

@mountiny I checked, and sometimes the last paid expense report doesn't exist when we logout and login again. Can we return the fieldList of the last paid expense report in the data of policyExpenseChat report like the iouReportID field that is the last unsettled expense report?

[mountiny]

Hmm I am not sure if this is really a feasible approach for us. Is there a way we could just avoid showing the optimistic violation rather and wait for the onyx data to come from the backend to only then show this violation @nkdengineer

[nkdengineer]

@mountiny Yeah we can prevent adding the report violation in optimistic data when we create a new expense report. What do you think?

[mountiny]

could we do both:

• detect we have the fieldList data we need to potentially show the optimistic violation and show it
• if we do not have the necessary data locally do not show the violation optimistically

[melvin-bot[bot]]

@puneetlath, @mountiny, @VictoriaExpensify, @dukenv0307 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[nkdengineer]

detect we have the fieldList data we need to potentially show the optimistic violation and show it

@mountiny I think it's hard to detect, for example, we cannot know if the paid expense report is the latest or not or cannot know if we have the paid or not because in OpenApp API, the data may not contain all REPORTPREVIEW actions or expense reports of the policy expense chat

[mountiny]

@nkdengineer Yeah this is not easy thing to resolve. I need to think about this more. What would you propose now to handle this gracefully without adding any more data from backend?

[VictoriaExpensify]

Not overdue

[nkdengineer]

What would you propose now to handle this gracefully without adding any more data from backend?

@mountiny I propose to not add field violation to optimistic data here if we create a new expense report.

https://github.com/Expensify/App/blob/b9af3d1f7ab160f7d817102d081b0e6186ba2d02/src/libs/actions/IOU.ts#L888-L890

[mountiny]

@nkdengineer Are none of the reliably computable optimistically? I agree it might be cleaner to not show this optimistically as they can be complex and wait for backend

[nkdengineer]

Are none of the reliably computable optimistically? I agree it might be cleaner to not show this optimistically as they can be complex and wait for backend

@mountiny Yes, only when we edit the report field we can computable the optimistic data correctly because in this case, we can know exactly the value of report field.

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[mountiny]

I think to fix this properly we would need to load all this data in OpenApp - which means the latest paid report fieldList for all control policies user is member of. I think that is something we should not add as it could slow down already stretched OpenApp call.

So i would vote to not create these optimistically as often the user wont have this data available when they sign in and create a new report. @puneetlath do you have any concerns about that?

[melvin-bot[bot]]

@puneetlath, @mountiny, @VictoriaExpensify, @dukenv0307 Eep! 4 days overdue now. Issues have feelings too...

[puneetlath]

Could we get it when opening the report instead of when opening the app?

[nkdengineer]

Could we get it when opening the report instead of when opening the app?

We can create the money request via FAB without opening the report.

[mountiny]

We can create the money request via FAB without opening the report.

+1

[puneetlath]

Ahh ok, sorry I didn't correctly understand. Would it be possible to create them optimistically if we have the data, but don't create them optimistically if we don't?

Otherwise, what you suggest makes sense to me.

[VictoriaExpensify]

Not overdue