firegento / firegento-pdf

Generates nicer and configurable PDF for invoices, creditmemos and shippings in Magento
100 stars 62 forks source link

Enable invoice comments only for certain methods #350

Closed peterukena closed 7 years ago

peterukena commented 7 years ago

We had an issue with the invoice comments. We needed them to be able to create B2B invoices. This was bound to special payment method codes. I changed the "show comments" setting to respect only certain payment methods. This was done more or less in a hurry, so please have a look :)

sprankhub commented 7 years ago

Thanks for your contribution!

To be honest, I am not sure if we should merge it. This seems rather specific/special and does not look like something many people need. What do you think @Schrank?

Schrank commented 7 years ago

Thank you very much @kortwotze

I think the same as @sprankhub. I think it is a special case. But I would like to link the PR (and this issue) in the FAQ. If someone else needs it, maybe we include it. But I never heard of this issue.

Or someone™ implements it as an extension and link it.

TODO:

sprankhub commented 7 years ago

I just updated the FAQs accordingly.

peterukena commented 7 years ago

Hey, thanks for handling my pull request - just to elaborate about this a bit more. We are using invoice/creditmemo and shipment comments for our automatic order handling. So most of these entities have comments like "Created automatically by #externalthing @ #datetime" which we obviously do not want on our PDFs. ;) Since we introduced B2B orders, we have a special payment that we use that should have comments on the incoice PDF, since they are created manually and contain information for the B2B partner.

I agree on it beeing a not so general case, but I still think that making this setting more granular is a good thing to do.

And yes, I tried using the "visible_on_front" or "Add comments" function, but as far as I can see there are no proper getters and setters for that.

You guys have any idea to make this a thing?

sprankhub commented 7 years ago

I still think this case is too special to include it in the extension. The issue is that now, the comments are shown for every payment method. If we merge your changes, one would have to change the setting, so that the comments are still shown. Additionally, one would need to change this setting for each new payment method, which is installed.

A more compatible approach in my opinion would be to leave the show_comments as is and add an additional config option hide_comments_for_methods, which is only shown if show_comments is true. But still, I would advise against adding even more options...

I think you can implement a nice and clean implementation for your shop. If you need any more event or something, just tell us.

peterukena commented 7 years ago

The hide_comments_for_methods approach seems a better idea, even for our shop. That is a nice idea - if I need another event or something else, I will scream here :)

Thanks a lot :)