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

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

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

--- Comment #68 from Katrin Fischer <katrin.fischer at bsz-bw.de> ---
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.