From 22a215e58811a5a34420550effc8a4ab3d32b420 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Thu, 25 Mar 2021 17:22:29 +0000 Subject: [PATCH] Bug 28031: (follow-up) Clarify check methods The check methods were positioned under the 'Internal methods' section of the meodule but are used externally. It also felt strange to have a noop or die method. Instead, I propose renaming them to `repeatable_ok` and `unique_ok` and returning a boolean denoting their state. --- Koha/Patron/Attribute.pm | 42 +++++++++++++++++++++------------------- members/memberentry.pl | 3 +-- opac/opac-memberentry.pl | 3 +-- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/Koha/Patron/Attribute.pm b/Koha/Patron/Attribute.pm index e8b0faa673..a8dcd64b92 100644 --- a/Koha/Patron/Attribute.pm +++ b/Koha/Patron/Attribute.pm @@ -51,8 +51,11 @@ sub store { Koha::Exceptions::Patron::Attribute::InvalidType->throw( type => $self->code ) unless $type; - $self->check_repeatable($type); - $self->check_unique_id($type); + Koha::Exceptions::Patron::Attribute::NonRepeatable->throw( attribute => $self ) + unless $self->repeatable_ok($type); + + Koha::Exceptions::Patron::Attribute::UniqueIDConstraint->throw( attribute => $self ) + unless $self->unique_ok($type); return $self->SUPER::store(); } @@ -133,20 +136,18 @@ sub to_api_mapping { }; } -=head2 Internal methods - -=head3 check_repeatable +=head3 repeatable_ok -check_repeatable checks if the attribute type is repeatable and throws and exception -if the attribute type isn't repeatable and there's already an attribute with the same -code for the given patron. +Checks if the attribute type is repeatable and returns a boolean representing +whether storing the current object state would break the repeatable constraint. =cut -sub check_repeatable { +sub repeatable_ok { my ( $self, $type ) = @_; + my $ok = 1; if ( !$type->repeatable ) { my $params = { borrowernumber => $self->borrowernumber, @@ -156,25 +157,24 @@ sub check_repeatable { $params->{id} = { '!=' => $self->id } if $self->in_storage; - Koha::Exceptions::Patron::Attribute::NonRepeatable->throw( attribute => $self ) - if Koha::Patron::Attributes->search($params)->count > 0; + $ok = 0 if Koha::Patron::Attributes->search($params)->count > 0; } - return $self; + return $ok; } -=head3 check_unique_id +=head3 unique_ok -check_unique_id checks if the attribute type is marked as unique id and throws and exception -if the attribute type is a unique id and there's already an attribute with the same -code and value on the database. +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. =cut -sub check_unique_id { +sub unique_ok { my ( $self, $type ) = @_; + my $ok = 1; if ( $type->unique_id ) { my $params = { code => $self->code, attribute => $self->attribute }; @@ -184,13 +184,15 @@ sub check_unique_id { my $unique_count = Koha::Patron::Attributes ->search( $params ) ->count; - Koha::Exceptions::Patron::Attribute::UniqueIDConstraint->throw( attribute => $self ) - if $unique_count > 0; + + $ok = 0 if $unique_count > 0; } - return $self; + return $ok; } +=head2 Internal methods + =head3 _type =cut diff --git a/members/memberentry.pl b/members/memberentry.pl index e6be7854a0..4a1a4316cc 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -411,8 +411,7 @@ 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); - eval {$attribute->check_unique_id}; - if ( $@ ) { + if ( !$attribute->unique_ok ) { push @errors, "ERROR_extended_unique_id_failed"; my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code}); $template->param( diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl index dbfd6f83dd..015c56a3da 100755 --- a/opac/opac-memberentry.pl +++ b/opac/opac-memberentry.pl @@ -107,8 +107,7 @@ my $conflicting_attribute = 0; foreach my $attr (@$attributes) { my $attribute = Koha::Patron::Attribute->new($attr); - eval {$attribute->check_unique_id}; - if ( $@ ) { + if ( !$$attribute->unique_ok ) { my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code}); $template->param( extended_unique_id_failed_code => $attr->{code}, -- 2.20.1