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

Hide transaction detail and dismiss the edit modal when the request is deleted #24590

Merged
merged 8 commits into from
Aug 16, 2023
6 changes: 6 additions & 0 deletions src/components/ReportActionItem/MoneyRequestView.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ function MoneyRequestView({report, parentReport, shouldShowHorizontalRule, polic
description += ` • ${translate('iou.pending')}`;
}

// A temporary solution to hide the transaction detail
Copy link
Contributor

Choose a reason for hiding this comment

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

@dukenv0307 What was the plan for this once the transaction is added as a prop (which it is now)?

// This will be removed after we probably add the transaction as a prop
dukenv0307 marked this conversation as resolved.
Show resolved Hide resolved
if (ReportActionsUtils.isDeletedAction(parentReportAction)) {
return null;
}

return (
<View>
<View style={[StyleUtils.getReportWelcomeContainerStyle(isSmallScreenWidth), StyleUtils.getMinimumHeight(CONST.EMPTY_STATE_BACKGROUND.MONEY_REPORT.MIN_HEIGHT)]}>
Expand Down
6 changes: 3 additions & 3 deletions src/libs/TransactionUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ function getCurrency(transaction) {
if (currency) {
return currency;
}
return lodashGet(transaction, 'currency', '');
return lodashGet(transaction, 'currency', CONST.CURRENCY.USD);
}

/**
Expand All @@ -150,11 +150,11 @@ function getCurrency(transaction) {
* @returns {String}
*/
function getCreated(transaction) {
const created = lodashGet(transaction, 'modifiedCreated', '');
const created = lodashGet(transaction, 'modifiedCreated', '') || lodashGet(transaction, 'created', '');
if (created) {
return format(new Date(created), CONST.DATE.FNS_FORMAT_STRING);
}
return format(new Date(lodashGet(transaction, 'created', '')), CONST.DATE.FNS_FORMAT_STRING);
return format(new Date(), CONST.DATE.FNS_FORMAT_STRING);
dukenv0307 marked this conversation as resolved.
Show resolved Hide resolved
}

export {buildOptimisticTransaction, getUpdatedTransaction, getTransaction, getDescription, getAmount, getCurrency, getCreated};
42 changes: 33 additions & 9 deletions src/pages/EditRequestPage.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import React from 'react';
import React, {useEffect} from 'react';
import PropTypes from 'prop-types';
import lodashGet from 'lodash/get';
import {withOnyx} from 'react-native-onyx';
import {format} from 'date-fns';
import compose from '../libs/compose';
import CONST from '../CONST';
import Navigation from '../libs/Navigation/Navigation';
import ONYXKEYS from '../ONYXKEYS';
Expand Down Expand Up @@ -31,24 +32,40 @@ const propTypes = {

/** The report object for the thread report */
report: reportPropTypes,

/** The parent report object for the thread report */
parentReport: reportPropTypes,
};

const defaultProps = {
report: {},
parentReport: {},
};

function EditRequestPage({report, route}) {
const transactionID = lodashGet(ReportActionsUtils.getParentReportAction(report), 'originalMessage.IOUTransactionID', '');
function EditRequestPage({report, route, parentReport}) {
const parentReportAction = ReportActionsUtils.getParentReportAction(report);
const transactionID = lodashGet(parentReportAction, 'originalMessage.IOUTransactionID', '');
const transaction = TransactionUtils.getTransaction(transactionID);
const transactionDescription = TransactionUtils.getDescription(transaction);
const transactionAmount = TransactionUtils.getAmount(transaction, ReportUtils.isExpenseReport(ReportUtils.getParentReport(report)));
const transactionAmount = TransactionUtils.getAmount(transaction, ReportUtils.isExpenseReport(parentReport));
const transactionCurrency = TransactionUtils.getCurrency(transaction);

// Take only the YYYY-MM-DD value
const transactionCreatedDate = new Date(TransactionUtils.getCreated(transaction));
const transactionCreated = format(transactionCreatedDate, CONST.DATE.FNS_FORMAT_STRING);
const fieldToEdit = lodashGet(route, ['params', 'field'], '');

const isDeleted = ReportActionsUtils.isDeletedAction(parentReportAction);
const isSetted = ReportUtils.isSettled(parentReport.reportID);

// Dismiss the modal when the request is paid or deleted
useEffect(() => {
if (!isDeleted && !isSetted) {
return;
}
Navigation.dismissModal();
}, [isDeleted, isSetted]);
Copy link
Contributor

@Ollyws Ollyws Aug 15, 2023

Choose a reason for hiding this comment

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

With the modal open, If you delete a money request without any message you'll be lead to the "Hmm it's not here" page. Is this intended?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should go to the parent report then

Copy link
Contributor

Choose a reason for hiding this comment

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

Same logic as when deleting the request without the modal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When we delete the request if the report of the request has a comment, we don't navigate to parent report. If not, report will be deleted, and then we cannot get the parent report from the report, so I think dismissing the modal will be fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Besides, if user A is currently in the thread transaction report and on another device user A deletes the request, not found page will also appear.

Copy link
Contributor

Choose a reason for hiding this comment

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

lets just mimic the same behaviour as if the modal would not be opened (+dismiss it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In EditRequestPage, for the case, we delete the request from another device if the report doesn't have any message, report will be deleted so we cannot get the parentReport through report to navigate to parent report.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And the behavior is already the same when we delete the request from another device.

Screencast.from.16-08-2023.07.25.58.webm

Copy link
Contributor

Choose a reason for hiding this comment

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

Seeing as this is the current behaviour on production, perhaps it's out of scope here.


// Update the transaction object and close the modal
function editMoneyRequest(transactionChanges) {
IOU.editMoneyRequest(transactionID, report.reportID, transactionChanges);
Expand Down Expand Up @@ -116,8 +133,15 @@ function EditRequestPage({report, route}) {
EditRequestPage.displayName = 'EditRequestPage';
EditRequestPage.propTypes = propTypes;
EditRequestPage.defaultProps = defaultProps;
export default withOnyx({
report: {
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${route.params.threadReportID}`,
},
})(EditRequestPage);
export default compose(
withOnyx({
report: {
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${route.params.threadReportID}`,
},
}),
withOnyx({
parentReport: {
key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT}${report ? report.parentReportID : '0'}`,
},
}),
)(EditRequestPage);
Loading