Possible bug: Order status 'Attach Invoice PDF' option is not respected

[Chandra]

Hi,

The invoice pdf is attached in customer email whatever option is checked in order status.

Niara theme on TB1.1.0

Anyone experiencing this?

Thanks.

Edited January 27, 2020 by Chandra

[Chandra]

Anyone else facing this issue please?

[AndyC]

No, but am still waiting for wrong country under company name 

[Chandra]

https://github.com/thirtybees/thirtybees/issues/1155

[toplakd]

Have just tested it few times on my live shop, and on my installation it works.

Same test performed also on fresh thirty bees install (Stable 1.1.0 + niara) also does not have this issues.

If checkbox enabled, pdf is attached to email, if checkbox is not enabled, no pdf attachment.

Bottom picture shows 1st mail with checkbox disabled - no attachment, second mail with checkbox enabled and attachment is there.

Have tested also with payment accepted status (with or without checkbox) and it works as it should

 

You can also check your shop for database_schema differences with core updater, not sure it could be related to that, but it wont hurt.

However, I strongly suggest, that all changes are always tested on backup instance of your shop, in case something does get wrong.

Edited March 16, 2020 by toplakd

[toplakd]

I do also suggest, before posting a "Possible BUG" that you try to replicate same thing on Clean thirty bees installation, to see if the problem is also on original installation or it just shows up in your installation.

Edited March 16, 2020 by toplakd

[wakabayashi]

I agree with @toplakd Invoices are attached correctly in our case too.

[Chandra]

Thanks @toplakd and @wakabayashi

I had zero db differences when I ran core updater against 1.1.0 yesterday

I updated to bleeding edge overnight and have a clean code base from that sense.

I ran the db diff of Core Updater and see a bunch of diffs, mainly field lengths and autoincrements. I have applied all changes - let me test this further on bleeding edge and post back.

[toplakd]

There is also a module from @datakick which makes some consistency checks.

You can also try it.

https://store.getdatakick.com/en/modules/consistency

[Chandra]

DB and Consistency all clear.

Now waiting for an order to come 🙂

 

 

[datakick]

You should also install Override Check module to see what overrides are installed on your system

[Chandra]

Not many related ones, really.

Now I will stop to bug you and report back once I have the next order.

[toplakd]

For hunting down small bugs (or my mistakes) I always have 3 installation of thirty bees on the server. All latest bleeding edge

One is live, one is exact copy of live shop for testing, and 3rd is always the clean stock thirty bees installation.

So if something does not work on live but it works on stock install, i can compare testing shop with clean stock, and make adjustments or fixes until it works 100% like stock.

Only after that, I apply it to the real live shop.

[Chandra]

1 minute ago, toplakd said:

For hunting down small bugs (or my mistakes) I always have 3 installation of thirty bees on the server. All latest bleeding edge

One is live, one is exact copy of live shop for testing, and 3rd is always the clean stock thirty bees installation.

So if something does not work on live but it works on stock install, i can compare testing shop with clean stock, and make adjustments or fixes until it works 100% like stock.

Only after that, I apply it to the real live shop.

Good plan.

I never got to set them up but will do shortly.

[Chandra]

I still have the invoice attached to the customer email. I have reconfirmed Attach invoice PDF to email is unchecked for the order status.

Code base: up to date (bleeding edge)
DB: Up to date
Overrides: None relevant
I have mailalerts module installed if that is of any significance

 

@datakick @toplakd @wakabayashi

Can any of you give me a hand on which file generates the email so I can dig a bit.
I looked up the classes folder - Order related php files and did not find code that actually sent the email. Thanks.

 

 

 

Edited March 18, 2020 by Chandra

[datakick]

@Chandra you are mixing two things.

The settings on Order Statuses has impact on 'Order Status Changed' email only (ie. Awaiting bank wire email)

What you are talking about is the 'Order confirmation' email, and that has nothing to do with order status. This email is always sent, and it contains all current information available.

I guess we could change the current semantics, and attach invoice to 'Order confirmation' email only if current order status has the send invoice checkbox enabled. However, I'm sure there would be plenty of merchants that would not want this behaviour by default.

[toplakd]

Yes, the pic Chandra added to github, was showing Order Processing status, so we all checked if issue was in statuses.

You should have told you are talking about Order confirmation email, that would have saved us some time spent on searching for issue that is does not exist.

If you don't want to attach any invoice to the customer (even if you have invoice checked in status messages), you should disable invoices.

And if invoices are enabled, it's only attached to "order confirmation" email if it was generated, this means, if paid with bankwire, and status is "awaiting payment" then no invoice is attached to "order confirmation" email as invoice was not yet generated.

This issue is not an issue, its designed to be so.

So, if invoices disabled, there is no invoice attached to confirmation email.

However invoices do get generated once status is changed, and attaching invoices are then operated with the checkbox under status messages.

Case closed.

Edited March 18, 2020 by toplakd

[Chandra]

Thanks all.

3 hours ago, datakick said:

...you are mixing two things.

The settings on Order Statuses has impact on 'Order Status Changed' email only (ie. Awaiting bank wire email)

What you are talking about is the 'Order confirmation' email, and that has nothing to do with order status. This email is always sent, and it contains all current information available.

I guess we could change the current semantics, and attach invoice to 'Order confirmation' email only if current order status has the send invoice checkbox enabled. However, I'm sure there would be plenty of merchants that would not want this behaviour by default.

In my head, a new order meant an order status change from nothing to one of Processing/Payment accepted/Awaiting check payment etc. Hence I had thought the pdf checkbox would be respected.

My view is that it should be an option to be respected, even if it defaults to yes since you reckon most merchants want it that way.

To me, having the same content in the email body and the pdf is no additional value to the customer. I note the pdf delays order process by a few seconds (you will remember I was trying separately to make it faster by queuing up emails even).

 

1 hour ago, toplakd said:

Yes, the pic Chandra added to github, was showing Order Processing status, so we all checked if issue was in statuses.

You should have told you are talking about Order confirmation email, that would have saved us some time spent on searching for issue that is does not exist.

If you don't want to attach any invoice to the customer (even if you have invoice checked in status messages), you should disable invoices.

And if invoices are enabled, it's only attached to "order confirmation" email if it was generated, this means, if paid with bankwire, and status is "awaiting payment" then no invoice is attached to "order confirmation" email as invoice was not yet generated.

This issue is not an issue, its designed to be so.

So, if invoices disabled, there is no invoice attached to confirmation email.

However invoices do get generated once status is changed, and attaching invoices are then operated with the checkbox under status messages.

Case closed.

Sorry you had to take the trouble. Please see my explanation above on why I thought/think it should be related to the status.

Disabling invoice for a status does not let the customer download it. I only think it makes sense to have an option to not to email it by status when the same info is in the email body.

If it is the design, I request we review it.

[toplakd]

Than you can set "payment accepted" status to not attach the invoice to the order confirmation for orders which are automatically marked as payment accepted.

The best thing you can do, is to setup up spare testing shop environment, where you can change things regarding orders, than placing orders by yourself and checking what the change brought you.

 

Edited March 18, 2020 by toplakd

[Chandra]

1 hour ago, toplakd said:

Than you can set "payment accepted" status to not attach the invoice to the order confirmation for orders which are automatically marked as payment accepted.

It is already set that way. Let me explain.

1. Customer creates cart.
2. Customer chooses either COD or Online payment module and confirms order.
3. If COD -> Order status is 'Processing in progress'.
4. If Online paid -> Order status is 'Payment Accepted'.
5. Both statuses have been set to not attach invoice to order  confirmation email.
6. However, both order confirmation emails have attached pdf.

Hope that clarifies.

[toplakd]

If order status (that is automatically set when order is placed) is also the status that first generates invoice.pdf, than this status will automatically also include pdf attachment.

However you can prevent that, by removing the second checkbox on that particular status. All other status changes afterwards, that are also generating invoice.pdf will not send .pdf if there is no checkbox by "attach invoice", but is one on "allow a customer to download"

You have to remove bottom checkbox on order status which is the first one shown on placed order.

[Chandra]

Well, I still need the customer to be able to login and view the invoices (in any order status, actually).

All I need switched off is the pdf in order confirmation.

 

[datakick]

7 minutes ago, Chandra said:

All I need switched off is the pdf in order confirmation

Well, there is no switch for this. You can only remove the invoice from 'Order status change' emails, not from the 'Order confirmation' email.

You can submit enhancement request for this on github.

[Chandra]

Will do, thanks.

Edit, here it is: https://github.com/thirtybees/thirtybees/issues/1156

Edited March 18, 2020 by Chandra

[toplakd]

@Chandra You have 2 options.
1.You rearange your statuses that once order is made it gets "Payment accepted/Awaiting payment" status, with no generation of the invoice at that time.

And once you mark order as processing, than invoice is generated and one can download it.

2. Make the override for for the controller that is sending the Order confirmation mail.