From 91b472b1a0c37e03640903198044a3661edaadf2 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 --- Koha/Item.pm | 12 ++++++ 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, 101 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 b9e35ae3527..35f7c5231ae 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1853,6 +1853,18 @@ sub effective_not_for_loan_status { my $itemtype = $self->itemtype; my $itype_notforloan = $itemtype ? $itemtype->notforloan : undef; 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 diff --git a/Koha/Items.pm b/Koha/Items.pm index 53e056e6909..82df7df3413 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 ba1a8a890e1..f44b3d5ae3c 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 005d75ec231..6433df44900 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 62209a4eda7..2051db24caf 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 5b092763dfa..087dd98f8aa 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 642505fd2fd..a289e189ae7 --- 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 3ac78f14827..29e90c42067 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -2750,7 +2750,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 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)', @@ -4008,7 +4008,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 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 17979f9f379..1a7cd033e90 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 d93cee3b87f..707237449ac 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 291313e44fb..76983576a6a 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 @@ -1076,13 +1076,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 f5175e81ffa..91e18e13dac 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 @@ - + [% 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 cf0f9e2edb8..ef0ac3ea381 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.30.2