Attachment 105385 Details for Bug 25444 – Bug 25444: Simplify the code using a loop

[patch] Bug 25444: Simplify the code using a loop

Bug-25444-Simplify-the-code-using-a-loop.patch (text/plain), 4.90 KB, created by Jonathan Druart on 2020-05-27 10:43:40 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2020-05-27 10:43:40 UTC

Size: 4.90 KB

patch

obsolete

>From 78293cd76821ec379a0c19571f273cc71e5f26e1 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Wed, 27 May 2020 10:25:15 +0200
>Subject: [PATCH] Bug 25444: Simplify the code using a loop
>
>In order to prevent typos or further regressions it is better (I think)
>to have this code into a loop
>
>Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>---
> C4/CourseReserves.pm | 74 +++++++++++---------------------------------
> 1 file changed, 18 insertions(+), 56 deletions(-)
>
>diff --git a/C4/CourseReserves.pm b/C4/CourseReserves.pm
>index ed12690fe8..6bcb342876 100644
>--- a/C4/CourseReserves.pm
>+++ b/C4/CourseReserves.pm
>@@ -541,64 +541,26 @@ sub _UpdateCourseItem {
>     if ( $course_item->is_enabled ) {
>         my $item_fields = {};
> 
>-        # Find newly enabled field and add item value to storage
>-        if ( $params{itype_enabled} && !$course_item->itype_enabled ) {
>-            $enabled{itype_storage} = $item->itype;
>-            $item_fields->{itype} = $params{itype};
>-        }
>-        # Find newly disabled field and copy the storage value to the item, unset storage value
>-        elsif ( !$params{itype_enabled} && $course_item->itype_enabled ) {
>-            $item_fields->{itype} = $course_item->itype_storage;
>-            $enabled{itype_storage} = undef;
>-        }
>-        # The field was already enabled, copy the incoming value to the item.
>-        # The "original" ( when not on course reserve ) value is already in the storage field
>-        elsif ( $course_item->itype_enabled) {
>-            $item_fields->{itype} = $params{itype};
>-        }
>-
>-        if ( $params{ccode_enabled} && !$course_item->ccode_enabled ) {
>-            $enabled{ccode_storage} = $item->ccode;
>-            $item_fields->{ccode} = $params{ccode};
>-        }
>-        elsif ( !$params{ccode_enabled} && $course_item->ccode_enabled ) {
>-            $item_fields->{ccode} = $course_item->ccode_storage;
>-            $enabled{ccode_storage} = undef;
>-        } elsif ( $course_item->ccode_enabled) {
>-            $item_fields->{ccode} = $params{ccode};
>-        }
>+        for my $field ( qw( itype ccode location homebranch holdingbranch ) ) {
> 
>-        if ( $params{location_enabled} && !$course_item->location_enabled ) {
>-            $enabled{location_storage} = $item->location;
>-            $item_fields->{location} = $params{location};
>-        }
>-        elsif ( !$params{location_enabled} && $course_item->location_enabled ) {
>-            $item_fields->{location} = $course_item->location_storage;
>-            $enabled{location_storage} = undef;
>-        } elsif ( $course_item->location_enabled) {
>-            $item_fields->{location} = $params{location};
>-        }
>+            my $field_enabled = sprintf "%s_enabled", $field;
>+            my $field_storage = sprintf "%s_storage", $field;
> 
>-        if ( $params{homebranch_enabled} && !$course_item->homebranch_enabled ) {
>-            $enabled{homebranch_storage} = $item->homebranch;
>-            $item_fields->{homebranch} = $params{homebranch};
>-        }
>-        elsif ( !$params{homebranch_enabled} && $course_item->homebranch_enabled ) {
>-            $item_fields->{homebranch} = $course_item->homebranch_storage;
>-            $enabled{homebranch_storage} = undef;
>-        } elsif ( $course_item->homebranch_enabled) {
>-            $item_fields->{homebranch} = $params{homebranch};
>-        }
>-
>-        if ( $params{holdingbranch_enabled} && !$course_item->holdingbranch_enabled ) {
>-            $enabled{holdingbranch_storage} = $item->holdingbranch;
>-            $item_fields->{holdingbranch} = $params{holdingbranch};
>-        }
>-        elsif ( !$params{holdingbranch_enabled} && $course_item->holdingbranch_enabled ) {
>-            $item_fields->{holdingbranch} = $course_item->holdingbranch_storage;
>-            $enabled{holdingbranch_storage} = undef;
>-        } elsif ( $course_item->holdingbranch_enabled) {
>-            $item_fields->{holdingbranch} = $params{holdingbranch};
>+            # Find newly enabled field and add item value to storage
>+            if ( $params{$field_enabled} && !$course_item->$field_enabled ) {
>+                $enabled{$field_storage} = $item->$field;
>+                $item_fields->{$field}   = $params{$field};
>+            }
>+            # Find newly disabled field and copy the storage value to the item, unset storage value
>+            elsif ( !$params{$field_enabled} && $course_item->$field_enabled ) {
>+                $item_fields->{$field}   = $course_item->$field_storage;
>+                $enabled{$field_storage} = undef;
>+            }
>+            # The field was already enabled, copy the incoming value to the item.
>+            # The "original" ( when not on course reserve ) value is already in the storage field
>+            elsif ( $course_item->$field_enabled) {
>+                $item_fields->{$field} = $params{$field};
>+            }
>         }
> 
>         $item->set( $item_fields )->store
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 25444: 105361 | 105362 | 105363 | 105364 | 105370 | 105371 | 105378 | 105379 | 105383 | 105384 | 105385 | 105386