Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimistic report previews #21578

Merged
merged 28 commits into from
Jul 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
b70230f
Optimistic report preview when sending money.
cristipaval Jun 21, 2023
623b8a1
Merge remote-tracking branch 'origin/main' into cristi_optimistic-rep…
cristipaval Jun 23, 2023
7b177a5
Improve optimistic report preview when requesting money.
cristipaval Jun 23, 2023
1685f56
Optimistically update report preview when requesting money in an exis…
cristipaval Jun 23, 2023
62d0420
Optimistic report preview when pying money requests
cristipaval Jun 26, 2023
aa3a4b0
Merge remote-tracking branch 'origin/main' into cristi_optimistic-rep…
cristipaval Jun 27, 2023
723a5d6
Fix optimistic report preview when sending money.
cristipaval Jun 27, 2023
50ddd45
Fix optimistic report preview when paying money request
cristipaval Jun 27, 2023
f5e4f02
Fix lint.
cristipaval Jun 28, 2023
279732d
run prettier
cristipaval Jun 28, 2023
769b91b
Update src/libs/ReportUtils.js
cristipaval Jun 30, 2023
f3631c1
Merge remote-tracking branch 'origin/main' into cristi_optimistic-rep…
cristipaval Jun 30, 2023
b4a361e
Reuse logic to build report preview message.
cristipaval Jul 3, 2023
1679480
Merge remote-tracking branch 'origin/main' into cristi_optimistic-rep…
cristipaval Jul 3, 2023
daf707f
Remove unecessary param
cristipaval Jul 3, 2023
8b4ec46
update report preview when already exists at money request
cristipaval Jul 3, 2023
91e9371
improve optimistic report preview when paying money request
cristipaval Jul 3, 2023
6f88bd1
Cleanup report preview when splitting the bill.
cristipaval Jul 3, 2023
544604f
Fix console warning.
cristipaval Jul 3, 2023
80c24b1
Make Lint happy.
cristipaval Jul 4, 2023
40a62ad
Improve code readability in buildOptimisticReportPreview
cristipaval Jul 5, 2023
e17a359
Merge remote-tracking branch 'origin/main' into cristi_optimistic-rep…
cristipaval Jul 5, 2023
70bff8c
Update src/libs/ReportUtils.js
cristipaval Jul 6, 2023
7c97057
Merge remote-tracking branch 'origin/main' into cristi_optimistic-rep…
cristipaval Jul 6, 2023
bfe57c0
Fix.
cristipaval Jul 6, 2023
1f20f9a
Make optimistic reportActions data more readable
cristipaval Jul 6, 2023
201f426
Merge remote-tracking branch 'origin/main' into cristi_optimistic-rep…
cristipaval Jul 8, 2023
c379a5c
Always update successData
cristipaval Jul 10, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1081,10 +1081,10 @@ function getTransactionReportName(reportAction) {
* Get money request message for an IOU report
*
* @param {Object} report
* @param {Object} reportAction
* @param {Object} [reportAction={}]
* @returns {String}
*/
function getReportPreviewMessage(report, reportAction) {
function getReportPreviewMessage(report, reportAction = {}) {
const reportActionMessage = lodashGet(reportAction, 'message[0].html', '');

if (_.isEmpty(report) || !report.reportID) {
Expand Down Expand Up @@ -1561,26 +1561,27 @@ function buildOptimisticIOUReportAction(type, amount, currency, comment, partici
};
}

function buildOptimisticReportPreview(reportID, iouReportID, payeeAccountID) {
function buildOptimisticReportPreview(chatReport, iouReport) {
const message = getReportPreviewMessage(iouReport);
return {
reportActionID: NumberUtils.rand64(),
reportID,
created: DateUtils.getDBTime(),
reportID: chatReport.reportID,
actionName: CONST.REPORT.ACTIONS.TYPE.REPORTPREVIEW,
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
accountID: payeeAccountID,
originalMessage: {
linkedReportID: iouReport.reportID,
},
message: [
{
html: '',
text: '',
html: message,
text: message,
isEdited: false,
type: CONST.REPORT.MESSAGE.TYPE.COMMENT,
},
],
originalMessage: {
linkedReportID: iouReportID,
},
actorAccountID: currentUserAccountID,
created: DateUtils.getDBTime(),
accountID: iouReport.managerID || 0,
actorAccountID: iouReport.managerID || 0,
Comment on lines +1583 to +1584
Copy link
Member

@parasharrajat parasharrajat Jul 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be the same? Also, should we be using 0 as the default for IDs?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For when the action is created other request or send money the actor acountID will be the person creating it so that should be correct.

As far as it goes for the default I am not sure what else we should use here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's expected to be 0 for the default, and it happens the case when the workspace owes money. The workspace doesn't have an accountID and we have logic that shows the workspace as the actor for the report preview when the accountID is 0.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to know. Thanks.

};
}

Expand Down
91 changes: 64 additions & 27 deletions src/libs/actions/IOU.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ function buildOnyxDataForMoneyRequest(
reportPreviewAction,
isNewChatReport,
isNewIOUReport,
isNewReportPreviewAction,
) {
const optimisticData = [
{
Expand Down Expand Up @@ -124,9 +123,7 @@ function buildOnyxDataForMoneyRequest(
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport.reportID}`,
value: {
...(isNewChatReport ? {[chatCreatedAction.reportActionID]: chatCreatedAction} : {}),
[reportPreviewAction.reportActionID]: {
...(isNewReportPreviewAction ? reportPreviewAction : {created: DateUtils.getDBTime()}),
},
[reportPreviewAction.reportActionID]: reportPreviewAction,
Julesssss marked this conversation as resolved.
Show resolved Hide resolved
},
},
{
Expand Down Expand Up @@ -189,13 +186,9 @@ function buildOnyxDataForMoneyRequest(
},
}
: {}),
...(isNewReportPreviewAction
? {
[reportPreviewAction.reportActionID]: {
pendingAction: null,
},
}
: {}),
[reportPreviewAction.reportActionID]: {
pendingAction: null,
},
},
},
{
Expand Down Expand Up @@ -387,11 +380,14 @@ function requestMoney(report, amount, currency, payeeEmail, payeeAccountID, part
},
};

let isNewReportPreviewAction = false;
let reportPreviewAction = isNewIOUReport ? null : ReportActionsUtils.getReportPreviewAction(chatReport.reportID, iouReport.reportID);
if (!reportPreviewAction) {
isNewReportPreviewAction = true;
reportPreviewAction = ReportUtils.buildOptimisticReportPreview(chatReport.reportID, iouReport.reportID);
if (reportPreviewAction) {
reportPreviewAction.created = DateUtils.getDBTime();
const message = ReportUtils.getReportPreviewMessage(iouReport, reportPreviewAction);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reportPreviewAction.message[0].html = message;
reportPreviewAction.message[0].text = message;
Comment on lines +385 to +388
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAB we can do this in a follow up, but I see this logic is duplicated in a few places, so maybe we can create a function updateReportPreviewAction to do this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do it in a follow up PR 👍

} else {
reportPreviewAction = ReportUtils.buildOptimisticReportPreview(chatReport, iouReport);
}

// STEP 5: Build Onyx Data
Expand All @@ -406,7 +402,6 @@ function requestMoney(report, amount, currency, payeeEmail, payeeAccountID, part
reportPreviewAction,
isNewChatReport,
isNewIOUReport,
isNewReportPreviewAction,
);

// STEP 6: Make the request
Expand Down Expand Up @@ -649,11 +644,14 @@ function createSplitsAndOnyxData(participants, currentUserLogin, currentUserAcco
},
};

let isNewOneOnOneReportPreviewAction = false;
let oneOnOneReportPreviewAction = ReportActionsUtils.getReportPreviewAction(oneOnOneChatReport.reportID, oneOnOneIOUReport.reportID);
if (!oneOnOneReportPreviewAction) {
isNewOneOnOneReportPreviewAction = true;
oneOnOneReportPreviewAction = ReportUtils.buildOptimisticReportPreview(oneOnOneChatReport.reportID, oneOnOneIOUReport.reportID);
if (oneOnOneReportPreviewAction) {
oneOnOneReportPreviewAction.created = DateUtils.getDBTime();
const message = ReportUtils.getReportPreviewMessage(oneOnOneIOUReport, oneOnOneReportPreviewAction);
oneOnOneReportPreviewAction.message[0].html = message;
oneOnOneReportPreviewAction.message[0].text = message;
} else {
oneOnOneReportPreviewAction = ReportUtils.buildOptimisticReportPreview(oneOnOneChatReport, oneOnOneIOUReport);
}

// STEP 5: Build Onyx Data
Expand All @@ -668,7 +666,6 @@ function createSplitsAndOnyxData(participants, currentUserLogin, currentUserAcco
oneOnOneReportPreviewAction,
isNewOneOnOneChatReport,
isNewOneOnOneIOUReport,
isNewOneOnOneReportPreviewAction,
);

const splitData = {
Expand Down Expand Up @@ -944,14 +941,16 @@ function getSendMoneyParams(report, amount, currency, comment, paymentMethodType
true,
);

const reportPreviewAction = ReportUtils.buildOptimisticReportPreview(chatReport, optimisticIOUReport);

// First, add data that will be used in all cases
const optimisticChatReportData = {
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${chatReport.reportID}`,
value: {
...chatReport,
lastReadTime: DateUtils.getDBTime(),
lastVisibleActionCreated: optimisticIOUReportAction.created,
lastVisibleActionCreated: reportPreviewAction.created,
},
};
const optimisticIOUReportData = {
Expand All @@ -963,7 +962,7 @@ function getSendMoneyParams(report, amount, currency, comment, paymentMethodType
lastMessageHtml: optimisticIOUReportAction.message[0].html,
},
};
const optimisticReportActionsData = {
const optimisticIOUReportActionsData = {
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${optimisticIOUReport.reportID}`,
value: {
Expand All @@ -973,6 +972,13 @@ function getSendMoneyParams(report, amount, currency, comment, paymentMethodType
},
},
};
const optimisticChatReportActionsData = {
onyxMethod: Onyx.METHOD.SET,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport.reportID}`,
value: {
[reportPreviewAction.reportActionID]: reportPreviewAction,
},
};

const successData = [
{
Expand All @@ -989,6 +995,15 @@ function getSendMoneyParams(report, amount, currency, comment, paymentMethodType
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${optimisticTransaction.transactionID}`,
value: {pendingAction: null},
},
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport.reportID}`,
value: {
[reportPreviewAction.reportActionID]: {
pendingAction: null,
},
},
},
];

const failureData = [
Expand All @@ -1008,7 +1023,6 @@ function getSendMoneyParams(report, amount, currency, comment, paymentMethodType
// Change the method to set for new reports because it doesn't exist yet, is faster,
// and we need the data to be available when we navigate to the chat page
optimisticChatReportData.onyxMethod = Onyx.METHOD.SET;
optimisticReportActionsData.onyxMethod = Onyx.METHOD.SET;
optimisticIOUReportData.onyxMethod = Onyx.METHOD.SET;

// Set and clear pending fields on the chat report
Expand Down Expand Up @@ -1053,8 +1067,8 @@ function getSendMoneyParams(report, amount, currency, comment, paymentMethodType
},
};

// Add an optimistic created action to the optimistic reportActions data
optimisticReportActionsData.value[optimisticCreatedAction.reportActionID] = optimisticCreatedAction;
// Add an optimistic created action to the optimistic chat reportActions data
optimisticChatReportActionsData.value[optimisticCreatedAction.reportActionID] = optimisticCreatedAction;
} else {
failureData.push({
onyxMethod: Onyx.METHOD.MERGE,
Expand All @@ -1067,7 +1081,7 @@ function getSendMoneyParams(report, amount, currency, comment, paymentMethodType
});
}

const optimisticData = [optimisticChatReportData, optimisticIOUReportData, optimisticReportActionsData, optimisticTransactionData];
const optimisticData = [optimisticChatReportData, optimisticIOUReportData, optimisticChatReportActionsData, optimisticIOUReportActionsData, optimisticTransactionData];
if (!_.isEmpty(optimisticPersonalDetailListData)) {
optimisticData.push(optimisticPersonalDetailListData);
}
Expand All @@ -1081,6 +1095,7 @@ function getSendMoneyParams(report, amount, currency, comment, paymentMethodType
transactionID: optimisticTransaction.transactionID,
newIOUReportDetails,
createdReportActionID: isNewChat ? optimisticCreatedAction.reportActionID : 0,
reportPreviewReportActionID: reportPreviewAction.reportActionID,
},
optimisticData,
successData,
Expand Down Expand Up @@ -1115,6 +1130,12 @@ function getPayMoneyRequestParams(chatReport, iouReport, recipient, paymentMetho
login: recipient.login,
};

const optimisticReportPreviewAction = ReportActionsUtils.getReportPreviewAction(chatReport.reportID, iouReport.reportID);
optimisticReportPreviewAction.created = DateUtils.getDBTime();
const message = ReportUtils.getReportPreviewMessage(iouReport, optimisticReportPreviewAction);
optimisticReportPreviewAction.message[0].html = message;
optimisticReportPreviewAction.message[0].text = message;

const optimisticData = [
{
onyxMethod: Onyx.METHOD.MERGE,
Expand All @@ -1139,6 +1160,13 @@ function getPayMoneyRequestParams(chatReport, iouReport, recipient, paymentMetho
},
},
},
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport.reportID}`,
value: {
[optimisticReportPreviewAction.reportActionID]: optimisticReportPreviewAction,
},
},
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${iouReport.reportID}`,
Expand Down Expand Up @@ -1196,6 +1224,15 @@ function getPayMoneyRequestParams(chatReport, iouReport, recipient, paymentMetho
},
},
},
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport.reportID}`,
value: {
[optimisticReportPreviewAction.reportActionID]: {
created: optimisticReportPreviewAction.created,
},
},
},
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${optimisticTransaction.transactionID}`,
Expand Down