Skip to content

Fix split bill flow in App #13385

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.

Sign up for GitHub

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

Merged
luacmartins merged 6 commits into from
Dec 20, 2022
Merged

Fix split bill flow in App #13385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ae98613
add reportID param
luacmartins Dec 6, 2022
9dd1fec
rename param
luacmartins Dec 6, 2022
68b626c
get existing group
luacmartins Dec 6, 2022
6aa864f
rm hasMultipleParticipants logic
luacmartins Dec 8, 2022
5380b4a
fix logic with single participant
luacmartins Dec 8, 2022
96d5544
fix typo
luacmartins Dec 9, 2022
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
128 changes: 57 additions & 71 deletions src/libs/actions/IOU.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,13 +242,16 @@ function requestMoney(report, amount, currency, recipientEmail, participant, com
* @param {String} comment
* @param {String} currency
* @param {String} locale
* @param {String} existingGroupChatReportID
*
* @return {Object}
*/
function createSplitsAndOnyxData(participants, currentUserLogin, amount, comment, currency, locale) {
function createSplitsAndOnyxData(participants, currentUserLogin, amount, comment, currency, locale, existingGroupChatReportID = '') {
const currentUserEmail = OptionsListUtils.addSMSDomainIfPhoneNumber(currentUserLogin);
const participantLogins = _.map(participants, participant => OptionsListUtils.addSMSDomainIfPhoneNumber(participant.login).toLowerCase());
const existingGroupChatReport = ReportUtils.getChatByParticipants(participantLogins);
const existingGroupChatReport = existingGroupChatReportID
? chatReports[`${ONYXKEYS.COLLECTION.REPORT}${existingGroupChatReportID}`]
: ReportUtils.getChatByParticipants(participantLogins);
const groupChatReport = existingGroupChatReport || ReportUtils.buildOptimisticChatReport(participantLogins);
const groupCreatedReportAction = existingGroupChatReport ? {} : ReportUtils.buildOptimisticCreatedReportAction(currentUserEmail);
const groupChatReportMaxSequenceNumber = lodashGet(groupChatReport, 'maxSequenceNumber', 0);
Expand Down Expand Up @@ -333,8 +336,8 @@ function createSplitsAndOnyxData(participants, currentUserLogin, amount, comment
return;
}

// If we only have one participant, the oneOnOneChatReport is the groupChatReport
const existingOneOnOneChatReport = hasMultipleParticipants ? ReportUtils.getChatByParticipants([email]) : groupChatReport;
// If we only have one participant and the request was initiated from the global create menu, i.e. !existingGroupChatReportID, the oneOnOneChatReport is the groupChatReport
const existingOneOnOneChatReport = (!hasMultipleParticipants && !existingGroupChatReportID) ? groupChatReport : ReportUtils.getChatByParticipants([email]);
const oneOnOneChatReport = existingOneOnOneChatReport || ReportUtils.buildOptimisticChatReport([email]);
let oneOnOneIOUReport;
let existingIOUReport = null;
Expand Down Expand Up @@ -382,77 +385,59 @@ function createSplitsAndOnyxData(participants, currentUserLogin, amount, comment
createChat: existingOneOnOneChatReport ? null : CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
};

// If we only have one other participant, we just need to update onyxData for the groupChatReport and add an iouReportAction of type = create
// If we have more participants, we need to push the new oneOnOneChaReport, the create reportAction and iouReportAction of type = create to onyxData
if (!hasMultipleParticipants) {
optimisticData[0].value = oneOnOneChatReport;
optimisticData[1].value = {
...optimisticData[1].value,
[oneOnOneIOUReportAction.sequenceNumber]: oneOnOneIOUReportAction,
};
successData[1].value = {
...successData[1].value,
[oneOnOneIOUReportAction.sequenceNumber]: {pendingAction: null},
};
failureData[1].value = {
...failureData[1].value,
[oneOnOneIOUReportAction.sequenceNumber]: {pendingAction: null},
};
} else {
optimisticData.push(
{
onyxMethod: existingOneOnOneChatReport ? CONST.ONYX.METHOD.MERGE : CONST.ONYX.METHOD.SET,
key: `${ONYXKEYS.COLLECTION.REPORT}${oneOnOneChatReport.reportID}`,
value: oneOnOneChatReport,
},
{
onyxMethod: existingOneOnOneChatReport ? CONST.ONYX.METHOD.MERGE : CONST.ONYX.METHOD.SET,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${oneOnOneChatReport.reportID}`,
value: {
...oneOnOneCreatedReportAction,
[oneOnOneIOUReportAction.sequenceNumber]: oneOnOneIOUReportAction,
},
optimisticData.push(
{
onyxMethod: existingOneOnOneChatReport ? CONST.ONYX.METHOD.MERGE : CONST.ONYX.METHOD.SET,
key: `${ONYXKEYS.COLLECTION.REPORT}${oneOnOneChatReport.reportID}`,
value: oneOnOneChatReport,
},
{
onyxMethod: existingOneOnOneChatReport ? CONST.ONYX.METHOD.MERGE : CONST.ONYX.METHOD.SET,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${oneOnOneChatReport.reportID}`,
value: {
...oneOnOneCreatedReportAction,
[oneOnOneIOUReportAction.sequenceNumber]: oneOnOneIOUReportAction,
},
);
},
);

successData.push(
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${oneOnOneChatReport.reportID}`,
value: {pendingFields: {createChat: null}},
},
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${oneOnOneChatReport.reportID}`,
value: {
0: {pendingAction: null},
[oneOnOneIOUReportAction.sequenceNumber]: {pendingAction: null},
},
successData.push(
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${oneOnOneChatReport.reportID}`,
value: {pendingFields: {createChat: null}},
},
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${oneOnOneChatReport.reportID}`,
value: {
0: {pendingAction: null},
[oneOnOneIOUReportAction.sequenceNumber]: {pendingAction: null},
},
);
},
);

failureData.push(
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${oneOnOneChatReport.reportID}`,
value: {
pendingFields: {createChat: null},
hasOutstandingIOU: existingOneOnOneChatReport ? existingOneOnOneChatReport.hasOutstandingIOU : false,
iouReportID: existingOneOnOneChatReport ? existingOneOnOneChatReport.iouReportID : null,
maxSequenceNumber: oneOnOneChatReportMaxSequenceNumber,
lastReadSequenceNumber: oneOnOneChatReportMaxSequenceNumber,
},
failureData.push(
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${oneOnOneChatReport.reportID}`,
value: {
pendingFields: {createChat: null},
hasOutstandingIOU: existingOneOnOneChatReport ? existingOneOnOneChatReport.hasOutstandingIOU : false,
iouReportID: existingOneOnOneChatReport ? existingOneOnOneChatReport.iouReportID : null,
maxSequenceNumber: oneOnOneChatReportMaxSequenceNumber,
lastReadSequenceNumber: oneOnOneChatReportMaxSequenceNumber,
},
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${oneOnOneChatReport.reportID}`,
value: {
0: {pendingAction: null},
[oneOnOneIOUReportAction.sequenceNumber]: {pendingAction: null},
},
},
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${oneOnOneChatReport.reportID}`,
value: {
0: {pendingAction: null},
[oneOnOneIOUReportAction.sequenceNumber]: {pendingAction: null},
},
);
}
},
);

// Regardless of the number of participants, we always want to push the iouReport update to onyxData
optimisticData.push({
Expand Down Expand Up @@ -500,9 +485,10 @@ function createSplitsAndOnyxData(participants, currentUserLogin, amount, comment
* @param {String} comment
* @param {String} currency
* @param {String} locale
* @param {String} existingGroupChatReportID
*/
function splitBill(participants, currentUserLogin, amount, comment, currency, locale) {
const {groupData, splits, onyxData} = createSplitsAndOnyxData(participants, currentUserLogin, amount, comment, currency, locale);
function splitBill(participants, currentUserLogin, amount, comment, currency, locale, existingGroupChatReportID = '') {
const {groupData, splits, onyxData} = createSplitsAndOnyxData(participants, currentUserLogin, amount, comment, currency, locale, existingGroupChatReportID);

API.write('SplitBill', {
reportID: groupData.chatReportID,
Comment on lines +490 to 494

@mountiny mountiny Dec 9, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@luacmartins As Monil pointed out, we are not passing the existingGroupChatReportID to the API here.

@mountiny mountiny Dec 9, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

requesting a change

@luacmartins luacmartins Dec 9, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We actually don't need to pass this param to the API. We have both SplitBill and SplitBillAndOpenReport commands and that's enough for the API to figure out what it needs to do without the need to pass this param.

@luacmartins luacmartins Dec 9, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The front end needs the ID to get the groupChat when it's created from within a chat though

@mountiny mountiny Dec 9, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That makes sense!

Expand Down
1 change: 1 addition & 0 deletions src/pages/iou/IOUModal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ class IOUModal extends Component {
this.state.comment,
this.props.iou.selectedCurrencyCode,
this.props.preferredLocale,
reportID,
);
return;
}
Expand Down