Bugzilla – Attachment 186308 Details for
Bug 32748
Library limitations will cause data loss when editing items
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 32748: Fix item edition when authorised values is restricted
Bug-32748-Fix-item-edition-when-authorised-values-.patch (text/plain), 10.48 KB, created by
Thibaud Guillot (thibaud_g)
on 2025-09-10 07:37:48 UTC
(
hide
)
Description:
Bug 32748: Fix item edition when authorised values is restricted
Filename:
MIME Type:
Creator:
Thibaud Guillot (thibaud_g)
Created:
2025-09-10 07:37:48 UTC
Size:
10.48 KB
patch
obsolete
>From 81218dcb39e55d54c352393e842fcc24181614fe Mon Sep 17 00:00:00 2001 >From: Thibaud Guillot <thibaud.guillot@biblibre.com> >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 %] >- <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 -%] ⚠</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 -%] ⚠</option> >+ [% END %] >+ [% END %] >+ [% END %] > [% END %] > </select> > [% UNLESS matched || ( ( kohafield == 'items.damaged' || kohafield == 'items.itemlost' || kohafield == 'items.withdrawn' || kohafield == 'items.notforloan' ) && mv.default == '0' ) %] >-- >2.39.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 32748
:
148409
|
148491
|
150155
|
151989
|
154039
|
154040
|
154041
|
154042
|
186006
|
186007
|
186008
|
186009
|
186303
|
186307
| 186308 |
186312
|
186313
|
186314
|
186315
|
186316
|
186317