Display fees name instead of type on invoices

[jibees]

What? Why?

Closes #8904

Two steps:

• change the way the adjustment label is computed/formatted. There's no migration here, so, to test, this needs a new order (in order to create new adjustment). We now use the fee name instead of the fee type to create the adjustment label
• Do not merge "Admin & Handling" fees for an order, but let's keep the adjustments split.

What should we test?

• As a hub manager, edit fee name () and add fees to an order
• As a customer, checkout, see that invoice clearly display adjustment fees by name and not merged:

Release notes

Changelog Category: User facing changes

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

[jibees]

I'm wondering why ./spec/helpers/checkout_helper_spec.rb:52 constently failed on Github CI but not with my local setup. In addition, if I understand correctly the spec, it should be 123 and not 3 ...

(ref: https://github.com/openfoodfoundation/openfoodnetwork/runs/7501482753?check_suite_focus=true)

@filipefurtad0 do you have any idea?

[filipefurtad0]

I've pulled this branch to my local system now and cannot reproduce the missing spec. I'm running bundle exec rspec --bisect spec/ to see if something fails...

The only thing I can think of is some order dependent issue, or some leaking of configurations. Maybe it's time I pick this one up again: #8461 🙈

[jibees]

Thanks @filipefurtad0 for having a look at it.
I'll try a rebase, didn't try for now, let's see how it goes.

Will take a look at #8461 🙈

[filipefurtad0]

There's some mystery around this: suddenly, I can reproduce it. It is happening because there are two adjustments with label Shipping:

(byebug) Spree::Adjustment.all
#<ActiveRecord::Relation [#<Spree::Adjustment id: 1235, amount: 0.123e3, label: "Shipping", adjustable_id: 1423, created_at: "2022-07-26 01:53:57.330498000 +1000", updated_at: "2022-07-26 01:53:57.353426000 +1000", mandatory: nil, originator_id: 488, originator_type: "EnterpriseFee", eligible: true, adjustable_type: "Spree::Order", included_tax: 0.0, state: "open", order_id: 1423, included: false, tax_category_id: nil>, #<Spree::Adjustment id: 1234, amount: 0.3e1, label: "Shipping", adjustable_id: 1088, created_at: "2022-07-26 01:53:57.177449000 +1000", updated_at: "2022-07-26 01:53:57.369201000 +1000", mandatory: true, originator_id: 1334, originator_type: "Spree::ShippingMethod", eligible: true, adjustable_type: "Spree::Shipment", included_tax: 0.0, state: "open", order_id: 1423, included: false, tax_category_id: nil>]>
(byebug) Spree::Adjustment.first.amount
0.3e1
(byebug) Spree::Adjustment.second.amount
0.123e3

I'm not sure why this is failing now though... I think if we define admin_fee_summary on line 58 as

admin_fee_summary = adjustments.first

Then it passes - at least locally.

[jibees]

Thanks a lot @filipefurtad0 .
You're right, there are two adjustments. I've then selected the one which is not the shipping adjustment

admin_fee_summary = adjustments.reject { |a| a.id == shipping_adjustment.id }.first

It should work now!
Again, thanks! 😊 🙏

[mkllnk]

I wonder if this can cause problems. If other languages are not updated then the old variable is required but won't be set. The solution would be to introduce a new translation key. But let's monitor this in the release and see what Transifex does. Maybe it updates this automatically which would be great.

[jibees]

@mkllnk so, this cause issue: https://app.bugsnag.com/yaycode/openfoodnetwork-uk/errors/633477967f4d38000830e14e?event_id=6334779600979f7f70b60000&i=sk&m=nw

I'll update the PR by creating a new key.

[jibees]

I've introduced a new key enterprise_fee_by_name: 5e1d769

Moving to Test Ready, I assume this is small enough to not be reviewed.

[mkllnk]

We got some related spec failures.

[jibees]

I rebased it onto master. Let see how it goes.

[jibees]

@filipefurtad0
Could you please run exec spec spec/system/admin/invoice_print_spec.rb:245 on your own setup on this branch and copy/paste the results here? I don't have the same result than the CI here ...

My result:

As an administrator
    I want to print a invoice as PDF
 an order with taxes included alternative invoice displays the taxes correctly
     Failure/Error: expect(page).to have_content "Item Qty Unit price (Incl. tax)"
       expected to find text "Item Qty Unit price (Incl. tax)" in "TAX INVOICE Enterprise 23 10 Lovely Street, Northwest Herndon, 20170, Victoria Billing address: Invoice issued on: 2022-09-14 John Doe Date of transaction: 2022-09-14 10 Lovely Street, Northwest Invoice number: R017822862 Herndon, 20170, Victoria 123-456-7890 [email protected] Item Qty Unit price (Incl. tTotal price (Incl. tax) Tax rate Product #27 - 9150 (1g) 1 $12.54 $12.54 0.0% Enterprise 24 Product #28 - 9213 (1g) 3 $500.15 $1,500.45 20.0% Enterprise 24 Whole order - Enterprise fee 4 fee by coordinator $120.00 15.0% Enterprise 23 Shipping $100.55 10.0% Total (Incl. tax): $1,733.54 Total tax (10.0%): $9.14 Total tax (15.0%): $15.65 Total tax (20.0%): $250.08 Total (Excl. tax): $1,458.67 Payment summary Balance due : $1,733.54 Date/time Payment Method Payment State Amount Sep 14, 2022 23:08 Check checkout $1,733.54 Payment Description at Checkout"

[filipefurtad0]

Hey @jibees ,

After pulling and resetting this branch to the latest commit 090cac0, running bundle exec rspec spec/system/admin/invoice_print_spec.rb:245 always fails with:

  1) 
    As an administrator
    I want to print a invoice as PDF
 an order with taxes included alternative invoice displays the taxes correctly
     Failure/Error: expect(page).to have_content "#{enterprise_fee.name} fee by coordinator $120.00"
       expected to find text "Enterprise fee 1 fee by coordinator $120.00" in "TAX INVOICE Enterprise 1 10 Lovely Street, Northwest Herndon, 20170, Victoria Billing address: Invoice issued on: 2022-09-15 John Doe Date of transaction: 2022-09-15 10 Lovely Street, Northwest Invoice number: R303877401 Herndon, 20170, Victoria 123-456-7890 [email protected] Item Qty Unit price (Incl. tax) Total price (Incl. taTax rate Product #1 - 5386 1 $12.54 $12.54 0.0% (1g) Enterprise 2 Product #2 - 5358 3 $500.15 $1,500.45 20.0% (1g) Enterprise 2 Shipping $100.55 10.0% Whole order - Enterprise fee 1 fee by coordinator Enterprise 1 $120.00 15.0% Total (Incl. tax): $1,733.54 Total tax (10.0%): $9.14 Total tax (15.0%): $15.65 Total tax (20.0%): $250.08 Total (Excl. tax): $1,458.67 Payment summary Balance due : $1,733.54 Date/time Payment Method Payment Amount State Sep 15, 2022 Check checkout $1,733.54 02:26 Payment Description at Checkout"

[filipefurtad0]

Ok, it seems you are getting:

Item Qty Unit price (Incl. tTotal price (Incl. tax) Tax rate Product #27 - 9150 (1g) 1 $12.54 $12.54 0.0%

I'm getting:

Item Qty Unit price (Incl. tax) Total price (Incl. taTax rate Product #1 - 5367 1 $12.54 $12.54 0.0%

So in your case, this assertion fails:
expect(page).to have_content "Item Qty Unit price (Incl. tax)"

For some reason, your rack_test is overlapping and clipping the text in a slightly different way, then in my setup. In my/GH setup it clips in only on the Total price line. For some reason, your rendering is slightly different and your clipping starts slightly earlier, already in the Unit price (Incl. tbit.

I have no idea why this happens, I've quickly searched for a solution to harmonize our results but could not really find anything.

If you comment out this assertion only for the sake of making your test pass locally, would you then get the same error I'm getting?

[jibees]

Hi @filipefurtad0
You seems to have the same error that the CI, which is a good news.

If you comment out this assertion only for the sake of making your test pass locally, would you then get the same error I'm getting?

No, actually if I comment those lines before the assertion that failed on your side (and on CI), it actually pass...

It seems that my pdf is rendered in a very strange way inside specs, with text overlapping some others, or misplaced text...

But maybe it is also misplaced in the CI...
CI, text found in the PDF via the spec:

Whole order - Enterprise fee 11 fee by $120.00 15.0%  coordinator Enterprise 403

When I looked at an invoice (alternative model):

I would say that coordinator Enterprise 403 that comes from the tax rate is completely misplaced...
So, I assume that we have some errors with PDF rendering inside specs!

EDIT: and this spec actually fails on master too for me

[jibees]

PDF is rendered like this:

TAX     INVOICE


Enterprise      1
10      Lovely  Street, Northwest
Herndon,        20170,  Victoria


                                                                                                                  Billing       address:
Invoice issued  on:     2022-09-15                                                                                                John  Doe
Date    of      transaction:    2022-09-15                                                                             10       Lovely  Street, Northwest
Invoice number: R626307743                                                                               Herndon,       20170,  Victoria

                                                                                                                    123-456-7890
                                                                                                            [email protected]
Item                                                                Qty   Unit  price   (Incl.  tTotal  price   (Incl.  tax)       Tax  rate


Product #1      -       4958

(1g)                                                                     1             $12.54              $12.54               0.0%
Enterprise      2

Product #2      -       0

(1g)                                                                     3            $500.15           $1,500.45              20.0%
Enterprise      2

Whole   order   -       Enterprise      fee     1       fee     by      coordinator                                                      $120.00              15.0%
Enterprise      1
Shipping                                                                                               $100.55              10.0%

                                                                          Total (Incl.  tax):                             $1,733.54
                                                                         Total  tax     (10.0%):                                  $9.14

                                                                         Total  tax     (15.0%):                                 $15.65
                                                                         Total  tax     (20.0%):                               $250.08
                                                                           Total        (Excl.  tax):                            $1,458.67




Payment summary                                                                                             Balance     due     :       $1,733.54

     Date/time     Payment      Method Payment  State Amount

Sep     15,     2022    18:02 Check             checkout        $1,733.54



Payment Description     at      Checkout

We can clearly see that this seems to be a overlapping issue. Let's try to increase the size of the PDF?

[filipefurtad0]

We can clearly see that this seems to be a overlapping issue.

Fully agree.

Let's try to increase the size of the PDF?

This would be ideal. Maybe this is a setting from wicked-pdf or wkhtmltopdf. I'll try to find it.

[filipefurtad0]

One option could be to set :page_size => 'A3', on config/initializers/wicked_pdf.rb. This is not really what we have in staging/prod, but since we are only interested in pdf contents and not format (rack_test) I think it could be acceptable solution to set this option for a test environment only.

After doing this, the text is not clipped and the spec is green with the correct assertion, i.e.:

Total price (Incl. tax) Tax rate Product

instead of:

Total price (Incl. taTax rate

Edit: PR #9674 tries to achieve this as simply as possible. The setting works (the pdf page size changes indeed). I hope this passes on your system as well 🤞

[jibees]

Ok, thanks @drummer83 : I've found this issue, and this was well related to this PR.

Now moving to Test Ready (waiting for specs to be green)

[drummer83]

Thanks @jibees, this looks much better now. I can populate and view the cart with all fees enabled now.

It seems we still need to update the /cart page. Here we can see both: the separate fees AND the combined Admin & Handling:

Just a note (but I think this is ok): We do see the fee names instead of the fee type also for all existing (old) orders.

Moving to In Dev. We're close! 💪

[jibees]

Back into 'Code Review' since it has changed.

[mkllnk]

Cool. 😎

[drummer83]

Hi @jibees,
Thanks for having another look! I have created an order with all kind of fees and taxes. Here are my test results.

Fees setup

Places checked

Before	After
Shopfront: Price chart	✔️
Shopfront: Cart overview	✔️
Shopfront: Edit cart	✔️
Shopfront: Checkout legacy	✔️
Shopfront: Checkout split	✔️
Shopfront: order confirmation	✔️
Order confirmation email hub	✔️
Order confirmation email customer	✔️
Backend: Edit order	✔️
Invoice regular	✔️
Invoice alternative	✔️

Observations (before and after staging)

• All fees per ITEM (or weight) are added to the price in the shopfront and on invoices ✔️
• All fees per ITEM (or weight) are displayed in Line Item Adjustments ✔️
• All fees per ORDER are NOT added to the price in the shop ✔️
• All fees per ORDER are displayed in Order Adjustments ✔️
• All fees per ORDER are grouped together (except for Shipping and Payment fees) before staging and are displayed seperately after staging ✔️

Findings (before and after staging)

• Shopfront: When having multiple fees per ITEM of the same type the fees are summed up, but only one name is displayed. ➡️ will create separate issue ➡️ Multiple fees per item of same fee type are merged in shopfront but only one name is displayed #9751
• I was not able to follow the tax calculation 🤯
• I don't really like the very long wording of the separate fees, but first of all I have long names for fees and coordinator and second this can be improved in Transifex.

Summary

I think we are good here. Thanks JB for your patience!
Moving to Ready to go! 🚀 🎉

[RachL]

@jibees @drummer83 I'm not able to make this work in FR production. I can't even see "admin & handling" line anymore.

I do see the pies on the shopfront and fees in the backend. But nothing in the cart or on the invoice. What am I missing: is there a toggle somewhere?

[drummer83]

@RachL I am not aware of a toggle. You need to set up an enterprise fee which is per order. Fees per item are included in the product price and thus not displayed separately in the cart and invoices. And the fee needs to be added to the order cycle of course (step 2 or 3). After updating the fees you might need to empty your cart and start over again.
Could this be it?

[jibees]

@RachL You mean that on invoices, you don't see any fees?

There is no toggle.

[RachL]

@jibees yes, I only see shipping fees... I need to dig more