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 2023-10-23] 🟡 [$500] Add stylized receipts for distance requests #27050

Closed
JmillsExpensify opened this issue Sep 8, 2023 · 41 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Design Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item.

Comments

@JmillsExpensify
Copy link

JmillsExpensify commented Sep 8, 2023

One thing I think we need to re-visit now that we shipped distance v1 is the receipt. For instance, this was recommended in the original design doc, though during the review process it was deemed unnecessary.

I think in hindsight that decision hasn’t turned out to be a great one. Reason being, you have to kind of deduce what the points on the map are, so what is point A and what is point B. For SBE, it was somewhat easy since the event looks like it was close to the Burbank airport? But yeah, I think a core part of mileage is knowing where someone went, whether it’s from A to B, or A to B to C.

As a result, I think we need to:

  • Utilize the stylized receipt pattern that we've settled on for eReceipts
  • Apply the same foundational styling for distance request
  • Print the sum of a user's Start, Stop, and Finish way points, each on their own row
  • All while still showing the core information about the request, such as amount, date and distance merchant.

I'm going to assign the internal label, as this is dependent on related internal initiatives and Expensify employees have the most context. We can involve external as soon as the scope and todos are clear.

cc distance project team @neil-marcellini @hayata-suenaga @arosiclair @trjExpensify

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b483120a2e8ed4dd
  • Upwork Job ID: 1700221182911156224
  • Last Price Increase: 2023-09-11
@JmillsExpensify JmillsExpensify added Daily KSv2 NewFeature Something to build that is a new item. labels Sep 8, 2023
@JmillsExpensify JmillsExpensify self-assigned this Sep 8, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 8, 2023

Current assignee @JmillsExpensify is eligible for the NewFeature assigner, not assigning anyone new.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Sep 8, 2023
@JmillsExpensify JmillsExpensify added the Internal Requires API changes or must be handled by Expensify staff label Sep 8, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 8, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Sep 8, 2023

Triggered auto assignment to Contributor Plus for review of internal employee PR - @mollfpr (Internal)

@hayata-suenaga
Copy link
Contributor

I think the waypoints are stored in Transaction that is returned from the OpenReport and other relevant API commands?

I think edits that need to be made are in the NewDot side. Let's make this external 🚀

@Expensify/design could you double check the design?

@hayata-suenaga hayata-suenaga self-assigned this Sep 11, 2023
@hayata-suenaga hayata-suenaga added the External Added to denote the issue can be worked on by a contributor label Sep 11, 2023
@melvin-bot melvin-bot bot changed the title Add stylized receipts for distance requests [$500] Add stylized receipts for distance requests Sep 11, 2023
@hayata-suenaga hayata-suenaga removed the Internal Requires API changes or must be handled by Expensify staff label Sep 11, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 11, 2023

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Sep 11, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 11, 2023

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

@mollfpr
Copy link
Contributor

mollfpr commented Sep 11, 2023

@hayata-suenaga @JmillsExpensify I think this issue missing Help Wanted label.

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

Thank you for pointing that out. I added the Help Wanted label 🙇

@trjExpensify
Copy link
Contributor

Let's assign the Design label to this and make sure we complete getting an updated style for this in-line with the eReceipts.

@melvin-bot
Copy link

melvin-bot bot commented Sep 11, 2023

Triggered auto assignment to @shawnborton (Design), see these Stack Overflow questions for more details.

@neil-marcellini neil-marcellini 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 11, 2023
@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Oct 4, 2023
@neil-marcellini
Copy link
Contributor

The PR was merged!

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Oct 16, 2023
@melvin-bot melvin-bot bot changed the title 🟡 [$500] Add stylized receipts for distance requests [HOLD for payment 2023-10-23] 🟡 [$500] Add stylized receipts for distance requests Oct 16, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Oct 16, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 16, 2023

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot
Copy link

melvin-bot bot commented Oct 16, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.84-10 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 2023-10-23. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

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

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Oct 16, 2023
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2023-10-23] 🟡 [$500] Add stylized receipts for distance requests [HOLD for payment 2023-10-23] [HOLD for payment 2023-10-23] 🟡 [$500] Add stylized receipts for distance requests Oct 16, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 16, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.84-10 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 2023-10-23. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

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

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@JmillsExpensify JmillsExpensify changed the title [HOLD for payment 2023-10-23] [HOLD for payment 2023-10-23] 🟡 [$500] Add stylized receipts for distance requests [HOLD for payment 2023-10-23] 🟡 [$500] Add stylized receipts for distance requests Oct 17, 2023
@JmillsExpensify
Copy link
Author

@mollfpr I think we should have a regression test for this one, as it's a core flow and piece of compliance in the app. Do you agree and can you suggest a test?

@neil-marcellini
Copy link
Contributor

100% we need a regression test, especially since it's already broken 🙈; I'm working on a fix.

@mollfpr
Copy link
Contributor

mollfpr commented Oct 18, 2023

Regression step for this issue:

Distance request with 2 waypoints

  1. Click the green plus, request money, distance
  2. Enter valid start and finish locations
  3. Create the request
  4. Open the money request report
  5. Click on the distance request to open the transaction report
  6. Click on the route image
  7. Verify that the distance eReceipt is shown and looks good

Distance request with 12 waypoints

  1. Click the green plus, request money, distance
  2. Enter valid start and finish locations, plus 10 stops
  3. Create the request
  4. Open the money request report
  5. Click on the distance request to open the transaction report
  6. Click on the route image
  7. Verify that the distance eReceipt is shown and that you can scroll down to see the entire eReceipt

Regression step for #28289 and #28949

  1. Go offline
  2. Click the green plus, request money, distance
  3. Enter a valid address for start and finish waypoints
  4. Verify that you can click next
  5. Select a workspace
  6. Verify that all information shows or is TBD
  7. Click request
  8. Verify that the report preview shows a pending receipt image with the amount and miles as TBD
  9. Click the report preview
  10. Verify that the request shows as distance, with the amount and miles as TBD
  11. Click on the request
  12. Verify that the request shows as distance, with the amount and miles as TBD
  13. Verify that there's not Merchant input in the forms
  14. Go online
  15. Verify that all pending data fills

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 Daily KSv2 labels Oct 23, 2023
@JmillsExpensify
Copy link
Author

Those look good to me. I'll get an issue created and add them.

@JmillsExpensify
Copy link
Author

100% we need a regression test, especially since it's already broken 🙈; I'm working on a fix.

@neil-marcellini for the payment, are you saying that we've already had a regression and the should be reflected in payment?

@neil-marcellini
Copy link
Contributor

Yes there was already a regression but we think it was due to some weird git merging behavior, and the mistake appears to have come from another PR. I commented about the regression here. No need to reduce pay for this issue.

@JmillsExpensify
Copy link
Author

Payment summary: $500 to @mollfpr for PR review.

@JmillsExpensify
Copy link
Author

@mollfpr Please raise a request in NewDot, going to close this one as I've created the regression test.

@JmillsExpensify
Copy link
Author

$500 payment approved for @mollfpr based on summary above.

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 Design Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item.
Projects
None yet
Development

No branches or pull requests

7 participants