Migrate the next steps to client side by rezkiy37 · Pull Request #34450 · Expensify/App

rezkiy37 · 2024-01-12T16:23:09Z

Details

Generates an optimistic nextStep based on a current report status and other properties. Includes unit-tests that cover 100% of a main function - buildNextStep.

Fixed Issues

$ #28771
PROPOSAL: N/A

Tests

Note: we migrate next step prediction as much as possible. However, the app cannot predict all violations or some other complex states. So, the backend can respond with a little bit detailed next step or even another message like an error or warning. I describe all possible cases that we calculate on the client side in "Tests". Also, since we test how optimistic data works, I recommend to enable slow Internet or offline mode.

Pre requirements:

Create/have 2 users at least. One of them is a member of workspaces. The 2nd one is an admin.
Create/have 2 paid workspaces via OD: Collect and Control. The feature does not work for a free workspace.
Use OD to manipulate with the workspaces.

1. Generates an optimistic `nextStep` once a report has been OPENED

1. Self review

The member: opens a workspace chat.
The member: requests money.
The member: Verify that there is a next step with a comment: "Waiting for you to submit these expenses." and label is "Next Steps".

2. Scheduled submit enabled

The admin: sets "Scheduled Submit" enabled via OD in the "Reports" section.
The admin: selects each value one by one. The label is "Next Steps" and corresponding comments:

Daily: "These expenses are scheduled to automatically submit later today! No further action required!";
Weekly: "These expenses are scheduled to automatically submit on Sunday! No further action required!";
Twice a month: "These expenses are scheduled to automatically submit on the 1st and 16th of each month! No further action required!";
Monthly: "These expenses are scheduled to automatically submit on the [Selected report submission date] of each month! No further action required!";
Trip: "These expenses are scheduled to automatically submit at the end of your trip! No further action required!";
Manual: "Waiting for you to submit these expenses.";

The member: requests money.
The member: Verify that there is a next step with a corresponding comment mentioned above.

3. Self submitting prevented

The admin: sets "Prevent Self-Approval" enabled via OD in the "Members" section > "Workflow Enforcement".
The admin: requests money.
The admin: Verify that there is a next step with a comment: "Oops! Looks like you're submitting to yourself. Approving your own reports is forbidden by your policy. Please submit this report to someone else or contact your admin to change the person you submit to." and label is "Next Steps".

2. Generates an optimistic `nextStep` once a report has been SUBMITTED

1. Self review

Do steps: 1.3.
The admin: clicks on a "Submit" button.
The admin: Verify that there is a next step with a comment: "Waiting for you* to review these expenses." and label is "Next Steps".

2. Another reviewer

Do steps: 1.1.
The member: clicks on a "Submit" button.
The member: Verify that there is a next step with a comment: "Waiting for [the admin's display name]* to approve these expenses." and label is "Next Steps".

3. Another owner

Do steps: 1.1.
The admin: clicks on a "Submit" button.
The admin: Verify that there is a next step with a comment: "[the member's login]* is waiting for you to review these expenses." and label is "Next Steps" and label is "Next Steps". However, when you are not a manager a verb review is replaced with approve.

3. Generates an optimistic `nextStep` once a report has been APPROVED

1. Self review

Do steps: 1.3 and 2.1.
The admin: clicks on an "Approve" button.
The admin: Verify that there is a next step with a comment: "Waiting for you* to review these expenses." and label is "Next Steps".

2. Another owner

Do steps: 1.1 and 2.3.
The admin: clicks on an "Approve" button.
The admin: Verify that there is a next step with a comment: "No further action required!" and label is "Finished!".

4. Generates an optimistic `nextStep` once a report has been PAID

1. Paid with wallet

Do steps: 1.1 and 2.3, 3.1 or 3.2.
The admin: clicks on a "Pay elsewhere" button.
The admin: Verify that there is a next step with a comment: "You have marked these expenses as paid." and label is "Finished!".

2. Paid outside of Expensify

Do steps: 1.1 and 2.3, 3.1 or 3.2.
The admin: clicks on a "Pay with Expensify" button.
The admin: Verify that there is a next step with a comment: "You have marked these expenses as paid outside of Expensify." and label is "Finished!".

Verify that no errors appear in the JS console

Offline tests

Same as "Tests".

QA Steps

Same as "Tests".

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android.1.1.mp4

Android.1.2.Last.business.day.mp4

Android.1.3.mp4

Android: mWeb Chrome

Android.Chrome.1.1.mp4

Android.Chrome.1.2.Trip.mov

Android.Chrome.1.3.mp4

iOS: Native

IOS.1.1.mov

IOS.1.2.Last.business.day.mp4

IOS.1.3.mov

iOS: mWeb Safari

IOS.Safari.1.1.mov

IOS.Safari.1.2.Trip.mov

IOS.Safari.1.3.mov

MacOS: Chrome / Safari

Chrome.1.1.mp4

Chrome.1.2.Daily.mp4

Chrome.1.2.Monthly.3rd.mp4

Chrome.1.3.mov

Chrome.2.1.mov

Chrome.2.2.mov

Chrome.2.3.mov

MacOS: Desktop

Desktop.1.1.mp4

Desktop.1.2.Twice.a.month.mov

Desktop.1.3.mov

…ture/28771-migrate-next-steps-to-client

allroundexperts

Testing well. Thanks for resolving the conflicts!

…ture/28771-migrate-next-steps-to-client

mountiny

will try to review tomorrow

mountiny

will try to review tomorrow

…ture/28771-migrate-next-steps-to-client

mountiny

@rezkiy37 ok couple comments

mountiny · 2024-02-08T19:25:38Z

+            // Another owner
+            if (!isOwner) {
+                optimisticNextStep.title = 'Finished!';
+                optimisticNextStep.message = [
+                    {
+                        text: 'No further action required!',
+                    },
+                ];
+            }


An approver could also be an admin who still needs to reimburse I think, how would this work in this case? It would show them they are done but they still need to pay it, right?

An approver could also be an admin...
Yes, could be. I think it is always an admin, it is not?

Based on a message that we get from the backend, it should be "No further action....". So, looks like it works properly.

Approved.mov

Ok lets go with this and see if some issues will be reported!

allroundexperts

~~I see conflicts but still tested it so that this does not get blocked.~~ It is working good. The chat switching is much snappier now.

…ture/28771-migrate-next-steps-to-client

rezkiy37 · 2024-02-09T13:08:34Z

@mountiny, please take a look. I've applied the suggestions.

mountiny · 2024-02-09T14:12:38Z

@allroundexperts was this a comment for wrong PR? 😄

mountiny

Great work @rezkiy37 really appreciate it!

@allroundexperts thank you for testing!

mountiny · 2024-02-09T14:25:56Z

+            // Another owner
+            if (!isOwner) {
+                optimisticNextStep.title = 'Finished!';
+                optimisticNextStep.message = [
+                    {
+                        text: 'No further action required!',
+                    },
+                ];
+            }


Ok lets go with this and see if some issues will be reported!

OSBotify · 2024-02-09T14:31:51Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2024-02-12T14:20:01Z

🚀 Deployed to staging by https://github.com/mountiny in version: 1.4.40-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	failure ❌

OSBotify · 2024-02-13T18:25:22Z

🚀 Deployed to production by https://github.com/mountiny in version: 1.4.40-5 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	failure ❌

rezkiy37 added 2 commits January 12, 2024 17:22

add a new report flag

0230d23

draft implementation of buildNextStep

c2f14f8

rezkiy37 mentioned this pull request Jan 12, 2024

[HOLD for payment 2024-02-20] [LOW] Drafts: Migrate the next steps to client side wherever it makes sense #28771

Closed

mountiny assigned rezkiy37 Jan 15, 2024

rezkiy37 added 25 commits January 15, 2024 16:22

Merge branch 'main' of https://github.com/rezkiy37/Expensify into fea…

c2c25b2

…ture/28771-migrate-next-steps-to-client

cover all basic scenarios

04d2c44

Merge branch 'main' of https://github.com/rezkiy37/Expensify into fea…

24552b0

…ture/28771-migrate-next-steps-to-client

rename const

a32e22e

clear todo

bbe860d

integrate timestamp handling

9387b10

add a case with random for manual approval

090d8ff

add comment

f77cea1

integrate predictedNextStatus argument

58baa0a

replace review with approve once submit

13d2d7f

handle manager case

81616da

improve adding the random for manual approval message

003d387

integrate optimistic next step generation

85731fb

Merge branch 'main' of https://github.com/rezkiy37/Expensify into fea…

9942130

…ture/28771-migrate-next-steps-to-client

minor improvements

fa2c8fb

remove log

dfec8f6

improve and fix tests

59a036d

Merge branch 'main' of https://github.com/rezkiy37/Expensify into fea…

ba8e6ff

…ture/28771-migrate-next-steps-to-client

Merge branch 'main' of https://github.com/rezkiy37/Expensify into fea…

9ea9a31

…ture/28771-migrate-next-steps-to-client

integrate last business day calculation

8481f66

Merge branch 'main' of https://github.com/rezkiy37/Expensify into fea…

ae300a4

…ture/28771-migrate-next-steps-to-client

prettify codebase

7467e2c

remove one case for now

38bfb70

Merge branch 'main' of https://github.com/rezkiy37/Expensify into fea…

8703eb1

…ture/28771-migrate-next-steps-to-client

Merge branch 'main' of https://github.com/rezkiy37/Expensify into fea…

f2041bf

…ture/28771-migrate-next-steps-to-client

rezkiy37 marked this pull request as ready for review January 29, 2024 09:51

Merge branch 'main' of https://github.com/rezkiy37/Expensify into fea…

9e3ad05

…ture/28771-migrate-next-steps-to-client

allroundexperts approved these changes Feb 7, 2024

View reviewed changes

melvin-bot Bot requested a review from mountiny February 7, 2024 06:26

Merge branch 'main' of https://github.com/rezkiy37/Expensify into fea…

ea00b24

…ture/28771-migrate-next-steps-to-client

mountiny reviewed Feb 8, 2024

View reviewed changes

Comment thread src/libs/NextStepUtils.ts Outdated

Comment thread src/libs/NextStepUtils.ts Outdated

mountiny reviewed Feb 8, 2024

View reviewed changes

rezkiy37 added 2 commits February 8, 2024 10:40

Merge branch 'main' of https://github.com/rezkiy37/Expensify into fea…

afe058a

…ture/28771-migrate-next-steps-to-client

remove isHarvestingEnabled ans use another default value for accountID

bebeea9

rezkiy37 requested a review from mountiny February 8, 2024 09:48

integrate harvesting value

bbe1426

mountiny reviewed Feb 8, 2024

View reviewed changes

allroundexperts approved these changes Feb 9, 2024

View reviewed changes

melvin-bot Bot requested a review from mountiny February 9, 2024 00:02

rezkiy37 added 5 commits February 9, 2024 13:43

Merge branch 'main' of https://github.com/rezkiy37/Expensify into fea…

4e0bb21

…ture/28771-migrate-next-steps-to-client

use merge method

655d9de

check if a report created as submitted

e70ec81

check if a report is expense

12a28c1

flip a condition once submit

b9ac4a3

mountiny approved these changes Feb 9, 2024

View reviewed changes

mountiny merged commit bb1fc0a into Expensify:main Feb 9, 2024

github-actions Bot mentioned this pull request Feb 12, 2024

Deploy Checklist: New Expensify 2024-02-12 #36335

Closed

90 tasks

This was referenced Feb 13, 2024

[$500] Expense - Approver gets "Finished" instead of "Next step" after approving report #36419

Closed

[$500] Web - Admin can submit and approve their own report when "Prevent Self-Approval" is on #36425

Closed

rlinoz mentioned this pull request Feb 14, 2024

Expense - "Next step" message changes when requesting money offline #36510

Closed

6 tasks

rayane-d mentioned this pull request Feb 17, 2024

[HOLD for payment 2024-02-26] [$500] Cancel payment - "Next step" does not update after cancelling payment until page is refreshed #36705

Closed

6 tasks

Conversation

rezkiy37 commented Jan 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details

Fixed Issues

Tests

1. Generates an optimistic nextStep once a report has been OPENED

1. Self review

2. Scheduled submit enabled

3. Self submitting prevented

2. Generates an optimistic nextStep once a report has been SUBMITTED

1. Self review

2. Another reviewer

3. Another owner

3. Generates an optimistic nextStep once a report has been APPROVED

1. Self review

2. Another owner

4. Generates an optimistic nextStep once a report has been PAID

1. Paid with wallet

2. Paid outside of Expensify

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

allroundexperts left a comment

Choose a reason for hiding this comment

Uh oh!

mountiny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mountiny left a comment

Choose a reason for hiding this comment

Uh oh!

mountiny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mountiny Feb 8, 2024

Choose a reason for hiding this comment

Uh oh!

rezkiy37 Feb 9, 2024

Choose a reason for hiding this comment

Uh oh!

mountiny Feb 9, 2024

Choose a reason for hiding this comment

Uh oh!

allroundexperts left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rezkiy37 commented Feb 9, 2024

Uh oh!

mountiny commented Feb 9, 2024

Uh oh!

mountiny left a comment

Choose a reason for hiding this comment

Uh oh!

mountiny Feb 9, 2024

Choose a reason for hiding this comment

Uh oh!

OSBotify commented Feb 9, 2024

Uh oh!

OSBotify commented Feb 12, 2024

Uh oh!

OSBotify commented Feb 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

rezkiy37 commented Jan 12, 2024 •

edited

Loading

1. Generates an optimistic `nextStep` once a report has been OPENED

2. Generates an optimistic `nextStep` once a report has been SUBMITTED

3. Generates an optimistic `nextStep` once a report has been APPROVED

4. Generates an optimistic `nextStep` once a report has been PAID

allroundexperts left a comment •

edited

Loading