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

Payment Method Description dropped off Invoices #6359

Closed
emilyjeanrogers opened this issue Nov 10, 2020 · 11 comments · Fixed by #7542
Closed

Payment Method Description dropped off Invoices #6359

emilyjeanrogers opened this issue Nov 10, 2020 · 11 comments · Fixed by #7542
Assignees
Labels
AU Selected to be done by the Australian active instance bug-s3 The bug is stopping a critical or non-critical feature but there is a usable workaround.

Comments

@emilyjeanrogers
Copy link

emilyjeanrogers commented Nov 10, 2020

Description

Payment Method Description information was dropped off invoices by the resolution of Issue #5208

Issue #5208 changed invoices so that payment details were displayed in a table. The decision was made to leave the Payment Method Description out of this table as it can include lengthy content which would throw out the layout of the table. Payment Method Description was not moved to another section of the invoice however, and it now no longer appears on Invoices.

One of the main uses cases for the Payment Method Description field is to provide How to Pay information such as bank details for making a deposit. This information appeared at checkout and also in invoices. There is not currently a formal way for Enterprises to include How to pay information in Invoices so this was a common workaround NOTE: there is an open issue exploring a resolution of the How to Pay issue (#5209) which may overlap this issue.

Enterprises previously used the Payment Method Description field to provide How to Pay information, which would then be displayed at the bottom of the invoice. Now that this field no longer appears on Invoices, Enterprises currently have no way of including additional payment details relating to the Payment Method on Invoices.

Expected Behavior

Prior to issue #5208 the Payment Method Description field appeared in invoices. Enterprises that relied on this feature have now lost it. This is how the Invoice used to look:

Screen Shot 2020-11-09 at 12 46 36 PM

Actual Behaviour

The invoice now looks like this

Screen Shot 2020-11-11 at 9 40 03 AM

Steps to Reproduce

  1. Create an Order / access a shop with existing orders
  2. Log in as a Manager of the Enterprise
  3. Go to Orders
  4. Select the checkbox next to an Order
  5. Click Print Invoice

Workaround

The Enterprise can manually provide additional details such as How to Pay information to customers by email or as a handwritten note in posted invoices.

Severity

S3

-->

Your Environment

  • Version used:
  • Browser name and version:
  • Operating System and version (desktop or mobile):

Possible Fix

Under Payment Summary there is a field called Payment Description at Checkout: which then prints the description info, not in the table just width of the page

@emilyjeanrogers emilyjeanrogers added the bug-s3 The bug is stopping a critical or non-critical feature but there is a usable workaround. label Nov 10, 2020
@emilyjeanrogers
Copy link
Author

FYI @tschumilas

Also @kirstenalarsen solving this has a link / dependency with #5209. If including How to Pay information is the MAIN use case of the Payment Method Description field on Invoices, then a resolution to #5209 would make this issue less of a priority to resolve.

@kirstenalarsen
Copy link
Contributor

I think we need to look at fixing this (which is likely a papercut (rather than waiting on #5209 - which is definitely not) unfortunately. I don't think we can assess as papercut tonight though as it doesn't have a proposed solution @emilyjeanrogers

Would it work to have the Payment Method Description displayed just for the 1st and/or last Payment method or something? I think probably the first - the one used when the order was placed. This would fix the problem that we have created and would likely be very easy to do.

For purposes of papercut tonight that's what I'm proposing. Under
Payment Summary there is a field called Payment Description at Checkout: which then prints the description info, not in the table just width of the page

@tschumilas
Copy link

likely too late with this - but just saying I like the solution you propose @kirstenalarsen

@kirstenalarsen kirstenalarsen added the AU Selected to be done by the Australian active instance label Dec 17, 2020
@kirstenalarsen
Copy link
Contributor

kirstenalarsen commented Feb 3, 2021

I have just had pushback from the Aus customer support team about this being our papercut nomination as they argue that it is a regression introduced by #5208 and was picked up straight away. I think this is a valid point and arguably should be fixed in delivery pipe without being an AU voted papercut. This proposal would mean it potentially goes into dev ready above proposed papercuts or s3s? Thoughts @RachL @lin-d-hop @jaycmb ?

I have noted to myself for discussion at next delivery train

@RachL
Copy link
Contributor

RachL commented Feb 4, 2021

@kirstenalarsen we don't have a process for "regressions". I know it can be confusing because when doing rails upgrade we have a time frame during which we tackle non s1/s2. The rest of the time its our s3/papercuts process that deal with s3 (all our s3 are regressions ;) ).

The quickest if you want this fixed, is to change your vote for your current s3 that is in dev ready: #5018

@kirstenalarsen
Copy link
Contributor

kirstenalarsen commented Feb 4, 2021 via email

@jibees jibees assigned jibees and unassigned jibees Mar 22, 2021
@jibees jibees self-assigned this May 4, 2021
@jibees
Copy link
Contributor

jibees commented May 4, 2021

I've submitted a PR, and now here's the new invoice template (see Payment Description at Checkout part at the bottom):
Capture d’écran 2021-05-04 à 13 33 20

I hope it suits you.

@jibees
Copy link
Contributor

jibees commented May 5, 2021

@kirstenalarsen @RachL
Re.

Would it work to have the Payment Method Description displayed just for the 1st and/or last Payment method or something? I think probably the first - the one used when the order was placed.

When you said the one used when the order was placed, do you think about Payment State = Failed for example?
In the screenshot below, I've made 2 payments with credit cards that failed, and then I chose "Cash on collection". The Payment Description at Checkout is linked to the first one, ie. "Credit card (fake)". What to you think?
Capture d’écran 2021-05-05 à 09 54 36

Should the Payment Description at Checkout part be linked to the first payment, or the first that is actually in CHECKOUT state? Is there any other states? How can I replicate it to test each cases?

Thanks. 🙏

@kirstenalarsen
Copy link
Contributor

kirstenalarsen commented May 5, 2021 via email

@tschumilas
Copy link

Just a reminder that there are 2 invoice templates to consider. They are different. Below is an example of the current invoice template used in instances where tax is not included in prices. Here is a google sheet that compares the 2 invoice templates and what info is extracted - https://drive.google.com/file/d/1fVSUbJ6KaNY782bNN3GISiJzc1gk_aYz/view?usp=sharing
image

@jibees
Copy link
Contributor

jibees commented May 6, 2021

I think it should be the one that is connected to the order in checkout state i.e. the one actually used. Does that work?

Yes, I guess too. Then, I'll ask core-devs team what this means in term of state in code base.

Just a reminder that there are 2 invoice templates to consider.

I saw this, what as far as I understood they both use the same template to generate Payment summary (and then payment description at checkout). Thanks for pointing me that out! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AU Selected to be done by the Australian active instance bug-s3 The bug is stopping a critical or non-critical feature but there is a usable workaround.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants