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] [Distance] - Distance editor menu in IOU report shows Merchant instead of Distance #26559

Closed
6 tasks done
lanitochka17 opened this issue Sep 2, 2023 · 15 comments
Closed
6 tasks done
Assignees
Labels
Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@lanitochka17
Copy link

lanitochka17 commented Sep 2, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Issue found when executing PR #25707

Action Performed:

  1. Go to staging.new.expensify.com
  2. Create a distance request in workspace chat
  3. Open IOU report
  4. Click Distance

Expected Result:

The right hand menu shows Distance as the header

Actual Result:

The right hand menu shows Merchant as the header. The title of the edit field also shows Merchant

Workaround:

Unknown

Platforms:

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

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.3.62-1

Reproducible in staging?: Yes

Reproducible in production?: No

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

Notes/Photos/Videos: Any additional supporting documentation

Bug6186220_20230902_184437.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01c148f42c5bc47bd3
  • Upwork Job ID: 1698180072419635200
  • Last Price Increase: 2023-09-03
@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Sep 2, 2023
@OSBotify
Copy link
Contributor

OSBotify commented Sep 2, 2023

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@melvin-bot
Copy link

melvin-bot bot commented Sep 2, 2023

Triggered auto assignment to @jasperhuangg (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@ShogunFire
Copy link
Contributor

ShogunFire commented Sep 2, 2023

Proposal

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

The text of the header and of the input are "Merchant" instead of Distance

What is the root cause of that problem?

In MoneyRequestView here:

<MenuItemWithTopDescription
description={isDistanceRequest ? translate('common.distance') : translate('common.merchant')}
title={transactionMerchant}
interactive={canEdit}
shouldShowRightIcon={canEdit}
titleStyle={styles.flex1}
onPress={() => Navigation.navigate(ROUTES.getEditRequestRoute(report.reportID, CONST.EDIT_REQUEST_FIELD.MERCHANT))}
brickRoadIndicator={hasErrors && transactionMerchant === CONST.TRANSACTION.UNKNOWN_MERCHANT ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''}
subtitle={hasErrors && transactionMerchant === CONST.TRANSACTION.UNKNOWN_MERCHANT ? translate('common.error.enterMerchant') : ''}
subtitleTextStyle={styles.textLabelError}
/>

We have only one MenuItemWithTopDescription for both Merchant and Distance and we navigate to the same page for both.

The page we navigate to is https://github.com/Expensify/App/blob/main/src/pages/EditRequestMerchantPage.js

In this page we display Merchant and never Distance

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

I think we should do a page specific for Distance because currently when we submit we actually save the distance in the merchant field here:

onSubmit={(transactionChanges) => {
// In case the merchant hasn't been changed, do not make the API request.
if (transactionChanges.merchant.trim() === transactionMerchant) {
Navigation.dismissModal();
return;
}
editMoneyRequest({merchant: transactionChanges.merchant.trim()});
}}

So here are the changes I suggest:
In MoneyRequestView, add a specific MenuItemWithTopDescription for Distance

Add CONST.EDIT_REQUEST_FIELD.DISTANCE in the const file

In EditRequestPage add a condition to test if the field is distance and navigate to the new page EditRequestDistancePage that will look like EditRequestMerchantPage but with the right text

What alternative solutions did you explore? (Optional)

The alternative is to test if this is a distance request in EditRequestMerchantPage to display the right text, and we also would need to test that in the submit method to save in the right field

@situchan
Copy link
Contributor

situchan commented Sep 2, 2023

This will be fixed in Edit distance request PR

@hayata-suenaga hayata-suenaga added Daily KSv2 External Added to denote the issue can be worked on by a contributor and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Sep 3, 2023
@melvin-bot melvin-bot bot changed the title Distance IOU - Distance editor menu in IOU report shows Merchant instead of Distance [$500] Distance IOU - Distance editor menu in IOU report shows Merchant instead of Distance Sep 3, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 3, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 3, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 3, 2023

Triggered auto assignment to @anmurali (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Sep 3, 2023

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

@hayata-suenaga hayata-suenaga assigned anmurali and unassigned mollfpr and anmurali Sep 3, 2023
@hayata-suenaga hayata-suenaga added External Added to denote the issue can be worked on by a contributor and removed 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 labels Sep 3, 2023
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 3, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 3, 2023

Triggered auto assignment to @zanyrenney (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Sep 3, 2023

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

@hayata-suenaga
Copy link
Contributor

I'm so bad at assigning labels 😭

@b4s36t4
Copy link
Contributor

b4s36t4 commented Sep 3, 2023

Hey, as @situchan mentioned is in't going taken care at some other PR? is this up for proposals?

@hayata-suenaga
Copy link
Contributor

@b4s36t4 I need to confirm with that PR author but probably will be fixed in that PR

I gonna keep this issue open but will removed help wanted label

@hayata-suenaga hayata-suenaga removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 3, 2023
@b4s36t4
Copy link
Contributor

b4s36t4 commented Sep 3, 2023

Ok understood.

@lanitochka17 lanitochka17 changed the title [$500] Distance IOU - Distance editor menu in IOU report shows Merchant instead of Distance [$500] [Distance] - Distance editor menu in IOU report shows Merchant instead of Distance Sep 5, 2023
@melvin-bot melvin-bot bot added the Overdue label Sep 5, 2023
@zanyrenney
Copy link
Contributor

Hey @hayata-suenaga I'm a little lost on this one with the labels.

I think Anu is assigned as the BZ manager:

Triggered auto assignment to @anmurali (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

But then it looks like you unassigned her and then the external label again which has also now assigned me...

hayata-suenaga assigned hayata-suenaga and unassigned anmurali 2 days ago

Can you confirm what's happening here please? Thanks!

@hayata-suenaga
Copy link
Contributor

Screenshot 2023-09-05 at 10 19 32 AM

😓 this is probably the fault of my late-night brain... I constantly click wrong buttons 🔘

Anyway, this PR is going to fix the issue. I gonna close this issue for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests