From b64e6be1c4f3dbdcde41ac8f4766b4d1f962610d Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 29 Mar 2016 14:58:01 +0100 Subject: [PATCH] Bug 16157: Move the selected flag from GetAuthorisedValues to the templates MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit From C4::Koha::GetAuthorisedValues # TODO: the "selected" feature should be replaced by a utility function # somewhere else, it doesn't belong in here. For starters it makes # caching much more complicated. Or just let the UI logic handle it, it's # what it's for. Indeed, it's not a job for a subroutine, the template should take care of that. Note that a perf gain could be won with this patch \o/ Test plan: - Edit an itemtype and check the value of the "Search category" dropdown list - Edit a patron attribute type and check the value of the "Class" dropdown list - Detail for a catalogue record, the Status column should be correctly populated if items are damaged and/or lost - Item details for a catalogue record, the lost, damaged and withdrawn value should be correctly displayed - Edit a patron, the "street type" should be correctly selected - Create a patron attribute type linked to an authorised value list. - Edit a patron, set a value for this attribute, edit it again. The correct value should be selected. - Search for subscriptions. The 'Location' dropdown list should behave correctly (select the entry you have choosen before, etc.) - Edit a subscription, the location dropdown list should select the correct value. - Edit and view a suggestion with a 'reason for suggestion' set (you should have at least 1 OPAC_SUG AV defined) Followed test plan, works as expected Signed-off-by: Marc Véron Signed-off-by: Katrin Fischer Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com --- C4/Koha.pm | 26 ++----------- Koha/Template/Plugin/AuthorisedValues.pm | 4 +- admin/itemtypes.pl | 2 +- admin/patron-attr-types.pl | 18 ++++----- catalogue/detail.pl | 4 +- catalogue/moredetail.pl | 6 +-- .../prog/en/includes/av-build-dropbox.inc | 2 + .../includes/member-main-address-style-de.inc | 2 +- .../includes/member-main-address-style-us.inc | 2 +- .../prog/en/modules/admin/itemtypes.tt | 6 +-- .../en/modules/admin/patron-attr-types.tt | 2 +- .../prog/en/modules/catalogue/detail.tt | 4 +- .../prog/en/modules/catalogue/moredetail.tt | 8 ++-- .../prog/en/modules/members/memberentrygen.tt | 2 +- .../en/modules/serials/serials-collection.tt | 3 +- .../prog/en/modules/serials/serials-edit.tt | 3 +- .../prog/en/modules/serials/serials-search.tt | 38 ++++--------------- .../en/modules/serials/subscription-add.tt | 2 +- .../prog/en/modules/suggestion/suggestion.tt | 20 ++++++++-- .../bootstrap/en/includes/opac-topissues.inc | 4 +- members/memberentry.pl | 2 +- opac/opac-search.pl | 2 +- serials/serials-collection.pl | 12 +----- serials/serials-edit.pl | 8 +--- serials/serials-search.pl | 2 +- serials/subscription-add.pl | 2 +- suggestion/suggestion.pl | 2 +- t/db_dependent/Koha.t | 8 +--- 28 files changed, 74 insertions(+), 122 deletions(-) diff --git a/C4/Koha.pm b/C4/Koha.pm index 36e81dad19..7fe07f1050 100644 --- a/C4/Koha.pm +++ b/C4/Koha.pm @@ -998,35 +998,25 @@ sub GetAuthValCodeFromField { =head2 GetAuthorisedValues - $authvalues = GetAuthorisedValues([$category], [$selected]); + $authvalues = GetAuthorisedValues([$category]); This function returns all authorised values from the'authorised_value' table in a reference to array of hashrefs. C<$category> returns authorised values for just one category (optional). -C<$selected> adds a "selected => 1" entry to the hash if the -authorised_value matches it. B this feature should be considered -deprecated as it may be removed in the future. - C<$opac> If set to a true value, displays OPAC descriptions rather than normal ones when they exist. =cut sub GetAuthorisedValues { - my ( $category, $selected, $opac ) = @_; - - # TODO: the "selected" feature should be replaced by a utility function - # somewhere else, it doesn't belong in here. For starters it makes - # caching much more complicated. Or just let the UI logic handle it, it's - # what it's for. + my ( $category, $opac ) = @_; # Is this cached already? $opac = $opac ? 1 : 0; # normalise to be safe my $branch_limit = C4::Context->userenv ? C4::Context->userenv->{"branch"} : ""; - my $selected_key = defined($selected) ? $selected : ''; my $cache_key = - "AuthorisedValues-$category-$selected_key-$opac-$branch_limit"; + "AuthorisedValues-$category-$opac-$branch_limit"; my $cache = Koha::Cache->get_instance(); my $result = $cache->get_from_cache($cache_key); return $result if $result; @@ -1063,13 +1053,6 @@ sub GetAuthorisedValues { $sth->execute( @where_args ); while (my $data=$sth->fetchrow_hashref) { - if ( defined $selected and $selected eq $data->{authorised_value} ) { - $data->{selected} = 1; - } - else { - $data->{selected} = 0; - } - if ($opac && $data->{lib_opac}) { $data->{lib} = $data->{lib_opac}; } @@ -1077,9 +1060,6 @@ sub GetAuthorisedValues { } $sth->finish; - # We can't cache for long because of that "selected" thing which - # makes it impossible to clear the cache without iterating through every - # value, which sucks. This'll cover this request, and not a whole lot more. $cache->set_in_cache( $cache_key, \@results, { deepcopy => 1, expiry => 5 } ); return \@results; } diff --git a/Koha/Template/Plugin/AuthorisedValues.pm b/Koha/Template/Plugin/AuthorisedValues.pm index 36ddce1b5c..62565e05f7 100644 --- a/Koha/Template/Plugin/AuthorisedValues.pm +++ b/Koha/Template/Plugin/AuthorisedValues.pm @@ -29,8 +29,8 @@ sub GetByCode { } sub Get { - my ( $self, $category, $selected, $opac ) = @_; - return GetAuthorisedValues( $category, $selected, $opac ); + my ( $self, $category, $opac ) = @_; + return GetAuthorisedValues( $category, $opac ); } sub GetAuthValueDropbox { diff --git a/admin/itemtypes.pl b/admin/itemtypes.pl index c762acc76b..c8c224e703 100755 --- a/admin/itemtypes.pl +++ b/admin/itemtypes.pl @@ -59,7 +59,7 @@ undef($sip_media_type) if defined($sip_media_type) and $sip_media_type =~ /^\s*$ if ( $op eq 'add_form' ) { my $itemtype = Koha::ItemTypes->find($itemtype_code); my $imagesets = C4::Koha::getImageSets( checked => ( $itemtype ? $itemtype->imageurl : undef ) ); - my $searchcategory = GetAuthorisedValues("ITEMTYPECAT", ( $itemtype ? $itemtype->searchcategory : '' ) ); + my $searchcategory = GetAuthorisedValues("ITEMTYPECAT"); my $translated_languages = C4::Languages::getTranslatedLanguages( undef , C4::Context->preference('template') ); $template->param( itemtype => $itemtype, diff --git a/admin/patron-attr-types.pl b/admin/patron-attr-types.pl index 8ccf927d42..b233854f74 100755 --- a/admin/patron-attr-types.pl +++ b/admin/patron-attr-types.pl @@ -99,7 +99,7 @@ sub add_attribute_type_form { branches_loop => \@branches_loop, ); authorised_value_category_list($template); - pa_classes($template); + $template->param(classes_val_loop => GetAuthorisedValues( 'PA_CLASS')); } sub error_add_attribute_type_form { @@ -255,7 +255,7 @@ sub edit_attribute_type_form { $template->param(display_checkout_checked => 'checked="checked"'); } authorised_value_category_list($template, $attr_type->authorised_value_category()); - pa_classes( $template, $attr_type->class ); + $template->param(classes_val_loop => GetAuthorisedValues( 'PA_CLASS' )); my $branches = GetBranches; @@ -271,8 +271,11 @@ sub edit_attribute_type_form { } $template->param( branches_loop => \@branches_loop ); - $template->param ( category_code => $attr_type->category_code ); - $template->param ( category_description => $attr_type->category_description ); + $template->param( + category_code => $attr_type->category_code, + category_class => $attr_type->class, + category_description => $attr_type->category_description, + ); $template->param( attribute_type_form => 1, @@ -325,10 +328,3 @@ sub authorised_value_category_list { } $template->param(authorised_value_categories => \@list); } - -sub pa_classes { - my $template = shift; - my $selected = @_ ? shift : ''; - - $template->param(classes_val_loop => GetAuthorisedValues( 'PA_CLASS', $selected ) ); -} diff --git a/catalogue/detail.pl b/catalogue/detail.pl index c4afb62f03..23d6d04aaf 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -218,9 +218,9 @@ foreach my $item (@items) { $item->{datedue} = format_sqldatetime($item->{datedue}); # item damaged, lost, withdrawn loops - $item->{itemlostloop} = GetAuthorisedValues($authvalcode_items_itemlost, $item->{itemlost}) if $authvalcode_items_itemlost; + $item->{itemlostloop} = GetAuthorisedValues($authvalcode_items_itemlost) if $authvalcode_items_itemlost; if ($item->{damaged}) { - $item->{itemdamagedloop} = GetAuthorisedValues($authvalcode_items_damaged, $item->{damaged}) if $authvalcode_items_damaged; + $item->{itemdamagedloop} = GetAuthorisedValues($authvalcode_items_damaged) if $authvalcode_items_damaged; } #get shelf location and collection code description if they are authorised value. # same thing for copy number diff --git a/catalogue/moredetail.pl b/catalogue/moredetail.pl index aa23799d35..82044ed1c5 100755 --- a/catalogue/moredetail.pl +++ b/catalogue/moredetail.pl @@ -137,9 +137,9 @@ foreach ( keys %{$data} ) { ($itemnumber) and @items = (grep {$_->{'itemnumber'} == $itemnumber} @items); foreach my $item (@items){ $item->{object} = Koha::Items->find( $item->{itemnumber} ); - $item->{itemlostloop}= GetAuthorisedValues(GetAuthValCode('items.itemlost',$fw),$item->{itemlost}) if GetAuthValCode('items.itemlost',$fw); - $item->{itemdamagedloop}= GetAuthorisedValues(GetAuthValCode('items.damaged',$fw),$item->{damaged}) if GetAuthValCode('items.damaged',$fw); - $item->{itemwithdrawnloop}= GetAuthorisedValues(GetAuthValCode('items.withdrawn',$fw),$item->{withdrawn}) if GetAuthValCode('items.withdrawn',$fw); + $item->{itemlostloop}= GetAuthorisedValues(GetAuthValCode('items.itemlost',$fw)) if GetAuthValCode('items.itemlost',$fw); + $item->{itemdamagedloop}= GetAuthorisedValues(GetAuthValCode('items.damaged',$fw)) if GetAuthValCode('items.damaged',$fw); + $item->{itemwithdrawnloop}= GetAuthorisedValues(GetAuthValCode('items.withdrawn',$fw)) if GetAuthValCode('items.withdrawn',$fw); $item->{'collection'} = $ccodes->{ $item->{ccode} } if ($ccodes); $item->{'itype'} = $itemtypes->{ $item->{'itype'} }->{'translated_description'}; $item->{'replacementprice'} = sprintf( "%.2f", $item->{'replacementprice'} ); diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/av-build-dropbox.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/av-build-dropbox.inc index 08536857a8..b2d754e3be 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/av-build-dropbox.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/av-build-dropbox.inc @@ -6,6 +6,7 @@ default: the default authorised value to select class: the CSS class of the select element size: the size to use for the input (generated if the authorised value category does not exist). + all: add a "All" entry %] [% SET avs = AuthorisedValues.GetAuthValueDropbox( category, default ) %] @@ -16,6 +17,7 @@ [% IF avs %] [% FOR roadtype IN roadtypes %] - [% IF roadtype.selected %] + [% IF roadtype.authorised_value == streettype %] [% ELSE %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-us.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-us.inc index 32116845be..b7115c690c 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-us.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/member-main-address-style-us.inc @@ -24,7 +24,7 @@ [% FOREACH cat IN searchcategory %] - [% IF cat.selected %] + [% IF cat.authorised_value == itemtype.searchcategory %] @@ -278,8 +278,8 @@ Item types administration diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt index a12d721fc3..e314fa10fe 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt @@ -650,7 +650,7 @@ function verify_images() { [% IF ( item.itemlost ) %] [% IF ( item.itemlostloop ) %] [% FOREACH itemlostloo IN item.itemlostloop %] - [% IF ( itemlostloo.selected ) %] + [% IF itemlostloo.authorised_value == item.itemlost %] [% itemlostloo.lib %] [% END %] [% END %] @@ -666,7 +666,7 @@ function verify_images() { [% IF ( item.damaged ) %] [% IF ( item.itemdamagedloop ) %] [% FOREACH itemdamagedloo IN item.itemdamagedloop %] - [% IF ( itemdamagedloo.selected ) %] + [% IF itemdamagedloo.authorised_value == item.damaged %] [% itemdamagedloo.lib %] [% END %] [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt index d3caf166d9..cea9caedbe 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt @@ -109,7 +109,7 @@ [% FOREACH itemdamagedloo IN ITEM_DAT.itemdamagedloop %] - [% IF ( itemdamagedloo.selected ) %] + [% IF itemdamagedloo.authorised_value == ITEM_DAT.damaged %] [% ELSE %] @@ -173,7 +173,7 @@ [% ELSE %] [% FOREACH itemwithdrawn IN ITEM_DAT.itemwithdrawnloop %] - [% IF ( itemwithdrawn.selected ) %] + [% IF itemwithdrawn.authorised_value == ITEM_DAT.withdrawn %] [% itemwithdrawn.lib %] [% END %] [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt index aeb7f128ee..d54c29ea7f 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt @@ -1046,7 +1046,7 @@ function select_user(borrowernumber, borrower) { - [% IF locations %] -
  • - - -
    • - [% END %] +
  • + + [% PROCESS 'av-build-dropbox.inc' name="location_filter", category="LOC", default=location_filter, all=1 %] +
  • @@ -418,21 +407,10 @@ [% END %]
    • - [% IF locations %] -
  • - - -
    • - [% END %] +
  • + + [% PROCESS 'av-build-dropbox.inc' name="location_filter", category="LOC", default=location_filter, all=1 %] +
  • diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt index a3ea8ef8a5..29ae21a7f1 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt @@ -600,7 +600,7 @@ $(document).ready(function() { [% FOREACH patron_reason_loo IN patron_reason_loop %] - [% IF ( patron_reason_loo.selected ) %][% ELSE %][% END %] - [% END %]
    • [% END %] + [% IF patron_reason_loop %] +
  • + + +
    • + [% END %]
    • diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-topissues.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-topissues.inc index 7324e08ff5..55b57a35e8 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-topissues.inc +++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-topissues.inc @@ -53,8 +53,8 @@