From 327e704b9d5bb501e66a54f489b803689de924bc Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
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