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 <mathsabypro@gmail.com>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
Thibaud Guillot 2023-04-06 17:16:22 +02:00 committed by Pedro Amorim
parent f2aa1ecb53
commit b443b31998
No known key found for this signature in database
GPG Key ID: 06B75BA7F3894D55
3 changed files with 56 additions and 22 deletions

View File

@ -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, '
. (

View File

@ -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

View File

@ -135,20 +135,32 @@
[% FOREACH aval IN mv.values %]
[% IF aval == mv.default %]
[% SET matched = 1 %]
<option value="[%- aval | html -%]" selected="selected">[%- mv.labels.$aval | html -%]</option>
[% IF mv.restricted.$aval %]
<option value="[%- aval | html -%]" selected="selected" style="float:right" title="This value ([%- mv.labels.$aval | html -%]) is limited to other librairies">[%- mv.labels.$aval | html -%] &#9888;</option>
[% ELSE %]
<option value="[%- aval | html -%]" selected="selected">[%- mv.labels.$aval | html -%]</option>
[% END %]
[% ELSE %]
[% IF subfield.IS_LOST_AV && Koha.Preference("ClaimReturnedLostValue") && aval == Koha.Preference("ClaimReturnedLostValue") %]
<option disabled="disabled" value="[%- aval | html -%]" title="Return claims must be processed from the patron details page">[%- mv.labels.$aval | html -%]</option>
[% ELSIF subfield.IS_LOST_AV && Koha.Preference("BundleLostValue") && aval == Koha.Preference("BundleLostValue") %]
<option disabled="disabled" value="[%- aval | html -%]" title="Bundle losses are set at checkin automatically">[%- mv.labels.$aval | html -%]</option>
[% ELSE %]
<option value="[%- aval | html -%]">[%- mv.labels.$aval | html -%]</option>
[% UNLESS mv.restricted.$aval %]
<option value="[%- aval | html -%]">[%- mv.labels.$aval | html -%]</option>
[% 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 #%]
<option value="[%- mv.default | html -%]" selected="selected">[%- mv.default | html -%] (Not an authorised value)</option>
[% IF mv.restricted %]
[% FOREACH code IN mv.restricted.keys %]
[% IF mv.default == code %]
<option value="[%- code | html -%]" selected="selected" style="float:right" title="This value ([%- mv.restricted.$code | html -%]) is limited to other librairies">[%- mv.restricted.$code | html -%] &#9888;</option>
[% END %]
[% END %]
[% END %]
[% END %]
</select>
[% UNLESS matched || ( ( kohafield == 'items.damaged' || kohafield == 'items.itemlost' || kohafield == 'items.withdrawn' || kohafield == 'items.notforloan' ) && mv.default == '0' ) %]