From c44a377d9cc44538b6a7134f48763184c9a110ba Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Mon, 30 Jan 2017 12:36:57 -0300
Subject: [PATCH] Bug 13757: (QA followup) Make opac-memberentry.pl handle
 attrs deletion

The original code on this bug skipped empty-valued attributes. But
emptying attributes is the only way to tell the controller script that
the user wants to delete them.

This patch makes opac-memberentry.pl check the existence of attributes
sharing the code of the empty for the given patron, and it stores the
deletion on the Koha::Patron::Modification as needed. Otherwise
deletions got skipped.

To test:
- Verify setting/deleting attributes that are opac-editable and verify
the results are sound.

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
---
 opac/opac-memberentry.pl | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl
index b3fed91623..4b00623c94 100755
--- a/opac/opac-memberentry.pl
+++ b/opac/opac-memberentry.pl
@@ -89,7 +89,7 @@ $template->param(
     OPACPatronDetails => C4::Context->preference('OPACPatronDetails'),
 );
 
-my $attributes = ParsePatronAttributes($cgi);
+my $attributes = ParsePatronAttributes($borrowernumber,$cgi);
 my $conflicting_attribute = 0;
 
 foreach my $attr (@$attributes) {
@@ -560,17 +560,22 @@ sub GeneratePatronAttributesForm {
 }
 
 sub ParsePatronAttributes {
-    my ( $cgi ) = @_;
+    my ($borrowernumber,$cgi) = @_;
 
-    my @codes = $cgi->multi_param('patron_attribute_code');
+    my @codes  = $cgi->multi_param('patron_attribute_code');
     my @values = $cgi->multi_param('patron_attribute_value');
 
     my $ea = each_array( @codes, @values );
     my @attributes;
     my %dups = ();
 
-    while ( my ( $code, $value, $password ) = $ea->() ) {
-        next unless defined($value) and $value ne '';
+    while ( my ( $code, $value ) = $ea->() ) {
+        # Don't skip if the patron already has attributes with $code, because
+        # it means we are being requested to remove the attributes.
+        next
+            unless defined($value) and $value ne ''
+            or Koha::Patron::Attributes->search(
+            { borrowernumber => $borrowernumber, code => $code } )->count > 0;
         next if exists $dups{$code}->{$value};
         $dups{$code}->{$value} = 1;
 
@@ -579,3 +584,5 @@ sub ParsePatronAttributes {
 
     return \@attributes;
 }
+
+1;