From b499ca27f376be1b00cd23e88329cde7b42cd858 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Thu, 2 Feb 2017 10:14:25 -0300 Subject: [PATCH] Bug 13757: (QA followup) Fix non-editable attrs on failed save When a field is not editable but displayable in the OPAC, and you submit an incomplete/wrong update, those attributes are displayed as empty. This patch fixes that. Signed-off-by: Tomas Cohen Arazi --- .../bootstrap/en/modules/opac-memberentry.tt | 11 ++- opac/opac-memberentry.pl | 97 ++++++++++++---------- 2 files changed, 57 insertions(+), 51 deletions(-) diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt index 84e9a6ce9a..231e2bf70c 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt @@ -894,7 +894,7 @@ [% FOREACH pa_class IN patron_attribute_classes %] [% IF pa_class.class %]
- [% pa_loo.lib %] + [% pa_class.lib %] [% ELSE %]
Additional information @@ -912,9 +912,8 @@ [% ELSE %] - + [% END %] Clear [% IF ( pa.type.repeatable ) %] @@ -933,9 +932,9 @@ [% END %] [% ELSE %] [% IF ( pa.type.authorised_value_category ) %] - [% AuthorisedValues.GetByCode( pa.type.authorised_value_category, pa_value.value, 1 ) | html_line_break %] + [% AuthorisedValues.GetByCode( pa.type.authorised_value_category, pa_value, 1 ) | html_line_break %] [% ELSE %] - [% pa_value.value | html_line_break %] + [% pa_value | html_line_break %] [% END %] [% END %] diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl index 4b00623c94..b70bb31ce2 100755 --- a/opac/opac-memberentry.pl +++ b/opac/opac-memberentry.pl @@ -29,14 +29,16 @@ use C4::Output; use C4::Members; use C4::Members::Attributes qw( GetBorrowerAttributes ); use C4::Form::MessagingPreferences; -use Koha::Patrons; -use Koha::Patron::Modification; -use Koha::Patron::Modifications; use C4::Scrubber; use Email::Valid; use Koha::DateUtils; use Koha::Libraries; +use Koha::Patron::Attribute::Types; +use Koha::Patron::Attributes; use Koha::Patron::Images; +use Koha::Patron::Modification; +use Koha::Patron::Modifications; +use Koha::Patrons; use Koha::Token; my $cgi = new CGI; @@ -246,7 +248,7 @@ elsif ( $action eq 'update' ) { secret => md5_base64( Encode::encode( 'UTF-8', C4::Context->config('pass') ) ), }), ); - $template->param( patron_attribute_classes => GeneratePatronAttributesForm( undef, $attributes ) ); + $template->param( patron_attribute_classes => GeneratePatronAttributesForm( $borrowernumber, $attributes ) ); $template->param( action => 'edit' ); } @@ -477,7 +479,6 @@ sub ExtendedAttributesMatch { my @patron_attributes_arr = GetBorrowerAttributes( $borrowernumber, 1 ); my $patron_attributes = $patron_attributes_arr[0]; - if ( scalar @{$entered_attributes} != scalar @{$patron_attributes} ) { return 1; } @@ -493,51 +494,50 @@ sub ExtendedAttributesMatch { return 0; } - sub GeneratePatronAttributesForm { my ( $borrowernumber, $entered_attributes ) = @_; # Get all attribute types and the values for this patron (if applicable) - my @types = C4::Members::AttributeTypes::GetAttributeTypes(); - - if (scalar(@types) == 0) { + my @types = grep { $_->opac_editable() or $_->opac_display } + Koha::Patron::Attribute::Types->search()->as_list(); + if ( scalar(@types) == 0 ) { return []; } - my %attr_values = (); + my @displayable_attributes = grep { $_->opac_display } + Koha::Patron::Attributes->search( { borrowernumber => $borrowernumber } )->as_list; - if ( $borrowernumber ) { - my $attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber); + my %attr_values = (); - # Remap the patron's attributes into a hash of arrayrefs per attribute (depends on - # autovivification) - foreach my $attr (@$attributes) { - push @{ $attr_values{ $attr->{code} } }, $attr; + # Build the attribute values list either from the passed values + # or taken from the patron itself + if ( defined $entered_attributes ) { + foreach my $attr (@$entered_attributes) { + push @{ $attr_values{ $attr->{code} } }, $attr->{value}; } } - - if ( $entered_attributes ) { - foreach my $attr (@$entered_attributes) { - push @{ $attr_values{ $attr->{code} } }, $attr; + elsif ( defined $borrowernumber ) { + my @editable_attributes = grep { $_->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 ) { + push @{ $attr_values{ $attr->code } }, $attr->attribute; + } + # Find all existing classes - my @classes = uniq( map { $_->{class} } @types ); - @classes = sort @classes; + my @classes = sort( uniq( map { $_->class } @types ) ); my %items_by_class; - foreach my $attr_type_desc (@types) { - my $attr_type = C4::Members::AttributeTypes->fetch( $attr_type_desc->{code} ); - # Make sure this attribute should be displayed in the OPAC - next unless ( $attr_type->opac_display() ); - # Then, make sure it either has values or is editable - next unless ( $attr_values{ $attr_type->code() } || $attr_type->opac_editable() ); - + foreach my $attr_type (@types) { push @{ $items_by_class{ $attr_type->class() } }, { type => $attr_type, - # If editable, make sure there's at least one empty entry, to make the template's job easier - values => $attr_values{ $attr_type->code() } || [{}] + # If editable, make sure there's at least one empty entry, + # to make the template's job easier + values => $attr_values{ $attr_type->code() } || [''] }; } @@ -546,14 +546,17 @@ sub GeneratePatronAttributesForm { foreach my $class (@classes) { next unless ( $items_by_class{$class} ); - my $av = Koha::AuthorisedValues->search({ category => 'PA_CLASS', authorised_value => $class }); + my $av = Koha::AuthorisedValues->search( + { category => 'PA_CLASS', authorised_value => $class } ); + my $lib = $av->count ? $av->next->opac_description : $class; - push @class_loop, { + push @class_loop, + { class => $class, items => $items_by_class{$class}, lib => $lib, - }; + }; } return \@class_loop; @@ -567,19 +570,23 @@ sub ParsePatronAttributes { my $ea = each_array( @codes, @values ); my @attributes; - my %dups = (); + + my $delete_candidates = {}; while ( my ( $code, $value ) = $ea->() ) { - # Don't skip if the patron already has attributes with $code, because - # it means we are being requested to remove the attributes. - next - unless defined($value) and $value ne '' - or Koha::Patron::Attributes->search( - { borrowernumber => $borrowernumber, code => $code } )->count > 0; - next if exists $dups{$code}->{$value}; - $dups{$code}->{$value} = 1; - - push @attributes, { code => $code, value => $value }; + if ( !defined($value) or $value eq '' ) { + $delete_candidates->{$code} = 1 + unless $delete_candidates->{$code}; + } else { + # we've got a value + push @attributes, { code => $code, value => $value }; + # 'code' is no longer a delete candidate + delete $delete_candidates->{$code}; + } + } + + foreach my $code ( keys %{ $delete_candidates } ) { + push @attributes, { code => $code, value => '' }; } return \@attributes; -- 2.11.0