[Koha-bugs] [Bug 11651] Add possibility to print holds from holds queue

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11651

Rafal Kopaczka <rkopaczka at afm.edu.pl> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Failed QA                   |Needs Signoff

--- Comment #11 from Rafal Kopaczka <rkopaczka at afm.edu.pl> ---
(In reply to Jonathan Druart from comment #9)
> Comment on attachment 26262 [details] [review]
> Bug 11651: Add possibility to print holds from holds queue
> 
> Review of attachment 26262 [details] [review]:
> -----------------------------------------------------------------
> 
> Please take a look at the following.
> Marked as Failed QA.
> > +        &MarkHoldPrinted
> > +        &GetItemNumberFromTmpHold
> 
> New unit tests should be provided for these 2 new subroutines.
> @@ +136,5 @@
> > +    if (C4::Context->preference('printSlipFromHoldsQueue')){
> > +        $query .= $branchli
> 
> New unit tests should be provided for this subroutine change.
> 
> ::: C4/Reserves.pm
> @@ +2255,5 @@
> > +            'biblio'      => $reserve->{biblionumber},
> > +            'items'       => $itemnumber,
> At least one unit test should be provided for this new subroutine.

Fixed. But please, take a look to this test. I'm new in writing unit tests. So
any feedback may be helpful. 


> ::: circ/hold-pull-print.pl
> License should be GPLv3.
> > +use strict;
> > +#use warnings; #FIXME - Bug 2505
> 
> replace use strict/warnings with use Modern::Perl.
> 
Fixed.

> ::: circ/view_holdsqueue.pl
> @@ +76,4 @@
> >  $template->param(
> >       branchloop => GetBranchesLoop(C4::Context->userenv->{'branch'}),
> >     itemtypeloop => \@itemtypesloop,
> > +   printenable => C4::Context->preference('printSlipFromHoldsQueue'),
> 
> Same as previously.
I don't get it, what is wrong here? Not unit test I think?

> 
> ::: installer/data/mysql/updatedatabase.pl

> > +    print "Upgrade to $DBversion done \n";
> 
> you forgot the SetVersion call here.

Fixed, also rebased to master.

> you can use [un]checkCheckboxes functions provided by
> jquery.checkboxes.min.js to do that.
Fixed.
> 
> @@ +93,2 @@
> >      [% IF ( itemsloop ) %]
> > +        <form id="hold_print" name="hold_print" method="get" action="/cgi-bin/koha/circ/hold-pull-print.pl" target="_blank">
> 
> why do you use target="_blank" here?
Print slip must be open in new window it is in form to print, same as all
printable reports in Koha (Reserve slip/Borrower sumary etc.). Why here? It
works, at least for me :)

> @@ +142,5 @@
> >          [% IF ( printenable ) %]
> > +        </form>
> 
> This close tag is in a IF but not the <form>.

Fixed.

-- 
You are receiving this mail because:
You are watching all bug changes.