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

[$250] Pay someone flow doesn’t include a reportAction to distinguish the payment method used (Expensify or elsewhere) #41645

Open
1 of 6 tasks
m-natarajan opened this issue May 4, 2024 · 50 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@m-natarajan
Copy link

m-natarajan commented May 4, 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.70-2
Reproducible in staging?: Yes
Reproducible in production?: Yes
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: @trjExpensify
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1714729020479179

Action Performed:

Prerequisite: Pay someone with either Expensify or elsewhere option

  1. Login as a user who got paid
  2. Open the DM and observe the preview for money received
  3. Click to open the payment details and observe

Expected Result:

Method of payment displayed in both DM and details page (Expensify or elsewhere)

Actual Result:

Only xyz paid displayed and method of payment not displayed

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 (1)
Snip - (2) New Expensify

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~014f39f2dacd55f2a4
  • Upwork Job ID: 1787959016608243712
  • Last Price Increase: 2024-05-21
  • Automatic offers:
    • ahmedGaber93 | Reviewer | 102528931
    • nkdengineer | Contributor | 102528935
Issue OwnerCurrent Issue Owner: @ahmedGaber93
@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels May 4, 2024
Copy link

melvin-bot bot commented May 4, 2024

Triggered auto assignment to @sakluger (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.

@jainilparikh
Copy link

Proposal

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

Payment type is not mentioned in the details page.

What is the root cause of that problem?

We do not have a field for payment type. We only have fields for 'amount', 'comment', 'merchant', 'date', 'category' and 'tag'.

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

Add a new field similar to the one below for 'PaymentType' info:

{canUseViolations && <ViolationMessages violations={getViolationsForField('receipt')} />}
<OfflineWithFeedback pendingAction={getPendingFieldAction('amount')}>
<MenuItemWithTopDescription
title={formattedTransactionAmount ? formattedTransactionAmount.toString() : ''}
shouldShowTitleIcon={isSettled}
titleIcon={Expensicons.Checkmark}
description={amountDescription}
titleStyle={styles.textHeadlineH2}
interactive={canEditAmount}
shouldShowRightIcon={canEditAmount}
onPress={() => Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_AMOUNT.getRoute(CONST.IOU.ACTION.EDIT, iouType, transaction?.transactionID ?? '', report.reportID))}
brickRoadIndicator={getErrorForField('amount') ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
error={getErrorForField('amount')}
/>

PaymentType info is available in ParentReportAction array that's passed to this component.

What alternative solutions did you explore? (Optional)

@melvin-bot melvin-bot bot added the Overdue label May 6, 2024
Copy link

melvin-bot bot commented May 7, 2024

@sakluger Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@sakluger sakluger added the External Added to denote the issue can be worked on by a contributor label May 7, 2024
@melvin-bot melvin-bot bot changed the title Pay someone flow doesn’t include a reportAction to distinguish the payment method used (Expensify or elsewhere) [$250] Pay someone flow doesn’t include a reportAction to distinguish the payment method used (Expensify or elsewhere) May 7, 2024
Copy link

melvin-bot bot commented May 7, 2024

Job added to Upwork: https://www.upwork.com/jobs/~014f39f2dacd55f2a4

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

melvin-bot bot commented May 7, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label May 7, 2024
@ahmedGaber93

This comment was marked as outdated.

@jainilparikh
Copy link

@ahmedGaber93 , the reportAction does not have any information about how payment is done (Via Expensify or via Elsewhere). This is what we get:


avatarUrl: undefined
chatReportID: "8115357106458061"
chatType: ""
currency: "INR"
description: ""
errorFields: []
fieldList:  undefined
invoiceReceiver:  undefined
iouReportID: undefined
isDeletedParentAction: undefined
isOptimisticReport: undefined
isOwnPolicyExpenseChat: false
isPinned: false
isPolicyExpenseChat: undefined
isWaitingOnBankAccount: false
lastMentionedTime: undefined
lastReadTime: "2024-05-12 05:03:17.881"
lastVisibleActionCreated: "2024-05-12 05:03:17.881"
managerID: 16778387
nonReimbursableTotal: 0
notificationPreference: "always"
oldPolicyName: ""
ownerAccountID: 17102600
parentReportActionID: "189825184571791248"
parentReportID: "8115357106458061"
participantAccountIDs: (2) [16778387, 17102600]
pendingFields: undefined
permissions: (2) ['read', 'write']
policyID: "E1269E8555168358"
policyName: undefined
reportID: "7799079344254003"
reportName: "IOU"
stateNum: 2

Do we first need the BE to send this info in the parentReportActions ?

@melvin-bot melvin-bot bot added the Overdue label May 12, 2024
@sakluger
Copy link
Contributor

Not overdue, we're still discussing the expected behavior.

Copy link

melvin-bot bot commented May 14, 2024

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

Copy link

melvin-bot bot commented May 14, 2024

@sakluger, @ahmedGaber93 Huh... This is 4 days overdue. Who can take care of this?

@ahmedGaber93
Copy link
Contributor

I checked it today, we already have the paymentType in FE but not with this reportAction, I will complete checking tomorrow to know if we can catch it from FE, or it requires BE changes

@melvin-bot melvin-bot bot removed the Overdue label May 14, 2024
@sakluger sakluger removed the Bug Something is broken. Auto assigns a BugZero manager. label May 16, 2024
@sakluger sakluger removed their assignment May 16, 2024
@sakluger sakluger added the Bug Something is broken. Auto assigns a BugZero manager. label May 16, 2024
Copy link

melvin-bot bot commented May 16, 2024

Triggered auto assignment to @sakluger (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.

@sakluger sakluger removed their assignment May 16, 2024
@sakluger sakluger added Bug Something is broken. Auto assigns a BugZero manager. and removed Bug Something is broken. Auto assigns a BugZero manager. labels May 16, 2024
@ahmedGaber93
Copy link
Contributor

Waiting on proposals

@johncschuster
Copy link
Contributor

@nkdengineer
Copy link
Contributor

nkdengineer commented May 22, 2024

Proposal

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

Only xyz paid displayed and method of payment not displayed

What is the root cause of that problem?

  1. We filter out the send money request action when we combine report actions here

return actionType !== CONST.IOU.REPORT_ACTION_TYPE.CREATE && actionType !== CONST.IOU.REPORT_ACTION_TYPE.TRACK && !isSentMoneyReportAction(action);

  1. The pay action in this case is send money so even we don't filter out this action in the point 1, it still shows as preview not a pay content.

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

  1. We shouldn't filter out the send money request action here
return actionType !== CONST.IOU.REPORT_ACTION_TYPE.CREATE && actionType !== CONST.IOU.REPORT_ACTION_TYPE.TRACK;

return actionType !== CONST.IOU.REPORT_ACTION_TYPE.CREATE && actionType !== CONST.IOU.REPORT_ACTION_TYPE.TRACK && !isSentMoneyReportAction(action);

  1. We should remove isSendMoney condition here, so this action can be displayed as pay content instead of a preview. Or we can only display it as preview if the report isn't a combine report but I think it's unnecessary because for send money it's always is a single transaction.
(action.originalMessage.type === CONST.IOU.REPORT_ACTION_TYPE.CREATE ||
action.originalMessage.type === CONST.IOU.REPORT_ACTION_TYPE.SPLIT ||
action.originalMessage.type === CONST.IOU.REPORT_ACTION_TYPE.TRACK)

or

(action.originalMessage.type === CONST.IOU.REPORT_ACTION_TYPE.CREATE ||
action.originalMessage.type === CONST.IOU.REPORT_ACTION_TYPE.SPLIT ||
action.originalMessage.type === CONST.IOU.REPORT_ACTION_TYPE.TRACK ||
(isSendingMoney && !transactionThreadReport?.reportID))

What alternative solutions did you explore? (Optional)

NA

Result

Screen.Recording.2024-05-24.at.11.13.52.mov

@ahmedGaber93
Copy link
Contributor

Reviewing today

@ahmedGaber93
Copy link
Contributor

@nkdengineer Thanks for the proposal.

I don't think your RCA is correct, as we already create optimistic pay action CONST.IOU.REPORT_ACTION_TYPE.PAY here

App/src/libs/actions/IOU.ts

Lines 5561 to 5564 in 8375abe

[optimisticIOUReportAction.reportActionID]: {
...(optimisticIOUReportAction as OnyxTypes.ReportAction),
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
},

@nkdengineer
Copy link
Contributor

@ahmedGaber93 Thanks for your feedback, I updated proposal #41645 (comment)

@ahmedGaber93
Copy link
Contributor

@nkdengineer Thanks for the update.

@nkdengineer's proposal using cancel filtering "send money pay action" in combinedReportActions then display it as a normal message not money request preview in expense report page LGTM.

🎀 👀 🎀 C+ reviewed

Copy link

melvin-bot bot commented May 26, 2024

Triggered auto assignment to @stitesExpensify, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@ahmedGaber93
Copy link
Contributor

@stitesExpensify please take a look here when you have a time.

@stitesExpensify
Copy link
Contributor

then display it as a normal message not money request preview in expense report page

Can you post screenshots of what it used to look like, and what it will look like now? This seems like a significant design change

@nkdengineer
Copy link
Contributor

@stitesExpensify This will show the message like the result in my proposal here #41645 (comment)

@stitesExpensify
Copy link
Contributor

Okay, I see that now at the end. What does it look like currently?

@nkdengineer
Copy link
Contributor

@stitesExpensify Actually, Nothing is shown in combine report of send money request because the send money action is filtered out.

Screenshot 2024-05-30 at 01 59 02

@stitesExpensify
Copy link
Contributor

So it was never showing money request preview like this comment suggested?

@ahmedGaber93
Copy link
Contributor

So it was never showing money request preview like #41645 (comment)?

@stitesExpensify Yes, It was never showing because we filter it here, but now In this issue, we are going to cancel filtering it because it has the payment method and message we need Paid USD 12.00 elswhere.

The problem now is after cancel filtering it, it will display as money request preview

// Filter out request and send money request actions because we don't want to show any preview actions for one transaction reports

so the point 2 in @nkdengineer's solution is to display it as a normal message Paid USD 12.00 elswhere not money request preview.

@stitesExpensify
Copy link
Contributor

Great, thank you both for helping me catch up! Assigning @nkdengineer

@melvin-bot melvin-bot bot removed 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

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

Offer link
Upwork job

Copy link

melvin-bot bot commented May 30, 2024

📣 @nkdengineer 🎉 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 📖

@nkdengineer
Copy link
Contributor

@ahmedGaber93 The PR is here.

Copy link

melvin-bot bot commented Jun 6, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@ahmedGaber93
Copy link
Contributor

@nkdengineer the PR is reverted because of this regression #43183. Can you investigate the issue?

@nkdengineer
Copy link
Contributor

@ahmedGaber93 Coming from #43183 I want to discuss again about the expected behavior because the child report of send money request action is transaction thread report:

  1. Not display as combine report for iou report of send money request so send money request action will display as preview and click on this will open the transaction thread report

  2. Still display send money request as Paid system message and fix this issue Pay someone - Replies sent to paid system message appear again in transaction thread #43183 to display the child report of send money action as normal thread

I prefer option 1 since the child report is transaction thread report, it doesn't make sense if we display it as a normal thread

Please help to confirm the expected behavior that we want here cc @trjExpensify @stitesExpensify

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. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
Status: No status
Development

No branches or pull requests

9 participants