[Koha-bugs] [Bug 14570] Make it possible to add multiple guarantors to a record

bugzilla-daemon at bugs.koha-community.org bugzilla-daemon at bugs.koha-community.org
Tue May 1 18:03:19 CEST 2018


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

--- Comment #73 from Kyle M Hall <kyle at bywatersolutions.com> ---
> 1) Release notes
> Should state consequences to:
> - patron import (no change, but can only import with one guarantor?)
> - LDAP and SIP (lines where changed, but only in POD?)
> - patron REST API (guarantor information can't be changed or asked for)
> - Reports using borrowers.guarantorid have to be changed/rewritten

Added. Left out LDAP/SIP since there is literally no change in behavior. I
updated the documentation since it's there but it should really be removed from
the codebase altogether.

> 2) QA script
>  FAIL	opac/opac-user.pl
>    FAIL	  valid
> 		"my" variable $patron masks earlier declaration in same scope
> 
>  FAIL	Koha/Patron/Relationships.pm
>    FAIL	  pod 
> 		Spurious =cut command
> 		*** ERROR: 
> 		 in file Koha/Patron/Relationships.pm
>    FAIL	  pod coverage
> 		POD is missing for 'object_class'
> 
>  FAIL	Koha/Patron.pm
>    FAIL	  pod coverage
> 		POD is missing for 'has_permission'

Fixed! QA script is giving me invalid TT syntax failures for some reason.
xt/tt_valid.t passes and everything works in actual testing.


> a) I tried adding a guarantor from a Teacher (Professional) and from a Kid
> (Child) account. Both don't offer the form to me.
> 
> b) Using the 'add child' from an existing patron (Adult), a new form opens
> for an adult patron category, the guarantor information is empty.

Fixed!

> 
> 5) Code review (splinter):
> 
> ::: Koha/Patron.pm
> @@ +119,5 @@
> >  
> >      return scalar Koha::Patron::Images->find( $self->borrowernumber );
> >  }
> >  
> > +=head3 library
> 
> Please add a description here too, not just an empty entry. QA tools will be
> happy with this, but the missing POD never fixed.

Fixed!

> @@ +135,5 @@
> >  
> > +Returns the set of relationships for the patrons that are guarantors for this patron.
> > +
> > +This is returned instead of a Koha::Patron object because the guarantor
> > +may not exist as a patron in Koha. If this is true, the guarantors name
> 
> This is quite a change - is it really implemented in this patch set? What's
> the use case? Just a name is not a lot to locate the person if needed.

It's not a change, it's a new subroutine. I'm not sure I understand what you
mean. The use case is explained in the comment. We can't return Koha::Patron's
because not every guarantor is one. So instead we return the guarantor
relationships from which a Koha::Patron can be gotten if there is one,
otherwise the guarantor info ( name, relationship, etc ) can be read directly
from the guarantor relationship object.

> ::: koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc
> @@ -17,5 @@
> >      [% IF CAN_user_borrowers_edit_borrowers %]
> >          [% IF patron.is_adult AND Koha.Preference("borrowerRelationship") %]
> >              <a id="addchild" class="btn btn-default btn-sm" href="/cgi-bin/koha/members/memberentry.pl?op=add&guarantorid=[% patron.borrowernumber %]"><i class="fa fa-plus"></i> Add child</a>
> >          [% END %]
> > -        [% IF CAN_user_borrowers_edit_borrowers %]
> 
> This change doesn't look like it was intentional/related. Please check.

This change is intentional. Looks at the  IF above it. We have two nested if
statements checking the same thing. There was no need for the inner one so I
removed it.

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


More information about the Koha-bugs mailing list