From 12092ac2ad1e54ef98378c6cc70cefe91474a484 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 Signed-off-by: Martin Renvoize --- 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 e453a4328a..e8b0faa673 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.20.1