Attachment 118830 Details for Bug 28031 – Bug 28031: (follow-up) Retrieve type only once

[patch] Bug 28031: (follow-up) Retrieve type only once

Bug-28031-follow-up-Retrieve-type-only-once.patch (text/plain), 2.78 KB, created by Martin Renvoize (ashimema) on 2021-03-25 17:26:15 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2021-03-25 17:26:15 UTC

Size: 2.78 KB

patch

obsolete

>From 12092ac2ad1e54ef98378c6cc70cefe91474a484 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
>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> 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

Actions: View | Diff | Splinter Review

Attachments on bug 28031: 118792 | 118793 | 118794 | 118828 | 118829 | 118830 | 118831 | 118833 | 118859 | 118860 | 118861 | 118862 | 118863 | 119377 | 119378 | 119379 | 119380 | 119382