From c8be11a89adfd9f2f2578effca2e7377de3e2b0b Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 27 Mar 2017 13:07:26 -0300 Subject: [PATCH] Bug 18339: Remove ->opac_editable ->opac_display from Koha::Patron::Attribute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trying to simplify the implementation of 13757, I replaced some legacy code in favour of a Koha::Object-based implementation for the OPAC controller scripts. On doing so I went too far with this methods, as what was needed was a way to retrieve the type of the attribute and ask it about such properties. This patch removes those methods, its tests, and replaces its uses in the codebase like this: $attribute->opac_editable for $attribute->type->opac_editable To test: - Run $ prove t/db_dependent/Koha/Patron/Attributes.t => SUCCESS: Tests still pass - Try changing the opac_editable and opac_display properties and play in the OPAC with those attributes. Verify that everything works as expected. - Sign off :-D Signed-off-by: Marc VĂ©ron --- Koha/Patron/Attribute.pm | 28 ------------ opac/opac-memberentry.pl | 18 +++++--- t/db_dependent/Koha/Patron/Attributes.t | 80 +-------------------------------- 3 files changed, 13 insertions(+), 113 deletions(-) diff --git a/Koha/Patron/Attribute.pm b/Koha/Patron/Attribute.pm index 906fc6d..4c880cb 100644 --- a/Koha/Patron/Attribute.pm +++ b/Koha/Patron/Attribute.pm @@ -51,34 +51,6 @@ sub store { return $self->SUPER::store(); } -=head3 opac_display - - my $attribute = Koha::Patron::Attribute->new({ code => 'a_code', ... }); - if ( $attribute->opac_display ) { ... } - -=cut - -sub opac_display { - - my $self = shift; - - return Koha::Patron::Attribute::Types->find( $self->code )->opac_display; -} - -=head3 opac_editable - - my $attribute = Koha::Patron::Attribute->new({ code => 'a_code', ... }); - if ( $attribute->is_opac_editable ) { ... } - -=cut - -sub opac_editable { - - my $self = shift; - - return Koha::Patron::Attribute::Types->find( $self->code )->opac_editable; -} - =head3 type my $attribute_type = $attribute->type; diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl index 64f9e03..5f4cb4d 100755 --- a/opac/opac-memberentry.pl +++ b/opac/opac-memberentry.pl @@ -473,7 +473,7 @@ sub DelEmptyFields { sub FilterUnchangedAttributes { my ( $borrowernumber, $entered_attributes ) = @_; - my @patron_attributes = grep {$_->opac_editable} Koha::Patron::Attributes->search({ borrowernumber => $borrowernumber })->as_list; + my @patron_attributes = grep {$_->type->opac_editable} Koha::Patron::Attributes->search({ borrowernumber => $borrowernumber })->as_list; my $patron_attribute_types; foreach my $attr (@patron_attributes) { @@ -530,13 +530,19 @@ sub GeneratePatronAttributesForm { my ( $borrowernumber, $entered_attributes ) = @_; # Get all attribute types and the values for this patron (if applicable) - my @types = grep { $_->opac_editable() or $_->opac_display } - Koha::Patron::Attribute::Types->search()->as_list(); + my @types = Koha::Patron::Attribute::Types->search( + { -or => [ + opac_editable => 1, + opac_display => 1 + ] + + } + ); if ( scalar(@types) == 0 ) { return []; } - my @displayable_attributes = grep { $_->opac_display } + my @displayable_attributes = grep { $_->type->opac_display } Koha::Patron::Attributes->search({ borrowernumber => $borrowernumber })->as_list; my %attr_values = (); @@ -549,14 +555,14 @@ sub GeneratePatronAttributesForm { } } elsif ( defined $borrowernumber ) { - my @editable_attributes = grep { $_->opac_editable } @displayable_attributes; + my @editable_attributes = grep { $_->type->opac_editable } @displayable_attributes; foreach my $attr (@editable_attributes) { push @{ $attr_values{ $attr->code } }, $attr->attribute; } } # Add the non-editable attributes (that don't come from the form) - foreach my $attr ( grep { !$_->opac_editable } @displayable_attributes ) { + foreach my $attr ( grep { !$_->type->opac_editable } @displayable_attributes ) { push @{ $attr_values{ $attr->code } }, $attr->attribute; } diff --git a/t/db_dependent/Koha/Patron/Attributes.t b/t/db_dependent/Koha/Patron/Attributes.t index cbb02b9..ff3b659 100644 --- a/t/db_dependent/Koha/Patron/Attributes.t +++ b/t/db_dependent/Koha/Patron/Attributes.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 5; +use Test::More tests => 3; use t::lib::TestBuilder; use Test::Exception; @@ -157,84 +157,6 @@ subtest 'store() unique_id attributes tests' => sub { $schema->storage->txn_rollback; }; -subtest 'opac_display() tests' => sub { - - plan tests => 2; - - $schema->storage->txn_begin; - - my $patron - = $builder->build( { source => 'Borrower' } )->{borrowernumber}; - my $attribute_type_1 = $builder->build( - { source => 'BorrowerAttributeType', - value => { opac_display => 1 } - } - ); - - my $attribute_1 = Koha::Patron::Attribute->new( - { borrowernumber => $patron, - code => $attribute_type_1->{code}, - attribute => $patron - } - ); - is( $attribute_1->opac_display, 1, '->opac_display returns 1' ); - - my $attribute_type_2 = $builder->build( - { source => 'BorrowerAttributeType', - value => { opac_display => 0 } - } - ); - - my $attribute_2 = Koha::Patron::Attribute->new( - { borrowernumber => $patron, - code => $attribute_type_2->{code}, - attribute => $patron - } - ); - is( $attribute_2->opac_display, 0, '->opac_display returns 0' ); - - $schema->storage->txn_rollback; -}; - -subtest 'opac_editable() tests' => sub { - - plan tests => 2; - - $schema->storage->txn_begin; - - my $patron - = $builder->build( { source => 'Borrower' } )->{borrowernumber}; - my $attribute_type_1 = $builder->build( - { source => 'BorrowerAttributeType', - value => { opac_editable => 1 } - } - ); - - my $attribute_1 = Koha::Patron::Attribute->new( - { borrowernumber => $patron, - code => $attribute_type_1->{code}, - attribute => $patron - } - ); - is( $attribute_1->opac_editable, 1, '->opac_editable returns 1' ); - - my $attribute_type_2 = $builder->build( - { source => 'BorrowerAttributeType', - value => { opac_editable => 0 } - } - ); - - my $attribute_2 = Koha::Patron::Attribute->new( - { borrowernumber => $patron, - code => $attribute_type_2->{code}, - attribute => $patron - } - ); - is( $attribute_2->opac_editable, 0, '->opac_editable returns 0' ); - - $schema->storage->txn_rollback; -}; - subtest 'type() tests' => sub { plan tests => 4; -- 2.1.4