Bug 8033

Summary: remove unused sco/receipt template file
Product: Koha Reporter: Melia Meggs <melia>
Component: Self checkoutAssignee: Elliott Davis <elliott>
Status: CLOSED FIXED QA Contact: Paul Poulain <paul.poulain>
Severity: minor    
Priority: P5 - low CC: chris, dcook, derrynj, elliott, gitbot, jonathan.druart, katrin.fischer, kyle, mtj, mtompset, nengard, paul.poulain, savitra.sirohi, tomascohen, veron
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Adds a print receipt to Self Checkout
This patch adds print slips to self checkout.
This patch adds print slips to self checkout.
Bug 8033 : This patch adds print slips to self checkout.
[Followup] Modified POD and copyright. Also perltidied
Bug 8033 : This patch adds print slips to self checkout.
[Follow-up]
Change javascipt behavior to degrade nicely
Bug 8033 : This patch adds print slips to self checkout.
Change javascipt behavior to degrade nicely
Bug 8033 follow-up removing tabs in indentation
Bug 8033: Change javascipt behavior to degrade nicely
Bug 8033 : This patch adds print slips to self checkout.
Bug 8033 follow-up removing tabs in indentation
Bug 8033: Change javascipt behavior to degrade nicely
Bug 8033: Change javascipt behavior to degrade nicely
Bug 8033 follow-up removing tabs in indentation
Bug 8033 : This patch adds print slips to self checkout.
Bug 8033 : This patch adds print slips to self checkout.
Bug 8033 follow-up removing tabs in indentation
Bug 8033: Change javascipt behavior to degrade nicely
Bug 8033 [Follow-up] add print receipt option to Koha self-check
Bug 8033 [Follow-up] add print receipt option to Koha self-check
[SIGNED-OFF] Bug 8033 [Follow-up] add print receipt option to Koha self-check
Bug 8033 [Follow-up] add print receipt option to Koha self-check
Bug 8033 - add print receipt option to Koha self-check (edit)
Bug 8033 [Follow-up] add print receipt option to Koha self-check
Bug 8033 - add print receipt option to Koha self-check (edit)
Bug 8033 [Follow-up] add print receipt option to Koha self-check
Bug 8033 - add print receipt option to Koha self-check (edit)
Bug 8033 - Remove unused template file.
Bug 8033 - Remove unused template file.
[Signed-off] Bug 8033 - Remove unused template file.
[PASSED QA] Bug 8033 - Remove unused template file.

Description Melia Meggs 2012-05-01 01:05:08 UTC
Add print receipt option to Koha Self-check.

It should:

* Bring up alert box upon hitting 'finish' ( like transfer/hold alert in CKI
module)
* "Would you like a receipt?" - options 'Yes' / 'No'
* Print receipt if 'Yes' --- else, no receipt.
Comment 1 Elliott Davis 2012-07-05 17:48:56 UTC Comment hidden (obsolete)
Comment 2 Elliott Davis 2012-07-05 18:11:35 UTC Comment hidden (obsolete)
Comment 3 Chris Cormack 2012-07-06 02:42:14 UTC
Hi Elliott 

This looks good, but a few little things that need to be fixed

in opac/sco/printslip.pl
1/ Copyright headers (it says biblibre and katipo at the moment)
2/ use warnings is commented out, we shouldn't be making new scripts with warnings commented out
3/ The pod is still referring to moremember.pl which is where the script was copied from I guess
4/ $template->param( is a HTML::Template thing, they still exist in old scripts and we have a shim in C4::Templates to fix it, but its best to make no ones like this
$template->{VARS}->{'slip'} = $slip;
Comment 4 Elliott Davis 2012-07-06 17:48:27 UTC Comment hidden (obsolete)
Comment 5 Elliott Davis 2012-07-06 17:51:12 UTC
Chris,

I made the changes you requested.  Let me know if it looks ok.  The reason I left the copyright headers was because it was basically a direct copy and I didn't want to take credit for someone else's work.
Comment 6 Chris Cormack 2012-07-06 20:10:01 UTC
(In reply to comment #5)
> Chris,
> 
> I made the changes you requested.  Let me know if it looks ok.  The reason I
> left the copyright headers was because it was basically a direct copy and I
> didn't want to take credit for someone else's work.

Yep, you could add parts copyright Bywater Solutions in that case. Also now we have no copyright headers :( And no POD.

A follow up fixing those would be great. But I'll sign off on it because functionality wise it works, but you will probably need that follow up to get passed QA. 

(The POD is handy for this http://perldoc.koha-community.org/members/patronimage.html)

http://perldoc.koha-community.org/ is a great resource for developing and it gets better the better we document our code.

But that will be up to QA not me :)
Comment 7 Chris Cormack 2012-07-06 20:12:15 UTC Comment hidden (obsolete)
Comment 8 Paul Poulain 2012-07-13 15:32:55 UTC
QA comments
 * new script passes perlcritic
 * Eliott, if you don't want to copyright your code to bywatersolutions, please copyright it to HLT
 * your new script sco/printslip.pl seems not to be perltidy-ed (see PERL1 coding guidelines http://wiki.koha-community.org/wiki/Coding_Guidelines)
 * I tested, got the message, but the printed paper was empty. Filling ISSUEQSLIP notice make things looking better !

failed QA for now, when you've fixed 2 and 3, switch back to signed off, as the QA comments are just coding style related
Comment 9 Elliott Davis 2012-07-24 15:07:19 UTC Comment hidden (obsolete)
Comment 10 Owen Leonard 2012-07-26 15:09:14 UTC
I find that submitting a barcode to check out results in a "Would you like to print a receipt" dialog. Shouldn't it only offer to print when you click "Finish?"
Comment 11 Elliott Davis 2012-07-26 18:54:46 UTC
Owen, 

The reason I added it to the submit button rather than the finish button was because I feared someone would not click finish and just leave the terminal. If you think that this is an improbable case it is a pretty easy change. 

(In reply to comment #10)
> I find that submitting a barcode to check out results in a "Would you like
> to print a receipt" dialog. Shouldn't it only offer to print when you click
> "Finish?"
Comment 12 Owen Leonard 2012-08-21 16:04:24 UTC
(In reply to comment #11)
> The reason I added it to the submit button rather than the finish button was
> because I feared someone would not click finish and just leave the terminal.

This was a potential problem before this patch, wasn't it? I think it's too much of a disruption to the process to pop up a message after each transaction. I think it would be better to leave it up to the user to decide when to print their receipt. Perhaps another button alongside "finish," "Finish and print receipt?"
Comment 13 Jared Camins-Esakov 2012-08-28 02:31:03 UTC
(In reply to comment #12)
> (In reply to comment #11)
> > The reason I added it to the submit button rather than the finish button was
> > because I feared someone would not click finish and just leave the terminal.
> 
> This was a potential problem before this patch, wasn't it? I think it's too
> much of a disruption to the process to pop up a message after each
> transaction. I think it would be better to leave it up to the user to decide
> when to print their receipt. Perhaps another button alongside "finish,"
> "Finish and print receipt?"

I agree with Owen. If someone skips the receipt printing step, that's their problem.
Comment 14 David Cook 2012-09-06 04:35:02 UTC
(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > The reason I added it to the submit button rather than the finish button was
> > > because I feared someone would not click finish and just leave the terminal.
> > 
> > This was a potential problem before this patch, wasn't it? I think it's too
> > much of a disruption to the process to pop up a message after each
> > transaction. I think it would be better to leave it up to the user to decide
> > when to print their receipt. Perhaps another button alongside "finish,"
> > "Finish and print receipt?"
> 
> I agree with Owen. If someone skips the receipt printing step, that's their
> problem.

Additionally, you're skipping the server-side validation when you have your script on the submit button. You generate a slip even though the item might not actually be checked out (because it is reference book, currently checked out to someone else, on hold for someone else, a renewal, and so on). While this isn't a crippling deficiency, it is certainly problematic.

I also wanted to mention that I agree with Owen as well. I use the self-checkout at my local branch all the time, and you print the receipt by pressing the "Print Receipt" button on the screen. It's a much smoother behaviour. 

(I've also noticed recently that there is an "Email Receipt" button as an alternative to the paper receipt.)
Comment 15 David Cook 2012-09-06 04:57:27 UTC
Not to hassle you too much, but it's also worth noting that renewals aren't included in the print out. Just check outs. I haven't looked too much into the guts of this script, but they should probably be included too.
Comment 16 Elliott Davis 2012-09-18 15:45:44 UTC Comment hidden (obsolete)
Comment 17 Owen Leonard 2012-10-24 16:02:52 UTC
This seems to work fine, but I would feel more comfortable signing off if the "Finish" action wasn't so dependent on JavaScript. Instead of using a click handler on the submit button, I suggest using a submit handler:

    $("#logout_form").submit(function(){
      if(confirm("Would you like to print a receipt?")){
          window.open("/cgi-bin/koha/sco/printslip.pl?borrowernumber=[% borrowernumber %]&amp;print=qslip");
      }
      return true;
    });

You don't have to rewrite the "action" of the form because whether the user wants to print a receipt or not they want to log out. Keep the default action of the form and pop up the window if the user has JavaScript on and chooses to confirm.
Comment 18 Elliott Davis 2012-11-20 15:23:32 UTC Comment hidden (obsolete)
Comment 19 Elliott Davis 2012-11-20 15:26:34 UTC
Please apply both the original patch and the follow up.  The testing plan is the same as before.
Comment 20 Mason James 2012-11-28 04:33:29 UTC Comment hidden (obsolete)
Comment 21 Mason James 2012-11-28 04:38:26 UTC
(In reply to comment #20)
> Created attachment 13716 [details] [review]
> Change javascipt behavior to degrade nicely
> 
> http://bugs.koha-community.org/show_bug.cgi?id=8033
> Signed-off-by: Mason James <mtj@kohaaloha.com>


with jscript disabled, 'finish' button works as expected
... but printing pop-up is disabled


Elliot, i assume this is the expected behaviour?
Comment 22 Owen Leonard 2012-11-28 14:48:12 UTC
(In reply to comment #21)
> with jscript disabled, 'finish' button works as expected
> ... but printing pop-up is disabled

This is to be expected since the trigger for the pop-up is JavaScript. If we wanted to be sure we offered a print view to non-JS users we might add a separate button and hide it with JavaScript.
Comment 23 Elliott Davis 2012-12-03 19:21:59 UTC
Owen is right, it is expected behavior as the pop-up is javascript.  Is adding a seperate button something that needs to be done before this patch sees some QA love?

Elliott
Comment 24 Owen Leonard 2012-12-03 20:23:07 UTC
(In reply to comment #23)
> Is adding a seperate button something that needs to be done before this patch
> sees some QA love?

I would think it could be left for a follow-up.
Comment 25 Paul Poulain 2012-12-12 07:51:56 UTC
QA comment:
 * passes koha-qa.pl although there are a few tabs I've replaced by 4 spaces in a follow-up
 * work as expected/announced, no side effect risk I can see
 * there are some missing images with ccsr theme, but this problem is not related to this patch
 * Idea for an improvement = make sco slip printing compatible with silent printing (see bug 7563)

passed QA
Comment 26 Paul Poulain 2012-12-12 07:52:28 UTC Comment hidden (obsolete)
Comment 27 Paul Poulain 2012-12-12 07:52:34 UTC Comment hidden (obsolete)
Comment 28 Paul Poulain 2012-12-12 07:52:38 UTC Comment hidden (obsolete)
Comment 29 Jared Camins-Esakov 2012-12-12 13:04:28 UTC
I followed the test plan, and I am definitely not asked if I want to print out a receipt. Is there a syspref that needs to be set?
Comment 30 Jared Camins-Esakov 2012-12-13 16:02:03 UTC
Marking as "Failed QA" until guidance is provided on how to test this.
Comment 31 Jared Camins-Esakov 2012-12-13 17:49:12 UTC Comment hidden (obsolete)
Comment 32 Jared Camins-Esakov 2012-12-13 18:48:26 UTC Comment hidden (obsolete)
Comment 33 Jared Camins-Esakov 2012-12-13 18:48:35 UTC Comment hidden (obsolete)
Comment 34 Jared Camins-Esakov 2012-12-13 18:48:44 UTC Comment hidden (obsolete)
Comment 35 Elliott Davis 2012-12-13 18:58:31 UTC Comment hidden (obsolete)
Comment 36 Elliott Davis 2012-12-13 18:59:03 UTC Comment hidden (obsolete)
Comment 37 Elliott Davis 2012-12-13 18:59:22 UTC Comment hidden (obsolete)
Comment 38 Elliott Davis 2012-12-13 19:02:39 UTC Comment hidden (obsolete)
Comment 39 Elliott Davis 2012-12-13 19:03:04 UTC Comment hidden (obsolete)
Comment 40 Elliott Davis 2012-12-13 19:03:18 UTC Comment hidden (obsolete)
Comment 41 Jared Camins-Esakov 2012-12-13 22:54:12 UTC
This patch has been pushed to master.
Comment 42 Owen Leonard 2012-12-14 15:50:23 UTC Comment hidden (obsolete)
Comment 43 Nicole C. Engard 2013-01-09 14:26:17 UTC
Bug 8033 : This patch adds print slips to self checkout.
Apply? [yn] y

Applying: Bug 8033 : This patch adds print slips to self checkout.
error: koha-tmpl/opac-tmpl/prog/en/modules/sco/printslip.tt: already exists in index
error: patch failed: koha-tmpl/opac-tmpl/prog/en/modules/sco/sco-main.tt:8
error: koha-tmpl/opac-tmpl/prog/en/modules/sco/sco-main.tt: patch does not apply
error: opac/sco/printslip.pl: already exists in index
error: patch failed: opac/sco/sco-main.pl:239
error: opac/sco/sco-main.pl: patch does not apply
Patch failed at 0001 Bug 8033 : This patch adds print slips to self checkout.
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Patch left in /tmp/Bug-8033--This-patch-adds-print-slips-to-self-chec-33SuJQ.patch
nengard@kohavm:~/kohaclone$
Comment 44 Owen Leonard 2013-01-09 14:52:19 UTC Comment hidden (obsolete)
Comment 45 Nicole C. Engard 2013-01-09 15:18:31 UTC Comment hidden (obsolete)
Comment 46 Nicole C. Engard 2013-01-09 15:19:08 UTC
Obsoleted all patches that appeared to have been pushed already - if I was wrong feel free to restore.

Follow up signed off.

Nicole
Comment 47 Jonathan Druart 2013-01-10 12:47:06 UTC
I cannot test the modifications on the sco/receipt.tt file.
Where is it used?
Comment 48 Nicole C. Engard 2013-01-10 14:12:12 UTC
Click 'Finish' on /cgi-bin/koha/sco/sco-main.pl and it will prompt to print.

Nicole
Comment 49 Jonathan Druart 2013-01-10 14:38:15 UTC
(In reply to comment #48)
> Click 'Finish' on /cgi-bin/koha/sco/sco-main.pl and it will prompt to print.
> 
> Nicole

According to the code this action uses the sco/printslip.tt file, not sco/receipt.tt
Comment 50 Jonathan Druart 2013-01-31 09:58:03 UTC
Could someone confirm this file (sco/receipt.tt) can be removed?
Comment 51 David Cook 2013-02-22 04:57:52 UTC
(In reply to comment #50)
> Could someone confirm this file (sco/receipt.tt) can be removed?

I've searched in my text editor and grepped the files within the sco directories in cgi-bin and htdocs, and I cannot see any reference to sco/receipt.tt
Comment 52 David Cook 2013-02-22 05:25:15 UTC
Also, it looks like there is an issue with the HTML <title> tags. Chrome and IE9 (I haven't checked out browsers) seem to add a <title i18n-content="title"> tag, which causes the opening <title> tag to be rendered as part of the innerHTML, so you wind up with "<title> Print Receipt for..."

I don't know if the best solution is to remove the opening <title> tag or if others have other ideas, but it would be worth a quick follow-up...
Comment 53 Mason James 2013-04-12 03:39:26 UTC
(In reply to comment #51)
> (In reply to comment #50)
> > Could someone confirm this file (sco/receipt.tt) can be removed?
> 
> I've searched in my text editor and grepped the files within the sco
> directories in cgi-bin and htdocs, and I cannot see any reference to
> sco/receipt.tt

interesting...

despite sco/receipt.tt being added to the repo in commit a0b220f8551
it looks like it's probably never been used/referred to?

so, delete this file for sure :)
Comment 54 Mason James 2013-04-12 03:45:41 UTC
(In reply to comment #53)
> (In reply to comment #51)
> > (In reply to comment #50)
> > > Could someone confirm this file (sco/receipt.tt) can be removed?
> > 
> > I've searched in my text editor and grepped the files within the sco
> > directories in cgi-bin and htdocs, and I cannot see any reference to
> > sco/receipt.tt
> 
> interesting...
> 
> despite sco/receipt.tt being added to the repo in commit a0b220f8551
> it looks like it's probably never been used/referred to?
> 
> so, delete this file for sure :)

ah, i see now...

the orig. file was opac/sco/receipt.tmpl and was used by opac/sco/printer.pl
opac/sco/printer.pl was removed, but its tmpl/tt file was not

the file can still be removed safely
Comment 55 Mason James 2013-04-12 04:08:16 UTC Comment hidden (obsolete)
Comment 56 Mason James 2013-04-12 04:10:53 UTC
(In reply to comment #55)
> Created attachment 17373 [details] [review] [review]
> Bug 8033 [Follow-up] add print receipt option to Koha self-check
> 
> This follow-up makes some corrections to JavaScript, most importantly
> by enabling translation of strings embedded in the script. Other
> corrections: Trailing whitespace, proper <![CDATA[ commenting,
> and === comparisons (Following coding/JSHint guidelines).
> 
> To test, check something out in self checkout and click the "Finish"
> button. The JavaScript confirmation dialog should appear and the receipt
> should appear and self-close correctly.
> 
> Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
> Works as expected.
> Signed-off-by: Mason James <mtj@kohaaloha.com>

passing QA on this

...with a follow-up patch to remove sco/receipt.tt
Comment 57 Mason James 2013-04-12 04:20:34 UTC Comment hidden (obsolete)
Comment 58 Nicole C. Engard 2013-05-09 19:37:06 UTC
I was going to sign off on the follow up but I can't apply the original signed off patch anymore.

Bug 8033 - add print receipt option to Koha self-check (edit)
Apply? [yn] y

Applying: Bug 8033 - add print receipt option to Koha self-check (edit)
error: patch failed: koha-tmpl/opac-tmpl/prog/en/modules/sco/receipt.tt:1
error: koha-tmpl/opac-tmpl/prog/en/modules/sco/receipt.tt: patch does not apply
Patch failed at 0001 Bug 8033 - add print receipt option to Koha self-check (edit)
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Patch left in /tmp/Bug-8033---add-print-receipt-option-to-Koha-self-c-gJKXQ0.patch
Comment 59 Kyle M Hall 2013-05-09 20:18:52 UTC Comment hidden (obsolete)
Comment 60 Kyle M Hall 2013-05-09 20:19:06 UTC Comment hidden (obsolete)
Comment 61 Owen Leonard 2013-05-15 15:48:49 UTC
(In reply to comment #60)

> 2/ fixed <title i18n-content="title"> display bug

What is 'i18n-content="title"' supposed to do? This template should not have a beginning <title> tag. <title> is supplied by doc-head-open.inc.
Comment 62 Derryn 2013-07-31 07:24:38 UTC
I would like to voice my annoyance at this bug.

We have no printer, no need or want for receipts. 
Why should our users be forced to get out the mouse/keyboard and hit no every time they log out?
This should be an option in the preferences and not forced upon everyone.
Comment 63 Chris Cormack 2013-07-31 07:55:34 UTC
(In reply to Derryn from comment #62)
> I would like to voice my annoyance at this bug.
> 
> We have no printer, no need or want for receipts. 
> Why should our users be forced to get out the mouse/keyboard and hit no
> every time they log out?
> This should be an option in the preferences and not forced upon everyone.

Awesome, I look forward to your patch implementing this.
Comment 64 Owen Leonard 2013-10-22 22:03:17 UTC Comment hidden (obsolete)
Comment 65 Owen Leonard 2013-10-22 22:05:25 UTC Comment hidden (obsolete)
Comment 66 I'm just a bot 2013-10-30 07:50:42 UTC
Patch applied cleanly, go forth and signoff
Comment 67 Marc Véron 2014-03-06 15:49:44 UTC
Attachment 22301 [details]: Merge conflict in sco-main.tt
Comment 68 Mark Tompsett 2015-04-03 18:27:32 UTC Comment hidden (obsolete)
Comment 69 Mark Tompsett 2015-04-03 18:32:53 UTC Comment hidden (obsolete)
Comment 70 Marc Véron 2015-04-04 06:32:32 UTC Comment hidden (obsolete)
Comment 71 Kyle M Hall 2015-04-10 14:11:07 UTC
Created attachment 37651 [details] [review]
[PASSED QA] Bug 8033 - Remove unused template file.

All prog related patches were obsoleted from this bug.

Bug 11349 changed the code to have the proper template name
used (sco/printslip.tt instead of sco/printsplit.tmpl).

Bug 12062 moved the hard coded string logic into the template,
and passed the borrower number.

This only leaves the patch to remove the unused receipt.tt file.

TEST PLAN
---------
1) Confirm the file isn't used.
   $ git grep "sco/receipt" | grep -v "\.po:"
   -- this should list nothing, because there are still po
      files referencing it, but they aren't needed either.
2) ls ./koha-tmpl/opac-tmpl/bootstrap/en/modules/sco/receipt.tt
   -- there should be a file
3) apply patch
4) repeat step 2
   -- the file should be removed

Followed test plan, file does no longer exist.
Signed-off-by: Marc Veron <veron@veron.ch>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 72 Tomás Cohen Arazi 2015-04-13 14:55:12 UTC
Patch pushed to master.

Thanks Owen! We miss you!