Bugzilla – Attachment 59813 Details for
Bug 13757
Make patron attributes editable in the opac if set to 'editable in OPAC'
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 13757: (QA followup) Fix non-editable attrs on failed save
Bug-13757-QA-followup-Fix-non-editable-attrs-on-fa.patch (text/plain), 10.53 KB, created by
Tomás Cohen Arazi (tcohen)
on 2017-02-02 13:16:35 UTC
(
hide
)
Description:
Bug 13757: (QA followup) Fix non-editable attrs on failed save
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2017-02-02 13:16:35 UTC
Size:
10.53 KB
patch
obsolete
>From b499ca27f376be1b00cd23e88329cde7b42cd858 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >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 <tomascohen@theke.io> >--- > .../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 %] > <fieldset id="aai_[% pa_loo.class %]" class="rows patron-attributes"> >- <legend>[% pa_loo.lib %]</legend> >+ <legend>[% pa_class.lib %]</legend> > [% ELSE %] > <fieldset class="rows patron-attributes"> > <legend>Additional information</legend> >@@ -912,9 +912,8 @@ > <select id="[% form_id %]" name="patron_attribute_value"> > <option value=""></option> > [% FOREACH auth_val IN AuthorisedValues.Get( pa.type.authorised_value_category, 1 ) %] >- [% IF ( auth_val.authorised_value == pa_value.value ) %] >+ [% IF ( auth_val.authorised_value == pa_value ) %] > <option value="[% auth_val.authorised_value %]" selected="selected"> >- [%# Yes, lib; GetAuthorisedValues takes care of intelligently setting this from lib_opac %] > [% auth_val.lib %] > </option> > [% ELSE %] >@@ -925,7 +924,7 @@ > [% END %] > </select> > [% ELSE %] >- <textarea rows="2" cols="30" id="[% form_id %]" name="patron_attribute_value">[% pa_value.value %]</textarea> >+ <textarea rows="2" cols="30" id="[% form_id %]" name="patron_attribute_value">[% pa_value %]</textarea> > [% END %] > <a href="#" class="clear-attribute">Clear</a> > [% 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 %] > </li> >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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 13757
:
42254
|
42626
|
42627
|
47625
|
48040
|
48041
|
48042
|
48043
|
48379
|
48380
|
48381
|
48382
|
56924
|
56925
|
56926
|
56927
|
56928
|
56929
|
57041
|
57042
|
57043
|
57044
|
57045
|
57046
|
58252
|
58253
|
58254
|
58255
|
58351
|
58352
|
58353
|
58354
|
58355
|
58356
|
58357
|
58428
|
58429
|
58581
|
58703
|
58706
|
58707
|
58708
|
58709
|
58710
|
58711
|
58712
|
58713
|
58714
|
58715
|
59376
|
59737
|
59738
|
59739
|
59813
|
59896
|
59897
|
59898
|
60359
|
60360
|
61132
|
61133
|
61134
|
61135
|
61136
|
61137
|
61138
|
61139
|
61140
|
61141
|
61142
|
61143
|
61144
|
61145
|
61146
|
61147
|
61148
|
61149
|
61150
|
61492
|
61493
|
61555
|
61556
|
61557
|
61558
|
61559
|
61560
|
61561
|
61562
|
61563
|
61564
|
61565
|
61566
|
61567
|
61568
|
61569
|
61570
|
61571
|
61572
|
61573
|
61574
|
61575