From 81e556d17cb71d42a64f0869c11b50a0cd8d0a36 Mon Sep 17 00:00:00 2001
From: Emmi Takkinen <emmi.takkinen@outlook.com>
Date: Tue, 3 Nov 2020 15:03:11 +0200
Subject: [PATCH 1/2] Bug 12133: Fix issues with modifying child patron and
 adding guarantors

Child patrons couldn't be modified when guarantor was
patron due missing guarantor_id value. Also if multiple
guarantors were added one could easily add child patron
as a guarantor.

This patch adds Instead of one guarantor id all new and
existing guarantor ids are checked in case of child patron.

To test:
1. Add child patron with syspref ChildNeedsGuarantor and
GuarantorNeedsToBePatron on.
2. Add two guarantors for the patron, first one should be
an adult patron and second a child.
3. Save patron.
=> Error "A guarantor cannot be a guarantee." should be
raised but isn't.
4. Modify patron and save.
=> Error "Child needs guarantor" is raised even if (valid)
guarantor exists.
5. Apply patch.
6. Repeat steps 1 to 4.
=> This time while saving a new patron error "A guarantor
cannot be a guarantee." is raised but modifying is
successful.

Sponsored-by: Koha-Suomi Oy
---
 .../prog/en/modules/members/memberentrygen.tt |  4 ++--
 members/memberentry.pl                        | 23 ++++++++++++-------
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
index 42e4dcfeba..37c410d4a6 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
@@ -573,7 +573,7 @@ legend:hover {
                                         </fieldset> <!-- /#memberentry_guarantor -->
                                     </div> <!-- #/memberentry_guarantor_anchor -->
 
-                                    [% UNLESS nocontactname && nocontactfirstname && norelationship %]
+                                    [% UNLESS nocontactname && nocontactfirstname && norelationship || Koha.Preference('GuarantorHasToBePatron') %]
                                         <fieldset class="rows" id="non_patron_guarantor">
                                             <legend>Non-patron guarantor</legend>
                                             <ol>
@@ -637,7 +637,7 @@ legend:hover {
                                     [% PROCESS 'main-address-style' %]
                                 [% END # /UNLESS nostreet && nocity etc group%]
 
-                                [% UNLESS nophone && nophonepro && nomobile && noemail && noemailpro && nofax || Koha.Preference('GuarantorHasToBePatron') %]
+                                [% UNLESS nophone && nophonepro && nomobile && noemail && noemailpro && nofax %]
                                     <fieldset class="rows" id="memberentry_contact">
                                         <legend id="contact_lgd">Contact information</legend>
                                         <ol>
diff --git a/members/memberentry.pl b/members/memberentry.pl
index 3787f5d5ad..c2099c283e 100755
--- a/members/memberentry.pl
+++ b/members/memberentry.pl
@@ -110,19 +110,25 @@ $template->param( relationships => scalar $patron->guarantor_relationships ) if
 my @relations = split /,|\|/, C4::Context->preference('borrowerRelationship');
 my $empty_relationship_allowed = grep {$_ eq ""} @relations;
 $template->param( empty_relationship_allowed => $empty_relationship_allowed );
-
-my $guarantorinfo = $input->param('guarantorinfo');
 my $guarantor_id = $input->param('new_guarantor_id');
 my $guarantor = undef;
 $guarantor = Koha::Patrons->find( $guarantor_id ) if $guarantor_id;
 $template->param( guarantor => $guarantor );
-
 my @delete_guarantor = $input->multi_param('delete_guarantor');
 foreach my $id ( @delete_guarantor ) {
     my $r = Koha::Patron::Relationships->find( $id );
     $r->delete() if $r;
 }
 
+#Search existing guarantor id(s) and new ones from params
+my @guarantor_ids;
+my @guarantors = $patron->guarantor_relationships()->guarantors unless !$patron;
+foreach my $guarantor (@guarantors){
+    push @guarantor_ids, $guarantor->id;
+};
+my @new_guarantor_ids = grep { $_ ne '' } $input->multi_param('new_guarantor_id');
+push (@guarantor_ids, @new_guarantor_ids);
+
 ## Deal with debarments
 $template->param(
     debarments => scalar GetDebarments( { borrowernumber => $borrowernumber } ) );
@@ -287,15 +293,16 @@ if ( ( $op eq 'insert' ) and !$nodouble ) {
     }
 }
 
-if ( $guarantor_id ) {
-    if (my $guarantor = Koha::Patrons->find( $guarantor_id )) {
-        my $guarantor_category = $guarantor->category->category_type;
-        push @errors, 'ERROR_guarantor_is_guarantee' if ( ($guarantor_category eq 'C') &&
+if ( @guarantor_ids ) {
+    foreach my $guarantor_id ( @guarantor_ids ){
+        if (my $guarantor = Koha::Patrons->find( $guarantor_id )){
+            push @errors, 'ERROR_guarantor_is_guarantee' if ( ($guarantor->is_child) &&
                                                           ($op eq 'save' || $op eq 'insert') );
+        }
     }
 }
 
-my $valid_guarantor = $guarantor_id ? $guarantor_id : $newdata{'contactname'};
+my $valid_guarantor = @guarantor_ids or $newdata{'contactname'} ? 1 : 0;
 
 if($category_type eq 'C' && ($op eq 'save' ||  $op eq 'insert') && C4::Context->preference('ChildNeedsGuarantor')){
     if(!$valid_guarantor){
-- 
2.17.1