Bug 7646 - Printing w/o the SQL Code showing
Summary: Printing w/o the SQL Code showing
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Shane Sammons
QA Contact: Ian Walls
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-05 18:45 UTC by Shane Sammons
Modified: 2018-12-03 20:04 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Modified the template to have a JS link to change the print media class and launch a print dialog. (1.57 KB, patch)
2012-03-06 03:28 UTC, Shane Sammons
Details | Diff | Splinter Review
Added missing </style> (1.58 KB, patch)
2012-03-09 01:04 UTC, Shane Sammons
Details | Diff | Splinter Review
should be correct this time...I hope... (1.58 KB, patch)
2012-03-09 01:46 UTC, Shane Sammons
Details | Diff | Splinter Review
complete <style></style> tag this time.. (1.61 KB, patch)
2012-03-09 01:51 UTC, Shane Sammons
Details | Diff | Splinter Review
Bug 7646: Add "do not print SQL" link to reports (1.97 KB, patch)
2012-03-19 13:16 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 7646: Add "do not print SQL" link to reports (1.51 KB, patch)
2015-04-13 19:08 UTC, Marc Véron
Details | Diff | Splinter Review
Screenshot showing position of link after applying patch (60.23 KB, image/png)
2015-04-14 04:46 UTC, Marc Véron
Details
Bug 7646 - Printing w/o the SQL Code showing (1.39 KB, patch)
2015-04-15 21:26 UTC, Nick Clemens
Details | Diff | Splinter Review
[Signed-off] Bug 7646 - Printing w/o the SQL Code showing (1.60 KB, patch)
2015-04-16 08:33 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 7646 [QA Followup] - Tweak button to match other Koha print buttons (1.23 KB, patch)
2015-04-17 12:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
screenshot-applied-patch (19.57 KB, image/png)
2015-07-19 18:16 UTC, Mirko Tietgen
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Shane Sammons 2012-03-05 18:45:18 UTC
I have made modifications to the template file. It added a text link to remove the SQL statement from the print view. Thus saving the need to use CSV/TSV exports.

The print text will not show on the print out as is. I added a little .noprint class to the template file. Then I added my text with a print dialog and class change JS.

Please see: http://pastebin.com/LWR3BqiW for the changes specifically lines 22-26 & line 501
Comment 1 Shane Sammons 2012-03-06 03:28:02 UTC Comment hidden (obsolete)
Comment 2 Jared Camins-Esakov 2012-03-08 03:59:09 UTC
When I apply this patch and attempt to test it, the guided reports page (/cgi-bin/koha/reports/guided_reports.pl?phase=Use%20saved) has no content in the <body> tag.
Comment 3 Shane Sammons 2012-03-08 04:17:09 UTC
i see my mistake i will correct and re-submit. Thanks!
Comment 4 Shane Sammons 2012-03-09 01:04:01 UTC Comment hidden (obsolete)
Comment 5 Shane Sammons 2012-03-09 01:46:09 UTC Comment hidden (obsolete)
Comment 6 Shane Sammons 2012-03-09 01:51:18 UTC Comment hidden (obsolete)
Comment 7 Shane Sammons 2012-03-09 16:53:15 UTC
I tested it on the sandbox http://pro.test9.biblibre.com/ the link appeared on the top an SQL generated report, it launched my print box, and when I previewed the print out it did not have the SQL visable.
Comment 8 Ian Walls 2012-03-19 12:29:17 UTC
Dropping back to Needs Signoff, since signoff by someone other than the author is required.

This looks to be a pretty easy one to test, but I'm about to switch trains, so I'll leave it up to another community member.  Should pass QA quickly once signed off.
Comment 9 Jared Camins-Esakov 2012-03-19 13:16:19 UTC Comment hidden (obsolete)
Comment 10 Ian Walls 2012-03-19 13:20:46 UTC
Easy to read template only change.  Marking Passed QA
Comment 11 Paul Poulain 2012-03-20 11:10:00 UTC
QA comment:

All pages include a 
<link rel="stylesheet" type="text/css" media="print" href="/intranet-tmpl/prog/en/css/print.css" />
for printing stylesheet

that already has a .noprint class:

div#breadcrumbs,
div#toplevelnav,
div#login,
div#header_search,
div#toolbar,
div#changelanguage,
div#menu,
div.yui-b,
.noprint,
form#sortbyform,
#cartDetails,
fieldset.action,
.list-actions,
.print {
    display: none;
}

So this code:
+<style type="text/css" media="print">
+/* ### FIX ME, PUT ME IN MAIN CSS IF USED OFTEN! */
+.noprint {
+display: none;
+}
+</style>
in your patch is useless

please resubmit with this duplicate class fixed
Comment 12 Marc Véron 2015-04-13 19:08:53 UTC Comment hidden (obsolete)
Comment 13 Aleisha Amohia 2015-04-14 00:21:14 UTC
Patch applied cleanly but can't seem to see the print link?
Setting as Failed QA
Comment 14 Marc Véron 2015-04-14 04:46:50 UTC Comment hidden (obsolete)
Comment 15 Nick Clemens 2015-04-15 21:26:12 UTC Comment hidden (obsolete)
Comment 16 Nick Clemens 2015-04-15 21:27:53 UTC
I tested, but the link looked very odd to me, added a patch to put a 'print results' button with the other action buttons.    I did not obsolete the old patch so anyone can test both and see which they prefer
Comment 17 Marc Véron 2015-04-16 08:33:04 UTC
Created attachment 37945 [details] [review]
[Signed-off] Bug 7646 - Printing w/o the SQL Code showing

This patch provides the same function but adds a 'Print results' button rather than the link.
Looks a little cleaner to my eye and there doesn't seem to be an option to print with SQL.

To test:
Go to Home > Reports > Guided reports wizard > Saved reports
Select a report and run it
You will find a new link "Print w/o SQL" above the title
Hit this link.
The report (this page) will print without the SQL statement.

Works as expected. Print function is displayed as Button.
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 18 Kyle M Hall 2015-04-17 12:30:29 UTC
Created attachment 38043 [details] [review]
Bug 7646 [QA Followup] - Tweak button to match other Koha print buttons
Comment 19 Kyle M Hall 2015-04-17 12:33:28 UTC
A few issues:

1) Javascript embedded in html. Javascript should be moved outside the html, include the template, and include, or a js file, whichever is best suited.

2) The noprint class is added when clicking the print button, but not removed afterward

3) Only the visible results are printed, but to me the print button implies that it will print all results. If you don't want to go this far, add a tooltip to the print button so it shows "Print visible results" when you hover over it.
Comment 20 Mirko Tietgen 2015-07-19 18:16:56 UTC
Created attachment 41093 [details]
screenshot-applied-patch

I printed to PDF and under the report name i see the comment, the message »Total number of rows matching the (unlimited) query is 1.« and »Rows per page« including the dropdown menu and the »Update« button. Is that expected? I attached a screenshot.

I agree with Kyle that I'd expect all results to be printed, not only the first page.
Comment 21 Katrin Fischer 2018-04-29 22:12:58 UTC
I just tested the print layout on master, I think the only bit to remove now is the 'rows per page'. I've filed bug 20679 for this.