[HOLD for payment 2024-03-19] [$500] [CRITICAL] [Backwards Compatibility] Update UI for closed expense reports with no expenses

[amyevans]

Problem

When a report is deleted in OldDot, it is currently hard-deleted, along with any comments on the report. As we move to a chat-centric NewDot, it will be more common to have reports with a comment history, and losing that context upon report deletion would be confusing for participants.

Solution

For reports with comments, we will update the backend (internal ref: https://github.com/Expensify/Expensify/issues/350244) to instead soft-delete the report, putting its statusNum to 2 (CLOSED). Note that reports without comments will still be hard-deleted.

On the front end, we need to update the UI so that the report preview shows [Deleted report] (similar to how we show [Deleted request] or [Deleted message] in other scenarios) and the report can still be commented on.

So for reference, the three UI states for a closed report, depending on a combo of the report's type and number of expenses, would be:

1. closed chat report: existing UI of an archived chat room
2. closed expense report with expenses: existing UI of a completed expense report with no action required
3. closed expense report with no expenses: (what we're building here) an archived expense report, which only exists to house comment history (and can still be commented on)

Setup/Reproduction steps

1. Go to OldDot and open the settings for the Collect/Control policy (create one first if needed)
2. In the JS console with the policy open, run:

p = Policy.getCurrent();
p.policy.isPolicyExpenseChatEnabled = "true";
p.save();

3. Create an open expense report (but do not add expenses to it)
4. Comment on it
5. In NewDot, open the report and grab its reportID from the URL
6. In the console, run:

window.Onyx.merge(`report_<myReportID>`, {
    statusNum: 2,
});

(or put this in a useEffect hook in a component)

Feature implementation requirements

• Report preview in the workspace chat should render [Deleted report]
• Option row in the LHN should render [Deleted report] as appropriate
• The expense report should render according to this mock:
  

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0105da3eb4f9af3f30
• Upwork Job ID: 1756061610101006336
• Last Price Increase: 2024-02-09
• Automatic offers:
• abdulrahuman5196 | Reviewer | 0
• rayane-djouah | Contributor | 0

[melvin-bot]

Triggered auto assignment to @alexpensify (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0105da3eb4f9af3f30

[melvin-bot]

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

[rayane-djouah]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Update UI for closed expense reports with no expenses.

What is the root cause of that problem?

This is a new feature.

What changes do you think we should make in order to solve the problem?

First, we should add an isDeletedReport function in https://github.com/Expensify/App/blob/main/src/libs/ReportUtils.ts

function isDeletedReport(report: OnyxEntry<Report>): boolean {
    return report?.statusNum === CONST.REPORT.STATUS_NUM.CLOSED && report?.stateNum === CONST.REPORT.STATE_NUM.OPEN;
}

To make the report preview in the workspace chat render [Deleted report] we should add the below condition here.

if (ReportUtils.isDeletedReport(ReportUtils.getReport(ReportActionsUtils.getIOUReportIDFromReportActionPreview(props.action) ?? ''))){
  children = (
      <ReportPreview
           ...
      />
);
}else{
    children = (
        <RenderHTML html={`<comment>${translate('parentReportAction.deletedReport')}</comment>`} />
    );
}

To make the option row in the LHN render [Deleted report] we should add the below code here

    if (isDeletedReport(report)) {
        return translate('parentReportAction.deletedReport');
    }

We should add translations for parentReportAction.deletedReport.

To make the expense report render according to the mock:

1. we should not render the money report total

we should add here the below condition:

<View style={[StyleUtils.getReportWelcomeTopMarginStyle(isSmallScreenWidth, true)]}>
      {!isDeletedReport(report) &&
         <>
         //content
        </>
        }
    </View>

2. we should not render next steps in the report header, we should add this condition here:

const shouldShowNextStep = !ReportUtils.isDeletedReport(moneyRequestReport) && isFromPaidPolicy && !!nextStep?.message?.length;

3. To display the manager/workspace icon and name only without the owner/member icon and name, we should:
   add here this:

        if(isDeletedReport(report)){
            return [workspaceIcon];
        }

and add here this:

        if(isDeletedReport(report)){
            return [workspaceIcon];
        }

and add here this:

        if(isDeletedReport(report)){
            return [managerIcon];
        }

and we should adjust the condition here to:

const displayAllActors = useMemo(() => action.actionName === CONST.REPORT.ACTIONS.TYPE.REPORTPREVIEW && iouReport && !ReportUtils.isDeletedReport(iouReport), [action.actionName, iouReport]);

Result:

delete.reportRecording.2024-02-10.015537.mp4

deleted.report.Recording.2024-02-10.020230.mp4

What alternative solutions did you explore? (Optional)

N/A

[amyevans]

@abdulrahuman5196 could you review the above proposal please?

[greg-schroeder]

bump @abdulrahuman5196

[abdulrahuman5196]

Hi, @greg-schroeder sorry I haven't looked into this yet. Will work on review today.

[abdulrahuman5196]

Reviewing now

[abdulrahuman5196]

Meanwhile questions on the usecase I have?

closed expense report with no expenses

Does this mean settled expenses? I am unable to delete a approved report in oldDot also currently. Only able to delete expense report in processing state

Is this usecase only for transition of oldDot to newDot?
In new dot i don't see a way of creating expense report without expense directly. One way to achieve this is to create expense request and add comments and delete the request alone without deleting the report.
But still I don't have a option to delete the total expense report in new dot.

Could you kindly provide information on how this expense report with no expenses can be achieved for testing? Kindly do let me know if I had misunderstood something here.

@amyevans

[abdulrahuman5196]

Anyways I don't want to block the proposal review phase on the above questions. Since the proposal is to do a straightforward implementation after the closed expense report with no expenses state is achieved.

The proposal by @rayane-djouah here looks good #36190 (comment) and has good amount of information/cases to implement the feature. We can do through testing in PR phase.

🎀 👀 🎀
C+ Reviewed

cc: @amyevans

[melvin-bot]

Current assignee @amyevans is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[amyevans]

closed expense report with no expenses

Does this mean settled expenses?

No, not settled expenses, literally no expenses (an empty report).

I am unable to delete a approved report in oldDot also currently. Only able to delete expense report in processing state

Being unable to delete an approved report in OldDot is expected. You shouldn't be able to delete a report in the processing state either though (you need to retract it first), so I am confused about how you are seeing that. Maybe share a recording and the email(s) you're testing with? Currently when deleting an Open report, this is what happens:

delete-report.mov

1. any expenses are removed from the report (become unreported) and 2. the report is hard-deleted from the database

I will be updating the backend so that point 2 only happens in the future if the report has no comments, and if the report does have comments, then it will instead become Closed. Here's that scenario in action locally for me:

new-behavior.mov

But that backend work is not live yet and may not be for several weeks because it's held up on some other in-progress work. We shouldn't let that block progress here though.

---

Is this usecase only for transition of oldDot to newDot?

Yes, since you can't delete a report from NewDot, this only applies to someone taking the action in OldDot.

In new dot i don't see a way of creating expense report without expense directly. One way to achieve this is to create expense request and add comments and delete the request alone without deleting the report.
But still I don't have a option to delete the total expense report in new dot.

Yep, all correct.

Could you kindly provide information on how this expense report with no expenses can be achieved for testing?

1. create expense request
2. add comments
3. delete the request
4. add a useEffect hook containing the following to modify the status of the report to Closed:

useEffect(() => {
    window.Onyx.merge(`report_547026414937073`, {
        statusNum: 2,
    });
});

Hope that helps but definitely happy to clarify further if not!

[greg-schroeder]

Ready for review @abdulrahuman5196!

[alexpensify]

Catching up from being OOO, it looks like the PR is waiting for @abdulrahuman5196's feedback.

[greg-schroeder]

Bump @abdulrahuman5196, this is a Critical item so it would be great to get that review started!

[abdulrahuman5196]

Sorry for the delay. Will work on review today

Edit: Provided review and comments

[greg-schroeder]

Let us know your progress on this @rayane-djouah!

[rayane-djouah]

I updated the PR

[alexpensify]

Update: Making progress in the PR

[alexpensify]

Heads up, I will be offline until Tuesday, March 5, 2024. I will not be actively watching over this GitHub during that period. @mallenexpensify is stepping in as the BZ member here and will keep an eye that we get the design feedback needed to keep this one moving forward.

[mallenexpensify]

@abdulrahuman5196 , can you please provide an update, this in the PR is from 3 days ago.

I checked the working of the PR. Seems to be working fine. Will check on the response for the above question and proceed accordingly.

[abdulrahuman5196]

Hi, @mallenexpensify it is still under discussion #36767 (comment)

[alexpensify]

@mallenexpensify -- thanks for your help here, I'm taking this one back as the BZ member at this party.

[alexpensify]

Ok, I'm caught up here. The PR is now waiting for @amyevans's final review. Let's keep pushing forward here.

[alexpensify]

Awesome, this PR is ready to go to staging.

[alexpensify]

Looks like we are still waiting for this one to go to staging.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.50-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Backwards Compatibility: Update UI for closed expense reports with expenses #36767

If no regressions arise, payment will be issued on 2024-03-19. 🎊

For reference, here are some details about the assignees on this issue:

• @abdulrahuman5196 requires payment automatic offer (Reviewer)
• @rayane-djouah requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@abdulrahuman5196] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
• [@alexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

[alexpensify]

Here is the payment summary:

• External issue reporter N/A
• Contributor that fixed the issue @rayane-djouah $500
• Contributor+ that helped on the issue and/or PR @abdulrahuman5196 $500

Upwork Job: https://www.upwork.com/jobs/~0105da3eb4f9af3f30

Extra Notes regarding payment: Everyone has been paid in Upwork.