From 7c2ce30ad5ed4a152e5edea0ed9656a525236a3f 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 Signed-off-by: Mathieu Saby --- 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 ac43e3c03ef..e6fe6e75d46 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 c43483895df..958a6f5449d 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 be0af14ce17..d0708f1f6c9 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc @@ -139,20 +139,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