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

[HOLD #301281] [$1000] Web - Missing Default Merchant Statement in Split Bill Details #25885

Closed
1 of 6 tasks
izarutskaya opened this issue Aug 24, 2023 · 30 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering 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 Weekly KSv2

Comments

@izarutskaya
Copy link

izarutskaya commented Aug 24, 2023

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


Action Performed:

  1. Click on the plus sign and select "Split Bill."
  2. Enter the desired numbers and add an email address.
  3. Click on "See More" and observe the not-updated default statement
  4. Open the split bill details and click on "See More" again.
  5. Notice that the default merchant statement is not present.

Expected Result:

The default merchant statement should be present both in the split bill details and when splitting the bill.

Actual Result:

The default merchant statement is present when splitting the bill but is not displayed in the split bill details.

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: v1.3.57-2

Reproducible in staging?: Y

Reproducible in production?: N

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

girma.mp4
Recording.1316.mp4

Expensify/Expensify Issue URL:

Issue reported by: @tewodrosGirmaA

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1692855560586859

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~015da00ce15854b03b
  • Upwork Job ID: 1694858757030412288
  • Last Price Increase: 2023-09-13
@izarutskaya izarutskaya added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 24, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 24, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Aug 24, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@OSBotify
Copy link
Contributor

👋 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 Aug 24, 2023

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

@alphaboss1104
Copy link
Contributor

Proposal

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

Missing Default Merchant Statement in Split Bill Details

What is the root cause of that problem?

We displayed the merchant statement of split bill details here:

<MenuItemWithTopDescription
shouldShowRightIcon={!props.isReadOnly && isTypeRequest}
title={props.iouMerchant}
description={translate('common.merchant')}
style={[styles.moneyRequestMenuItem, styles.mb2]}
titleStyle={styles.flex1}
onPress={() => Navigation.navigate(ROUTES.getMoneyRequestMerchantRoute(props.iouType, props.reportID))}
disabled={didConfirm || props.isReadOnly || !isTypeRequest}
/>

But we didn't passed the props.iouMerchant from the parent.

{Boolean(participants.length) && (
<MoneyRequestConfirmationList
hasMultipleParticipants
payeePersonalDetails={payeePersonalDetails}
selectedParticipants={participantsExcludingPayee}
iouAmount={splitAmount}
iouComment={splitComment}
iouCurrencyCode={splitCurrency}
iouType={CONST.IOU.MONEY_REQUEST_TYPE.SPLIT}
isReadOnly
shouldShowFooter={false}
/>
)}

This is the root cause of this issue.

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

First of all we should get the merchant statement of split bill details here:

const {amount: splitAmount, currency: splitCurrency, comment: splitComment, merchant: splitMerchant} = ReportUtils.getTransactionDetails(transaction);

and then We should add iouMerchant as a prop to MoneyRequestConfirmationList component here:

                  <MoneyRequestConfirmationList
                      hasMultipleParticipants
                      payeePersonalDetails={payeePersonalDetails}
                      selectedParticipants={participantsExcludingPayee}
                      iouAmount={splitAmount}
                      iouComment={splitComment}
+                     iouMerchant={splitMerchant}
                      iouCurrencyCode={splitCurrency}
                      iouType={CONST.IOU.MONEY_REQUEST_TYPE.SPLIT}
                      isReadOnly
                      shouldShowFooter={false}
                  />

What alternative solutions did you explore? (Optional)

@melvin-bot melvin-bot bot added Daily KSv2 and removed Hourly KSv2 labels Aug 24, 2023
@pecanoro
Copy link
Contributor

Trying to figure out from which PR is coming

@pecanoro
Copy link
Contributor

@mountiny @luacmartins This deploy blocker is coming from this PR #25794

@luacmartins
Copy link
Contributor

I don't think this is a blocker. It's a really small issue and we're still making several changes to this flow.

@pecanoro pecanoro added External Added to denote the issue can be worked on by a contributor and removed DeployBlockerCash This issue or pull request should block deployment labels Aug 24, 2023
@melvin-bot melvin-bot bot changed the title Web - Missing Default Merchant Statement in Split Bill Details [$1000] Web - Missing Default Merchant Statement in Split Bill Details Aug 24, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 24, 2023

Job added to Upwork: https://www.upwork.com/jobs/~015da00ce15854b03b

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

melvin-bot bot commented Aug 24, 2023

Current assignee @muttmuure is eligible for the External assigner, not assigning anyone new.

@melvin-bot
Copy link

melvin-bot bot commented Aug 24, 2023

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

@melvin-bot melvin-bot bot added the Overdue label Aug 28, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 28, 2023

@pecanoro, @muttmuure, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!

@pecanoro
Copy link
Contributor

@situchan Could you review the proposals when you have a chance? Thank you!

@melvin-bot
Copy link

melvin-bot bot commented Sep 4, 2023

@pecanoro, @muttmuure, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!

@pecanoro
Copy link
Contributor

pecanoro commented Sep 4, 2023

Before split: Request
After split: Bill split with xxx, xxx

@mountiny @luacmartins Is this expected? I know we are still refactoring some of this stuff so I am not sure if it's expected or not.

@melvin-bot melvin-bot bot removed the Overdue label Sep 4, 2023
@mountiny
Copy link
Contributor

mountiny commented Sep 4, 2023

I would probably wait with the split bill after the conference, I dont think this was properly scoped in terms of how the merchant and fields should behave for split so this should be brought to slack first

@pecanoro
Copy link
Contributor

pecanoro commented Sep 4, 2023

Gotcha! Going to put a HOLD then and move it to weekly

@pecanoro pecanoro changed the title [$1000] Web - Missing Default Merchant Statement in Split Bill Details [HOLD][$1000] Web - Missing Default Merchant Statement in Split Bill Details Sep 4, 2023
@pecanoro pecanoro added Weekly KSv2 and removed Daily KSv2 labels Sep 4, 2023
@melvin-bot melvin-bot bot added the Overdue label Sep 13, 2023
@pecanoro
Copy link
Contributor

Bringing the discussion to Slack

@melvin-bot melvin-bot bot removed the Overdue label Sep 13, 2023
@pecanoro pecanoro changed the title [HOLD][$1000] Web - Missing Default Merchant Statement in Split Bill Details [$1000] Web - Missing Default Merchant Statement in Split Bill Details Sep 13, 2023
@pecanoro pecanoro added Daily KSv2 and removed Weekly KSv2 labels Sep 13, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 13, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot
Copy link

melvin-bot bot commented Sep 14, 2023

@pecanoro @muttmuure @situchan this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

@melvin-bot melvin-bot bot added the Overdue label Sep 18, 2023
@pecanoro
Copy link
Contributor

@melvin-bot melvin-bot bot removed the Overdue label Sep 18, 2023
@pecanoro pecanoro changed the title [$1000] Web - Missing Default Merchant Statement in Split Bill Details [HOLD #301281] [$1000] Web - Missing Default Merchant Statement in Split Bill Details Sep 18, 2023
@pecanoro pecanoro added Weekly KSv2 and removed Daily KSv2 labels Sep 18, 2023
@melvin-bot melvin-bot bot added the Overdue label Sep 27, 2023
@pecanoro
Copy link
Contributor

Still HOLDING

@melvin-bot melvin-bot bot removed the Overdue label Sep 29, 2023
@melvin-bot melvin-bot bot added the Overdue label Oct 9, 2023
@pecanoro
Copy link
Contributor

pecanoro commented Oct 9, 2023

Still HOLDING

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. Engineering 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 Weekly KSv2
Projects
None yet
Development

No branches or pull requests

8 participants