--- Comment #68 from Katrin Fischer <katrin.fisc...@bsz-bw.de> ---
Comment on attachment 73583
Bug 14570 - Make it possible to add multiple guarantors to a record
Review of attachment 73583:
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
"my" variable $patron masks earlier declaration in same scope
Spurious =cut command
in file Koha/Patron/Relationships.pm
FAIL pod coverage
POD is missing for 'object_class'
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):
@@ +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.
@@ -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"
> 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 : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/