financial#58: Don't crash the View Batches page after a failed batch …

[MegaphoneJon]

…export

Overview

When a batch fails to export, the View Batches page crashes.

Before

Datatables error.

After

No error.

Comments

I'm just wrapping three lines in an if statement. I'm also removing the "Download" link if the file doesn't exist.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@MegaphoneJon why would the batch not exist - do people need to be otherwise alerted?

Ping @pradpnayak @monishdeb

[monishdeb]

I agree with @eileenmcnaughton, from core perspective we must make the code right to prevent such issue. @MegaphoneJon is this something extension specific or something to do with core?

[MegaphoneJon]

In my case, a max_execution_time related timeout caused the batch to fail. I previously saw this issue with an extension that failed to generate the file to be downloaded (I believe it was a bug but I could also see this happening on a permissions issue).

While I agree that batch export shouldn't fail - when it does, it should fail gracefully. A failed export breaking batch view until someone edits the db feels a bit harsh. I would also be open to other approaches, such as a "Failed" status for bad exports.

[MegaphoneJon]

See this Stack Exchange discussion. The issue here is that any problem with the "Batch Export" activity causes the "View Batches" datatable to crash. In this case, the user had deleted a single "Batch Export" activity, which broke his ability to view ANY exported batches.

[eileenmcnaughton]

This seems safe enough & although obscure @MegaphoneJon obviously does see it hit -merging

[don-alejandro-z]

This patch also fixes a problem I encountered a couple months ago while creating a highly customized financial batch export to match a clients current accounting system.

I used hooks to modify the batch query and created a .csv export from the results, but if I started the download and exited, then I'd get the data-tables warning. I didn't know it at the time, but this was apparently because the export activity is not being created till after the point where the result hook fires. I had to save my custom export, setup a way for my client to download the custom export, and tell my client to ignore the .csv file exported by CiviCRM, otherwise the exported batch list would fail.

With this patch installed I can start the download and exit, so my client doesn't have to ignore the CiviCRM export (or go looking for the custom export)...