From 2841837a9c6e8e42c70088da377d129b4c55091c Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 9 Apr 2021 09:37:28 -0300 Subject: [PATCH] Bug 28031: (QA follow-up) Adapt controllers and class This patch adapts the *memberentry.pl controllers so they pass the $type parameter when calling unique_ok. The parameter would still be retrieved from the DB if not passed, and then fetched again for reporting back errors... So it makes sense to do it once and stick with this approach. The $type parameter is made mandatory, and exceptions are thrown if missing. Tests added only for this exception situation, as the other cases are already covered in the store() tests. Bonus: this patch also fixes a mistake made on a late follow-up for bug 27857 on the tests. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Patron.t \ t/db_dependent/Koha/Patron/Attribute* => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi --- Koha/Patron/Attribute.pm | 22 ++++++++++++++-- members/memberentry.pl | 4 +-- opac/opac-memberentry.pl | 4 +-- t/db_dependent/Koha/Patron.t | 2 +- t/db_dependent/Koha/Patron/Attribute.t | 36 +++++++++++++++++++++++++- 5 files changed, 60 insertions(+), 8 deletions(-) diff --git a/Koha/Patron/Attribute.pm b/Koha/Patron/Attribute.pm index f0777a6520a..3a5caa84eab 100644 --- a/Koha/Patron/Attribute.pm +++ b/Koha/Patron/Attribute.pm @@ -18,6 +18,7 @@ package Koha::Patron::Attribute; use Modern::Perl; use Koha::Database; +use Koha::Exceptions; use Koha::Exceptions::Patron::Attribute; use Koha::Patron::Attribute::Types; use Koha::AuthorisedValues; @@ -46,7 +47,14 @@ sub store { my $self = shift; - my $type = $self->type; + my $type; + + if ( $self->in_storage ) { + $type = $self->type; + } + else { + $type = Koha::Patron::Attribute::Types->find( $self->code ); + } Koha::Exceptions::Patron::Attribute::InvalidType->throw( type => $self->code ) unless $type; @@ -72,7 +80,7 @@ sub type { my $self = shift; - return scalar Koha::Patron::Attribute::Types->find( $self->code ); + return Koha::Patron::Attribute::Type->_new_from_dbic( $self->_result->code ); } =head3 authorised_value @@ -138,6 +146,8 @@ sub to_api_mapping { =head3 repeatable_ok + if ( $attr->repeatable_ok( $type ) ) { ... } + Checks if the attribute type is repeatable and returns a boolean representing whether storing the current object state would break the repeatable constraint. @@ -147,6 +157,9 @@ sub repeatable_ok { my ( $self, $type ) = @_; + Koha::Exceptions::MissingParameter->throw( "The type parameter is mandatory" ) + unless $type; + my $ok = 1; if ( !$type->repeatable ) { my $params = { @@ -165,6 +178,8 @@ sub repeatable_ok { =head3 unique_ok + if ( $attr->unique_ok( $type ) ) { ... } + Checks if the attribute type is marked as unique and returns a boolean representing whether storing the current object state would break the unique constraint. @@ -174,6 +189,9 @@ sub unique_ok { my ( $self, $type ) = @_; + Koha::Exceptions::MissingParameter->throw( "The type parameter is mandatory" ) + unless $type; + my $ok = 1; if ( $type->unique_id ) { my $params = { code => $self->code, attribute => $self->attribute }; diff --git a/members/memberentry.pl b/members/memberentry.pl index 6aad0c0dc46..f7c09997bf0 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -412,9 +412,9 @@ if ($op eq 'save' || $op eq 'insert'){ for my $attr ( @$extended_patron_attributes ) { $attr->{borrowernumber} = $borrowernumber if $borrowernumber; my $attribute = Koha::Patron::Attribute->new($attr); - if ( !$attribute->unique_ok ) { + my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code}); + if ( !$attribute->unique_ok( $attr_type ) ) { push @errors, "ERROR_extended_unique_id_failed"; - my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code}); $template->param( ERROR_extended_unique_id_failed_code => $attr->{code}, ERROR_extended_unique_id_failed_value => $attr->{attribute}, diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl index 015c56a3dad..250962872f8 100755 --- a/opac/opac-memberentry.pl +++ b/opac/opac-memberentry.pl @@ -107,8 +107,8 @@ my $conflicting_attribute = 0; foreach my $attr (@$attributes) { my $attribute = Koha::Patron::Attribute->new($attr); - if ( !$$attribute->unique_ok ) { - my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code}); + my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code}); + if ( !$$attribute->unique_ok( $attr_type ) ) { $template->param( extended_unique_id_failed_code => $attr->{code}, extended_unique_id_failed_value => $attr->{attribute}, diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t index a645eb37d94..c5a2cbfd900 100755 --- a/t/db_dependent/Koha/Patron.t +++ b/t/db_dependent/Koha/Patron.t @@ -647,7 +647,7 @@ subtest 'extended_attributes' => sub { 'Koha::Exceptions::Patron::MissingMandatoryExtendedAttribute', 'Exception thrown on missing mandatory attribute type'; - is( $@->value, $attribute_type_1->code, 'Exception parameters are correct' ); + is( $@->type, $attribute_type_1->code, 'Exception parameters are correct' ); is( $patron->extended_attributes->count, 0, 'Extended attributes storing rolled back' ); diff --git a/t/db_dependent/Koha/Patron/Attribute.t b/t/db_dependent/Koha/Patron/Attribute.t index 670bc87cbfd..a05809c4e14 100755 --- a/t/db_dependent/Koha/Patron/Attribute.t +++ b/t/db_dependent/Koha/Patron/Attribute.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 2; +use Test::More tests => 4; use t::lib::TestBuilder; use Test::Exception; @@ -288,3 +288,37 @@ subtest 'type() tests' => sub { $schema->storage->txn_rollback; }; + +subtest 'repeatable_ok() tests' => sub { + + plan tests => 1; + + my $attr = Koha::Patron::Attribute->new( + { + attribute => 'something', + code => 'some code' + } + ); + + throws_ok + { $attr->repeatable_ok(); } + 'Koha::Exceptions::MissingParameter', + 'Exception on missing type attribute'; +}; + +subtest 'unique_ok() tests' => sub { + + plan tests => 1; + + my $attr = Koha::Patron::Attribute->new( + { + attribute => 'something', + code => 'some code' + } + ); + + throws_ok + { $attr->unique_ok(); } + 'Koha::Exceptions::MissingParameter', + 'Exception on missing type attribute'; +}; -- 2.31.1