Attachment 66006 Details for Bug 18361 – Bug 18361: [SQUASHED FOR 16.11] Koha::Objects->find should accept composite primary keys

[patch] Bug 18361: [SQUASHED FOR 16.11] Koha::Objects->find should accept composite primary keys

Bug-18361-SQUASHED-FOR-1611-KohaObjects-find-shoul.patch (text/plain), 6.93 KB, created by Marcel de Rooy on 2017-08-15 12:12:22 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2017-08-15 12:12:22 UTC

Size: 6.93 KB

patch

obsolete

>From 5ba4ee84c52da01f89309819e4a5a784466890ee Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Fri, 31 Mar 2017 13:52:15 +0200
>Subject: [PATCH] Bug 18361: [SQUASHED FOR 16.11] Koha::Objects->find should
> accept composite primary keys
>Content-Type: text/plain; charset=utf-8
>
>Changes the $id parameter to an array. (IssuingRule has three keys.)
>The build_object method in TestBuilder.pm has been adjusted to pass
>multiple primary key values to find.
>
>Also adjusted the POD section to show more clearly that we accept
>the same parameters as DBIx ResultSet does.
>
>Test plan:
>Run t/db_dependent/Koha/Object.t
>Run t/db_dependent/Koha/Objects.t
>Run t/db_dependent/TestBuilder.t
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>
>Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>
>Bug 18361: Additional tests for Koha::Objects->find
>
>Adding a subtest find in t/db_dependent/Koha/Objects.t.
>
>Test plan:
>Run t/db_dependent/Koha/Objects.t
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>
>Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>
>Bug 18361: (QA followup) Add tests for the no params case
>
>This patch adds a test for the trivial case in which no param is passed
>and the ->find method returns undef.
>
>For completeness purposes.
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>
>Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>
>Bug 18361: [QA Follow-up] Resolve warning on undef values supplied
>
>The following warning was raised in Letters.t:
>DBIx::Class::ResultSource::_minimal_valueset_satisfying_constraint():
>NULL/undef values supplied for requested unique constraint 'primary' (NULL
>values in column(s): 'id'). This is almost certainly not what you wanted,
>though you can set DBIC_NULLABLE_KEY_NOWARN to disable this warning.
>
>This warning is triggered by this line in C4/Letters.pm:
>    Koha::SMS::Providers->find( $member->{'sms_provider_id'} );
>As you already guessed, the sms_provider_id returns undef.
>
>Resolved in sub find by testing if there are parameters and if so, they
>should not be all undefined. (In most cases there will be only one
>parameter; but this report is about composite keys.)
>
>Added a trivial test case in Objects.t too.
>
>Test plan:
>Run t/db_dependent/Koha/Object.t
>Run t/db_dependent/Koha/Objects.t
>Run t/db_dependent/Letters.t
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>---
> Koha/Objects.pm               | 17 ++++++++++++-----
> t/db_dependent/Koha/Objects.t | 43 +++++++++++++++++++++++++++++++++++++------
> 2 files changed, 49 insertions(+), 11 deletions(-)
>
>diff --git a/Koha/Objects.pm b/Koha/Objects.pm
>index df2ea90..114aaca 100644
>--- a/Koha/Objects.pm
>+++ b/Koha/Objects.pm
>@@ -20,6 +20,7 @@ package Koha::Objects;
> use Modern::Perl;
> 
> use Carp;
>+use List::MoreUtils qw( none );
> 
> use Koha::Database;
> 
>@@ -70,19 +71,25 @@ sub _new_from_dbic {
> 
> =head3 Koha::Objects->find();
> 
>-my $object = Koha::Objects->find($id);
>-my $object = Koha::Objects->find( { keypart1 => $keypart1, keypart2 => $keypart2 } );
>+Similar to DBIx::Class::ResultSet->find this method accepts:
>+    \%columns_values | @pk_values, { key => $unique_constraint, %attrs }?
>+Strictly speaking, columns_values should only refer to columns under an
>+unique constraint.
>+
>+my $object = Koha::Objects->find( { col1 => $val1, col2 => $val2 } );
>+my $object = Koha::Objects->find( $id );
>+my $object = Koha::Objects->find( $idpart1, $idpart2, $attrs ); # composite PK
> 
> =cut
> 
> sub find {
>-    my ( $self, $id ) = @_;
>+    my ( $self, @pars ) = @_;
> 
>     croak 'Cannot use "->find" in list context' if wantarray;
> 
>-    return unless defined($id);
>+    return if !@pars || none { defined($_) } @pars;
> 
>-    my $result = $self->_resultset()->find($id);
>+    my $result = $self->_resultset()->find( @pars );
> 
>     return unless $result;
> 
>diff --git a/t/db_dependent/Koha/Objects.t b/t/db_dependent/Koha/Objects.t
>index bb1fae6..e4e748d 100644
>--- a/t/db_dependent/Koha/Objects.t
>+++ b/t/db_dependent/Koha/Objects.t
>@@ -19,15 +19,12 @@
> 
> use Modern::Perl;
> 
>-<<<<<<< HEAD
>-use Test::More tests => 10;
>-=======
>-use Test::More tests => 14;
>->>>>>>> 5a26041... Bug 18539: Forbid list context calls for Koha::Objects->find
>+use Test::More tests => 12;
> use Test::Warn;
> 
> use Koha::Authority::Types;
> use Koha::Cities;
>+use Koha::IssuingRules;
> use Koha::Patron::Category;
> use Koha::Patron::Categories;
> use Koha::Patrons;
>@@ -48,13 +45,19 @@ my $borrowernumber_exists = grep { /^borrowernumber$/ } @columns;
> is( $borrowernumber_exists, 1, 'Koha::Objects->columns should return the table columns' );
> 
> subtest 'find' => sub {
>-    plan tests => 2;
>+    plan tests => 4;
>     my $patron = $builder->build({source => 'Borrower'});
>     my $patron_object = Koha::Patrons->find( $patron->{borrowernumber} );
>     is( $patron_object->borrowernumber, $patron->{borrowernumber}, '->find should return the correct object' );
> 
>     eval { my @patrons = Koha::Patrons->find( $patron->{borrowernumber} ); };
>     like( $@, qr|^Cannot use "->find" in list context|, "->find should not be called in list context to avoid side-effects" );
>+
>+    # Test sending undef to find; should not generate a warning
>+    warning_is { $patron = Koha::Patrons->find( undef ); }
>+        "", "Sending undef does not trigger a DBIx warning";
>+    warning_is { $patron = Koha::Patrons->find( undef, undef ); }
>+        "", "Sending two undefs does not trigger a DBIx warning too";
> };
> 
> subtest 'update' => sub {
>@@ -112,6 +115,34 @@ subtest 'new' => sub {
>     Koha::Patron::Categories->find($a_cat_code)->delete;
> };
> 
>+subtest 'find' => sub {
>+    plan tests => 5;
>+
>+    # check find on a single PK
>+    my $patron = $builder->build({ source => 'Borrower' });
>+    is( Koha::Patrons->find($patron->{borrowernumber})->surname,
>+        $patron->{surname}, "Checking an arbitrary patron column after find"
>+    );
>+    # check find with unique column
>+    my $obj = Koha::Patrons->find($patron->{cardnumber}, { key => 'cardnumber' });
>+    is( $obj->borrowernumber, $patron->{borrowernumber},
>+        'Find with unique column and key specified' );
>+    # check find with an additional where clause in the attrs hash
>+    # we do not expect to find something now
>+    is( Koha::Patrons->find(
>+        $patron->{borrowernumber},
>+        { where => { surname => { '!=', $patron->{surname} }}},
>+    ), undef, 'Additional where clause in find call' );
>+
>+    # check find with a composite FK
>+    my $rule = $builder->build({ source => 'Issuingrule' });
>+    my @pk = ( $rule->{branchcode}, $rule->{categorycode}, $rule->{itemtype} );
>+    is( ref(Koha::IssuingRules->find(@pk)), "Koha::IssuingRule",
>+        'Find returned a Koha object for composite primary key' );
>+
>+    is( Koha::Patrons->find(), undef, 'Find returns undef if no params passed' );
>+};
>+
> subtest 'search_related' => sub {
>     plan tests => 8;
>     my $builder   = t::lib::TestBuilder->new;
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 18361: 61754 | 62142 | 62143 | 62395 | 62396 | 62397 | 62909 | 62910 | 62911 | 64339 | 64340 | 64341 | 64347 | 64348 | 64349 | 64372 | 66006