Bugzilla – Attachment 172129 Details for
Bug 35906
Add bookable option on itemtypes
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 35906: Remove preference and add override handling
Bug-35906-Remove-preference-and-add-override-handl.patch (text/plain), 22.34 KB, created by
Thibaud Guillot (thibaud_g)
on 2024-09-27 15:31:37 UTC
(
hide
)
Description:
Bug 35906: Remove preference and add override handling
Filename:
MIME Type:
Creator:
Thibaud Guillot (thibaud_g)
Created:
2024-09-27 15:31:37 UTC
Size:
22.34 KB
patch
obsolete
>From a8c38220a4d04a81a74392eeb3cac3c42a2907ae Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Mon, 26 Feb 2024 16:14:28 +0000 >Subject: [PATCH] Bug 35906: Remove preference and add override handling > >This patch updates the bookable nature of items to allow setting at the >itemtype level and then overriding that setting at item level should you >so wish to do so. > >We also now properly handle the item_level-itypes preference such that >we look at item or biblioitem level appropriately. > >Signed-off-by: Esther <esther@bywatersolutions.com> >--- > Koha/Item.pm | 14 +++++++ > Koha/Items.pm | 15 ++++--- > api/v1/swagger/definitions/item.yaml | 14 +++++-- > api/v1/swagger/paths/biblios.yaml | 1 + > api/v1/swagger/paths/items.yaml | 4 +- > catalogue/updateitem.pl | 3 +- > .../Bug_35906_add-column-bookable-itemtype.pl | 28 +++++++++---- > installer/data/mysql/kohastructure.sql | 4 +- > installer/data/mysql/mandatory/sysprefs.sql | 1 - > .../prog/en/modules/admin/itemtypes.tt | 20 +++++----- > .../admin/preferences/circulation.pref | 7 ---- > .../prog/en/modules/catalogue/moredetail.tt | 17 +++++--- > t/db_dependent/Koha/Items.t | 40 +++++++++---------- > 13 files changed, 103 insertions(+), 65 deletions(-) > mode change 100644 => 100755 installer/data/mysql/atomicupdate/Bug_35906_add-column-bookable-itemtype.pl > >diff --git a/Koha/Item.pm b/Koha/Item.pm >index b325570910..e3ba7ed679 100644 >--- a/Koha/Item.pm >+++ b/Koha/Item.pm >@@ -1874,6 +1874,20 @@ sub effective_not_for_loan_status { > return ( defined($itype_notforloan) && !$self->notforloan ) ? $itype_notforloan : $self->notforloan; > } > >+=head3 effective_bookable >+ >+ my $bookable = $item->effective_bookable; >+ >+Returns the effective bookability of the current item, be that item or itemtype level >+ >+=cut >+ >+sub effective_bookable { >+ my ($self) = @_; >+ >+ return defined( $self->bookable ) ? $self->bookable : $self->itemtype->bookable; >+} >+ > =head3 orders > > my $orders = $item->orders(); >diff --git a/Koha/Items.pm b/Koha/Items.pm >index 53e056e690..82df7df341 100644 >--- a/Koha/Items.pm >+++ b/Koha/Items.pm >@@ -191,12 +191,17 @@ Returns a new resultset, containing only those items that are allowed to be book > sub filter_by_bookable { > my ($self) = @_; > >- my $params = >- C4::Context->preference('item-level_booking') == 1 >- ? { bookable => 1 } >- : { itype => { -in => \'SELECT itemtype FROM itemtypes WHERE bookable = 1' } }; >+ my @bookable_itemtypes = Koha::ItemTypes->search( { bookable => 1 } )->get_column('itemtype'); > >- return $self->search($params); >+ my ( $where, $attr ); >+ if ( C4::Context->preference("item-level_itypes") ) { >+ $where = [ { bookable => 1 }, { bookable => undef, itype => { -in => \@bookable_itemtypes } } ]; >+ } else { >+ $where = [ { bookable => 1 }, { bookable => undef, 'biblioitem.itemtype' => { -in => \@bookable_itemtypes } } ]; >+ $attr = { join => 'biblioitem' }; >+ } >+ >+ return $self->search( $where, $attr ); > } > > =head3 move_to_biblio >diff --git a/api/v1/swagger/definitions/item.yaml b/api/v1/swagger/definitions/item.yaml >index ba1a8a890e..f44b3d5ae3 100644 >--- a/api/v1/swagger/definitions/item.yaml >+++ b/api/v1/swagger/definitions/item.yaml >@@ -24,6 +24,11 @@ properties: > - "null" > description: Information about the acquisition source (it is not really a vendor id) > bookable: >+ type: >+ - boolean >+ - "null" >+ description: Item level bookability override. >+ effective_bookable: > type: boolean > description: Allow bookings on this item. > home_library_id: >@@ -90,7 +95,8 @@ properties: > - string > - "null" > format: date-time >- description: The date and time an item was last marked as withdrawn, NULL if not >+ description: >+ The date and time an item was last marked as withdrawn, NULL if not > withdrawn > callnumber: > type: >@@ -155,14 +161,16 @@ properties: > type: > - string > - "null" >- description: Linked to the CART and PROC temporary locations feature, stores the >+ description: >+ Linked to the CART and PROC temporary locations feature, stores the > permanent shelving location > checked_out_date: > type: > - string > - "null" > format: date >- description: Defines if item is checked out (NULL for not checked out, and checkout >+ description: >+ Defines if item is checked out (NULL for not checked out, and checkout > date for checked out) > call_number_source: > type: >diff --git a/api/v1/swagger/paths/biblios.yaml b/api/v1/swagger/paths/biblios.yaml >index 005d75ec23..6433df4490 100644 >--- a/api/v1/swagger/paths/biblios.yaml >+++ b/api/v1/swagger/paths/biblios.yaml >@@ -479,6 +479,7 @@ > - cover_image_ids > - item_group_item.item_group.description > - serial_item.serial >+ - effective_bookable > collectionFormat: csv > - $ref: "../swagger.yaml#/parameters/match" > - $ref: "../swagger.yaml#/parameters/order_by" >diff --git a/api/v1/swagger/paths/items.yaml b/api/v1/swagger/paths/items.yaml >index 62209a4eda..2051db24ca 100644 >--- a/api/v1/swagger/paths/items.yaml >+++ b/api/v1/swagger/paths/items.yaml >@@ -22,6 +22,7 @@ > enum: > - +strings > - biblio >+ - effective_bookable > collectionFormat: csv > - $ref: "../swagger.yaml#/parameters/match" > - $ref: "../swagger.yaml#/parameters/order_by" >@@ -88,6 +89,7 @@ > type: string > enum: > - +strings >+ - effective_bookable > collectionFormat: csv > consumes: > - application/json >@@ -157,7 +159,7 @@ > * linked_analytics: The item has linked analytic records > * not_same_branch: The item is blocked by independent branches > schema: >- $ref: "../swagger.yaml#/definitions/error" >+ $ref: "../swagger.yaml#/definitions/error" > "500": > description: | > Internal server error. Possible `error_code` attribute values: >diff --git a/catalogue/updateitem.pl b/catalogue/updateitem.pl >index 5b092763df..087dd98f8a 100755 >--- a/catalogue/updateitem.pl >+++ b/catalogue/updateitem.pl >@@ -39,7 +39,7 @@ my $itemnotes_nonpublic=$cgi->param('itemnotes_nonpublic'); > my $withdrawn=$cgi->param('withdrawn'); > my $damaged=$cgi->param('damaged'); > my $exclude_from_local_holds_priority = $cgi->param('exclude_from_local_holds_priority'); >-my $bookable = $cgi->param('bookable'); >+my $bookable = $cgi->param('bookable') // q{}; > > my $confirm=$cgi->param('confirm'); > my $dbh = C4::Context->dbh; >@@ -77,6 +77,7 @@ elsif ( $op eq "cud-set_public_note" ) { # i.e., itemnotes parameter passed from > $item->exclude_from_local_holds_priority($exclude_from_local_holds_priority); > $messages = "updated_exclude_from_local_holds_priority=$exclude_from_local_holds_priority&"; > } elsif ( $op eq "cud-set_bookable" && $bookable ne $item_data_hashref->{'bookable'} ) { >+ undef($bookable) if $bookable eq ""; > $item->bookable($bookable); > } elsif ( $op eq "cud-set_damaged" && $damaged ne $item_data_hashref->{'damaged'}) { > $item->damaged($damaged); >diff --git a/installer/data/mysql/atomicupdate/Bug_35906_add-column-bookable-itemtype.pl b/installer/data/mysql/atomicupdate/Bug_35906_add-column-bookable-itemtype.pl >old mode 100644 >new mode 100755 >index 642505fd2f..a289e189ae >--- a/installer/data/mysql/atomicupdate/Bug_35906_add-column-bookable-itemtype.pl >+++ b/installer/data/mysql/atomicupdate/Bug_35906_add-column-bookable-itemtype.pl >@@ -7,15 +7,29 @@ return { > my ($args) = @_; > my ( $dbh, $out ) = @$args{qw(dbh out)}; > >- $dbh->do(q{ >- INSERT IGNORE INTO systempreferences (`variable`, `value`, `options`, `explanation`, `type`) VALUES ('item-level_booking', 1, '', 'enables item type level for future booking', 'YesNo'); >- }); >- >- $dbh->do(q{ >+ $dbh->do( >+ q{ > ALTER TABLE itemtypes ADD IF NOT EXISTS bookable INT(1) DEFAULT 0 >- }); >+ } >+ ); > >- say $out "Added new system preference 'item-level_booking'"; > say $out "Added column 'itemtypes.bookable'"; >+ >+ $dbh->do( >+ q{ >+ ALTER TABLE items MODIFY COLUMN bookable tinyint(1) DEFAULT NULL COMMENT 'nullable boolean value defining whether this this item is available for bookings or not' >+ } >+ ); >+ >+ say $out "Updated column 'items.bookable' allow nullable"; >+ >+ $dbh->do( >+ q{ >+ ALTER TABLE deleteditems MODIFY COLUMN bookable tinyint(1) DEFAULT NULL COMMENT 'nullable boolean value defining whether this this item is available for bookings or not' >+ } >+ ); >+ >+ say $out "Updated column 'deleteditems.bookable' allow nullable"; >+ > }, > }; >diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql >index 3f0e67be6d..bf2232c95d 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -2754,7 +2754,7 @@ CREATE TABLE `deleteditems` ( > `biblionumber` int(11) NOT NULL DEFAULT 0 COMMENT 'foreign key from biblio table used to link this item to the right bib record', > `biblioitemnumber` int(11) NOT NULL DEFAULT 0 COMMENT 'foreign key from the biblioitems table to link to item to additional information', > `barcode` varchar(20) DEFAULT NULL COMMENT 'item barcode (MARC21 952$p)', >- `bookable` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'boolean value defining whether this item is available for bookings or not', >+ `bookable` tinyint(1) DEFAULT NULL COMMENT 'nullable boolean value defining whether this this item is available for bookings or not', > `dateaccessioned` date DEFAULT NULL COMMENT 'date the item was acquired or added to Koha (MARC21 952$d)', > `booksellerid` longtext DEFAULT NULL COMMENT 'where the item was purchased (MARC21 952$e)', > `homebranch` varchar(10) DEFAULT NULL COMMENT 'foreign key from the branches table for the library that owns this item (MARC21 952$a)', >@@ -4012,7 +4012,7 @@ CREATE TABLE `items` ( > `biblionumber` int(11) NOT NULL DEFAULT 0 COMMENT 'foreign key from biblio table used to link this item to the right bib record', > `biblioitemnumber` int(11) NOT NULL DEFAULT 0 COMMENT 'foreign key from the biblioitems table to link to item to additional information', > `barcode` varchar(20) DEFAULT NULL COMMENT 'item barcode (MARC21 952$p)', >- `bookable` tinyint(1) NOT NULL DEFAULT 0 COMMENT 'boolean value defining whether this item is available for bookings or not', >+ `bookable` tinyint(1) DEFAULT NULL COMMENT 'nullable boolean value defining whether this this item is available for bookings or not', > `dateaccessioned` date DEFAULT NULL COMMENT 'date the item was acquired or added to Koha (MARC21 952$d)', > `booksellerid` longtext DEFAULT NULL COMMENT 'where the item was purchased (MARC21 952$e)', > `homebranch` varchar(10) DEFAULT NULL COMMENT 'foreign key from the branches table for the library that owns this item (MARC21 952$a)', >diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql >index adac85c6b4..20d11d36ab 100644 >--- a/installer/data/mysql/mandatory/sysprefs.sql >+++ b/installer/data/mysql/mandatory/sysprefs.sql >@@ -354,7 +354,6 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` > ('IssueLostItem','alert','Defines what should be done when an attempt is made to issue an item that has been marked as lost.','alert|confirm|nothing','Choice'), > ('IssuingInProcess','0',NULL,'If ON, disables fines if the patron is issuing item that accumulate debt','YesNo'), > ('item-level_itypes','1','','If ON, enables Item-level Itemtype / Issuing Rules','YesNo'), >-('item-level_booking','1','','If ON, enables Item-level for future booking feature','YesNo'), > ('itemBarcodeFallbackSearch','0',NULL,'If set, uses scanned item barcodes as a catalogue search if not found as barcodes','YesNo'), > ('itemBarcodeInputFilter','','whitespace|T-prefix|cuecat|libsuite8|EAN13','If set, allows specification of a item barcode input filter','Choice'), > ('itemcallnumber','',NULL,'The MARC field/subfield that is used to calculate the itemcallnumber (Dewey would be 082ab or 092ab; LOC would be 050ab or 090ab) could be 852hi from an item record','free'), >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt >index df6772ceda..278d10815f 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt >@@ -258,17 +258,15 @@ > [% END %] > <span class="hint">If checked, items will be automatically checked in once they've reached their due date. This feature requires the <code>misc/cronjobs/automatic_checkin.pl</code> cronjob. Ask your system administrator to schedule it.</span> > </li> >- [% IF Koha.Preference('item-level_booking') == 0 %] >- <li> >- <label for="bookable">Bookable: </label> >- [% IF itemtype.bookable %] >- <input type="checkbox" id="bookable" name="bookable" checked="checked" value="1" /> >- [% ELSE %] >- <input type="checkbox" id="bookable" name="bookable" value="1" /> >- [% END %] >- <span class="hint">If checked, all items of this type will be enabled for future bookings.</span> >- </li> >- [% END %] >+ <li> >+ <label for="bookable">Bookable: </label> >+ [% IF itemtype.bookable %] >+ <input type="checkbox" id="bookable" name="bookable" checked="checked" value="1" /> >+ [% ELSE %] >+ <input type="checkbox" id="bookable" name="bookable" value="1" /> >+ [% END %] >+ <span class="hint">If checked, all items of this type will be enabled for future bookings unless overridden at the item level.</span> >+ </li> > <li> > <label for="rentalcharge">Rental charge: </label> > <input type="text" id="rentalcharge" name="rentalcharge" size="10" value="[% itemtype.rentalcharge | $Price on_editing => 1 %]" min="0" /> >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref >index 33f028a923..317e4238ec 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref >@@ -1082,13 +1082,6 @@ Circulation: > 1: "Do" > 0: "Don't" > - place a hold when ordering from a suggestion. >- - >- - Use the type of the >- - pref: item-level_booking >- choices: >- 1: item >- 0: itemtype >- - level to defined witch items can be bookable in future. > Patron restrictions: > - > - pref: PatronRestrictionTypes >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt >index 048fb0b7e9..4bfb260329 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt >@@ -359,21 +359,28 @@ > <input type="hidden" name="biblionumber" value="[% ITEM_DAT.biblionumber | html %]" /> > <input type="hidden" name="biblioitemnumber" value="[% ITEM_DAT.biblioitemnumber | html %]" /> > <input type="hidden" name="itemnumber" value="[% ITEM_DAT.itemnumber | html %]" /> >- <select name="bookable" [% IF ITEM_DAT.effective_itemtype.bookable == 1 && Koha.Preference('item-level_booking') == 0 %]disabled[% END %]> >- [% IF ITEM_DAT.bookable == 1 || ITEM_DAT.effective_itemtype.bookable == 1 %] >+ <select name="bookable"> >+ [% IF ITEM_DAT.bookable == 1 %] >+ <option value="">Follow item type</option> > <option value="1" selected="selected">Yes</option> > <option value="0">No</option> >- [% ELSE %] >+ [% ELSIF ITEM_DAT.bookable == 0 %] >+ <option value="">Follow item type</option> > <option value="1">Yes</option> > <option value="0" selected="selected">No</option> >- [% END %] >+ [% ELSE %] >+ <option value="" selected="selected">Follow item type</option> >+ <option value="1">Yes</option> >+ <option value="0">No</option> >+ [% END %] > </select> > <input type="hidden" name="op" value="cud-set_bookable" /> > <input type="submit" name="submit" class="btn btn-primary btn-xs" value="Update" /> > </form> > [% ELSE %] >- [% IF ITEM_DAT.bookable == 1 %] Yes [% ELSE %] No [% END %] >+ [% IF ITEM_DAT.bookable == 1 %] Yes [% ELSIF ITEM_DAT.bookable == 0 %] No [% ELSE %] Follow item type [% END %] > [% END %] >+ <span class="hint"> Item type bookable: [% IF ITEM_DAT.effective_itemtype.bookable == 1 %]Yes[% ELSE %]No[% END %]</span> > </li> > > </ol> <!-- /.bibliodetails --> >diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t >index cf0f9e2edb..ef0ac3ea38 100755 >--- a/t/db_dependent/Koha/Items.t >+++ b/t/db_dependent/Koha/Items.t >@@ -2112,41 +2112,37 @@ subtest 'filter_by_for_hold' => sub { > }; > > subtest 'filter_by_bookable' => sub { >- plan tests => 3; >+ plan tests => 4; > > $schema->storage->txn_begin; > >- my $biblio = $builder->build_sample_biblio; >+ t::lib::Mocks::mock_preference('item-level_itypes', 0); >+ >+ my $bookable_item_type = $builder->build_object( { class => 'Koha::ItemTypes', value => { bookable => 1 } } ); >+ my $non_bookable_item_type = $builder->build_object( { class => 'Koha::ItemTypes', value => { bookable => 0 } } ); >+ my $biblio = $builder->build_sample_biblio({ itemtype => $bookable_item_type->itemtype }); > > # bookable items >- my $bookable_item1 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber, bookable => 1 } ); >+ my $bookable_item1 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber, itype => $bookable_item_type->itemtype, bookable => 1 } ); > > # not bookable items >- my $non_bookable_item1 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber, bookable => 0 } ); >- my $non_bookable_item2 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber, bookable => 0 } ); >+ my $non_bookable_item1 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber, itype => $non_bookable_item_type->itemtype, bookable => 0 } ); >+ my $non_bookable_item2 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber, itype => $non_bookable_item_type->itemtype, bookable => 0 } ); > >- is( $biblio->items->filter_by_bookable->count, 1, "filter_by_bookable returns the correct number of items" ); >+ is( $biblio->items->filter_by_bookable->count, 1, "filter_by_bookable returns the correct number of items set at item level" ); > is( > $biblio->items->filter_by_bookable->next->itemnumber, $bookable_item1->itemnumber, >- "the correct item is returned from filter_by_bookable" >+ "the correct item is returned from filter_by_bookable at the item level" > ); > >- # unset level booking on item (for itemtype) >- t::lib::Mocks::mock_preference( 'item-level_booking', 0 ); >- >- # test with itemtype directly bookable >- my $item_type = $builder->build_object( { class => 'Koha::ItemTypes', value => { bookable => 1 } } ); >- my $biblio2 = $builder->build_sample_biblio( { itemtype => $item_type->itemtype } ); >+ $bookable_item1->bookable(undef)->store(); >+ $non_bookable_item1->bookable(undef)->store(); >+ $non_bookable_item2->bookable(undef)->store(); >+ $biblio->get_from_storage; >+ is( $biblio->items->filter_by_bookable->count, 3, "filter_by_bookable returns the correct number of items when not set at item level and using biblio level itemtypes" ); > >- # bookable items >- my $bookable_item3 = $builder->build_sample_item( >- { biblionumber => $biblio2->biblionumber, itype => $item_type->itemtype, bookable => 1 } ); >- my $bookable_item4 = $builder->build_sample_item( >- { biblionumber => $biblio2->biblionumber, itype => $item_type->itemtype, bookable => 0 } ); >- >- # items are bookable even if bookable => 0 on item (due to itemtype bookable => 1) >- is( $biblio2->items->filter_by_bookable->count, 2, "filter_by_bookable returns the correct number of items" ); >+ t::lib::Mocks::mock_preference('item-level_itypes', 1); >+ is( $biblio->items->filter_by_bookable->count, 1, "filter_by_bookable returns the correct number of items when not set at item level and using item level itemtypes" ); > > $schema->storage->txn_rollback; >- > }; >-- >2.25.1
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=172129">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 35906
:
161397
|
161638
|
161744
|
161961
|
162242
|
162365
|
162465
|
162466
|
162467
|
162468
|
162469
|
162470
|
162472
|
163291
|
163292
|
163293
|
163294
|
163295
|
163547
|
163548
|
163549
|
163550
|
163551
|
164579
|
164580
|
164581
|
164582
|
164583
|
164584
|
164585
|
165437
|
166866
|
166867
|
166868
|
166869
|
166870
|
166871
|
168120
|
168121
|
168122
|
168160
|
168161
|
168162
|
168163
|
168164
|
170187
|
170188
|
170189
|
170190
|
170191
|
170582
|
170583
|
170584
|
170585
|
170586
|
172127
|
172128
|
172129
|
172130
|
172131
|
172132
|
172941
|
172945
|
172950
|
172951
|
172952
|
172953
|
172954
|
172955
|
172956
|
172957
|
172958
|
172959
|
172960
|
172961
|
173324
|
173325
|
173326
|
173327
|
173328
|
173329
|
173330
|
173331
|
173332
|
173333
|
173334
|
173335
|
173893