Right now if you want to have HTML in your report you can do so - usually this is used to turn bib numbers into links or titles into links - etc. It would be great if the export to CSV function stripped out the html tags before exporting so that you could have a report that printed to the screen with links, but when downloaded just showed you the necessary data without any extraneous tags.
You would probably have to fully re-build the SQL for output as CSV. If you simply stripped out the concatenated text you would potentially be outputting two or more fields crammed together in one column: SELECT biblionumber, CONCAT('<a href=\"/cgi-bin/koha/catalogue/detail.pl?biblionumber=',biblionumber,'\">',title,'</a>') AS Title -------------------------------------------- | biblionumber | Title | -------------------------------------------- | 29839 | 29839The stand | --------------------------------------------
This is how we do it now - but that means 2 reports - one for viewing and clicking and one for exporting. The library wanted way to use the same report for both (clicking and exporting).
I wonder if there is a library or method somewhere that could just scrub the HTML tags from the results. I also wonder if this should be optional, in some cases keeping the HTML might be useful, depending on how you want to use the exported data.
I would want to be able to decide if the HTML was scrubbed or not. Sometimes I want the HTML output in the csv I download.
+1 on this, it'd be a really handy option to have
+1 - this would be a useful feature!
We have been using our own patch to strip HTML when downloading a report. It would be great if this can become part of mainstream Koha as an optional feature.
Addition: The patch from bug #5697 has solved the issue for us, as we were not using HTML for another purpose in these reports.
Created attachment 161709 [details] [review] Bug 5920: Strip HTML from report exports This patch uses HTML::Restrict to strip out HTML tags from the CSV download of reports. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 161710 [details] [review] Bug 5920: Add HTML::Restrict to cpanfile Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Attached patches from Mirjam's branch.. I wasn't aware of this bug prior to now, otherwise I would have submitted this up earlier.
Whilst I agree, the core reason for this has an alternative now, I could see other use cases for HTML in report output that may not be catered for in other ways yet, so I think this is still a worthwhile patch.
With this patch on ByWater's sandbox site I get the following error when I attempt to create a report via SQL: Could not compile /kohadevbox/koha/reports/guided_reports.pl: Can't locate HTML/Restrict.pm in @INC (you may need to install the HTML::Restrict module) (@INC contains: /kohadevbox/koha /kohadevbox/koha/lib /kohadevbox/koha/installer /kohadevbox/koha/lib/installer /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.32.1 /usr/local/share/perl/5.32.1 /usr/lib/x86_64-linux-gnu/perl5/5.32 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl-base /usr/lib/x86_64-linux-gnu/perl/5.32 /usr/share/perl/5.32 /usr/local/lib/site_perl /var/lib/koha/html/plugins) at /kohadevbox/koha/reports/guided_reports.pl line 43. BEGIN failed--compilation aborted at /kohadevbox/koha/reports/guided_reports.pl line 43. at /usr/share/perl5/CGI/Compile.pm line 144
Sandboxes don't pull new dependencies from the cpanfile I'm afraid, so this one can't easily be tested there :-(.
(In reply to Martin Renvoize from comment #14) > Sandboxes don't pull new dependencies from the cpanfile I'm afraid, so this > one can't easily be tested there :-(. Ahha! I'll try to get to it on my testing docker. Thanks!
Martin, can you please provide a test plan? What I've tried doesn't seem to work, but perhaps I'm misunderstanding your intent. Here's what I did: - apply patch - cpan HTML:Restrict - restart_all - create a new report from SQL: "select concat('<h1>','Hello world','</h1>') as message" - run report - click Download, select CSV format - open csv in text editor I expected my CSV to contain "Hello world" without the <hi> tags, but I got: message "<h1>Hello world</h1>" I also tried: SELECT biblionumber, CONCAT('<a href=\"/cgi-bin/koha/catalogue/detail.pl?biblionumber=',biblionumber,'\">',title,'</a>') AS Title from biblio and ended up with a csv containing: biblionumber,Title 12484,"<a href=""/cgi-bin/koha/catalogue/detail.pl?biblionumber=12484"">The mask</a>"
Clearly I've doing this too early in the morning. I applied your patches to my testing docker and then ran my reports on the wrong Koha. When I follow my test plan correctly, this works! However, for my test report: SELECT biblionumber, CONCAT('<a href=\"/cgi-bin/koha/catalogue/detail.pl?biblionumber=',biblionumber,'\">',title,'</a>') AS Title from biblio When I download CSV or ODS formats, the second column contains just the title. When I download TSV format, the second column contains just the biblionumber (so the report is two columns of the same data). It'd be best if all 3 options work the same way. But I'm happy to sign off on this and pursue that consistency in a new bug if desired; this is absolutely useful as-is.
It would also be great to have HTML-stipping available in reports run and emailed via the commandline. Martin, is that reasonably easy to add here, or would you rather I file a new bug?
I want to reiterate that whether or not the HTML is stripped from a report when it's downloaded should be optional. There are times that I actually want the raw HTML output in a downloaded report. Example: I have reports that a library can run and download that has HTML in the output so that the raw HTML can then be cut and pasted into the library's website. If this patch strips all of the HTML tags from -all- downloaded report output, then I'm going to have to re-write those reports to find a new way to create the raw HTML when I actually want the raw HTML output.
(In reply to George Williams (NEKLS) from comment #19) > I want to reiterate that whether or not the HTML is stripped from a report > when it's downloaded should be optional. Well-reiterated, thank you! Would you prefer a global option for this, or something more granular? As someone who probably never wants to export the HTML, I'd hate to add a to every report download in order to opt for stripping the HTML at that point.
How about switching the "Download" drop-down from a list of options to a checkbox drop-down that allows the user to specify the format and whether or not HTML is stripped from the output. That would be the most granular way of going about it.
Created attachment 161983 [details] [review] Bug 5920: Add HTML::Restrict to cpanfile Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 161984 [details] [review] Bug 5920: Strip HTML from report exports This patch uses HTML::Restrict to strip out HTML tags from the CSV download of reports. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 161985 [details] [review] Bug 5920: Add HTML::Restrict to cpanfile Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Created attachment 165689 [details] [review] Bug 5920: Strip HTML from report exports This patch uses HTML::Restrict to strip out HTML tags from the CSV download of reports. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 165690 [details] [review] Bug 5920: Add HTML::Restrict to cpanfile Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 165691 [details] [review] Bug 5920: Tidy guided_reports.pl Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Could we use HTML::Scrubber instead of HTML::Restrict here? It seems like it can accomplish the same thing and avoid adding another dependency. Otherwise consider this PQA.
Good call on HTML::Scrubber.. I jsut rebased the existing patches and modernised it a bit, but I should have spotted the overlap.
Created attachment 165832 [details] [review] Bug 5920: Strip HTML from report exports This patch uses HTML::Restrict to strip out HTML tags from the CSV download of reports. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 165833 [details] [review] Bug 5920: Add HTML::Restrict to cpanfile Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 165834 [details] [review] Bug 5920: (QA follow-up) Switch to use existing dependancy We already have a scrubber module we can use; This patch switches us from the newly introduced dependancy to using the existing and well respected Scrubber we already have. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Glad we could avoid a new dependency here!
Pushed for 24.05! Well done everyone, thank you!
Not backported to 23.11.x
This bug says in the release notes that HTML is removed when downloaded to CSV but the HTML is removed also when downloading to Tab separated text and Open document spreadsheet. Maybe those two should also be mentioned in documenting?