https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14570
--- Comment #68 from Katrin Fischer <[email protected]> --- Comment on attachment 73583 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=73583 Bug 14570 - Make it possible to add multiple guarantors to a record Review of attachment 73583: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=14570&attachment=73583) ----------------------------------------------------------------- Hi Kyle, this is a huge patch with several consequences. Lots to test. Not nearly done yet. 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 ... 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' 3) New unit tests pass. 4) Tests in GUI Testing this, something doesn't seem to be quite right. 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. 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. @@ +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. ::: 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. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
