Bug 27510 - Download the report in HTML format
Summary: Download the report in HTML format
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Ivan Dziuba
QA Contact: Kyle M Hall
URL:
Keywords: dependency
Depends on:
Blocks:
 
Reported: 2021-01-21 14:48 UTC by Ivan Dziuba
Modified: 2023-11-30 14:58 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 27510: Download the report in HTML format (4.77 KB, patch)
2021-01-22 13:40 UTC, Ivan Dziuba
Details | Diff | Splinter Review
Bug 27510: Download the report in HTML format (6.28 KB, patch)
2021-01-22 15:58 UTC, Ivan Dziuba
Details | Diff | Splinter Review
Bug 27510: Download the report in HTML format (7.02 KB, patch)
2021-01-29 20:09 UTC, Ivan Dziuba
Details | Diff | Splinter Review
Bug 27510: Download the report in HTML format (6.95 KB, patch)
2021-01-30 17:04 UTC, Ivan Dziuba
Details | Diff | Splinter Review
Bug 27510: Download the report in HTML format. (6.90 KB, text/plain)
2021-02-12 20:26 UTC, Ivan Dziuba
Details
Bug 27510: Download the report in HTML format (7.26 KB, patch)
2021-02-12 20:58 UTC, Ivan Dziuba
Details | Diff | Splinter Review
Bug 27510: Download the report in HTML format (9.50 KB, patch)
2023-02-03 14:01 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 27510: Followup (2.47 KB, patch)
2023-02-03 14:12 UTC, Magnus Enger
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Ivan Dziuba 2021-01-21 14:48:27 UTC

    
Comment 1 Ivan Dziuba 2021-01-22 13:40:43 UTC Comment hidden (obsolete)
Comment 2 Ivan Dziuba 2021-01-22 13:41:35 UTC
TEST PLAN:
1. Apply this patch
2. Update database ./updatedatabase.pl
Now we have new preference 'ReportUserCSS' where we can add style for our HTML report.
3. Go on the page - <intranet>/reports/guided_reports.pl
4. Download -> Document HTML
Comment 3 Owen Leonard 2021-01-22 14:57:32 UTC
I don't see any changes to the template in this patch. Shouldn't there be a new menu item under the "Download" button?
Comment 4 Ivan Dziuba 2021-01-22 15:58:36 UTC Comment hidden (obsolete)
Comment 5 Ivan Dziuba 2021-01-22 15:59:22 UTC
(In reply to Owen Leonard from comment #3)
> I don't see any changes to the template in this patch. Shouldn't there be a
> new menu item under the "Download" button?

Added! Thanks.
Comment 6 Sally 2021-01-25 10:38:03 UTC
Really like the idea of this.  I've applied the patch on a sandbox and restarted services, but I get an error.

Steps 1-2 are fine, and ReportUserCSS shows in the system preferences.

When attempting step 3, the failure below shows on all of the guided_reports.pl pages:

Could not compile /kohadevbox/koha/reports/guided_reports.pl: Can't locate HTML/Table.pm in @INC (you may need to install the HTML::Table module) (@INC contains: /kohadevbox/koha /kohadevbox/koha/installer /kohadevbox/koha/lib/installer /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.24.1 /usr/local/share/perl/5.24.1 /usr/lib/x86_64-linux-gnu/perl5/5.24 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.24 /usr/share/perl/5.24 /usr/local/lib/site_perl /usr/lib/x86_64-linux-gnu/perl-base /var/lib/koha/friday/plugins) at /kohadevbox/koha/reports/guided_reports.pl line 42.
BEGIN failed--compilation aborted at /kohadevbox/koha/reports/guided_reports.pl line 42.
 at /usr/share/perl5/CGI/Compile.pm line 132

If it's a patch that can't be applied to a sandbox, revert the status and I'll let someone with a devbox test.  :)
Comment 7 Ivan Dziuba 2021-01-25 13:44:35 UTC
(In reply to Sally from comment #6)
> Really like the idea of this.  I've applied the patch on a sandbox and
> restarted services, but I get an error.
> 
> Steps 1-2 are fine, and ReportUserCSS shows in the system preferences.
> 
> When attempting step 3, the failure below shows on all of the
> guided_reports.pl pages:
> 
> Could not compile /kohadevbox/koha/reports/guided_reports.pl: Can't locate
> HTML/Table.pm in @INC (you may need to install the HTML::Table module) (@INC
> contains: /kohadevbox/koha /kohadevbox/koha/installer
> /kohadevbox/koha/lib/installer /etc/perl
> /usr/local/lib/x86_64-linux-gnu/perl/5.24.1 /usr/local/share/perl/5.24.1
> /usr/lib/x86_64-linux-gnu/perl5/5.24 /usr/share/perl5
> /usr/lib/x86_64-linux-gnu/perl/5.24 /usr/share/perl/5.24
> /usr/local/lib/site_perl /usr/lib/x86_64-linux-gnu/perl-base
> /var/lib/koha/friday/plugins) at /kohadevbox/koha/reports/guided_reports.pl
> line 42.
> BEGIN failed--compilation aborted at
> /kohadevbox/koha/reports/guided_reports.pl line 42.
>  at /usr/share/perl5/CGI/Compile.pm line 132
> 
> If it's a patch that can't be applied to a sandbox, revert the status and
> I'll let someone with a devbox test.  :)


You need to install the HTML::Table module. 
Thanks!
Comment 8 Owen Leonard 2021-01-26 14:27:54 UTC
I think this looks good. I think the label for the menu item should say "HTML file" instead of "Document HTML."

My only other concern is that the generated file isn't a complete HTML document... No <html>, no <body>, etc.

Does that matter? The file opens fine in a browser.
Comment 9 Ivan Dziuba 2021-01-26 19:49:20 UTC
(In reply to Owen Leonard from comment #8)
> I think this looks good. I think the label for the menu item should say
> "HTML file" instead of "Document HTML."
> 
> My only other concern is that the generated file isn't a complete HTML
> document... No <html>, no <body>, etc.
> 
> Does that matter? The file opens fine in a browser.

If that matter I can add all tags.
Thanks!
Comment 10 Katrin Fischer 2021-01-26 19:58:25 UTC
I think it would be good if the file validated :)
Comment 11 Ivan Dziuba 2021-01-29 20:09:56 UTC Comment hidden (obsolete)
Comment 12 Ivan Dziuba 2021-01-30 17:04:47 UTC Comment hidden (obsolete)
Comment 13 Ivan Dziuba 2021-02-12 20:26:52 UTC Comment hidden (obsolete)
Comment 14 Ivan Dziuba 2021-02-12 20:58:01 UTC Comment hidden (obsolete)
Comment 15 Ivan Dziuba 2021-03-08 23:42:13 UTC
You need install the module HTML::Table
Comment 16 Katrin Fischer 2021-03-09 11:23:20 UTC
(In reply to Ivan Dziuba from comment #15)
> You need install the module HTML::Table

Hi Ivan, I've added he dependency keyword - have you checked that this is available as package on Debian?
Comment 17 Katrin Fischer 2021-03-09 11:24:11 UTC
Please also add a test plan to the commit message.
Comment 18 Katrin Fischer 2022-02-22 17:18:37 UTC
I still quite like the idea of this. I think it would be helpful for staff users and others that have no access to the Reports module, but could profit from the links in an HTML export file.
Comment 19 Magnus Enger 2023-02-03 08:42:46 UTC
It looks like HTML::Table is packaged, at least in Debian 9.13:

$ sudo apt-cache policy libhtml-table-perl
libhtml-table-perl:
  Installert: (ingen)
  Kandidat:   2.08a-3
  Versjonstabell:
     2.08a-3 500
        500 http://mirrors.linode.com/debian stretch/main amd64 Packages

So what is needed here? 

- A test plan in the commit message
- To add libhtml-table-perl to Koha's dpendencies

Should adding the dep be part of the patch, or is that something the RM adds?
Comment 20 Magnus Enger 2023-02-03 14:01:15 UTC
Created attachment 146135 [details] [review]
Bug 27510: Download the report in HTML format

TEST PLAN:
1. Apply this patch
2. Update database ./updatedatabase.pl
Now we have new preference 'ReportUserCSS' where we can add style for our HTML report.
3. Go on the page - <intranet>/reports/guided_reports.pl
4. Download -> Document HTML

Signed-off-by: Magnus Enger <magnus@libriotech.no>
I have fixed some conflicts and put in the test plan that was on
Bugzilla. Feature works as advertised. HTML file can be downloaded
and is a full HTML document. Formatting can be done with the syspref.
Links generated in SQL carry over to the document (but links need to
be absolute to work, of course.)
Comment 21 Magnus Enger 2023-02-03 14:12:08 UTC
Created attachment 146142 [details] [review]
Bug 27510: Followup

- Change "Document HTML" to "HTML file" as requested by Owen
- Add HTML::Table to cpanfile
Comment 22 Magnus Enger 2023-02-03 14:15:43 UTC
Not sure if my followup should be one or two patches? I am also taking a chance on leaving this at "signed off". Please reset to "needs signoff" if that is thought necessary.
Comment 23 Kyle M Hall 2023-03-03 18:19:50 UTC
Shouldn't this just use a Template Toolkit based template to generate the output instead of HTML::Table and a bunch of html embedded in a perl script?
Comment 24 Kyle M Hall 2023-03-24 16:47:13 UTC
Shouldn't this just use a Template Toolkit based template to generate the output instead of HTML::Table and a bunch of html embedded in a perl script?
Comment 25 Magnus Enger 2023-03-27 08:07:27 UTC
(In reply to Kyle M Hall from comment #24)
> Shouldn't this just use a Template Toolkit based template to generate the
> output instead of HTML::Table and a bunch of html embedded in a perl script?

Yes, that does sound like a sensible idea. 

Ivan, could you find the time to redo the patch with a template for generating the table?
Comment 26 Magnus Enger 2023-06-06 07:36:45 UTC
Just a note that if HTML::Table is added to cpanfile, HTML::Table 2.08a will break the About page.
Comment 27 Martin Renvoize 2023-11-21 09:41:24 UTC
(In reply to Kyle M Hall from comment #24)
> Shouldn't this just use a Template Toolkit based template to generate the
> output instead of HTML::Table and a bunch of html embedded in a perl script?

Did I imagine you did this somewhere already Kyle?  I'm struggling to find the bug if you did.. if so, we could close this as a duplicate
Comment 28 Kyle M Hall 2023-11-30 14:58:41 UTC
(In reply to Martin Renvoize from comment #27)
> (In reply to Kyle M Hall from comment #24)
> > Shouldn't this just use a Template Toolkit based template to generate the
> > output instead of HTML::Table and a bunch of html embedded in a perl script?
> 
> Did I imagine you did this somewhere already Kyle?  I'm struggling to find
> the bug if you did.. if so, we could close this as a duplicate

Bug 34136 could definitely be used as an alternative to this though this patchset is for guided reports and mine is for SQL reports.