From b443b319988352c21432c688878092d1367b356b 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 Signed-off-by: Lisette Scheer Signed-off-by: Pedro Amorim --- C4/Koha.pm | 29 +++++++++++------ Koha/UI/Form/Builder/Item.pm | 31 +++++++++++++------ .../prog/en/includes/html_helpers.inc | 18 +++++++++-- 3 files changed, 56 insertions(+), 22 deletions(-) diff --git a/C4/Koha.pm b/C4/Koha.pm index fbce23192e..74da5f00ac 100644 --- a/C4/Koha.pm +++ b/C4/Koha.pm @@ -503,8 +503,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(); @@ -515,26 +518,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 907a8ebb2c..6a16e8317e 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,15 +203,22 @@ sub generate_subfield_form { } } elsif ( $subfield->{authorised_value} eq "itemtypes" ) { push @authorised_values, ""; - my $itemtypes = Koha::ItemTypes->new; + my $all_itemtypes = Koha::ItemTypes->search_with_localization; + my $filtered_itemtypes; if ($branch_limit) { - $itemtypes = $itemtypes->search_with_library_limits( {}, {}, $branch_limit ); + $filtered_itemtypes = Koha::ItemTypes->search_with_localization( { branchcode => $branch_limit } ); + } else { + $filtered_itemtypes = $all_itemtypes; } - $itemtypes = $itemtypes->order_by_translated_description; - 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 = ?"); @@ -244,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}; } } @@ -265,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)) ) ? () @@ -277,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 5a555c8f6f..c1e8741063 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers.inc @@ -135,20 +135,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' ) %]