New Model: Spree::BookkeepingDocuments

[mamhoff]

This has turned into this massive exercise of trying to make spree_print_invoice more flexible by allowing it to print not only invoices for Orders, but also for other models like Reimbursements. The architecture really allows for any object to be made printable - a notable candidate would be packaging slips for Spree::Shipments.

At the moment, the views and templates for other things than order-invoices and order-packagingslips are not provided as they're not fully tested. They'll come at a later stage. OK? :)

[tvdeyen]

Unfortunately Travis realized there's been a change :)

Could you please look into the error and push again?

Thanks

[tvdeyen]

Besides that 👍

[futhr]

+1

[tvdeyen]

@mamhoff I just saw, that you added the switch to prawn-rails into this PR as well. I think it would be better to separate this from the rest. What do you think?

[mamhoff]

@tvdeyen No - I need prawn-rails for refactoring the views, and without that, the new model doesn't really have that much of a point (I need invoices for different kinds of things, and for that I need clean, maintainable views). So I'd rather make a PR that's actually a complete feature.

[mamhoff]

This has turned into a massive exercise. The API now definitely has changed, and the behaviour of the Gem as well. I've been debating with myself (and with @tvdeyen) whether this still makes sense as a PR, or whether it actually constitutes a fork and thus should be named differently.

What has changed in a Nutshell?

• A Spree::Invoice is a new model that gets generated after completing orders or creating reimbursements.
• A Spree::Pdf is a new model that leverages view objects to make PDFs of arbitrary Spree objects (Order, Reimbursement, Shipment, you name it).
• Use of the prawn-rails gem to support breaking the PDF templates into partials
• Remove the template option in the backend, as that's something that's more than covered by the flexibility of the PDF object.
• A PDF index view that shows all PDFs in the system (and will subsequently be searchable/sortable via Ransack)

@alepore, @futhr, @lucaferri what do you think?

[futhr]

@mamhoff :+1: Everything has to move on and adapt to production active use cases (like the speech dhh made about Rails 5 at RailsConf Atlanta), the only important part when it comes to new features and refactor is to make it migratable.

[alepore]

@mamhoff very good stuff, thanks for this. i see this is now more than just "spree print invoice" , maybe some code could become a separate "spree pdf" gem?

[lucaferri]

I agree with @alepore because when I read

A Spree::Pdf is a new model that leverages view objects to make PDFs of arbitrary Spree objects (Order, Reimbursement, Shipment, you name it).

I thought that it's not a _spree_printinvoice gem anymore but it's evolving to something else, something more generic.

[futhr]

Well it still does the same thing as before just better and more dynamic. spree_print_invoice has always been a confusing name because it evolved from just handle invoices years ago. Continue on a cloned superseded repo as spree_pdf could ofc make it more declarative.

[mamhoff]

I'll push more yak-shaving tomorrow (so good!), which will do the following:

• Remove the Invoice model (invalidating the name of the PR), because the other model (see below) now also holds a number, and having two interlinked polymorphic relations (invoice -> order, pdf -> order) turned out to be really, really cumbersome.
• Rename Spree::Pdf to Spree::BookkeepingDocument which will actually have columns for customer name, email, document number, and amount. The columns have to be there for the index view table to be searchable and sortable ("Just give me invoice #INV43212 real quick will ya?").
• Remove the Views for Reimbursements and Shipments (sadly, they're not done yet, and I need feedback from our client as to how they need them implemented).

I'm also providing - albeit deprecated - methods on Spree::Order that will mimic class spree_print_invoice behaviour, so the upgrade path is as painless as possible.

@futhr absolutely, same thing as before, just more dynamic (thank you for the "better"). Also, the whole thing clearly owes a lot of history to spree_print_invoice. I would not mind keeping the name, but I would find spree_bookkeeping_documents quite declarative (albeit long)

[tvdeyen]

:+1: * :100: from me :cake:

[mamhoff]

@futhr ready to merge.

[futhr]

@mamhoff Interesting that Travis CI returns green specs even if I clear Travis cache, I try to run this locally and get dependencies issues with sass_rails that currently use ~> 4.0.0 https://github.com/spree-contrib/spree_print_invoice/blob/master/spree-print-invoice.gemspec#L36 while Spree use ~> 5.0.0 https://github.com/spree/spree/blob/5c6d69dfed60722b89f5d42d63707ad00e3db1c2/common_spree_dependencies.rb#L7 resolving the dependency issues gives 2-5 failed specs that occur randomly. The most common failures are:

1) Admin print invoice feature with Config.store_pdf set to true with pdf file not yet present sends the stored file.
     Failure/Error: visit spree.admin_bookkeeping_document_path(id: order.invoice.id, format: :pdf)
     ActionView::Template::Error:
       no implicit conversion of Sprockets::Asset into String

2) Admin print invoice feature with Config.store_pdf set to false sends rendered pdf.
     Failure/Error: visit spree.admin_bookkeeping_document_path(id: order.invoice.id, format: :pdf)
     ActionView::Template::Error:
       no implicit conversion of Sprockets::Asset into String

[mamhoff]

Nice find - I should've run bundle update first. Turns out the preferences cache is not cleared between tests, which is why you were having spurious tests. And: Whenever the logo was valid, the interface for Sprockets::Asset seems to have changed.

[mamhoff]

I increased the scale on the total column to allow for values > 999,99 and rebased.

[futhr]

@mamhoff Merged to master branch, nice work.

[tvdeyen]

🍻

[catalin88]

So, where can I find an example with a Pdf generated when a Reimbursement is issued ? Seems like the current version of the gem doesn't provide this, yet, in this discussion, it's said numerous times it does produce Pdf for Reimbursements