-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Fix/72129b - No checkmark displayed for selected recipient in choose recipient page #78354
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
b7f5709
2e75d3e
8bba565
f522d00
0120ff6
07bca03
56451e2
a088c65
663deb9
8ef981a
af514f2
7d4740d
3a6e9fb
5ce8ae2
c2ab915
f398d6f
e110ad5
c3de042
669e39b
44da0d9
7f607c4
b062e93
ec0029c
65f524d
98d9f5a
dee4d85
43c3e01
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 |
|---|---|---|
|
|
@@ -34,6 +34,7 @@ import { | |
| resetSplitShares, | ||
| setDraftSplitTransaction, | ||
| setMoneyRequestAmount, | ||
| setMoneyRequestParticipants, | ||
| setMoneyRequestParticipantsFromReport, | ||
| setMoneyRequestTaxAmount, | ||
| setMoneyRequestTaxRate, | ||
|
|
@@ -316,12 +317,15 @@ function IOURequestStepAmount({ | |
|
|
||
| // Starting from global + menu means no participant context exists yet, | ||
| // so we need to handle participant selection based on available workspace settings | ||
| const isReturningFromConfirmationPage = !!transaction?.participants?.length; | ||
| const firstParticipant = transaction?.participants?.at(0); | ||
| const isP2PChat = isParticipantP2P(firstParticipant, currentUserPersonalDetails.accountID); | ||
| const isNegativeAmount = convertToBackendAmount(Number.parseFloat(amount)) < 0; | ||
| if (shouldUseDefaultExpensePolicy(iouType, defaultExpensePolicy)) { | ||
| const shouldAutoReport = !!defaultExpensePolicy?.autoReporting || !!personalPolicy?.autoReporting; | ||
| const targetReport = shouldAutoReport ? getPolicyExpenseChat(currentUserAccountIDParam, defaultExpensePolicy?.id) : selfDMReport; | ||
| const transactionReportID = isSelfDM(targetReport) ? CONST.REPORT.UNREPORTED_REPORT_ID : targetReport?.reportID; | ||
| const iouTypeTrackOrSubmit = transactionReportID === CONST.REPORT.UNREPORTED_REPORT_ID ? CONST.IOU.TYPE.TRACK : CONST.IOU.TYPE.SUBMIT; | ||
| const isReturningFromConfirmationPage = !!transaction?.participants?.length; | ||
|
|
||
| const resetToDefaultWorkspace = () => { | ||
| setTransactionReport(transactionID, {reportID: transactionReportID}, true); | ||
|
|
@@ -331,10 +335,6 @@ function IOURequestStepAmount({ | |
| }; | ||
|
|
||
| if (isReturningFromConfirmationPage) { | ||
| const firstParticipant = transaction?.participants?.at(0); | ||
| const isP2PChat = isParticipantP2P(firstParticipant); | ||
| const isNegativeAmount = convertToBackendAmount(Number.parseFloat(amount)) < 0; | ||
|
|
||
| // P2P chats don't support negative amounts, so reset to default workspace when amount is negative. | ||
| if (isP2PChat && isNegativeAmount) { | ||
| resetToDefaultWorkspace(); | ||
|
|
@@ -353,6 +353,13 @@ function IOURequestStepAmount({ | |
| } else { | ||
| resetToDefaultWorkspace(); | ||
| } | ||
| } | ||
| // P2P chats don't support negative amounts, so in cases where there is no default workspace when the amount is negative, we will remove the selected transaction participants. | ||
| else if (iouType === CONST.IOU.TYPE.CREATE && isP2PChat && isNegativeAmount && isReturningFromConfirmationPage) { | ||
| setTransactionReport(transactionID, {reportID: undefined}, true); | ||
| setMoneyRequestParticipants(transactionID, [], true).then(() => { | ||
| navigateToParticipantPage(iouType, transactionID, reportID); | ||
| }); | ||
|
Comment on lines
+357
to
+362
Contributor
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 think we can remove this logic and instead update the block at lines 344–347 from: to With this change, we ensure the transaction participants are always cleared when navigating from the amount page.
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. I believe we should avoid doing this. If we always clear the transaction participants, Test 4 will consistently fail. |
||
| } else { | ||
| Navigation.setNavigationActionToMicrotaskQueue(() => { | ||
| navigateToParticipantPage(iouType, transactionID, reportID); | ||
|
|
@@ -472,8 +479,8 @@ function IOURequestStepAmount({ | |
| /** | ||
| * Check if the participant is a P2P chat | ||
| */ | ||
| function isParticipantP2P(participant: {accountID?: number; isPolicyExpenseChat?: boolean} | undefined): boolean { | ||
| return !!(participant?.accountID && !participant.isPolicyExpenseChat); | ||
| function isParticipantP2P(participant: {accountID?: number; isPolicyExpenseChat?: boolean} | undefined, currentUserAccountID?: number): boolean { | ||
| return !!(participant?.accountID && !participant.isPolicyExpenseChat && (!currentUserAccountID || participant?.accountID !== currentUserAccountID)); | ||
| } | ||
|
|
||
| // eslint-disable-next-line rulesdir/no-negated-variables | ||
|
|
||
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.
You're mixing
loginandreportIDin the sameloginsToExcludeobject. This can be confusing since the name suggests it only contains logins.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.
I believe I have only corrected its functionality, since we have also used it together with the name
loginsToExcludehere:App/src/libs/OptionsListUtils/index.ts
Lines 2002 to 2005 in 2abf7ab