Attachment 102591 Details for Bug 25037 – Bug 25037: Validate checkout_type in Koha::Checkout->store

[patch] Bug 25037: Validate checkout_type in Koha::Checkout->store

Bug-25037-Validate-checkouttype-in-KohaCheckout-st.patch (text/plain), 4.89 KB, created by Lari Taskula on 2020-04-08 18:34:41 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Lari Taskula

Created: 2020-04-08 18:34:41 UTC

Size: 4.89 KB

patch

obsolete

>From ab07598b74bdadb92520963e38ee729cd229b266 Mon Sep 17 00:00:00 2001
>From: Lari Taskula <lari.taskula@hypernova.fi>
>Date: Thu, 2 Apr 2020 06:33:54 +0000
>Subject: [PATCH] Bug 25037: Validate checkout_type in Koha::Checkout->store
>
>This patch adds a validation for checkout_type value when storing
>a Koha::Checkout
>
>To test:
>1. prove t/db_dependent/Koha/Checkouts.t
>
>Sponsored-by: The National Library of Finland
>---
> Koha/Checkout.pm                | 26 ++++++++++++++++
> Koha/Checkouts.pm               | 20 +++++++++++++
> t/db_dependent/Koha/Checkouts.t | 53 ++++++++++++++++++++++++++++++++-
> 3 files changed, 98 insertions(+), 1 deletion(-)
>
>diff --git a/Koha/Checkout.pm b/Koha/Checkout.pm
>index 39eb1e119d..38e11ec024 100644
>--- a/Koha/Checkout.pm
>+++ b/Koha/Checkout.pm
>@@ -29,6 +29,8 @@ use Koha::Database;
> use Koha::DateUtils;
> use Koha::Items;
> 
>+use Koha::Exceptions::Object;
>+
> use base qw(Koha::Object);
> 
> =head1 NAME
>@@ -103,6 +105,30 @@ sub patron {
>     return Koha::Patron->_new_from_dbic( $patron_rs );
> }
> 
>+=head3 store
>+
>+my $checkout = $checkout->store
>+
>+Overrides the default store subroutine.
>+
>+=cut
>+
>+sub store {
>+    my ( $self ) = @_;
>+
>+    my $checkout_type = $self->checkout_type;
>+    if ( $checkout_type ) {
>+        unless ( Koha::Checkouts::is_valid_checkout_type( $checkout_type ) ) {
>+            Koha::Exceptions::Object::FKConstraint->throw(
>+                broken_fk => 'checkout_type',
>+                value     => $self->checkout_type,
>+            );
>+        }
>+    }
>+
>+    return $self->SUPER::store();
>+}
>+
> =head3 to_api_mapping
> 
> This method returns the mapping for representing a Koha::Checkout object
>diff --git a/Koha/Checkouts.pm b/Koha/Checkouts.pm
>index 0bad230b9e..db037a2ddc 100644
>--- a/Koha/Checkouts.pm
>+++ b/Koha/Checkouts.pm
>@@ -22,6 +22,7 @@ use Modern::Perl;
> use Carp;
> 
> use C4::Context;
>+use Koha::AuthorisedValues;
> use Koha::Checkout;
> use Koha::Database;
> 
>@@ -54,8 +55,27 @@ sub calculate_dropbox_date {
>     return $dropbox_date;
> }
> 
>+=head3 is_valid_checkout_type
>+
>+my $valid_checkout_type = Koha::Checkouts::is_valid_checkout_type( $type );
>+
>+Checks authorised values for valid checkout types.
>+
> =cut
> 
>+sub is_valid_checkout_type {
>+    my ( $type ) = @_;
>+
>+    return 1 if not defined $type; # undefined refers to normal checkout
>+
>+    my $av = Koha::AuthorisedValues->search({
>+        category         => 'CHECKOUT_TYPE',
>+        authorised_value => $type,
>+    });
>+
>+    return $av->count ? 1 : 0;
>+}
>+
> =head2 Name to code mappings
> 
> =head3 $checkout_type
>diff --git a/t/db_dependent/Koha/Checkouts.t b/t/db_dependent/Koha/Checkouts.t
>index e584dae1e6..72d2425749 100644
>--- a/t/db_dependent/Koha/Checkouts.t
>+++ b/t/db_dependent/Koha/Checkouts.t
>@@ -19,10 +19,12 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 9;
>+use Test::More tests => 10;
> use Test::Exception;
> 
> use C4::Circulation;
>+use Koha::AuthorisedValues;
>+use Koha::AuthorisedValueCategories;
> use Koha::Checkouts;
> use Koha::Database;
> use Koha::DateUtils qw( dt_from_string );
>@@ -141,6 +143,55 @@ subtest 'patron' => sub {
>     );
> };
> 
>+subtest 'store' => sub {
>+    plan tests => 1;
>+
>+    subtest 'validate checkout_type' => sub {
>+        plan tests => 3;
>+
>+        # This should exist on clean installations, but just make sure...
>+        unless ( Koha::AuthorisedValueCategories->search( {
>+            category_name => 'CHECKOUT_TYPE'
>+        } )->count ) {
>+            $builder->build_object( {
>+                class=> 'Koha::AuthorisedValueCategories',
>+                value => { category_name => 'CHECKOUT_TYPE' }
>+            } );
>+        }
>+
>+        my $av = $builder->build_object( {
>+            class=> 'Koha::AuthorisedValues',
>+            value => { category => 'CHECKOUT_TYPE' }
>+        } );
>+
>+        my $patron = $builder->build_object( {
>+            class=> 'Koha::Patrons',
>+            value => { branchcode => $library->{branchcode} }
>+        } );
>+        my $item   = $builder->build_object( { class=> 'Koha::Items' } );
>+        my $checkout = Koha::Checkout->new(
>+            {   borrowernumber => $patron->borrowernumber,
>+                itemnumber     => $item->itemnumber,
>+                branchcode     => $library->{branchcode},
>+            }
>+        )->store;
>+
>+        ok( $checkout->issue_id > 0,
>+            'store() was successful with undef checkout_type' );
>+
>+        $checkout->set({ checkout_type => $av->authorised_value })->store;
>+        is( $checkout->checkout_type, $av->authorised_value,
>+            'store() was successful with a valid checkout_type' );
>+
>+        throws_ok {
>+            $checkout->set({ checkout_type => 'NON_EXISTENT' })->store
>+        } 'Koha::Exceptions::Object::FKConstraint',
>+          'store() throws an exception with an invalid checkout_type';
>+
>+        $checkout->delete;
>+    };
>+};
>+
> $retrieved_checkout_1->delete;
> is( Koha::Checkouts->search->count, $nb_of_checkouts + 1, 'Delete should have deleted the checkout' );
> 
>-- 
>2.17.1

Actions: View | Diff | Splinter Review

Attachments on bug 25037: 102259 | 102260 | 102261 | 102262 | 102263 | 102264 | 102265 | 102266 | 102267 | 102268 | 102269 | 102272 | 102413 | 102414 | 102415 | 102416 | 102417 | 102418 | 102419 | 102420 | 102421 | 102422 | 102423 | 102424 | 102586 | 102587 | 102588 | 102589 | 102590 | 102591 | 102592 | 102593 | 102594 | 102595 | 102596 | 102597 | 102598 | 102599 | 102655 | 102656 | 102657 | 102658 | 102659 | 102660 | 102661 | 102662 | 102663 | 102664 | 102665 | 102666 | 102675 | 102676 | 104267 | 104268 | 105638 | 105780 | 105781 | 105782 | 105783 | 105784