Attachment 119377 Details for Bug 28031 – Bug 28031: (QA follow-up) Adapt controllers and class

[patch] Bug 28031: (QA follow-up) Adapt controllers and class

Bug-28031-QA-follow-up-Adapt-controllers-and-class.patch (text/plain), 6.83 KB, created by Tomás Cohen Arazi (tcohen) on 2021-04-09 12:54:59 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2021-04-09 12:54:59 UTC

Size: 6.83 KB

patch

obsolete

>From 2841837a9c6e8e42c70088da377d129b4c55091c Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>Date: Fri, 9 Apr 2021 09:37:28 -0300
>Subject: [PATCH] Bug 28031: (QA follow-up) Adapt controllers and class
>
>This patch adapts the *memberentry.pl controllers so they pass the $type
>parameter when calling unique_ok. The parameter would still be retrieved
>from the DB if not passed, and then fetched again for reporting back
>errors... So it makes sense to do it once and stick with this approach.
>
>The $type parameter is made mandatory, and exceptions are thrown if
>missing. Tests added only for this exception situation, as the other
>cases are already covered in the store() tests.
>
>Bonus: this patch also fixes a mistake made on a late follow-up for bug
>27857 on the tests.
>
>To test:
>1. Apply this patch
>2. Run:
>   $ kshell
>  k$ prove t/db_dependent/Koha/Patron.t \
>           t/db_dependent/Koha/Patron/Attribute*
>=> SUCCESS: Tests pass!
>3. Sign off :-D
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>---
> Koha/Patron/Attribute.pm               | 22 ++++++++++++++--
> members/memberentry.pl                 |  4 +--
> opac/opac-memberentry.pl               |  4 +--
> t/db_dependent/Koha/Patron.t           |  2 +-
> t/db_dependent/Koha/Patron/Attribute.t | 36 +++++++++++++++++++++++++-
> 5 files changed, 60 insertions(+), 8 deletions(-)
>
>diff --git a/Koha/Patron/Attribute.pm b/Koha/Patron/Attribute.pm
>index f0777a6520a..3a5caa84eab 100644
>--- a/Koha/Patron/Attribute.pm
>+++ b/Koha/Patron/Attribute.pm
>@@ -18,6 +18,7 @@ package Koha::Patron::Attribute;
> use Modern::Perl;
> 
> use Koha::Database;
>+use Koha::Exceptions;
> use Koha::Exceptions::Patron::Attribute;
> use Koha::Patron::Attribute::Types;
> use Koha::AuthorisedValues;
>@@ -46,7 +47,14 @@ sub store {
> 
>     my $self = shift;
> 
>-    my $type = $self->type;
>+    my $type;
>+
>+    if ( $self->in_storage ) {
>+        $type = $self->type;
>+    }
>+    else {
>+        $type = Koha::Patron::Attribute::Types->find( $self->code );
>+    }
> 
>     Koha::Exceptions::Patron::Attribute::InvalidType->throw( type => $self->code )
>         unless $type;
>@@ -72,7 +80,7 @@ sub type {
> 
>     my $self = shift;
> 
>-    return scalar Koha::Patron::Attribute::Types->find( $self->code );
>+    return Koha::Patron::Attribute::Type->_new_from_dbic( $self->_result->code );
> }
> 
> =head3 authorised_value
>@@ -138,6 +146,8 @@ sub to_api_mapping {
> 
> =head3 repeatable_ok
> 
>+    if ( $attr->repeatable_ok( $type ) ) { ... }
>+
> Checks if the attribute type is repeatable and returns a boolean representing
> whether storing the current object state would break the repeatable constraint.
> 
>@@ -147,6 +157,9 @@ sub repeatable_ok {
> 
>     my ( $self, $type ) = @_;
> 
>+    Koha::Exceptions::MissingParameter->throw( "The type parameter is mandatory" )
>+        unless $type;
>+
>     my $ok = 1;
>     if ( !$type->repeatable ) {
>         my $params = {
>@@ -165,6 +178,8 @@ sub repeatable_ok {
> 
> =head3 unique_ok
> 
>+    if ( $attr->unique_ok( $type ) ) { ... }
>+
> 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.
> 
>@@ -174,6 +189,9 @@ sub unique_ok {
> 
>     my ( $self, $type ) = @_;
> 
>+    Koha::Exceptions::MissingParameter->throw( "The type parameter is mandatory" )
>+        unless $type;
>+
>     my $ok = 1;
>     if ( $type->unique_id ) {
>         my $params = { code => $self->code, attribute => $self->attribute };
>diff --git a/members/memberentry.pl b/members/memberentry.pl
>index 6aad0c0dc46..f7c09997bf0 100755
>--- a/members/memberentry.pl
>+++ b/members/memberentry.pl
>@@ -412,9 +412,9 @@ 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);
>-          if ( !$attribute->unique_ok ) {
>+          my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code});
>+          if ( !$attribute->unique_ok( $attr_type ) ) {
>               push @errors, "ERROR_extended_unique_id_failed";
>-              my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code});
>               $template->param(
>                   ERROR_extended_unique_id_failed_code => $attr->{code},
>                   ERROR_extended_unique_id_failed_value => $attr->{attribute},
>diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl
>index 015c56a3dad..250962872f8 100755
>--- a/opac/opac-memberentry.pl
>+++ b/opac/opac-memberentry.pl
>@@ -107,8 +107,8 @@ my $conflicting_attribute = 0;
> 
> foreach my $attr (@$attributes) {
>     my $attribute = Koha::Patron::Attribute->new($attr);
>-    if ( !$$attribute->unique_ok ) {
>-        my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code});
>+    my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code});
>+    if ( !$$attribute->unique_ok( $attr_type ) ) {
>         $template->param(
>             extended_unique_id_failed_code => $attr->{code},
>             extended_unique_id_failed_value => $attr->{attribute},
>diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t
>index a645eb37d94..c5a2cbfd900 100755
>--- a/t/db_dependent/Koha/Patron.t
>+++ b/t/db_dependent/Koha/Patron.t
>@@ -647,7 +647,7 @@ subtest 'extended_attributes' => sub {
>             'Koha::Exceptions::Patron::MissingMandatoryExtendedAttribute',
>             'Exception thrown on missing mandatory attribute type';
> 
>-        is( $@->value, $attribute_type_1->code, 'Exception parameters are correct' );
>+        is( $@->type, $attribute_type_1->code, 'Exception parameters are correct' );
> 
>         is( $patron->extended_attributes->count, 0, 'Extended attributes storing rolled back' );
> 
>diff --git a/t/db_dependent/Koha/Patron/Attribute.t b/t/db_dependent/Koha/Patron/Attribute.t
>index 670bc87cbfd..a05809c4e14 100755
>--- a/t/db_dependent/Koha/Patron/Attribute.t
>+++ b/t/db_dependent/Koha/Patron/Attribute.t
>@@ -19,7 +19,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 2;
>+use Test::More tests => 4;
> 
> use t::lib::TestBuilder;
> use Test::Exception;
>@@ -288,3 +288,37 @@ subtest 'type() tests' => sub {
> 
>     $schema->storage->txn_rollback;
> };
>+
>+subtest 'repeatable_ok() tests' => sub {
>+
>+    plan tests => 1;
>+
>+    my $attr = Koha::Patron::Attribute->new(
>+        {
>+            attribute => 'something',
>+            code      => 'some code'
>+        }
>+    );
>+
>+    throws_ok
>+        { $attr->repeatable_ok(); }
>+        'Koha::Exceptions::MissingParameter',
>+        'Exception on missing type attribute';
>+};
>+
>+subtest 'unique_ok() tests' => sub {
>+
>+    plan tests => 1;
>+
>+    my $attr = Koha::Patron::Attribute->new(
>+        {
>+            attribute => 'something',
>+            code      => 'some code'
>+        }
>+    );
>+
>+    throws_ok
>+        { $attr->unique_ok(); }
>+        'Koha::Exceptions::MissingParameter',
>+        'Exception on missing type attribute';
>+};
>-- 
>2.31.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