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 for payment 2024-08-14] [$500][P2P Distance] Create UpdateMoneyRequestDistanceRate in App #36987

Closed
neil-marcellini opened this issue Feb 21, 2024 · 58 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.

Comments

@neil-marcellini
Copy link
Contributor

neil-marcellini commented Feb 21, 2024

Create UpdateMoneyRequestRate following this plan. Please be sure to read and understand the relevant sections of the entire plan as well.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01fde353d38f5fdf1b
  • Upwork Job ID: 1760136022052638720
  • Last Price Increase: 2024-02-21
  • Automatic offers:
    • paultsimura | Contributor | 0
Issue OwnerCurrent Issue Owner: @stephanieelliott
@neil-marcellini neil-marcellini changed the title [P2P Distance] Create UpdateMoneyRequestRate [P2P Distance] Create UpdateMoneyRequestRate in App Feb 21, 2024
@neil-marcellini neil-marcellini added External Added to denote the issue can be worked on by a contributor Engineering Daily KSv2 Help Wanted Apply this label when an issue is open to proposals by contributors labels Feb 21, 2024
@melvin-bot melvin-bot bot changed the title [P2P Distance] Create UpdateMoneyRequestRate in App [$500] [P2P Distance] Create UpdateMoneyRequestRate in App Feb 21, 2024
Copy link

melvin-bot bot commented Feb 21, 2024

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

Copy link

melvin-bot bot commented Feb 21, 2024

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

@neil-marcellini neil-marcellini added the NewFeature Something to build that is a new item. label Feb 21, 2024
Copy link

melvin-bot bot commented Feb 21, 2024

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Feb 21, 2024
@paultsimura
Copy link
Contributor

Proposal

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

Once the Distance and Rate are added as separate fields of a Distance request (not stored in a merchant as currently), we want to introduce the possibility to edit the rate once the request is created.

What is the root cause of that problem?

This is a new feature request

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

This should be done after the Rate field is added to the Money Requests.

First, we should add the EditRequestRatePage similar to other pages that modify IOU fields (IOURequestStepDescription/IOURequestStepTag) using the WorkspaceRatePage as a UI example.

We need to add the function that will call the BE UpdateMoneyRequestRate API when the page form is submitted. It should use the corresponding parameters that the Endpoint requires, as well as build optimistic data.

What alternative solutions did you explore? (Optional)

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Feb 21, 2024
@koko57
Copy link
Contributor

koko57 commented Feb 22, 2024

Commenting to help with the review.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 22, 2024
@neil-marcellini
Copy link
Contributor Author

@paultsimura's proposal looks good. Sounds like we will hold on #36985, so updating the title for that.

Copy link

melvin-bot bot commented Feb 22, 2024

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

@neil-marcellini neil-marcellini changed the title [$500] [P2P Distance] Create UpdateMoneyRequestRate in App [$500] [HOLD 36985] [P2P Distance] Create UpdateMoneyRequestRate in App Feb 22, 2024
@neil-marcellini neil-marcellini added Weekly KSv2 Help Wanted Apply this label when an issue is open to proposals by contributors and removed Daily KSv2 labels Feb 22, 2024
@paultsimura
Copy link
Contributor

Today's update: I've almost finished the PR. Only final testing remains.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Jul 22, 2024
@paultsimura
Copy link
Contributor

The PR is ready: #40021

@stephanieelliott
Copy link
Contributor

PR is awaiting your review @mananjadhav

@mananjadhav
Copy link
Collaborator

Thanks for the bump @stephanieelliott. I'll finish this today.

@stephanieelliott
Copy link
Contributor

PR was merged to Main earlier today!

Copy link

melvin-bot bot commented Aug 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.

Copy link

melvin-bot bot commented Aug 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.

@neil-marcellini neil-marcellini removed the Reviewing Has a PR in review label Aug 7, 2024
@neil-marcellini
Copy link
Contributor Author

@stephanieelliott will you please handle payment and then we can close this out?

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Aug 7, 2024
@melvin-bot melvin-bot bot changed the title [$500][P2P Distance] Create UpdateMoneyRequestDistanceRate in App [HOLD for payment 2024-08-14] [$500][P2P Distance] Create UpdateMoneyRequestDistanceRate in App Aug 7, 2024
Copy link

melvin-bot bot commented Aug 7, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.17-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-08-14. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Aug 7, 2024

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

@melvin-bot melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Aug 14, 2024
@stephanieelliott
Copy link
Contributor

Summarizing payment on this issue:

@manjesh-yadav @paultsimura as the last step on this issue, can you please propose a regression test?

Upwork job is here: https://www.upwork.com/jobs/~01fde353d38f5fdf1b

@melvin-bot melvin-bot bot removed the Overdue label Aug 16, 2024
@paultsimura
Copy link
Contributor

@mananjadhav was out sick these days, but I think he'll add the tests here as he was the C+.

@mananjadhav
Copy link
Collaborator

Thanks @stephanieelliott. This is more of a feature request so we don't have any offending PR.

Regression Test proposal.

  1. Open the app and have a workspace with multiple distance rates.
  2. Create a Distance request to the workspace
  3. Navigate to the request details page
  4. Click the "Rate" field and change the rate
  5. Verify the rate & amount are changed correctly
  6. Verify the "modified expense" action was added
  7. Now, create a Distance request to self via "track expense"
  8. Navigate to the request details page
  9. Verify the "Rate" field is not clickable

@stephanieelliott
Copy link
Contributor

Thanks @mananjadhav! Regression test issue created here: https://github.com/Expensify/Expensify/issues/421162

@JmillsExpensify
Copy link

$500 approved for @mananjadhav

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 Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.
Projects
No open projects
Development

No branches or pull requests

8 participants