[Koha-bugs] [Bug 5670] Housebound Readers Module

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

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

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

Review of attachment 51377:
 --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=5670&attachment=51377)
-----------------------------------------------------------------

::: C4/Auth.pm
@@ +435,4 @@
>      );
>      if ( $in->{'type'} eq "intranet" ) {
>          $template->param(
> +            useHouseboundModule                                                        => 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="active">[% ELSE %]<li>[% END %]<a href="/cgi-bin/koha/members/discharge.pl?borrowernumber=[% borrowernumber %]">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="friday">Friday</option>
> +                          <option value="saturday">Saturday</option>
> +                          <option value="sunday">Sunday</option>
> +                        [% END %]
> +                      </select>

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

@@ +122,5 @@
> +                            <option value="[% frequency.value %]" selected="selected">[% frequency.label %]</option>
> +                          [% ELSE %]
> +                            <option value="[% frequency.value %]">[% frequency.label %]</option>
> +                          [% END %]
> +                        [% END %]

Like this.

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

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

::: members/housebound.pl
@@ +82,5 @@
> +        fav_itemtypes  => $input->param('fav_itemtypes'),
> +        fav_subjects   => $input->param('fav_subjects'),
> +        fav_authors    => $input->param('fav_authors'),
> +        referral       => $input->param('referral'),
> +        notes          => $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?

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