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

[Awaiting Payment 22nd June] [$250] [Pending/Scanning] Card transactions show Cash in the expense preview instead of Card #42832

Closed
1 of 6 tasks
m-natarajan opened this issue May 30, 2024 · 15 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@m-natarajan
Copy link

m-natarajan commented May 30, 2024

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


Version Number: 1.4.77-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: @kevinksullivan
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1717026809436129

Action Performed:

  1. Incur a card transaction on an expensify card
  2. Add a manual request to that same report

Expected Result:

The expense preview should say Card for card transactions.

Actual Result:

The card transactions show as Cash

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

image (2)

Snip - (72) New Expensify - Google Chrome

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01fc32ffbea0b1bdc5
  • Upwork Job ID: 1796170573012758528
  • Last Price Increase: 2024-05-30
  • Automatic offers:
    • Nodebrute | Contributor | 102573646
Issue OwnerCurrent Issue Owner: @parasharrajat
@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels May 30, 2024
Copy link

melvin-bot bot commented May 30, 2024

Triggered auto assignment to @trjExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@Nodebrute
Copy link
Contributor

Nodebrute commented May 30, 2024

Propsal

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

card transactions show Cash in the expense preview

What is the root cause of that problem?

This is a feature request. We don't have any logic to show Card instead of cash

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

We should add another check in getPreviewHeaderText in MoneyRequestPreviewContent.tsx

const getPreviewHeaderText = (): string => {
let message = translate('iou.cash');
if (isDistanceRequest) {
message = translate('common.distance');

We can do something like this

if(isCardTransaction){
message = translate('iou.card')
}

We should also check for other places where we want to show card instead of cash

If we only want to use Card for expensify card transactions then we can use check isExpensifyCardTransaction

What alternative solutions did you explore?

We can also do it like this here

let message = isCardTransaction ? translate('iou.card') : translate('iou.cash')

@trjExpensify trjExpensify changed the title card transactions show Cash in the expense preview [Pending/Scanning] Card transactions show Cash in the expense preview instead of Card May 30, 2024
@trjExpensify trjExpensify added the External Added to denote the issue can be worked on by a contributor label May 30, 2024
@melvin-bot melvin-bot bot changed the title [Pending/Scanning] Card transactions show Cash in the expense preview instead of Card [$250] [Pending/Scanning] Card transactions show Cash in the expense preview instead of Card May 30, 2024
Copy link

melvin-bot bot commented May 30, 2024

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

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

melvin-bot bot commented May 30, 2024

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

@ShridharGoel
Copy link
Contributor

Proposal

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

Card transactions show Cash in the expense preview instead of Card.

What is the root cause of that problem?

We are missing card text implementation at below places:

accessibilityLabel={isBillSplit ? translate('iou.split') : translate('iou.cash')}

if (isCardTransaction) {
if (formattedOriginalAmount) {
amountDescription += ` ${CONST.DOT_SEPARATOR} ${translate('iou.original')} ${formattedOriginalAmount}`;
}
if (isCancelled) {
amountDescription += ` ${CONST.DOT_SEPARATOR} ${translate('iou.canceled')}`;
}
} else {

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

Update this to below:

let message = isCardTransaction ? translate('iou.card') : translate('iou.cash');

Check if we want to update this accessibility label to below:

accessibilityLabel={isBillSplit ? translate('iou.split') : (isCardTransaction ? translate('iou.card') : translate('iou.cash'))}

If we want to update this, then we can use something like the below:

if (isCardTransaction) {
    if (formattedOriginalAmount) {
        amountDescription += ` ${CONST.DOT_SEPARATOR} ${translate('iou.card')} ${translate('iou.original')} ${formattedOriginalAmount}`;
    }
    if (isCancelled) {
        amountDescription += ` ${CONST.DOT_SEPARATOR} ${translate('iou.card')} ${translate('iou.canceled')}`;
    }
}

@parasharrajat
Copy link
Member

This looks like a new change. I believe we need to groom those issue first and add more details in OP. cc: @trjExpensify

@trjExpensify
Copy link
Contributor

Oh hm, according to @kevinksullivan we'd implemented this one already with the original implementation, so this was a regression. Is that not the case?

We show Expensify Card transactions in NewDot now, but the expense preview component reads Cash not Card.

If we only want to use Card for expensify card transactions then we can use check isExpensifyCardTransaction

If we did this, it wouldn't be future proof for when we show third-party card transactions, so I think a isCardTransaction check is better than isExpensifyCardTransaction check personally.

@parasharrajat
Copy link
Member

Ok. Thanks. @Nodebrute's proposal looks good to me. let's make sure we modify the related places as well.

🎀 👀 🎀 C+ reviewed

Copy link

melvin-bot bot commented May 31, 2024

Current assignee @grgia is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

@melvin-bot melvin-bot bot added Overdue and removed Help Wanted Apply this label when an issue is open to proposals by contributors labels Jun 3, 2024
Copy link

melvin-bot bot commented Jun 3, 2024

📣 @Nodebrute 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 Overdue labels Jun 3, 2024
@parasharrajat
Copy link
Member

PR was deployed 5 days back on PROD. Not sure why the status was not updated here.

@Nodebrute
Copy link
Contributor

@grgia The PR went into production on June 15th. The payment was due on June 22nd. Unfortunately, this issue missed the automatic payment date. Kindly issue the payment.

@trjExpensify trjExpensify changed the title [$250] [Pending/Scanning] Card transactions show Cash in the expense preview instead of Card [Awaiting Payment 22nd June] [$250] [Pending/Scanning] Card transactions show Cash in the expense preview instead of Card Jun 24, 2024
@trjExpensify trjExpensify added Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review labels Jun 24, 2024
@trjExpensify
Copy link
Contributor

Payment summary as follows:

Regression tests for the project will be added centrally. Closing!

@github-project-automation github-project-automation bot moved this from Release 2: Summer 2024 (Aug) to Done in [#whatsnext] #wave-collect Jun 24, 2024
@parasharrajat
Copy link
Member

Payment requested as per #42832 (comment)

@JmillsExpensify
Copy link

$250 approved for @parasharrajat

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
No open projects
Archived in project
Development

No branches or pull requests

7 participants