Created attachment 99574 [details] [review]
Bug 24724: Add plugin hook for new SQL report export formats

This adds plugin hooks for adding new export formats for
SQL reports.

The plugin should implement two new methods:
- sql_report_get_export_formats
    Takes no parameters.
    Should return an array ref of hashes, with the following keys:
      shortname => 'dump',
      name => 'Name of this format',
      type => 'text/plain'
- sql_report_process_data
    Takes 3 parameters: format, headers, and data.
    - format is string the shortname from one of the hashes in
      sql_report_get_export_formats.
    - headers is array ref of strings, column names of the data.
    - data is the data as returned by perl DBI fetchall_arrayref()

Test plan:
1) Enable plugins, and install the example plugin for the bug
2) Create a new SQL report, eg. "SELECT * FROM BORROWERS LIMIT 30"
3) Run the report - you should see two new entries in the "Download"
   button: "Perl data dump (.dump)" and "HTML table (.htm)"
4) Download the new formats, and inspect the file contents that
   they look correct
5) Test the other entries in the download-button and that they work

Created attachment 99575 [details]
Plugin to test Bug 24724

I propose you to depend on Bug 20415 that removes UseKohaPlugins system preference

Ah really nice hook.

I think it could be useful for many other exports (checkouts table, ...).

Use of those plugins will only be implemented only for sql reports, but can be implemented somewhere else in a second time.

So I propose you some changes :

1)
Only one format per plugin. 2 formats = create 2 plugins ;)
Will make code more clear I bet.

2)
Change methods :
    sub sql_report_get_export_format
    sub sql_report_process_data
to :
    export_format
    process_export

3)
Better add a method to Koha/Template/Plugin/KohaPlugins.pm to fetch the plugins you need.

PS : you need to remove call to UseKohaPlugins system preference

Let me know if you need help.

Best regards,

Maybe we should keep sql in the hook name - I could imagine other spots it could be different data formats to export/convert?

(In reply to Katrin Fischer from comment #5)
> 
> Maybe we should keep sql in the hook name - I could imagine other spots it
> could be different data formats to export/convert?

Ah yes I see, method should indicate where the plugin impacts.

I suggest we keep "report" instead of "sql" :
report_export_format
report_process_export

So a another place, ie checkouts, with exports will add hooks :
checkouts_export_format
checkouts_process_export

Since a plugin can handle several hooks its fine :D