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.

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

Fix/34609: Remove old description page #35137

Merged
merged 34 commits into from
Feb 7, 2024
Merged

Fix/34609: Remove old description page #35137

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
199a826
fix Remove old description page
DylanDylann Jan 25, 2024
c983c5f
move change from old description page to new one
DylanDylann Jan 25, 2024
d93929e
update logic to save description in each case
DylanDylann Jan 25, 2024
e690103
fix lint
DylanDylann Jan 25, 2024
f1de3f2
fix remove redandunt changr
DylanDylann Jan 25, 2024
61632f5
move logic to IOU.js
DylanDylann Jan 27, 2024
df5b209
merge main
DylanDylann Jan 28, 2024
da8f760
remove comment
DylanDylann Jan 28, 2024
ccc3f5a
remove comment
DylanDylann Jan 29, 2024
481119f
update logic split flow
DylanDylann Jan 30, 2024
9d0e118
merge main
DylanDylann Jan 30, 2024
1c45441
early return and update name
DylanDylann Jan 30, 2024
37a2280
clean code
DylanDylann Jan 30, 2024
9cb9602
clean code
DylanDylann Jan 30, 2024
08ac5dc
Update src/pages/iou/request/step/IOURequestStepDescription.js
DylanDylann Jan 30, 2024
2233734
update iouType params
DylanDylann Jan 30, 2024
28af0b8
fix lint
DylanDylann Jan 30, 2024
c512a6e
merge main
DylanDylann Feb 2, 2024
0d60886
merge main
DylanDylann Feb 2, 2024
e3f6ecf
update param
DylanDylann Feb 2, 2024
6579704
merge main
DylanDylann Feb 5, 2024
7d8e0d5
fix types
DylanDylann Feb 5, 2024
e86aaa7
fix types
DylanDylann Feb 5, 2024
446a515
add draft trsanaction to get default value
DylanDylann Feb 5, 2024
aa29db2
merge main
DylanDylann Feb 5, 2024
937a390
merge main
DylanDylann Feb 5, 2024
75cec60
merge main
DylanDylann Feb 6, 2024
3e557ac
fix types
DylanDylann Feb 6, 2024
387113a
fix lint
DylanDylann Feb 6, 2024
c40f1d3
Update src/pages/iou/request/step/IOURequestStepDescription.js
DylanDylann Feb 6, 2024
2a8a2ff
merge main
DylanDylann Feb 6, 2024
79c4485
merge main
DylanDylann Feb 6, 2024
6c41870
merge main
DylanDylann Feb 7, 2024
4924c67
merge main
DylanDylann Feb 7, 2024
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
10 changes: 3 additions & 7 deletions src/ROUTES.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,10 +279,6 @@ const ROUTES = {
route: ':iouType/new/currency/:reportID?',
getRoute: (iouType: string, reportID: string, currency: string, backTo: string) => `${iouType}/new/currency/${reportID}?currency=${currency}&backTo=${backTo}` as const,
},
MONEY_REQUEST_DESCRIPTION: {
route: ':iouType/new/description/:reportID?',
getRoute: (iouType: string, reportID = '') => `${iouType}/new/description/${reportID}` as const,
},
MONEY_REQUEST_CATEGORY: {
route: ':iouType/new/category/:reportID?',
getRoute: (iouType: string, reportID = '') => `${iouType}/new/category/${reportID}` as const,
Expand Down Expand Up @@ -349,9 +345,9 @@ const ROUTES = {
getUrlWithBackToParam(`create/${iouType}/date/${transactionID}/${reportID}`, backTo),
},
MONEY_REQUEST_STEP_DESCRIPTION: {
route: 'create/:iouType/description/:transactionID/:reportID',
getRoute: (iouType: ValueOf<typeof CONST.IOU.TYPE>, transactionID: string, reportID: string, backTo = '') =>
getUrlWithBackToParam(`create/${iouType}/description/${transactionID}/${reportID}`, backTo),
route: ':action/:iouType/description/:transactionID/:reportID',
getRoute: (action: ValueOf<typeof CONST.IOU.ACTION>, iouType: ValueOf<typeof CONST.IOU.TYPE>, transactionID: string, reportID: string, backTo = '') =>
getUrlWithBackToParam(`${action}/${iouType}/description/${transactionID}/${reportID}`, backTo),
},
MONEY_REQUEST_STEP_DISTANCE: {
route: 'create/:iouType/distance/:transactionID/:reportID',
Expand Down
1 change: 0 additions & 1 deletion src/SCREENS.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ const SCREENS = {
CONFIRMATION: 'Money_Request_Confirmation',
CURRENCY: 'Money_Request_Currency',
DATE: 'Money_Request_Date',
DESCRIPTION: 'Money_Request_Description',
CATEGORY: 'Money_Request_Category',
TAG: 'Money_Request_Tag',
MERCHANT: 'Money_Request_Merchant',
Expand Down
13 changes: 10 additions & 3 deletions src/components/MoneyRequestConfirmationList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,11 +663,18 @@ function MoneyRequestConfirmationList(props) {
title={props.iouComment}
description={translate('common.description')}
onPress={() => {
if (props.isEditingSplitBill) {
Navigation.navigate(ROUTES.EDIT_SPLIT_BILL.getRoute(props.reportID, props.reportActionID, CONST.EDIT_REQUEST_FIELD.DESCRIPTION));
if (!props.isEditingSplitBill) {
return;
}
Navigation.navigate(ROUTES.MONEY_REQUEST_DESCRIPTION.getRoute(props.iouType, props.reportID));
Navigation.navigate(
ROUTES.MONEY_REQUEST_STEP_DESCRIPTION.getRoute(
CONST.IOU.ACTION.EDIT,
DylanDylann marked this conversation as resolved.
Show resolved Hide resolved
CONST.IOU.TYPE.SPLIT,
transaction.transactionID,
props.reportID,
Navigation.getActiveRouteWithoutParams(),
),
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why we are assuming that this is going to be used for edits only? (similar question for MoneyTemporaryForRefactorRequestConfirmationList.js)

Copy link
Contributor Author

@DylanDylann DylanDylann Jan 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@s77rt if props.isEditingSplitBill is false, we will disable this field by this logic

App/src/components/MoneyRequestConfirmationList.js

Line 754 in 9dbc389

interactive={!props.isReadOnly}

Because of isReadOnly={!isEditingSplitBill}

Copy link
Contributor

Choose a reason for hiding this comment

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

The isReadOnly is a different prop and we can't assume it will always be related to isEditingSplitBill.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why we are assuming that this is going to be used for edits only?

@s77rt In that time, there was only editing split bill that used this component and this component MoneyRequestConfirmationList also be removed in this refactor phrase. But you're right, we can't assume that this is going to be used for edits only. So I updated in this commit to remove condition

 if (!props.isEditingSplitBill) {
                        return;
}

and using iouType as param of navigate function

Copy link
Contributor

Choose a reason for hiding this comment

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

@DylanDylann Is there any need to change the logic there? Can't we just keep using the same logic (i.e. keep isEditingSplitBill conditions) and navigate to the correct pages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@s77rt We need to change logic to use a new route to IOURequestStepDescription for both case. The above code is outdated. This is code after updating

Screenshot 2024-01-31 at 23 51 43

}}
style={[styles.moneyRequestMenuItem]}
titleStyle={styles.flex1}
Expand Down
8 changes: 3 additions & 5 deletions src/components/MoneyTemporaryForRefactorRequestConfirmationList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -691,11 +691,9 @@ function MoneyTemporaryForRefactorRequestConfirmationList({
title={iouComment}
description={translate('common.description')}
onPress={() => {
if (isEditingSplitBill) {
Navigation.navigate(ROUTES.EDIT_SPLIT_BILL.getRoute(reportID, reportActionID, CONST.EDIT_REQUEST_FIELD.DESCRIPTION));
return;
}
Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_DESCRIPTION.getRoute(iouType, transaction.transactionID, reportID, Navigation.getActiveRouteWithoutParams()));
Navigation.navigate(
ROUTES.MONEY_REQUEST_STEP_DESCRIPTION.getRoute(CONST.IOU.ACTION.CREATE, iouType, transaction.transactionID, reportID, Navigation.getActiveRouteWithoutParams()),
);
}}
style={[styles.moneyRequestMenuItem]}
titleStyle={styles.flex1}
Expand Down
12 changes: 11 additions & 1 deletion src/components/ReportActionItem/MoneyRequestView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,17 @@ function MoneyRequestView({report, parentReport, parentReportActions, policyCate
interactive={canEdit}
shouldShowRightIcon={canEdit}
titleStyle={styles.flex1}
onPress={() => Navigation.navigate(ROUTES.EDIT_REQUEST.getRoute(report.reportID, CONST.EDIT_REQUEST_FIELD.DESCRIPTION))}
onPress={() =>
Navigation.navigate(
ROUTES.MONEY_REQUEST_STEP_DESCRIPTION.getRoute(
CONST.IOU.ACTION.EDIT,
CONST.IOU.TYPE.REQUEST,
transaction.transactionID,
report.reportID,
Navigation.getActiveRouteWithoutParams(),
),
)
}
wrapperStyle={[styles.pv2, styles.taskDescriptionMenuItem]}
brickRoadIndicator={hasViolations('comment') ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''}
numberOfLinesTitle={0}
Expand Down
1 change: 0 additions & 1 deletion src/libs/Navigation/AppNavigator/ModalStackNavigators.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ const MoneyRequestModalStackNavigator = createModalStackNavigator<MoneyRequestNa
[SCREENS.MONEY_REQUEST.CONFIRMATION]: () => require('../../../pages/iou/steps/MoneyRequestConfirmPage').default as React.ComponentType,
[SCREENS.MONEY_REQUEST.CURRENCY]: () => require('../../../pages/iou/IOUCurrencySelection').default as React.ComponentType,
[SCREENS.MONEY_REQUEST.DATE]: () => require('../../../pages/iou/MoneyRequestDatePage').default as React.ComponentType,
[SCREENS.MONEY_REQUEST.DESCRIPTION]: () => require('../../../pages/iou/MoneyRequestDescriptionPage').default as React.ComponentType,
[SCREENS.MONEY_REQUEST.CATEGORY]: () => require('../../../pages/iou/MoneyRequestCategoryPage').default as React.ComponentType,
[SCREENS.MONEY_REQUEST.TAG]: () => require('../../../pages/iou/MoneyRequestTagPage').default as React.ComponentType,
[SCREENS.MONEY_REQUEST.MERCHANT]: () => require('../../../pages/iou/MoneyRequestMerchantPage').default as React.ComponentType,
Expand Down
1 change: 0 additions & 1 deletion src/libs/Navigation/linkingConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,6 @@ const linkingConfig: LinkingOptions<RootStackParamList> = {
[SCREENS.MONEY_REQUEST.CONFIRMATION]: ROUTES.MONEY_REQUEST_CONFIRMATION.route,
[SCREENS.MONEY_REQUEST.DATE]: ROUTES.MONEY_REQUEST_DATE.route,
[SCREENS.MONEY_REQUEST.CURRENCY]: ROUTES.MONEY_REQUEST_CURRENCY.route,
[SCREENS.MONEY_REQUEST.DESCRIPTION]: ROUTES.MONEY_REQUEST_DESCRIPTION.route,
[SCREENS.MONEY_REQUEST.CATEGORY]: ROUTES.MONEY_REQUEST_CATEGORY.route,
[SCREENS.MONEY_REQUEST.TAG]: ROUTES.MONEY_REQUEST_TAG.route,
[SCREENS.MONEY_REQUEST.MERCHANT]: ROUTES.MONEY_REQUEST_MERCHANT.route,
Expand Down
3 changes: 2 additions & 1 deletion src/libs/Navigation/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ type MoneyRequestNavigatorParamList = {
field: string;
threadReportID: string;
};
[SCREENS.MONEY_REQUEST.DESCRIPTION]: {
[SCREENS.MONEY_REQUEST.STEP_DESCRIPTION]: {
DylanDylann marked this conversation as resolved.
Show resolved Hide resolved
action: string;
iouType: string;
reportID: string;
DylanDylann marked this conversation as resolved.
Show resolved Hide resolved
field: string;
Expand Down
32 changes: 24 additions & 8 deletions src/libs/actions/IOU.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,14 +198,6 @@ function setMoneyRequestCurrency_temporaryForRefactor(transactionID, currency) {
Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {currency});
}

/**
* @param {String} transactionID
* @param {String} comment
*/
function setMoneyRequestDescription_temporaryForRefactor(transactionID, comment) {
Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`, {comment: {comment: comment.trim()}});
}

/**
* @param {String} transactionID
* @param {String} merchant
Expand Down Expand Up @@ -3698,6 +3690,30 @@ function getIOUReportID(iou, route) {
return lodashGet(route, 'params.reportID') || lodashGet(iou, 'participants.0.reportID', '');
}

/**
* @param {Object} transaction
* @param {String} comment
* @param {Boolean} isDraft
* @param {String} iouType
* @param {String} reportID
*/
function setMoneyRequestDescription_temporaryForRefactor(transaction, comment, isDraft, iouType, reportID) {
// Only update comment if it has changed
if (comment.trim() === lodashGet(transaction, 'comment.comment', '')) {
return;
}
DylanDylann marked this conversation as resolved.
Show resolved Hide resolved
if (isDraft) {
Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transaction.transactionID}`, {comment: {comment}});
} else {
if (iouType === CONST.IOU.TYPE.REQUEST) {
updateMoneyRequestDescription(transaction.transactionID, reportID, comment);
}
if (iouType === CONST.IOU.TYPE.SPLIT) {
setDraftSplitTransaction(transaction.transactionID, {comment});
}
DylanDylann marked this conversation as resolved.
Show resolved Hide resolved
}
}

export {
setMoneyRequestParticipants,
createDistanceRequest,
Expand Down
91 changes: 0 additions & 91 deletions src/pages/EditRequestDescriptionPage.js
View file
Open in desktop

This file was deleted.

45 changes: 24 additions & 21 deletions src/pages/EditRequestPage.js
View file
Open in desktop
DylanDylann marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import ROUTES from '@src/ROUTES';
import EditRequestAmountPage from './EditRequestAmountPage';
import EditRequestCategoryPage from './EditRequestCategoryPage';
import EditRequestCreatedPage from './EditRequestCreatedPage';
import EditRequestDescriptionPage from './EditRequestDescriptionPage';
// import EditRequestDescriptionPage from './EditRequestDescriptionPage';
DylanDylann marked this conversation as resolved.
Show resolved Hide resolved
import EditRequestDistancePage from './EditRequestDistancePage';
import EditRequestMerchantPage from './EditRequestMerchantPage';
import EditRequestReceiptPage from './EditRequestReceiptPage';
Expand Down Expand Up @@ -74,7 +74,7 @@ function EditRequestPage({report, route, policyCategories, policyTags, parentRep
const {
amount: transactionAmount,
currency: transactionCurrency,
comment: transactionDescription,
// comment: transactionDescription,
DylanDylann marked this conversation as resolved.
Show resolved Hide resolved
merchant: transactionMerchant,
category: transactionCategory,
tag: transactionTag,
Expand Down Expand Up @@ -180,25 +180,28 @@ function EditRequestPage({report, route, policyCategories, policyTags, parentRep
[transactionCategory, transaction.transactionID, report.reportID],
);

const saveComment = useCallback(
({comment: newComment}) => {
// Only update comment if it has changed
if (newComment.trim() !== transactionDescription) {
IOU.updateMoneyRequestDescription(transaction.transactionID, report.reportID, newComment.trim());
}
Navigation.dismissModal();
},
[transactionDescription, transaction.transactionID, report.reportID],
);

if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.DESCRIPTION) {
return (
<EditRequestDescriptionPage
defaultDescription={transactionDescription}
onSubmit={saveComment}
/>
);
}
// I removed EditRequestDescriptionPage because we won't use this component anymore. This page EditRequestPage also be removed in https://github.com/Expensify/App/issues/29107

// const saveComment = useCallback(
// ({comment: newComment}) => {
// // Only update comment if it has changed
// if (newComment.trim() !== transactionDescription) {
// IOU.updateMoneyRequestDescription(transaction.transactionID, report.reportID, newComment.trim());
// }
// Navigation.dismissModal();
// },
// [transactionDescription, transaction.transactionID, report.reportID],
// );

//
// if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.DESCRIPTION) {
// return (
// <EditRequestDescriptionPage
// defaultDescription={transactionDescription}
// onSubmit={saveComment}
// />
// );
// }

if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.DATE) {
return (
Expand Down
29 changes: 15 additions & 14 deletions src/pages/EditSplitBillPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import ROUTES from '@src/ROUTES';
import EditRequestAmountPage from './EditRequestAmountPage';
import EditRequestCategoryPage from './EditRequestCategoryPage';
import EditRequestCreatedPage from './EditRequestCreatedPage';
import EditRequestDescriptionPage from './EditRequestDescriptionPage';
// import EditRequestDescriptionPage from './EditRequestDescriptionPage';
DylanDylann marked this conversation as resolved.
Show resolved Hide resolved
import EditRequestMerchantPage from './EditRequestMerchantPage';
import EditRequestTagPage from './EditRequestTagPage';
import reportPropTypes from './reportPropTypes';
Expand Down Expand Up @@ -58,7 +58,7 @@ function EditSplitBillPage({route, transaction, draftTransaction, report}) {
const {
amount: transactionAmount,
currency: transactionCurrency,
comment: transactionDescription,
// comment: transactionDescription,
DylanDylann marked this conversation as resolved.
Show resolved Hide resolved
merchant: transactionMerchant,
created: transactionCreated,
category: transactionCategory,
Expand All @@ -76,18 +76,19 @@ function EditSplitBillPage({route, transaction, draftTransaction, report}) {
navigateBackToSplitDetails();
};

if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.DESCRIPTION) {
return (
<EditRequestDescriptionPage
defaultDescription={transactionDescription}
onSubmit={(transactionChanges) => {
setDraftSplitTransaction({
comment: transactionChanges.comment.trim(),
});
}}
/>
);
}
// I removed EditRequestDescriptionPage because we won't use this component anymore. This page EditSplitBillPage also be removed in https://github.com/Expensify/App/issues/29107
// if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.DESCRIPTION) {
// return (
// <EditRequestDescriptionPage
// defaultDescription={transactionDescription}
// onSubmit={(transactionChanges) => {
// setDraftSplitTransaction({
// comment: transactionChanges.comment.trim(),
// });
// }}
// />
// );
// }

if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.DATE) {
return (
Expand Down
Loading
Loading