Bug 5920 - Remove HTML from downloaded reports in CSV format
Summary: Remove HTML from downloaded reports in CSV format
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Kyle M Hall
URL:
Keywords: release-notes-needed
Depends on:
Blocks:
 
Reported: 2011-03-21 18:15 UTC by Nicole C. Engard
Modified: 2024-04-30 12:33 UTC (History)
11 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:
24.05.00


Attachments
Bug 5920: Strip HTML from report exports (3.38 KB, patch)
2024-02-01 06:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 5920: Add HTML::Restrict to cpanfile (753 bytes, patch)
2024-02-01 06:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 5920: Add HTML::Restrict to cpanfile (806 bytes, patch)
2024-02-09 16:21 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 5920: Strip HTML from report exports (3.43 KB, patch)
2024-02-09 16:22 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 5920: Add HTML::Restrict to cpanfile (806 bytes, patch)
2024-02-09 16:22 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 5920: Strip HTML from report exports (3.47 KB, patch)
2024-04-26 18:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 5920: Add HTML::Restrict to cpanfile (864 bytes, patch)
2024-04-26 18:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 5920: Tidy guided_reports.pl (65.44 KB, patch)
2024-04-26 18:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 5920: Strip HTML from report exports (3.47 KB, patch)
2024-04-30 09:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 5920: Add HTML::Restrict to cpanfile (863 bytes, patch)
2024-04-30 09:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 5920: (QA follow-up) Switch to use existing dependancy (4.89 KB, patch)
2024-04-30 09:02 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2011-03-21 18:15:28 UTC
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.
Comment 1 Owen Leonard 2013-08-05 20:05:22 UTC
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            |
--------------------------------------------
Comment 2 Nicole C. Engard 2013-08-06 13:57:54 UTC
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).
Comment 3 Katrin Fischer 2019-05-04 11:54:42 UTC
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.
Comment 4 George Williams (NEKLS) 2021-09-07 22:48:36 UTC
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.
Comment 5 Andrew Fuerste-Henry 2021-10-01 13:35:25 UTC
+1 on this, it'd be a really handy option to have
Comment 6 Rebecca Coert 2023-08-08 15:29:40 UTC
+1 - this would be a useful feature!
Comment 7 Mirjam Vantieghem 2024-01-30 10:12:26 UTC
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.
Comment 8 Mirjam Vantieghem 2024-01-30 12:27:09 UTC
Addition: The patch from bug #5697 has solved the issue for us, as we were not using HTML for another purpose in these reports.
Comment 9 Martin Renvoize 2024-02-01 06:31:07 UTC
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>
Comment 10 Martin Renvoize 2024-02-01 06:31:11 UTC
Created attachment 161710 [details] [review]
Bug 5920: Add HTML::Restrict to cpanfile

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2024-02-01 06:32:09 UTC
Attached patches from Mirjam's branch.. I wasn't aware of this bug prior to now, otherwise I would have submitted this up earlier.
Comment 12 Martin Renvoize 2024-02-01 06:33:14 UTC
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.
Comment 13 Andrew Fuerste-Henry 2024-02-01 14:56:19 UTC
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
Comment 14 Martin Renvoize 2024-02-01 20:46:06 UTC
Sandboxes don't pull new dependencies from the cpanfile I'm afraid, so this one can't easily be tested there :-(.
Comment 15 Andrew Fuerste-Henry 2024-02-01 20:47:59 UTC
(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!
Comment 16 Andrew Fuerste-Henry 2024-02-02 16:06:00 UTC
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>"
Comment 17 Andrew Fuerste-Henry 2024-02-02 16:24:06 UTC
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.
Comment 18 Andrew Fuerste-Henry 2024-02-02 16:25:35 UTC
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?
Comment 19 George Williams (NEKLS) 2024-02-02 16:34:15 UTC
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.
Comment 20 Andrew Fuerste-Henry 2024-02-02 16:40:17 UTC
(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.
Comment 21 George Williams (NEKLS) 2024-02-02 19:43:56 UTC
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.
Comment 22 Lucas Gass 2024-02-09 16:21:15 UTC
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>
Comment 23 Lucas Gass 2024-02-09 16:22:41 UTC
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>
Comment 24 Lucas Gass 2024-02-09 16:22:43 UTC
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>
Comment 25 Kyle M Hall 2024-04-26 18:30:45 UTC
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>
Comment 26 Kyle M Hall 2024-04-26 18:30:52 UTC
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>
Comment 27 Kyle M Hall 2024-04-26 18:30:55 UTC
Created attachment 165691 [details] [review]
Bug 5920: Tidy guided_reports.pl

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 28 Kyle M Hall 2024-04-26 18:32:27 UTC
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.
Comment 29 Martin Renvoize 2024-04-30 08:51:04 UTC
Good call on HTML::Scrubber.. I jsut rebased the existing patches and modernised it a bit, but I should have spotted the overlap.
Comment 30 Martin Renvoize 2024-04-30 09:02:25 UTC
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>
Comment 31 Martin Renvoize 2024-04-30 09:02:28 UTC
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>
Comment 32 Martin Renvoize 2024-04-30 09:02:31 UTC
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>
Comment 33 Katrin Fischer 2024-04-30 11:50:09 UTC
Glad we could avoid a new dependency here!
Comment 34 Katrin Fischer 2024-04-30 12:33:02 UTC
Pushed for 24.05!

Well done everyone, thank you!