From bugzilla-daemon@bugs.koha-community.org Fri May 27 02:51:15 2016
From: bugzilla-daemon@bugs.koha-community.org
To: koha-bugs@lists.koha-community.org
Subject: [Koha-bugs] [Bug 5670] Housebound Readers Module
Date: Fri, 27 May 2016 00:51:10 +0000
Message-ID: <bug-5670-70-Oq3RWFAryW@https.bugs.koha-community.org/bugzilla3/>
In-Reply-To: <bug-5670-70@https.bugs.koha-community.org/bugzilla3/>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============1805109776628555113=="

--===============1805109776628555113==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=3D5670

--- Comment #27 from M. Tompsett <mtompset@hotmail.com> ---
Comment on attachment 51377
  --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=3D51377
Rebase on master

Review of attachment 51377:
 --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=3Dsplinter.html&b=
ug=3D5670&attachment=3D51377)
-----------------------------------------------------------------

::: C4/Auth.pm
@@ +435,4 @@
>      );
>      if ( $in->{'type'} eq "intranet" ) {
>          $template->param(
> +            useHouseboundModule                                           =
             =3D> C4::Context->preference("useHouseboundModule"),

There is no need to do this. The preferences are accessible in the template,
see other templates which grab preferences already.

::: koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc
@@ +105,4 @@
>      [% IF CAN_user_borrowers && useDischarge %]
>          [% IF dischargeview %]<li class=3D"active">[% ELSE %]<li>[% END %]=
<a href=3D"/cgi-bin/koha/members/discharge.pl?borrowernumber=3D[% borrowernum=
ber %]">Discharge</a></li>
>      [% END %]
> +    [% IF Koha.Preference('HouseboundModule') %]

See, like here.

::: koha-tmpl/intranet-tmpl/prog/en/modules/members/housebound.tt
@@ +110,5 @@
> +                          <option value=3D"friday">Friday</option>
> +                          <option value=3D"saturday">Saturday</option>
> +                          <option value=3D"sunday">Sunday</option>
> +                        [% END %]
> +                      </select>

Couldn't all this select logic be simplified with a loop?

@@ +122,5 @@
> +                            <option value=3D"[% frequency.value %]" select=
ed=3D"selected">[% frequency.label %]</option>
> +                          [% ELSE %]
> +                            <option value=3D"[% frequency.value %]">[% fre=
quency.label %]</option>
> +                          [% END %]
> +                        [% END %]

Like this.

@@ +335,5 @@
> +                    [% ELSIF hpd =3D=3D 'saturday' %]
> +                      Saturday
> +                    [% ELSIF hpd =3D=3D 'sunday' %]
> +                      Sunday
> +                    [% END %]

http://template-toolkit.org/docs/manual/VMethods.html#section_ucfirst

::: members/housebound.pl
@@ +82,5 @@
> +        fav_itemtypes  =3D> $input->param('fav_itemtypes'),
> +        fav_subjects   =3D> $input->param('fav_subjects'),
> +        fav_authors    =3D> $input->param('fav_authors'),
> +        referral       =3D> $input->param('referral'),
> +        notes          =3D> $input->param('notes'),

Perhaps // q{} or something similar might be nice, just in case there's
undefined parameters.

::: t/db_dependent/Patron/Housebound.t
@@ +64,5 @@
> +foreach my $cho ( @{$found_choosers} ) {
> +    $cho_counter ++
> +        if ( $cho->borrowernumber eq $patron_chooser->{borrowernumber} );
> +};
> +is(1, 1, "Return our patron_chooser!");

What is the point of this test?

@@ +73,5 @@
> +foreach my $del ( @{$found_deliverers} ) {
> +    $del_counter ++
> +        if ( $del->borrowernumber eq $patron_deliverer->{borrowernumber} );
> +};
> +is(1,1,"Return our patron_deliverer!");

What is the point of this test?

--=20
You are receiving this mail because:
You are watching all bug changes.
You are the QA Contact for the bug.

--===============1805109776628555113==--