gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 28031: (follow-up) Clarify check methods 

The check methods were positioned under the 'Internal methods' section
of the meodule but are used externally.

It also felt strange to have a noop or die method. Instead, I propose
renaming them to `repeatable_ok` and `unique_ok` and returning a
boolean denoting their state.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Martin Renvoize 2021-03-25 17:22:29 +00:00 committed by Jonathan Druart
parent a48cad0f62
commit 7b9b1b0e2a
3 changed files with 24 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
Koha/Patron/Attribute.pm
View File

@ -51,8 +51,11 @@ sub store {
Koha::Exceptions::Patron::Attribute::InvalidType->throw( type => $self->code )
unless $type;
$self->check_repeatable($type);
$self->check_unique_id($type);
Koha::Exceptions::Patron::Attribute::NonRepeatable->throw( attribute => $self )
unless $self->repeatable_ok($type);
Koha::Exceptions::Patron::Attribute::UniqueIDConstraint->throw( attribute => $self )
unless $self->unique_ok($type);
return $self->SUPER::store();
}
@ -133,20 +136,18 @@ sub to_api_mapping {
};
}
=head2 Internal methods
=head3 repeatable_ok
=head3 check_repeatable
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.
Checks if the attribute type is repeatable and returns a boolean representing
whether storing the current object state would break the repeatable constraint.
=cut
sub check_repeatable {
sub repeatable_ok {
my ( $self, $type ) = @_;
my $ok = 1;
if ( !$type->repeatable ) {
my $params = {
borrowernumber => $self->borrowernumber,
@ -156,25 +157,24 @@ sub check_repeatable {
$params->{id} = { '!=' => $self->id }
if $self->in_storage;
Koha::Exceptions::Patron::Attribute::NonRepeatable->throw( attribute => $self )
if Koha::Patron::Attributes->search($params)->count > 0;
$ok = 0 if Koha::Patron::Attributes->search($params)->count > 0;
}
return $self;
return $ok;
}
=head3 check_unique_id
=head3 unique_ok
check_unique_id checks if the attribute type is marked as unique id and throws and exception
if the attribute type is a unique id and there's already an attribute with the same
code and value on the database.
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.
=cut
sub check_unique_id {
sub unique_ok {
my ( $self, $type ) = @_;
my $ok = 1;
if ( $type->unique_id ) {
my $params = { code => $self->code, attribute => $self->attribute };
@ -184,13 +184,15 @@ sub check_unique_id {
my $unique_count = Koha::Patron::Attributes
->search( $params )
->count;
Koha::Exceptions::Patron::Attribute::UniqueIDConstraint->throw( attribute => $self )
if $unique_count > 0;
$ok = 0 if $unique_count > 0;
}
return $self;
return $ok;
}
=head2 Internal methods
=head3 _type
=cut

3
members/memberentry.pl
View File

@ -412,8 +412,7 @@ 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);
eval {$attribute->check_unique_id};
if ( $@ ) {
if ( !$attribute->unique_ok ) {
push @errors, "ERROR_extended_unique_id_failed";
my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code});
$template->param(

3
opac/opac-memberentry.pl
View File

@ -107,8 +107,7 @@ my $conflicting_attribute = 0;
foreach my $attr (@$attributes) {
my $attribute = Koha::Patron::Attribute->new($attr);
eval {$attribute->check_unique_id};
if ( $@ ) {
if ( !$$attribute->unique_ok ) {
my $attr_type = Koha::Patron::Attribute::Types->find($attr->{code});
$template->param(
extended_unique_id_failed_code => $attr->{code},