-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Optimistic report previews #21578
Changes from 27 commits
b70230f
623b8a1
7b177a5
1685f56
62d0420
aa3a4b0
723a5d6
50ddd45
f5e4f02
279732d
769b91b
f3631c1
b4a361e
1679480
daf707f
8b4ec46
91e9371
6f88bd1
544604f
80c24b1
40a62ad
e17a359
70bff8c
7c97057
bfe57c0
1f20f9a
201f426
c379a5c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -124,9 +124,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
|
||
}, | ||
}, | ||
{ | ||
|
@@ -389,9 +387,14 @@ function requestMoney(report, amount, currency, payeeEmail, payeeAccountID, part | |
|
||
let isNewReportPreviewAction = false; | ||
let reportPreviewAction = isNewIOUReport ? null : ReportActionsUtils.getReportPreviewAction(chatReport.reportID, iouReport.reportID); | ||
if (!reportPreviewAction) { | ||
if (reportPreviewAction) { | ||
reportPreviewAction.created = DateUtils.getDBTime(); | ||
const message = ReportUtils.getReportPreviewMessage(iouReport, reportPreviewAction); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi we missed an edge case - 1:1 DM disappears after opening IOU report page when Scan IOU is created offline More details here |
||
reportPreviewAction.message[0].html = message; | ||
reportPreviewAction.message[0].text = message; | ||
Comment on lines
+385
to
+388
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll do it in a follow up PR 👍 |
||
} else { | ||
isNewReportPreviewAction = true; | ||
reportPreviewAction = ReportUtils.buildOptimisticReportPreview(chatReport.reportID, iouReport.reportID); | ||
reportPreviewAction = ReportUtils.buildOptimisticReportPreview(chatReport, iouReport); | ||
} | ||
|
||
// STEP 5: Build Onyx Data | ||
|
@@ -651,9 +654,14 @@ function createSplitsAndOnyxData(participants, currentUserLogin, currentUserAcco | |
|
||
let isNewOneOnOneReportPreviewAction = false; | ||
let oneOnOneReportPreviewAction = ReportActionsUtils.getReportPreviewAction(oneOnOneChatReport.reportID, oneOnOneIOUReport.reportID); | ||
if (!oneOnOneReportPreviewAction) { | ||
if (oneOnOneReportPreviewAction) { | ||
oneOnOneReportPreviewAction.created = DateUtils.getDBTime(); | ||
const message = ReportUtils.getReportPreviewMessage(oneOnOneIOUReport, oneOnOneReportPreviewAction); | ||
oneOnOneReportPreviewAction.message[0].html = message; | ||
oneOnOneReportPreviewAction.message[0].text = message; | ||
} else { | ||
isNewOneOnOneReportPreviewAction = true; | ||
oneOnOneReportPreviewAction = ReportUtils.buildOptimisticReportPreview(oneOnOneChatReport.reportID, oneOnOneIOUReport.reportID); | ||
oneOnOneReportPreviewAction = ReportUtils.buildOptimisticReportPreview(oneOnOneChatReport, oneOnOneIOUReport); | ||
} | ||
|
||
// STEP 5: Build Onyx Data | ||
|
@@ -944,14 +952,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 = { | ||
|
@@ -963,7 +973,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: { | ||
|
@@ -973,6 +983,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 = [ | ||
{ | ||
|
@@ -989,6 +1006,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 = [ | ||
|
@@ -1008,7 +1034,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 | ||
|
@@ -1053,8 +1078,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, | ||
|
@@ -1067,7 +1092,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); | ||
} | ||
|
@@ -1081,6 +1106,7 @@ function getSendMoneyParams(report, amount, currency, comment, paymentMethodType | |
transactionID: optimisticTransaction.transactionID, | ||
newIOUReportDetails, | ||
createdReportActionID: isNewChat ? optimisticCreatedAction.reportActionID : 0, | ||
reportPreviewReportActionID: reportPreviewAction.reportActionID, | ||
}, | ||
optimisticData, | ||
successData, | ||
|
@@ -1115,6 +1141,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, | ||
|
@@ -1139,6 +1171,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}`, | ||
|
@@ -1196,6 +1235,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}`, | ||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 theaccountID
is 0.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know. Thanks.