From db6cce8852736802822534eba5c310dc5a841084 Mon Sep 17 00:00:00 2001 From: Martin Renvoize 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 Signed-off-by: Paul Derscheid Signed-off-by: Katrin Fischer --- 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 090fea5514..183f579967 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1917,6 +1917,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 f026836417..ba9d87da48 100644 --- a/Koha/Items.pm +++ b/Koha/Items.pm @@ -197,12 +197,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 c4d8efa6c5..036179cfa9 100644 --- a/api/v1/swagger/paths/biblios.yaml +++ b/api/v1/swagger/paths/biblios.yaml @@ -480,6 +480,7 @@ - item_group_item.item_group.description - serial_item.serial - return_claims + - 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 6fe27117c4..5acc18ad28 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 78c526c46f..49c1aeb59d 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -2755,7 +2755,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)', @@ -4014,7 +4014,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 95997fbc4f..ede74e6d3f 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -355,7 +355,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 %] If checked, items will be automatically checked in once they've reached their due date. This feature requires the misc/cronjobs/automatic_checkin.pl cronjob. Ask your system administrator to schedule it. - [% IF Koha.Preference('item-level_booking') == 0 %] -
  • - - [% IF itemtype.bookable %] - - [% ELSE %] - - [% END %] - If checked, all items of this type will be enabled for future bookings. -
    • - [% END %] +
  • + + [% IF itemtype.bookable %] + + [% ELSE %] + + [% END %] + If checked, all items of this type will be enabled for future bookings unless overridden at the item level. +
  • 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 db55342dbd..f08873bbd0 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 @@ -1089,13 +1089,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 9af3d7f46b..2020f9fb9d 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt @@ -353,21 +353,28 @@ - + [% IF ITEM_DAT.bookable == 1 %] + - [% ELSE %] + [% ELSIF ITEM_DAT.bookable == 0 %] + - [% END %] + [% ELSE %] + + + + [% END %] [% 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 %] + Item type bookable: [% IF ITEM_DAT.effective_itemtype.bookable == 1 %]Yes[% ELSE %]No[% END %]
    • diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t index 5619680113..61a3909a0b 100755 --- a/t/db_dependent/Koha/Items.t +++ b/t/db_dependent/Koha/Items.t @@ -2176,41 +2176,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.39.5