Attachment 105201 Details for Bug 21083 – Bug 21083: Handle repeatable patron attributes in batch patron modification tool

[patch] Bug 21083: Handle repeatable patron attributes in batch patron modification tool

Bug-21083-Handle-repeatable-patron-attributes-in-b.patch (text/plain), 4.62 KB, created by Jonathan Druart on 2020-05-21 14:17:37 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2020-05-21 14:17:37 UTC

Size: 4.62 KB

patch

obsolete

>From 5ba5940104b7ee21e7f5e0ab2809aa37e846718a Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Thu, 21 May 2020 16:12:52 +0200
>Subject: [PATCH] Bug 21083: Handle repeatable patron attributes in batch
> patron modification tool
>
>This patch adds the ability to set patron attributes marked as
>repeatable in the batch patron modification tool.
>Prior to this patch they were ignored.
>
>Test plan:
>You should try with several combinaisons and set patron attributes using
>the batch patron modification tool.
>Make sure there is no data lose and that the result is what you expect
>Please detail in a comment what you tested.
>---
> tools/modborrowers.pl | 40 +++++++++++++++++-----------------------
> 1 file changed, 17 insertions(+), 23 deletions(-)
>
>diff --git a/tools/modborrowers.pl b/tools/modborrowers.pl
>index bb41637f9f..6694008ded 100755
>--- a/tools/modborrowers.pl
>+++ b/tools/modborrowers.pl
>@@ -117,9 +117,6 @@ if ( $op eq 'show' ) {
>     my $patron_attribute_types = Koha::Patron::Attribute::Types->search_with_library_limits({}, {}, $library_id);
>     my @patron_categories = Koha::Patron::Categories->search_limited({}, {order_by => ['description']});
>     while ( my $attr_type = $patron_attribute_types->next ) {
>-        # TODO Repeatable attributes are not correctly managed and can cause data lost.
>-        # This should be implemented.
>-        next if $attr_type->repeatable;
>         next if $attr_type->unique_id; # Don't display patron attributes that must be unqiue
>         my $options = $attr_type->authorised_value_category
>             ? GetAuthorisedValues( $attr_type->authorised_value_category )
>@@ -333,9 +330,6 @@ if ( $op eq 'do' ) {
>         $infos->{$field} = dt_from_string($infos->{$field}) if $infos->{$field};
>     }
> 
>-    my @attributes = $input->multi_param('patron_attributes');
>-    my @attr_values = $input->multi_param('patron_attributes_value');
>-
>     my @errors;
>     my @borrowernumbers = $input->multi_param('borrowernumber');
>     # For each borrower selected
>@@ -373,33 +367,33 @@ if ( $op eq 'do' ) {
>         }
> 
>         my $patron = Koha::Patrons->find( $borrowernumber );
>-        my $i=0;
>-        for ( @attributes ) {
>-            next unless $_;
>-            my $attribute;
>-            $attribute->{code} = $_;
>-            $attribute->{attribute} = $attr_values[$i];
>-            my $attr_type = Koha::Patron::Attribute::Types->find($_);
>+        my @attributes = $input->multi_param('patron_attributes');
>+        my @attr_values = $input->multi_param('patron_attributes_value');
>+        my $attributes;
>+        my $i = 0;
>+        for my $code ( @attributes ) {
>+            push @{ $attributes->{$code}->{values} }, shift @attr_values; # Handling repeatables
>+            $attributes->{$code}->{disabled} = grep { $_ eq sprintf("attr%s_value", ++$i) } @disabled;
>+        }
>+
>+        for my $code ( keys %$attributes ) {
>+            my $attr_type = Koha::Patron::Attribute::Types->find($code);
>             # If this borrower is not in the category of this attribute, we don't want to modify this attribute
>-            ++$i and next if $attr_type->category_code and $attr_type->category_code ne $patron->categorycode;
>-            my $valuename = "attr" . $i . "_value";
>-            if ( grep { $_ eq $valuename } @disabled ) {
>+            next if $attr_type->category_code and $attr_type->category_code ne $patron->categorycode;
>+
>+            if ( $attributes->{$code}->{disabled} ) {
>                 # The attribute is disabled, we remove it for this borrower !
>                 eval {
>-                    $patron->get_extended_attribute($attribute->{code})->delete;
>+                    $patron->get_extended_attribute($code)->delete;
>                 };
>                 push @errors, { error => $@ } if $@;
>             } else {
>                 eval {
>-                    # Note:
>-                    # We should not need to filter by branch, but stay on the safe side
>-                    # Repeatable are not supported so we can do that - TODO
>-                    $patron->extended_attributes->search({'me.code' => $attribute->{code}})->filter_by_branch_limitations->delete;
>-                    $patron->add_extended_attribute($attribute);
>+                    $patron->extended_attributes->search({'me.code' => $code})->filter_by_branch_limitations->delete;
>+                    $patron->add_extended_attribute({ code => $code, attribute => $_ }) for @{$attributes->{$code}->{values}};
>                 };
>                 push @errors, { error => $@ } if $@;
>             }
>-            $i++;
>         }
>     }
>     $op = "show_results"; # We have process modifications, the user want to view its
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 21083: 105201 | 133104 | 149749 | 149750 | 152707 | 152708 | 153421 | 153422 | 153423