From 327e704b9d5bb501e66a54f489b803689de924bc Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Thu, 25 Mar 2021 10:02:22 -0300 Subject: [PATCH] Bug 28031: (follow-up) Retrieve type only once The current implementation of store+check_repeatable+check_unique_id notably retrieves the related Koha::Patron::Attribute::Type object three times. This can be easily solved by retrieving it once and reusing. This patch does that. This changes the signature for the helper methods. To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Patron/Attribute.t => SUCCESS: Tests pass! 2. Apply this patch 3. Repeat 1 => SUCCESS: Tests pass! 4. Verify the old _check_repeatable method is not used anywhere $ git grep _check_repeatable => SUCCESS: It is not! 5. Verify check_unique_id is not used anywhere, so no risk changing the signature $ git grep check_unique_id => SUCCESS: It is safe to update it! 6. Sign off :-D --- Koha/Patron/Attribute.pm | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/Koha/Patron/Attribute.pm b/Koha/Patron/Attribute.pm index e453a4328a6..e8b0faa673a 100644 --- a/Koha/Patron/Attribute.pm +++ b/Koha/Patron/Attribute.pm @@ -46,11 +46,13 @@ sub store { my $self = shift; + my $type = $self->type; + Koha::Exceptions::Patron::Attribute::InvalidType->throw( type => $self->code ) - unless Koha::Patron::Attribute::Types->find( $self->code ); + unless $type; - $self->_check_repeatable; - $self->check_unique_id; + $self->check_repeatable($type); + $self->check_unique_id($type); return $self->SUPER::store(); } @@ -133,19 +135,19 @@ sub to_api_mapping { =head2 Internal methods -=head3 _check_repeatable +=head3 check_repeatable -_check_repeatable checks if the attribute type is repeatable and throws and exception +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. =cut -sub _check_repeatable { +sub check_repeatable { - my $self = shift; + my ( $self, $type ) = @_; - if ( !$self->type->repeatable ) { + if ( !$type->repeatable ) { my $params = { borrowernumber => $self->borrowernumber, code => $self->code @@ -171,9 +173,9 @@ code and value on the database. sub check_unique_id { - my $self = shift; + my ( $self, $type ) = @_; - if ( $self->type->unique_id ) { + if ( $type->unique_id ) { my $params = { code => $self->code, attribute => $self->attribute }; $params->{borrowernumber} = { '!=' => $self->borrowernumber } if $self->borrowernumber; -- 2.31.0