Repository navigation
[NOQA] perf: stabilized chatReport into MoneyRequestReportPreview #93950
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
Changes from all commits
1198d56
a3c1d25
03bab5b
0b39fea
3d631f4
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 |
|---|---|---|
|
|
@@ -75,6 +75,9 @@ type ActionContentRouterProps = { | |
| /** Report for this action */ | ||
| report: OnyxEntry<OnyxTypes.Report>; | ||
|
|
||
| /** The chat report associated with the report for this action */ | ||
| chatReport: OnyxEntry<OnyxTypes.Report>; | ||
|
|
||
| /** ID of the original report from which the given reportAction is first created */ | ||
| originalReportID?: string; | ||
|
|
||
|
|
@@ -124,6 +127,7 @@ type ActionContentRouterProps = { | |
| function ActionContentRouter({ | ||
| action, | ||
| report, | ||
| chatReport, | ||
| originalReportID, | ||
| iouReport, | ||
| reportID, | ||
|
|
@@ -167,7 +171,7 @@ function ActionContentRouter({ | |
| <ChatTransactionPreview | ||
| action={action} | ||
| reportID={reportID} | ||
| chatReportID={report?.type === CONST.REPORT.TYPE.CHAT ? reportID : report?.chatReportID} | ||
| chatReport={report?.type === CONST.REPORT.TYPE.CHAT ? report : chatReport} | ||
| iouReport={report?.type === CONST.REPORT.TYPE.CHAT ? iouReport : report} | ||
| shouldShowSplitPreview={shouldShowSplitPreview} | ||
| transactionID={shouldShowSplitPreview ? moneyRequestOriginalMessage?.IOUTransactionID : undefined} | ||
|
|
@@ -192,6 +196,7 @@ function ActionContentRouter({ | |
| iouReportID={getIOUReportIDFromReportActionPreview(action)} | ||
| policyID={policyID} | ||
| chatReportID={reportID} | ||
| chatReport={report} | ||
|
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.
When the preview is rendered from the report list, Useful? React with 👍 / 👎.
Contributor
Author
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. Good catch, fixed 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.
When this preview is rendered from Useful? React with 👍 / 👎.
Contributor
Author
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. Fixed |
||
| action={action} | ||
| isHovered={hovered} | ||
| isWhisper={isWhisper} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -109,7 +109,8 @@ const renderPage = ({isWhisper = false, isHovered = false}: Partial<MoneyRequest | |
| policyID={mockChatReport.policyID} | ||
| action={mockAction} | ||
| iouReportID={mockIOUReport.reportID} | ||
| chatReportID={mockChatReport.chatReportID} | ||
| chatReportID={mockChatReport.reportID} | ||
| chatReport={mockChatReport} | ||
| onPaymentOptionsShow={() => {}} | ||
| onPaymentOptionsHide={() => {}} | ||
| isHovered={isHovered} | ||
|
|
@@ -130,7 +131,7 @@ const getTransactionDisplayAmountAndHeaderText = (transaction: Transaction) => { | |
| const isTransactionMadeWithCard = isManagedCardTransaction(transaction); | ||
| const cashOrCard = isTransactionMadeWithCard ? TestHelper.translateLocal('iou.card') : TestHelper.translateLocal('iou.cash'); | ||
| const transactionHeaderText = `${date} ${CONST.DOT_SEPARATOR} ${cashOrCard}`; | ||
| const transactionDisplayAmount = convertToDisplayString(transaction.amount, transaction.currency); | ||
| const transactionDisplayAmount = convertToDisplayString(-transaction.amount, transaction.currency); | ||
|
Collaborator
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. question: Any reason we're making the amount negative?
Contributor
Author
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. The test showed positive before only because it passed the wrong |
||
| return {transactionHeaderText, transactionDisplayAmount}; | ||
| }; | ||
|
|
||
|
|
@@ -179,7 +180,7 @@ const setReportPreviewData = ( | |
| }; | ||
|
|
||
| const setHasOnceLoadedReportActions = async (hasOnceLoadedReportActions: boolean) => { | ||
| await Onyx.merge(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${mockChatReport.chatReportID}`, { | ||
| await Onyx.merge(`${ONYXKEYS.COLLECTION.RAM_ONLY_REPORT_LOADING_STATE}${mockChatReport.reportID}`, { | ||
| hasOnceLoadedReportActions, | ||
| }); | ||
| }; | ||
|
|
@@ -223,7 +224,7 @@ describe('MoneyRequestReportPreview', () => { | |
| for (const transaction of arrayOfTransactions) { | ||
| const {transactionDisplayAmount, transactionHeaderText} = getTransactionDisplayAmountAndHeaderText(transaction); | ||
|
|
||
| expect(screen.getByText(transactionDisplayAmount)).toBeOnTheScreen(); | ||
| expect(screen.getAllByText(transactionDisplayAmount).length).toBeGreaterThan(0); | ||
|
Collaborator
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. question: Is this needed?
Contributor
Author
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. Yes, with the correct |
||
| expect(screen.getAllByText(transactionHeaderText)).toHaveLength(arrayOfTransactions.length); | ||
| expect(screen.getAllByText(transaction.merchant)).toHaveLength(arrayOfTransactions.length); | ||
| } | ||
|
|
||
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.
question: Should we update the
ReportActionItemTest? It still passeschatReport={undefined}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.
chatReportis only read when rendering a REPORT_PREVIEW action, and these tests don't render that action, so it's never used here.undefinedis valid and a real report wouldn't change anything