Problem / Motivation

When a PDF file is generated for an invoice, we should store a reference to it instead of trying to figure out whether it exists via a query.

The patch from #3106014: Allow altering the invoice PDF filename via event subscribers is adding a field, but we're missing getters / setters as well as tests.

Also, the "challenging" part is that the field wasn't present before, which means if we're always trying get the file from the reference field, it won't work for existing invoices.

I'm not sure about the naming for that new field though, the patch from #3106014: Allow altering the invoice PDF filename via event subscribers went with "invoice_file".

To sum up, we need:

  • A file reference field, translatable
  • Getters/setters for that field (probably getFile(), setFile())
  • Update savePrintable() to call a getFile() method that returns the file reference field if not empty, and fallback to the logic that we have today (In case a file is found, we need to set the reference.

Open questions:

  • Should we delete the reference to the file on Invoice::postDelete()? Or do we always want to keep the files?

Comments

jsacksick created an issue. See original summary.

jsacksick’s picture

Issue summary: View changes
jsacksick’s picture

Issue summary: View changes
jsacksick’s picture

Issue summary: View changes
jsacksick’s picture

Status: Active » Needs review
StatusFileSize
new18.46 KB

Ideally, we'd write a post update function to backfill the file reference, but what I did instead is that I populate the reference when the download url is accessed.

jsacksick’s picture

StatusFileSize
new18.83 KB

Same patch, with tweaked field descriptions.

jsacksick’s picture

jsacksick’s picture

StatusFileSize
new22.38 KB
new3.82 KB

I'm expanding the tests to ensure the file reference field is populated when downloading the file, and I fixed an issue with the logic in the invoice controller.

  • jsacksick committed db12c23 on 8.x-2.x
    Issue #3129042 by jsacksick, archnode: Store a reference to the...
jsacksick’s picture

Status: Needs review » Fixed

I went ahead and committed a slightly different patch with coding standard fixes (Removing unused use statements mainly, due to the tests refactoring).

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.