Attachment 117616 Details for Bug 27545 – Bug 27545: Keep the location if passed

[patch] Bug 27545: Keep the location if passed

Bug-27545-Keep-the-location-if-passed.patch (text/plain), 9.14 KB, created by Tomás Cohen Arazi (tcohen) on 2021-03-03 15:23:15 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2021-03-03 15:23:15 UTC

Size: 9.14 KB

patch

obsolete

>From 0c52e17667d6d80aeb8c49a8b13aa20239f62c2c Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Mon, 25 Jan 2021 15:15:00 +0100
>Subject: [PATCH] Bug 27545: Keep the location if passed
>
>Change in behaviour here, but expect. We must keep the location value if
>passed to store.
>Same for the permanent_location
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>---
> Koha/Item.pm                |   5 +-
> t/db_dependent/Koha/Items.t | 162 ++++++++++++++++++++++++------------
> 2 files changed, 112 insertions(+), 55 deletions(-)
>
>diff --git a/Koha/Item.pm b/Koha/Item.pm
>index 9ce848d9f04..c0da07c4b94 100644
>--- a/Koha/Item.pm
>+++ b/Koha/Item.pm
>@@ -100,8 +100,9 @@ sub store {
>         }
> 
>         my $default_location = C4::Context->preference('NewItemsDefaultLocation');
>-        if ( $default_location ) {
>-            $self->permanent_location( $self->location ); # FIXME To confirm - even if passed?
>+        unless ( $self->location || !$default_location ) {
>+            $self->permanent_location( $self->location || $default_location )
>+              unless $self->permanent_location;
>             $self->location($default_location);
>         }
> 
>diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t
>index 2a3fdc5b1d2..d1d910b0202 100755
>--- a/t/db_dependent/Koha/Items.t
>+++ b/t/db_dependent/Koha/Items.t
>@@ -92,63 +92,119 @@ subtest 'store' => sub {
>     $item->delete;
> 
>     subtest 'permanent_location' => sub {
>-        plan tests => 7;
>+        plan tests => 2;
> 
>-        my $location = 'my_loc';
>-        my $attributes = {
>-            homebranch    => $library->{branchcode},
>-            holdingbranch => $library->{branchcode},
>-            biblionumber  => $biblio->biblionumber,
>-            location      => $location,
>+        subtest 'location passed to ->store' => sub {
>+            plan tests => 7;
>+
>+            my $location = 'my_loc';
>+            my $attributes = {
>+                homebranch    => $library->{branchcode},
>+                holdingbranch => $library->{branchcode},
>+                biblionumber  => $biblio->biblionumber,
>+                location      => $location,
>+            };
>+
>+            {
>+                # NewItemsDefaultLocation not set
>+                t::lib::Mocks::mock_preference( 'NewItemsDefaultLocation', '' );
>+
>+                # Not passing permanent_location on creating the item
>+                my $item = Koha::Item->new($attributes)->store->get_from_storage;
>+                is( $item->location, $location,
>+                    'location must have been set to location if given' );
>+                is( $item->permanent_location, $item->location,
>+                    'permanent_location must have been set to location if not given' );
>+                $item->delete;
>+
>+                # Passing permanent_location on creating the item
>+                $item = Koha::Item->new(
>+                    { %$attributes, permanent_location => 'perm_loc' } )
>+                  ->store->get_from_storage;
>+                is( $item->permanent_location, 'perm_loc',
>+                    'permanent_location must have been kept if given' );
>+                $item->delete;
>+            }
>+
>+            {
>+                # NewItemsDefaultLocation set
>+                my $default_location = 'default_location';
>+                t::lib::Mocks::mock_preference( 'NewItemsDefaultLocation', $default_location );
>+
>+                # Not passing permanent_location on creating the item
>+                my $item = Koha::Item->new($attributes)->store->get_from_storage;
>+                is( $item->location, $location,
>+                    'location must have been kept if given' );
>+                is( $item->permanent_location, $location,
>+                    'permanent_location must have been set to the location given' );
>+                $item->delete;
>+
>+                # Passing permanent_location on creating the item
>+                $item = Koha::Item->new(
>+                    { %$attributes, permanent_location => 'perm_loc' } )
>+                  ->store->get_from_storage;
>+                is( $item->location, $location,
>+                    'location must have been kept if given' );
>+                is( $item->permanent_location, 'perm_loc',
>+                    'permanent_location must have been kept if given' );
>+                $item->delete;
>+            }
>         };
> 
>-        {
>-            # NewItemsDefaultLocation not set
>-            t::lib::Mocks::mock_preference( 'NewItemsDefaultLocation', '' );
>-
>-            # Not passing permanent_location on creating the item
>-            my $item = Koha::Item->new($attributes)->store->get_from_storage;
>-            is( $item->location, $location,
>-                'location must have been set to location if given' );
>-            is( $item->permanent_location, $item->location,
>-                'permanent_location must have been set to location if not given' );
>-            $item->delete;
>-
>-            # Passing permanent_location on creating the item
>-            $item = Koha::Item->new(
>-                { %$attributes, permanent_location => 'perm_loc' } )
>-              ->store->get_from_storage;
>-            is( $item->permanent_location, 'perm_loc',
>-                'permanent_location must have been kept if given' );
>-            $item->delete;
>-        }
>+        subtest 'location NOT passed to ->store' => sub {
>+            plan tests => 7;
> 
>-        {
>-            # NewItemsDefaultLocation set
>-            my $default_location = 'default_location';
>-            t::lib::Mocks::mock_preference( 'NewItemsDefaultLocation', $default_location );
>-
>-            # Not passing permanent_location on creating the item
>-            my $item = Koha::Item->new($attributes)->store->get_from_storage;
>-            is( $item->location, $default_location,
>-                'location must have been set to default_location even if given' # FIXME this sounds wrong! Must be done in any cases?
>-            );
>-            is( $item->permanent_location, $location,
>-                'permanent_location must have been set to the location given' );
>-            $item->delete;
>-
>-            # Passing permanent_location on creating the item
>-            $item = Koha::Item->new(
>-                { %$attributes, permanent_location => 'perm_loc' } )
>-              ->store->get_from_storage;
>-            is( $item->location, $default_location,
>-                'location must have been set to default_location even if given' # FIXME this sounds wrong! Must be done in any cases?
>-            );
>-            is( $item->permanent_location, $location,
>-                'permanent_location must have been set to the location given'
>-            );
>-            $item->delete;
>-        }
>+            my $attributes = {
>+                homebranch    => $library->{branchcode},
>+                holdingbranch => $library->{branchcode},
>+                biblionumber  => $biblio->biblionumber,
>+            };
>+
>+            {
>+                # NewItemsDefaultLocation not set
>+                t::lib::Mocks::mock_preference( 'NewItemsDefaultLocation', '' );
>+
>+                # Not passing permanent_location on creating the item
>+                my $item = Koha::Item->new($attributes)->store->get_from_storage;
>+                is( $item->location, undef,
>+                    'location not passed and no default, it is undef' );
>+                is( $item->permanent_location, $item->location,
>+                    'permanent_location must have been set to location if not given' );
>+                $item->delete;
>+
>+                # Passing permanent_location on creating the item
>+                $item = Koha::Item->new(
>+                    { %$attributes, permanent_location => 'perm_loc' } )
>+                  ->store->get_from_storage;
>+                is( $item->permanent_location, 'perm_loc',
>+                    'permanent_location must have been kept if given' );
>+                $item->delete;
>+            }
>+
>+            {
>+                # NewItemsDefaultLocation set
>+                my $default_location = 'default_location';
>+                t::lib::Mocks::mock_preference( 'NewItemsDefaultLocation', $default_location );
>+
>+                # Not passing permanent_location on creating the item
>+                my $item = Koha::Item->new($attributes)->store->get_from_storage;
>+                is( $item->location, $default_location,
>+                    'location must have been set to default location if not given' );
>+                is( $item->permanent_location, $default_location,
>+                    'permanent_location must have been set to the default location as well' );
>+                $item->delete;
>+
>+                # Passing permanent_location on creating the item
>+                $item = Koha::Item->new(
>+                    { %$attributes, permanent_location => 'perm_loc' } )
>+                  ->store->get_from_storage;
>+                is( $item->location, $default_location,
>+                    'location must have been set to default location if not given' );
>+                is( $item->permanent_location, 'perm_loc',
>+                    'permanent_location must have been kept if given' );
>+                $item->delete;
>+            }
>+        };
> 
>     };
> 
>-- 
>2.30.1

Actions: View | Diff | Splinter Review

Attachments on bug 27545: 115775 | 115776 | 117615 | 117616 | 117804 | 117805