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

[$500] IOU - Description, merchant, date order changed after creating split bill in group chat. #34566

Closed
3 of 6 tasks
kbecciv opened this issue Jan 16, 2024 · 10 comments
Closed
3 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@kbecciv
Copy link

kbecciv commented Jan 16, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 1.4.25-4
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

  1. Launch app
  2. Tap on a group chat
  3. Tap plus icon near compose
  4. Tap split bill
  5. Enter an amount and tap next
  6. Tap show more & enter description, merchant
  7. Note the order of display of description, merchant and date
  8. Tap split
  9. Tap the split created to open details page
    10.Tap show more
  10. Note the order of display changed to description, date and merchant

Expected Result:

Description, merchant, date order must not be changed after creating split bill in group chat.

Actual Result:

Description, merchant, date order changed after creating split bill in group chat.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6343672_1705357833481.az_recorder_20240116_003150.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01bf7e0226a4739f95
  • Upwork Job ID: 1747238369577791488
  • Last Price Increase: 2024-01-16
@kbecciv kbecciv added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 16, 2024
@melvin-bot melvin-bot bot changed the title IOU - Description, merchant, date order changed after creating split bill in group chat. [$500] IOU - Description, merchant, date order changed after creating split bill in group chat. Jan 16, 2024
Copy link

melvin-bot bot commented Jan 16, 2024

Job added to Upwork: https://www.upwork.com/jobs/~01bf7e0226a4739f95

Copy link

melvin-bot bot commented Jan 16, 2024

Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 16, 2024
Copy link

melvin-bot bot commented Jan 16, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @rushatgabhane (External)

@abzokhattab
Copy link
Contributor

abzokhattab commented Jan 16, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

IOU - Description, merchant, date order changed after creating split bill in group chat

What is the root cause of that problem?

Date is switched with the merchant in this componet:

{shouldShowDate && (
<MenuItemWithTopDescription
shouldShowRightIcon={!props.isReadOnly}
title={props.iouCreated || format(new Date(), CONST.DATE.FNS_FORMAT_STRING)}
description={translate('common.date')}
style={[styles.moneyRequestMenuItem]}
titleStyle={styles.flex1}
onPress={() => {
if (props.isEditingSplitBill) {
Navigation.navigate(ROUTES.EDIT_SPLIT_BILL.getRoute(props.reportID, props.reportActionID, CONST.EDIT_REQUEST_FIELD.DATE));
return;
}
Navigation.navigate(ROUTES.MONEY_REQUEST_DATE.getRoute(props.iouType, props.reportID));
}}
disabled={didConfirm}
interactive={!props.isReadOnly}
brickRoadIndicator={shouldDisplayFieldError && TransactionUtils.isCreatedMissing(transaction) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''}
error={shouldDisplayFieldError && TransactionUtils.isCreatedMissing(transaction) ? translate('common.error.enterDate') : ''}
/>
)}

What changes do you think we should make in order to solve the problem?

Date should be move to the button of the component to be the last item:

details
 {shouldShowAllFields && (
                <>
                    {props.isDistanceRequest && (
                        <MenuItemWithTopDescription
                            shouldShowRightIcon={!props.isReadOnly && isTypeRequest}
                            title={props.iouMerchant}
                            description={translate('common.distance')}
                            style={[styles.moneyRequestMenuItem]}
                            titleStyle={styles.flex1}
                            onPress={() => Navigation.navigate(ROUTES.MONEY_REQUEST_DISTANCE.getRoute(props.iouType, props.reportID))}
                            disabled={didConfirm || !isTypeRequest}
                            interactive={!props.isReadOnly}
                        />
                    )}
                    {shouldShowMerchant && (
                        <MenuItemWithTopDescription
                            shouldShowRightIcon={!props.isReadOnly}
                            title={isMerchantEmpty ? '' : props.iouMerchant}
                            description={translate('common.merchant')}
                            style={[styles.moneyRequestMenuItem]}
                            titleStyle={styles.flex1}
                            onPress={() => {
                                if (props.isEditingSplitBill) {
                                    Navigation.navigate(ROUTES.EDIT_SPLIT_BILL.getRoute(props.reportID, props.reportActionID, CONST.EDIT_REQUEST_FIELD.MERCHANT));
                                    return;
                                }
                                Navigation.navigate(ROUTES.MONEY_REQUEST_MERCHANT.getRoute(props.iouType, props.reportID));
                            }}
                            disabled={didConfirm}
                            interactive={!props.isReadOnly}
                            brickRoadIndicator={shouldDisplayFieldError && TransactionUtils.isMerchantMissing(transaction) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''}
                            error={shouldDisplayMerchantError || (shouldDisplayFieldError && TransactionUtils.isMerchantMissing(transaction)) ? translate('common.error.enterMerchant') : ''}
                        />
                    )}
                    {shouldShowCategories && (
                        <MenuItemWithTopDescription
                            shouldShowRightIcon={!props.isReadOnly}
                            title={props.iouCategory}
                            description={translate('common.category')}
                            numberOfLinesTitle={2}
                            onPress={() => {
                                if (props.isEditingSplitBill) {
                                    Navigation.navigate(ROUTES.EDIT_SPLIT_BILL.getRoute(props.reportID, props.reportActionID, CONST.EDIT_REQUEST_FIELD.CATEGORY));
                                    return;
                                }
                                Navigation.navigate(ROUTES.MONEY_REQUEST_CATEGORY.getRoute(props.iouType, props.reportID));
                            }}
                            style={[styles.moneyRequestMenuItem]}
                            titleStyle={styles.flex1}
                            disabled={didConfirm}
                            interactive={!props.isReadOnly}
                            rightLabel={canUseViolations && Boolean(props.policy.requiresCategory) ? translate('common.required') : ''}
                        />
                    )}
                    {shouldShowTags && (
                        <MenuItemWithTopDescription
                            shouldShowRightIcon={!props.isReadOnly}
                            title={props.iouTag}
                            description={policyTagListName}
                            numberOfLinesTitle={2}
                            onPress={() => {
                                if (props.isEditingSplitBill) {
                                    Navigation.navigate(ROUTES.EDIT_SPLIT_BILL.getRoute(props.reportID, props.reportActionID, CONST.EDIT_REQUEST_FIELD.TAG));
                                    return;
                                }
                                Navigation.navigate(ROUTES.MONEY_REQUEST_TAG.getRoute(props.iouType, props.reportID));
                            }}
                            style={[styles.moneyRequestMenuItem]}
                            disabled={didConfirm}
                            interactive={!props.isReadOnly}
                            rightLabel={canUseViolations && Boolean(props.policy.requiresTag) ? translate('common.required') : ''}
                        />
                    )}

                    {shouldShowTax && (
                        <MenuItemWithTopDescription
                            shouldShowRightIcon={!props.isReadOnly}
                            title={taxRateTitle}
                            description={props.policyTaxRates.name}
                            style={[styles.moneyRequestMenuItem]}
                            titleStyle={styles.flex1}
                            onPress={() =>
                                Navigation.navigate(
                                    ROUTES.MONEY_REQUEST_STEP_TAX_RATE.getRoute(props.iouType, props.transaction.transactionID, props.reportID, Navigation.getActiveRouteWithoutParams()),
                                )
                            }
                            disabled={didConfirm}
                            interactive={!props.isReadOnly}
                        />
                    )}

                    {shouldShowTax && (
                        <MenuItemWithTopDescription
                            shouldShowRightIcon={!props.isReadOnly}
                            title={formattedTaxAmount}
                            description={props.policyTaxRates.name}
                            style={[styles.moneyRequestMenuItem]}
                            titleStyle={styles.flex1}
                            onPress={() =>
                                Navigation.navigate(
                                    ROUTES.MONEY_REQUEST_STEP_TAX_AMOUNT.getRoute(props.iouType, props.transaction.transactionID, props.reportID, Navigation.getActiveRouteWithoutParams()),
                                )
                            }
                            disabled={didConfirm}
                            interactive={!props.isReadOnly}
                        />
                    )}

                    {shouldShowBillable && (
                        <View style={[styles.flexRow, styles.justifyContentBetween, styles.alignItemsCenter, styles.ml5, styles.mr8, styles.optionRow]}>
                            <Text color={!props.iouIsBillable ? theme.textSupporting : undefined}>{translate('common.billable')}</Text>
                            <Switch
                                accessibilityLabel={translate('common.billable')}
                                isOn={props.iouIsBillable}
                                onToggle={props.onToggleBillable}
                            />
                        </View>
                    )}
                    {shouldShowDate && (
                        <MenuItemWithTopDescription
                            shouldShowRightIcon={!props.isReadOnly}
                            title={props.iouCreated || format(new Date(), CONST.DATE.FNS_FORMAT_STRING)}
                            description={translate('common.date')}
                            style={[styles.moneyRequestMenuItem]}
                            titleStyle={styles.flex1}
                            onPress={() => {
                                if (props.isEditingSplitBill) {
                                    Navigation.navigate(ROUTES.EDIT_SPLIT_BILL.getRoute(props.reportID, props.reportActionID, CONST.EDIT_REQUEST_FIELD.DATE));
                                    return;
                                }
                                Navigation.navigate(ROUTES.MONEY_REQUEST_DATE.getRoute(props.iouType, props.reportID));
                            }}
                            disabled={didConfirm}
                            interactive={!props.isReadOnly}
                            brickRoadIndicator={shouldDisplayFieldError && TransactionUtils.isCreatedMissing(transaction) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''}
                            error={shouldDisplayFieldError && TransactionUtils.isCreatedMissing(transaction) ? translate('common.error.enterDate') : ''}
                        />
                    )}
                </>
            )}

OR We can just put the date after the merchant item to be in sync with the other confrimationlist:

details ```js
                {props.isDistanceRequest && (
                    <MenuItemWithTopDescription
                        shouldShowRightIcon={!props.isReadOnly && isTypeRequest}
                        title={props.iouMerchant}
                        description={translate('common.distance')}
                        style={[styles.moneyRequestMenuItem]}
                        titleStyle={styles.flex1}
                        onPress={() => Navigation.navigate(ROUTES.MONEY_REQUEST_DISTANCE.getRoute(props.iouType, props.reportID))}
                        disabled={didConfirm || !isTypeRequest}
                        interactive={!props.isReadOnly}
                    />
                )}
                {shouldShowMerchant && (
                    <MenuItemWithTopDescription
                        shouldShowRightIcon={!props.isReadOnly}
                        title={isMerchantEmpty ? '' : props.iouMerchant}
                        description={translate('common.merchant')}
                        style={[styles.moneyRequestMenuItem]}
                        titleStyle={styles.flex1}
                        onPress={() => {
                            if (props.isEditingSplitBill) {
                                Navigation.navigate(ROUTES.EDIT_SPLIT_BILL.getRoute(props.reportID, props.reportActionID, CONST.EDIT_REQUEST_FIELD.MERCHANT));
                                return;
                            }
                            Navigation.navigate(ROUTES.MONEY_REQUEST_MERCHANT.getRoute(props.iouType, props.reportID));
                        }}
                        disabled={didConfirm}
                        interactive={!props.isReadOnly}
                        brickRoadIndicator={shouldDisplayFieldError && TransactionUtils.isMerchantMissing(transaction) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''}
                        error={shouldDisplayMerchantError || (shouldDisplayFieldError && TransactionUtils.isMerchantMissing(transaction)) ? translate('common.error.enterMerchant') : ''}
                    />
                )}

{shouldShowDate && (
<MenuItemWithTopDescription
shouldShowRightIcon={!props.isReadOnly}
title={props.iouCreated || format(new Date(), CONST.DATE.FNS_FORMAT_STRING)}
description={translate('common.date')}
style={[styles.moneyRequestMenuItem]}
titleStyle={styles.flex1}
onPress={() => {
if (props.isEditingSplitBill) {
Navigation.navigate(ROUTES.EDIT_SPLIT_BILL.getRoute(props.reportID, props.reportActionID, CONST.EDIT_REQUEST_FIELD.DATE));
return;
}
Navigation.navigate(ROUTES.MONEY_REQUEST_DATE.getRoute(props.iouType, props.reportID));
}}
disabled={didConfirm}
interactive={!props.isReadOnly}
brickRoadIndicator={shouldDisplayFieldError && TransactionUtils.isCreatedMissing(transaction) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''}
error={shouldDisplayFieldError && TransactionUtils.isCreatedMissing(transaction) ? translate('common.error.enterDate') : ''}
/>
)}


</details>


Result: 
<img width="1295" alt="image" src="https://github.com/Expensify/App/assets/59809993/fc0ce217-b2c7-4535-b949-f3daa3429f07">

@DylanDylann
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

Description, merchant, date order changed after creating split bill in group chat.

What is the root cause of that problem?

The order in 2 places are different

<MenuItemWithTopDescription
shouldShowRightIcon={!isReadOnly}
title={isMerchantEmpty ? '' : iouMerchant}
description={translate('common.merchant')}
style={[styles.moneyRequestMenuItem]}
titleStyle={styles.flex1}
onPress={() => {
if (isEditingSplitBill) {
Navigation.navigate(ROUTES.EDIT_SPLIT_BILL.getRoute(reportID, reportActionID, CONST.EDIT_REQUEST_FIELD.MERCHANT));
return;
}
Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_MERCHANT.getRoute(iouType, transaction.transactionID, reportID, Navigation.getActiveRouteWithoutParams()));
}}
disabled={didConfirm}
interactive={!isReadOnly}
brickRoadIndicator={merchantError ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''}
error={merchantError ? translate('common.error.fieldRequired') : ''}
/>
)}
{shouldShowDate && (
<MenuItemWithTopDescription
shouldShowRightIcon={!isReadOnly}
title={iouCreated || format(new Date(), CONST.DATE.FNS_FORMAT_STRING)}
description={translate('common.date')}
style={[styles.moneyRequestMenuItem]}
titleStyle={styles.flex1}
onPress={() => {
if (isEditingSplitBill) {
Navigation.navigate(ROUTES.EDIT_SPLIT_BILL.getRoute(reportID, reportActionID, CONST.EDIT_REQUEST_FIELD.DATE));
return;
}
Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_DATE.getRoute(iouType, transaction.transactionID, reportID, Navigation.getActiveRouteWithoutParams()));
}}
disabled={didConfirm}
interactive={!isReadOnly}
brickRoadIndicator={shouldDisplayFieldError && TransactionUtils.isCreatedMissing(transaction) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''}
error={shouldDisplayFieldError && TransactionUtils.isCreatedMissing(transaction) ? translate('common.error.enterDate') : ''}

{shouldShowDate && (
<MenuItemWithTopDescription
shouldShowRightIcon={!props.isReadOnly}
title={props.iouCreated || format(new Date(), CONST.DATE.FNS_FORMAT_STRING)}
description={translate('common.date')}
style={[styles.moneyRequestMenuItem]}
titleStyle={styles.flex1}
onPress={() => {
if (props.isEditingSplitBill) {
Navigation.navigate(ROUTES.EDIT_SPLIT_BILL.getRoute(props.reportID, props.reportActionID, CONST.EDIT_REQUEST_FIELD.DATE));
return;
}
Navigation.navigate(ROUTES.MONEY_REQUEST_DATE.getRoute(props.iouType, props.reportID));
}}
disabled={didConfirm}
interactive={!props.isReadOnly}
brickRoadIndicator={shouldDisplayFieldError && TransactionUtils.isCreatedMissing(transaction) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''}
error={shouldDisplayFieldError && TransactionUtils.isCreatedMissing(transaction) ? translate('common.error.enterDate') : ''}
/>

In the Confirmation Page, we migrate to use the new component MoneyTemporaryForRefactorRequestConfirmationList instead of MoneyRequestConfirmationList but in Split Detail Page we still use old component MoneyRequestConfirmationList , It caused inconsistency.

What changes do you think we should make in order to solve the problem?

I suggest in the split bill page we also should use MoneyTemporaryForRefactorRequestConfirmationList instead of MoneyRequestConfirmationList

What alternative solutions did you explore? (Optional)

NA

@rushatgabhane
Copy link
Member

Yes, looks like we created a temporary copy of MoneyRequestConfirmationList to avoid merge conflicts.
#28618

@rushatgabhane
Copy link
Member

rushatgabhane commented Jan 17, 2024

@tgolen im trying to gain clarity here. In #28618 we created a temporary copy of MoneyRequestConfirmationList.

But MoneyRequestConfirmationList and MoneyTemporaryForRefactorRequestConfirmationList are out of sync now, it is causing this inconsistency "bug".

I'm sure there is a plan to sync them when refactor is done. Could you please let us know if we should hold this issue or proceed with fixing it? Thank you so much 🙇

@aimane-chnaif
Copy link
Contributor

yes, this will be handled in #29107

sync

@rushatgabhane
Copy link
Member

rushatgabhane commented Jan 17, 2024

Thank you so much @aimane-chnaif!

@lschurr let's close this issue :)

@lschurr
Copy link
Contributor

lschurr commented Jan 18, 2024

Great, thanks!

@lschurr lschurr closed this as completed Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests

6 participants