From 5ba4ee84c52da01f89309819e4a5a784466890ee Mon Sep 17 00:00:00 2001 From: Marcel de Rooy 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 Signed-off-by: Tomas Cohen Arazi Signed-off-by: Jonathan Druart 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 Signed-off-by: Tomas Cohen Arazi Signed-off-by: Jonathan Druart 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 Signed-off-by: Jonathan Druart 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 --- 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