From 81218dcb39e55d54c352393e842fcc24181614fe Mon Sep 17 00:00:00 2001 From: Thibaud Guillot Date: Thu, 6 Apr 2023 17:16:22 +0200 Subject: [PATCH] Bug 32748: Fix item edition when authorised values is restricted When we set an authorised value linked to one library, users who do not belong to this library do not get the value and can therefore overwrite it. This patch add current value to the dropdown list even if it comes from an authorized value of another library. Test plan : 1) Edit an item with one of these fields has an authorized value from another library. 2) On 'edit' you will see AV but with '(Not an authorised value)' string and a warn 3) Apply this patch 4) Refresh page and edit item again 5) Now if the authorised value belongs to another lib and is the current value, you will see it with a tooltip and a warn. But if there are authorised values restricted to another lib and isn't current value, normally it will be not present in the option list. Sponsored-by: BibLibre --- C4/Koha.pm | 29 ++++++++++++------ Koha/UI/Form/Builder/Item.pm | 30 ++++++++++++------- .../prog/en/includes/html_helpers.inc | 18 +++++++++-- 3 files changed, 55 insertions(+), 22 deletions(-) diff --git a/C4/Koha.pm b/C4/Koha.pm index bee096db9fc..f6772217b1b 100644 --- a/C4/Koha.pm +++ b/C4/Koha.pm @@ -509,8 +509,11 @@ C<$opac> If set to a true value, displays OPAC descriptions rather than normal o =cut sub GetAuthorisedValues { - my $category = shift // ''; # optional parameter - my $opac = shift ? 1 : 0; # normalise to be safe + my $category = shift // ''; # optional parameter + my $opac = shift ? 1 : 0; # normalise to be safe + my $options = shift // ''; + my $no_limit; + if ($options) { $no_limit = $options->{'no_limit'}; } #optional parameter to ignore library limitation # Is this cached already? my $branch_limit = C4::Context::mybranch(); @@ -521,26 +524,34 @@ sub GetAuthorisedValues { my @results; my $dbh = C4::Context->dbh; + + my $select_clause = 'SELECT av.*'; + my @where_args; + if ( $branch_limit && $no_limit ) { + $select_clause .= ', IF (branchcode = ? OR branchcode IS NULL, 0, 1) as restricted'; + push @where_args, $branch_limit; + } + my $query = qq{ - SELECT DISTINCT av.* + $select_clause FROM authorised_values av }; $query .= qq{ LEFT JOIN authorised_values_branches ON ( id = av_id ) } if $branch_limit; my @where_strings; - my @where_args; - - if ($category) { + if($category) { push @where_strings, "category = ?"; push @where_args, $category; } - if ($branch_limit) { + + if ( $branch_limit && !defined $no_limit ) { push @where_strings, "( branchcode = ? OR branchcode IS NULL )"; push @where_args, $branch_limit; } - if ( @where_strings > 0 ) { - $query .= " WHERE " . join( " AND ", @where_strings ); + + if(@where_strings > 0) { + $query .= " WHERE " . join(" AND ", @where_strings); } $query .= ' ORDER BY category, ' . ( diff --git a/Koha/UI/Form/Builder/Item.pm b/Koha/UI/Form/Builder/Item.pm index 7a2f54466f6..3b0ec1be115 100644 --- a/Koha/UI/Form/Builder/Item.pm +++ b/Koha/UI/Form/Builder/Item.pm @@ -169,6 +169,7 @@ sub generate_subfield_form { if ( $subfield->{authorised_value} ) { my @authorised_values; my %authorised_lib; + my %restricted_values; # builds list, depending on authorised value... if ( $subfield->{authorised_value} eq "LOST" ) { @@ -202,16 +203,22 @@ sub generate_subfield_form { } } elsif ( $subfield->{authorised_value} eq "itemtypes" ) { push @authorised_values, ""; - my $itemtypes; + my $all_itemtypes = Koha::ItemTypes->search_with_localization; + my $filtered_itemtypes; if ($branch_limit) { - $itemtypes = Koha::ItemTypes->search_with_localization( { branchcode => $branch_limit } ); + $filtered_itemtypes = Koha::ItemTypes->search_with_localization( { branchcode => $branch_limit } ); } else { - $itemtypes = Koha::ItemTypes->search_with_localization; + $filtered_itemtypes = $all_itemtypes; } - while ( my $itemtype = $itemtypes->next ) { + while (my $itemtype = $filtered_itemtypes->next) { push @authorised_values, $itemtype->itemtype; $authorised_lib{ $itemtype->itemtype } = $itemtype->translated_description; } + while (my $itemtype = $all_itemtypes->next) { + if (!grep { $_ eq $itemtype->itemtype } @authorised_values) { + $restricted_values{ $itemtype->itemtype } = $itemtype->translated_description; + } + } if ( !$value && $biblionumber ) { my $itype_sth = $dbh->prepare("SELECT itemtype FROM biblioitems WHERE biblionumber = ?"); @@ -245,10 +252,11 @@ sub generate_subfield_form { #---- "true" authorised value } else { push @authorised_values, qq{}; - my $av = GetAuthorisedValues( $subfield->{authorised_value} ); + my $av = GetAuthorisedValues( $subfield->{authorised_value}, undef, { 'no_limit' => 1 } ); for my $r (@$av) { push @authorised_values, $r->{authorised_value}; $authorised_lib{ $r->{authorised_value} } = $r->{lib}; + $restricted_values{ $r->{authorised_value} } = $r->{restricted}; } } @@ -266,11 +274,12 @@ sub generate_subfield_form { }; } else { $subfield_data{marc_value} = { - type => 'select', - id => "tag_" . $tag . "_subfield_" . $subfieldtag . "_" . $index_subfield, - values => \@authorised_values, - labels => \%authorised_lib, - default => $value, + type => 'select', + id => "tag_" . $tag . "_subfield_" . $subfieldtag . "_" . $index_subfield, + values => \@authorised_values, + labels => \%authorised_lib, + restricted => \%restricted_values, + default => $value, ( ( grep { $_ eq $subfield->{authorised_value} } (qw(branches itemtypes cn_source)) ) ? () @@ -278,6 +287,7 @@ sub generate_subfield_form { ), }; } + } # it's a thesaurus / authority field diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc index 81a5d33da79..f94e3307ab1 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc @@ -124,20 +124,32 @@ [% FOREACH aval IN mv.values %] [% IF aval == mv.default %] [% SET matched = 1 %] - + [% IF mv.restricted.$aval %] + + [% ELSE %] + + [% END %] [% ELSE %] [% IF subfield.IS_LOST_AV && Koha.Preference("ClaimReturnedLostValue") && aval == Koha.Preference("ClaimReturnedLostValue") %] [% ELSIF subfield.IS_LOST_AV && Koha.Preference("BundleLostValue") && aval == Koha.Preference("BundleLostValue") %] [% ELSE %] - + [% UNLESS mv.restricted.$aval %] + + [% END %] [% END %] [% END %] [% END %] [% UNLESS matched || ( ( kohafield == 'items.damaged' || kohafield == 'items.itemlost' || kohafield == 'items.withdrawn' || kohafield == 'items.notforloan' ) && mv.default == '0' ) %] [%# If the current value is not in the authorised list and is not a field where 0 means unset #%] - + [% IF mv.restricted %] + [% FOREACH code IN mv.restricted.keys %] + [% IF mv.default == code %] + + [% END %] + [% END %] + [% END %] [% END %] [% UNLESS matched || ( ( kohafield == 'items.damaged' || kohafield == 'items.itemlost' || kohafield == 'items.withdrawn' || kohafield == 'items.notforloan' ) && mv.default == '0' ) %] -- 2.39.5