openfoodnetwork
openfoodnetwork copied to clipboard
openfoodfoundation
Display fees name instead of type on invoices
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
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?
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: https://github.com/openfoodfoundation/openfoodnetwork/pull/8461 :see_no_evil:
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 https://github.com/openfoodfoundation/openfoodnetwork/pull/8461 🙈
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.
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! 😊 🙏
@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"
Hey @jibees ,
After pulling and resetting this branch to the latest commit 090cac04d3203bc45b6a93bf9729fc492afc33e5, 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"
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?
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
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?
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.
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 :crossed_fingers:
Hi @jibees, Thanks for this PR! I couldn't quite make it work.
As soon as there is an enterprise fee per ITEM added to the order cycle, there is an Error 500 when populating the cart (see screen capture). Also the cart wont open anymore...
We have to move this back to In Dev...
Here are the logs:
{
"POST": {
"scheme": "https",
"host": "staging.openfoodnetwork.org.au",
"filename": "/cart/populate",
"remote": {
"Address": "65.99.217.140:443"
}
}
}
{
"Response Headers (591 B)": {
"headers": [
{
"name": "cache-control",
"value": "no-store"
},
{
"name": "content-type",
"value": "text/html; charset=utf-8"
},
{
"name": "date",
"value": "Wed, 28 Sep 2022 16:38:03 GMT"
},
{
"name": "expires",
"value": "Fri, 01 Jan 1990 00:00:00 GMT"
},
{
"name": "pragma",
"value": "no-cache"
},
{
"name": "referrer-policy",
"value": "strict-origin-when-cross-origin"
},
{
"name": "server",
"value": "nginx"
},
{
"name": "strict-transport-security",
"value": "max-age=63072000; includeSubDomains"
},
{
"name": "vary",
"value": "Accept"
},
{
"name": "x-content-type-options",
"value": "nosniff"
},
{
"name": "x-content-type-options",
"value": "nosniff"
},
{
"name": "x-download-options",
"value": "noopen"
},
{
"name": "X-Firefox-Spdy",
"value": "h2"
},
{
"name": "x-permitted-cross-domain-policies",
"value": "none"
},
{
"name": "x-request-id",
"value": "cafed9db-889f-4cc2-999d-27a64132b4f5"
},
{
"name": "x-runtime",
"value": "0.276014"
},
{
"name": "x-xss-protection",
"value": "1; mode=block"
},
{
"name": "x-xss-protection",
"value": "1; mode=block"
}
]
}
}
{
"Request Headers (1.048 KB)": {
"headers": [
{
"name": "Accept",
"value": "application/json, text/javascript, */*"
},
{
"name": "Accept-Encoding",
"value": "gzip, deflate, br"
},
{
"name": "Accept-Language",
"value": "en-US,en;q=0.5"
},
{
"name": "Connection",
"value": "keep-alive"
},
{
"name": "Content-Length",
"value": "54"
},
{
"name": "Content-Type",
"value": "application/json;charset=utf-8"
},
{
"name": "Cookie",
"value": "XSRF-TOKEN=giA1B8r5poUGtDlcwpkZlldisF69Owqi4DX7qdwQePGcz1MdXAID3ZjDCMZjsjsJP960PgBJLT0NJ8-tG41_7g; _ofn_session_id=95841932db288e7df9f9cbe738e875e8; _pk_id.3.cd81=472b733749e0bd88.1664382572.; _pk_ses.3.cd81=1; cookies_consent=cookies_consent; __stripe_mid=f65a40d4-0e78-42d1-916c-a4e2b741fd1246daf4; __stripe_sid=c67aed4e-b7ed-4729-b3df-0966e48b23b1b61438"
},
{
"name": "Host",
"value": "staging.openfoodnetwork.org.au"
},
{
"name": "Origin",
"value": "https://staging.openfoodnetwork.org.au"
},
{
"name": "Referer",
"value": "https://staging.openfoodnetwork.org.au/konrad-testhub-staging-au/shop"
},
{
"name": "Sec-Fetch-Dest",
"value": "empty"
},
{
"name": "Sec-Fetch-Mode",
"value": "cors"
},
{
"name": "Sec-Fetch-Site",
"value": "same-origin"
},
{
"name": "TE",
"value": "trailers"
},
{
"name": "User-Agent",
"value": "Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:104.0) Gecko/20100101 Firefox/104.0"
},
{
"name": "X-Requested-With",
"value": "XMLHttpRequest"
},
{
"name": "X-XSRF-TOKEN",
"value": "giA1B8r5poUGtDlcwpkZlldisF69Owqi4DX7qdwQePGcz1MdXAID3ZjDCMZjsjsJP960PgBJLT0NJ8-tG41_7g"
}
]
}
}
Notes to myself for testing next time
Observations (before 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)
Findings (before staging)
- Shopfront: When having multiple fees per ITEM the fees are summed up, but only one name is displayed.
- Edit order: Line item adjustments are still showing fee types, not fee names
- I was not able to follow the tax calculation
Places to check
- 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
I don't think this is related to this one, I'd purpose to let this one in Test Ready and probably open a new issue. Did you test on the same server with master?
For the request, I don't see any response body (only headers)
I have tested this on staging UK and AU because I thought it was due to my customer or whatever. But it was very clear that this happens as soon as I add the fee to the order cycle. I am not sure if I tried master as well...
Sorry for the incomplete logs... I have those on my other computer which I can't access now.
I think I'v found the right log: https://app.bugsnag.com/yaycode/openfoodnetwork-uk/errors/633477967f4d38000830e14e?event_id=6334779600979f7f70b60000&i=sk&m=nw
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)
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! 💪
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 | :heavy_check_mark: |
 |
 |
| Shopfront: Cart overview | :heavy_check_mark: |
 |
 |
 |
 |
| Shopfront: Edit cart | :heavy_check_mark: |
 |
|
| Shopfront: Checkout legacy | :heavy_check_mark: |
 |
 |
| Shopfront: Checkout split | :heavy_check_mark: |
 |
 |
| Shopfront: order confirmation | :heavy_check_mark: |
 |
 |
| Order confirmation email hub | :heavy_check_mark: |
 |
 |
| Order confirmation email customer | :heavy_check_mark: |
 |
 |
| Backend: Edit order | :heavy_check_mark: |
 |
 |
 |
 |
| Invoice regular | :heavy_check_mark: |
 |
 |
| Invoice alternative | :heavy_check_mark: |
 |
 |
Observations (before and after staging)
- All fees per ITEM (or weight) are added to the price in the shopfront and on invoices :heavy_check_mark:
- All fees per ITEM (or weight) are displayed in Line Item Adjustments :heavy_check_mark:
- All fees per ORDER are NOT added to the price in the shop :heavy_check_mark:
- All fees per ORDER are displayed in Order Adjustments :heavy_check_mark:
- All fees per ORDER are grouped together (except for Shipping and Payment fees) before staging and are displayed seperately after staging :heavy_check_mark:
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. :arrow_right: will create separate issue :arrow_right: #9751
- I was not able to follow the tax calculation :exploding_head:
- 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! :rocket: :tada:
@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?
@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?
@RachL You mean that on invoices, you don't see any fees?
There is no toggle.