--- Comment #68 from Katrin Fischer <> ---
Comment on attachment 73583
Bug 14570 - Make it possible to add multiple guarantors to a record

Review of attachment 73583:

Hi Kyle,

this is a huge patch with several consequences. Lots to test. Not nearly done

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/
   FAIL   valid
                "my" variable $patron masks earlier declaration in same scope

 FAIL   Koha/Patron/
   FAIL   pod 
                Spurious =cut command
                *** ERROR: 
                 in file Koha/Patron/
   FAIL   pod coverage
                POD is missing for 'object_class'

 FAIL   Koha/
   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/
@@ +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/
@@ -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/;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
website :
git :
bugs :

Reply via email to