From 9995cdf17a2e7432f7be54f3dfd00c84a12942aa Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Thu, 23 Sep 2021 10:01:23 +0100 Subject: [PATCH] Bug 28854: Expose functionality to attach items to bundles This patch adds methods the the Koha::Item object for managing item bundling operations and then exposes those methods via the REST API. We include the new `BundleNotLoanValue` preference for setting not for loan values when an item is added to a bundle. Finally, we expose bundle management via the catalogue details page. Test plan: 0) Apply patches up to this point 1) Creating a new bundle * Add a new bib record * Mark the bib record as a 'collection' type by setting leader position 7 to 'c' * Add a new item to this bib record * You should see a new 'Manage bundle' button available in the 'Actions' column of the Holdings table. * Clicking 'Manage bundle' should expand the table to include a new row directly beneath this one. * Use the new 'Add to bundle' button that appears in this row to trigger a modal that allows entering the barcode of items you wish to add to the bundle * Upon closing the modal, the bundle content table should reload and contain your newly associated items. * You can subsequently remove an item from a bundle using the new 'Remove' button. 2) Not for loan * Items that have been added into a bundle should now appear as 'Not for loan' from their original biblio record and note which bundle they belong to. 3) Error cases * Try adding an item that already belongs to a bundle to another bundle: Note an error is displayed in the modal form. TODO: Add Object Tests + API Tests https://bugs.koha-community.org/show_bug.cgi?id=24454 --- Koha/Item.pm | 165 ++++++++++++ Koha/REST/V1/Items.pm | 121 +++++++++ api/v1/swagger/definitions.json | 3 + api/v1/swagger/definitions/bundle_link.json | 14 + api/v1/swagger/definitions/item.json | 2 + api/v1/swagger/paths.json | 9 + api/v1/swagger/paths/items.json | 240 ++++++++++++++++++ catalogue/detail.pl | 14 + .../prog/css/src/staff-global.scss | 4 + .../admin/preferences/cataloguing.pref | 4 + .../prog/en/modules/catalogue/detail.tt | 228 ++++++++++++++++- 11 files changed, 803 insertions(+), 1 deletion(-) create mode 100644 api/v1/swagger/definitions/bundle_link.json diff --git a/Koha/Item.pm b/Koha/Item.pm index 60d80a5c3eb..2a69ab13d00 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -21,6 +21,7 @@ use Modern::Perl; use List::MoreUtils qw( any ); use Data::Dumper qw( Dumper ); +use Try::Tiny qw( catch try ); use Koha::Database; use Koha::DateUtils qw( dt_from_string ); @@ -1272,6 +1273,170 @@ sub move_to_biblio { return $to_biblionumber; } +=head3 bundle_items + + my $bundle_items = $item->bundle_items; + +Returns the items associated with this bundle + +=cut + +sub bundle_items { + my ($self) = @_; + + if ( !$self->{_bundle_items_cached} ) { + my $bundle_items = Koha::Items->search( + { 'item_bundles_item.host' => $self->itemnumber }, + { join => 'item_bundles_item' } ); + $self->{_bundle_items} = $bundle_items; + $self->{_bundle_items_cached} = 1; + } + + return $self->{_bundle_items}; +} + +=head3 is_bundle + + my $is_bundle = $item->is_bundle; + +Returns whether the item is a bundle or not + +=cut + +sub is_bundle { + my ($self) = @_; + return $self->bundle_items->count ? 1 : 0; +} + +=head3 bundle_host + + my $bundle = $item->bundle_host; + +Returns the bundle item this item is attached to + +=cut + +sub bundle_host { + my ($self) = @_; + + if ( !$self->{_bundle_host_cached} ) { + my $bundle_item_rs = $self->_result->item_bundles_item; + $self->{_bundle_host} = + $bundle_item_rs + ? Koha::Item->_new_from_dbic($bundle_item_rs->host) + : undef; + $self->{_bundle_host_cached} = 1; + } + + return $self->{_bundle_host}; +} + +=head3 in_bundle + + my $in_bundle = $item->in_bundle; + +Returns whether this item is currently in a bundle + +=cut + +sub in_bundle { + my ($self) = @_; + return $self->bundle_host ? 1 : 0; +} + +=head3 add_to_bundle + + my $link = $item->add_to_bundle($bundle_item); + +Adds the bundle_item passed to this item + +=cut + +sub add_to_bundle { + my ( $self, $bundle_item ) = @_; + + my $schema = Koha::Database->new->schema; + + my $BundleNotLoanValue = C4::Context->preference('BundleNotLoanValue'); + + try { + $schema->txn_do( + sub { + $self->_result->add_to_item_bundles_hosts( + { item => $bundle_item->itemnumber } ); + + $bundle_item->notforloan($BundleNotLoanValue)->store(); + } + ); + } + catch { + + # FIXME: See if we can move the below copy/paste from Koha::Object::store into it's own class and catch at a lower level in the Schema instantiation.. take inspiration fro DBIx::Error + if ( ref($_) eq 'DBIx::Class::Exception' ) { + warn $_->{msg}; + if ( $_->{msg} =~ /Cannot add or update a child row: a foreign key constraint fails/ ) { + # FK constraints + # FIXME: MySQL error, if we support more DB engines we should implement this for each + if ( $_->{msg} =~ /FOREIGN KEY \(`(?.*?)`\)/ ) { + Koha::Exceptions::Object::FKConstraint->throw( + error => 'Broken FK constraint', + broken_fk => $+{column} + ); + } + } + elsif ( + $_->{msg} =~ /Duplicate entry '(.*?)' for key '(?.*?)'/ ) + { + Koha::Exceptions::Object::DuplicateID->throw( + error => 'Duplicate ID', + duplicate_id => $+{key} + ); + } + elsif ( $_->{msg} =~ +/Incorrect (?\w+) value: '(?.*)' for column \W?(?\S+)/ + ) + { # The optional \W in the regex might be a quote or backtick + my $type = $+{type}; + my $value = $+{value}; + my $property = $+{property}; + $property =~ s/['`]//g; + Koha::Exceptions::Object::BadValue->throw( + type => $type, + value => $value, + property => $property =~ /(\w+\.\w+)$/ + ? $1 + : $property + , # results in table.column without quotes or backtics + ); + } + + # Catch-all for foreign key breakages. It will help find other use cases + $_->rethrow(); + } + else { + $_; + } + }; +} + +=head3 remove_from_bundle + +Remove this item from any bundle it may have been attached to. + +=cut + +sub remove_from_bundle { + my ($self) = @_; + + my $bundle_item_rs = $self->_result->item_bundles_item; + if ( $bundle_item_rs ) { + $bundle_item_rs->delete; + $self->notforloan(0)->store(); + return 1; + } + return 0; +} + =head2 Internal methods =head3 _after_item_action_hooks diff --git a/Koha/REST/V1/Items.pm b/Koha/REST/V1/Items.pm index 1887bce5145..61150013e16 100644 --- a/Koha/REST/V1/Items.pm +++ b/Koha/REST/V1/Items.pm @@ -152,4 +152,125 @@ sub pickup_locations { }; } +=head3 bundled_items + +Controller function that handles bundled_items Koha::Item objects + +=cut + +sub bundled_items { + my $c = shift->openapi->valid_input or return; + + my $item_id = $c->validation->param('item_id'); + my $item = Koha::Items->find( $item_id ); + + unless ($item) { + return $c->render( + status => 404, + openapi => { error => "Item not found" } + ); + } + + return try { + my $items_set = $item->bundle_items; + my $items = $c->objects->search( $items_set ); + return $c->render( + status => 200, + openapi => $items + ); + } + catch { + $c->unhandled_exception($_); + }; +} + +=head3 add_to_bundle + +Controller function that handles adding items to this bundle + +=cut + +sub add_to_bundle { + my $c = shift->openapi->valid_input or return; + + my $item_id = $c->validation->param('item_id'); + my $item = Koha::Items->find( $item_id ); + + unless ($item) { + return $c->render( + status => 404, + openapi => { error => "Item not found" } + ); + } + + + my $bundle_item_id = $c->validation->param('body')->{'external_id'}; + my $bundle_item = Koha::Items->find( { barcode => $bundle_item_id } ); + + unless ($bundle_item) { + return $c->render( + status => 404, + openapi => { error => "Bundle item not found" } + ); + } + + return try { + my $link = $item->add_to_bundle($bundle_item); + return $c->render( + status => 201, + openapi => $bundle_item + ); + } + catch { + if ( ref($_) eq 'Koha::Exceptions::Object::DuplicateID' ) { + return $c->render( + status => 409, + openapi => { + error => 'Item is already bundled', + key => $_->duplicate_id + } + ); + } + else { + $c->unhandled_exception($_); + } + }; +} + +=head3 remove_from_bundle + +Controller function that handles removing items from this bundle + +=cut + +sub remove_from_bundle { + my $c = shift->openapi->valid_input or return; + + my $item_id = $c->validation->param('item_id'); + my $item = Koha::Items->find( $item_id ); + + unless ($item) { + return $c->render( + status => 404, + openapi => { error => "Item not found" } + ); + } + + my $bundle_item_id = $c->validation->param('bundled_item_id'); + my $bundle_item = Koha::Items->find( { itemnumber => $bundle_item_id } ); + + unless ($bundle_item) { + return $c->render( + status => 404, + openapi => { error => "Bundle item not found" } + ); + } + + $bundle_item->remove_from_bundle; + return $c->render( + status => 204, + openapi => q{} + ); +} + 1; diff --git a/api/v1/swagger/definitions.json b/api/v1/swagger/definitions.json index 402eed6b256..ddd65a90e53 100644 --- a/api/v1/swagger/definitions.json +++ b/api/v1/swagger/definitions.json @@ -5,6 +5,9 @@ "basket": { "$ref": "definitions/basket.json" }, + "bundle_link": { + "$ref": "definitions/bundle_link.json" + }, "cashup": { "$ref": "definitions/cashup.json" }, diff --git a/api/v1/swagger/definitions/bundle_link.json b/api/v1/swagger/definitions/bundle_link.json new file mode 100644 index 00000000000..7b1274dfe4f --- /dev/null +++ b/api/v1/swagger/definitions/bundle_link.json @@ -0,0 +1,14 @@ +{ + "type": "object", + "properties": { + "item_id": { + "type": ["integer", "null"], + "description": "Internal item identifier" + }, + "external_id": { + "type": ["string", "null"], + "description": "Item barcode" + } + }, + "additionalProperties": false +} diff --git a/api/v1/swagger/definitions/item.json b/api/v1/swagger/definitions/item.json index 14c110714e3..61de43a6f00 100644 --- a/api/v1/swagger/definitions/item.json +++ b/api/v1/swagger/definitions/item.json @@ -9,6 +9,8 @@ "type": "integer", "description": "Internal identifier for the parent bibliographic record" }, + "biblio": { + }, "external_id": { "type": ["string", "null"], "description": "The item's barcode" diff --git a/api/v1/swagger/paths.json b/api/v1/swagger/paths.json index 23f420cc05f..fa30c86624a 100644 --- a/api/v1/swagger/paths.json +++ b/api/v1/swagger/paths.json @@ -83,6 +83,15 @@ "/items/{item_id}": { "$ref": "paths/items.json#/~1items~1{item_id}" }, + "/items/{item_id}/bundled_items": { + "$ref": "paths/items.json#/~1items~1{item_id}~1bundled_items" + }, + "/items/{item_id}/bundled_items/item": { + "$ref": "paths/items.json#/~1items~1{item_id}~1bundled_items~1item" + }, + "/items/{item_id}/bundled_items/item/{bundled_item_id}": { + "$ref": "paths/items.json#/~1items~1{item_id}~1bundled_items~1item~1{bundled_item_id}" + }, "/items/{item_id}/pickup_locations": { "$ref": "paths/items.json#/~1items~1{item_id}~1pickup_locations" }, diff --git a/api/v1/swagger/paths/items.json b/api/v1/swagger/paths/items.json index 7c18f625ee7..a0a2caf9d7b 100644 --- a/api/v1/swagger/paths/items.json +++ b/api/v1/swagger/paths/items.json @@ -127,6 +127,246 @@ } } }, + "/items/{item_id}/bundled_items": { + "get": { + "x-mojo-to": "Items#bundled_items", + "operationId": "bundledItems", + "tags": [ + "items" + ], + "summary": "List bundled items", + "parameters": [ + { + "$ref": "../parameters.json#/item_id_pp" + }, + { + "name": "external_id", + "in": "query", + "description": "Search on the item's barcode", + "required": false, + "type": "string" + }, + { + "$ref": "../parameters.json#/match" + }, + { + "$ref": "../parameters.json#/order_by" + }, + { + "$ref": "../parameters.json#/page" + }, + { + "$ref": "../parameters.json#/per_page" + }, + { + "$ref": "../parameters.json#/q_param" + }, + { + "$ref": "../parameters.json#/q_body" + }, + { + "$ref": "../parameters.json#/q_header" + } + ], + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "responses": { + "200": { + "description": "A list of item", + "schema": { + "type": "array", + "items": { + "$ref": "../definitions.json#/item" + } + } + }, + "401": { + "description": "Authentication required", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "403": { + "description": "Access forbidden", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "500": { + "description": "Internal server error", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "503": { + "description": "Under maintenance", + "schema": { + "$ref": "../definitions.json#/error" + } + } + }, + "x-koha-authorization": { + "permissions": { + "catalogue": "1" + } + }, + "x-koha-embed": [ + "biblio", + "checkout" + ] + } + }, + "/items/{item_id}/bundled_items/item": { + "post": { + "x-mojo-to": "Items#add_to_bundle", + "operationId": "addToBundle", + "tags": ["items"], + "summary": "Add item to bundle", + "parameters": [{ + "$ref": "../parameters.json#/item_id_pp" + }, + { + "name": "body", + "in": "body", + "description": "A JSON object containing information about the new bundle link", + "required": true, + "schema": { + "$ref": "../definitions.json#/bundle_link" + } + } + ], + "consumes": ["application/json"], + "produces": ["application/json"], + "responses": { + "201": { + "description": "A successfully created bundle link", + "schema": { + "items": { + "$ref": "../definitions.json#/item" + } + } + }, + "400": { + "description": "Bad parameter", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "401": { + "description": "Authentication required", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "403": { + "description": "Access forbidden", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "404": { + "description": "Resource not found", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "409": { + "description": "Conflict in creating resource", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "500": { + "description": "Internal server error", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "503": { + "description": "Under maintenance", + "schema": { + "$ref": "../definitions.json#/error" + } + } + }, + "x-koha-authorization": { + "permissions": { + "catalogue": 1 + } + } + } + }, + "/items/{item_id}/bundled_items/item/{bundled_item_id}": { + "delete": { + "x-mojo-to": "Items#remove_from_bundle", + "operationId": "removeFromBundle", + "tags": ["items"], + "summary": "Remove item from bundle", + "parameters": [{ + "$ref": "../parameters.json#/item_id_pp" + }, + { + "name": "bundled_item_id", + "in": "path", + "description": "Internal identifier for the bundled item", + "required": true, + "type": "string" + } + ], + "consumes": ["application/json"], + "produces": ["application/json"], + "responses": { + "204": { + "description": "Bundle link deleted" + }, + "400": { + "description": "Bad parameter", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "401": { + "description": "Authentication required", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "403": { + "description": "Access forbidden", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "404": { + "description": "Resource not found", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "500": { + "description": "Internal server error", + "schema": { + "$ref": "../definitions.json#/error" + } + }, + "503": { + "description": "Under maintenance", + "schema": { + "$ref": "../definitions.json#/error" + } + } + }, + "x-koha-authorization": { + "permissions": { + "catalogue": 1 + } + } + } + }, "/items/{item_id}/pickup_locations": { "get": { "x-mojo-to": "Items#pickup_locations", diff --git a/catalogue/detail.pl b/catalogue/detail.pl index 0cb204ced9d..ce594fbb512 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -203,6 +203,11 @@ if (@hostitems){ my $dat = &GetBiblioData($biblionumber); +#is biblio a collection +my $leader = $record->leader(); +$dat->{collection} = ( substr($leader,7,1) eq 'c' ) ? 1 : 0; + + #coping with subscriptions my $subscriptionsnumber = CountSubscriptionFromBiblionumber($biblionumber); my @subscriptions = SearchSubscriptions({ biblionumber => $biblionumber, orderby => 'title' }); @@ -395,6 +400,15 @@ foreach my $item (@items) { $item->{cover_images} = $item_object->cover_images; } + if ($item_object->is_bundle) { + $itemfields{bundles} = 1; + $item->{is_bundle} = 1; + } + + if ($item_object->in_bundle) { + $item->{bundle_host} = $item_object->bundle_host; + } + if ($currentbranch and C4::Context->preference('SeparateHoldings')) { if ($itembranchcode and $itembranchcode eq $currentbranch) { push @itemloop, $item; diff --git a/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss b/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss index 9d8f5e8069c..8ee0df0a57d 100644 --- a/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss +++ b/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss @@ -2293,6 +2293,10 @@ td { display: block; } +.bundled { + display: block; +} + .datedue { color: #999; display: block; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref index 5ffd9a22f3e..e0029bab616 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref @@ -158,6 +158,10 @@ Cataloging: - and record's last modifier name in MARC subfield - pref: MarcFieldForModifierName - ".
NOTE: Use a dollar sign between field and subfield like 123$a." + - + - Use the NOT_LOAN authorised value + - pref: BundleNotLoanValue + - to represent 'added to bundle' when an item is attached to bundle. Display: - - 'Separate main entry and subdivisions with ' diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt index 7c2b961d6ca..5c556335d5c 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt @@ -347,7 +347,7 @@ [% FOREACH item IN items %] - + [% IF (StaffDetailItemSelection) %] @@ -509,6 +509,10 @@ Note that permanent location is a code, and location may be an authval. ([% item.restrictedvalue | html %]) [% END %] + [% IF ( item.bundle_host ) %] + In bundle: [% INCLUDE 'biblio-title.inc' biblio = item.bundle_host.biblio link = 1 %] + [% END %] + [% item.datelastseen | $KohaDates %] [% item.dateaccessioned | $KohaDates %] @@ -593,6 +597,9 @@ Note that permanent location is a code, and location may be an authval. Edit [% END %] [% END %] + [% IF collection %] + + [% END %] [% END %] @@ -1020,6 +1027,35 @@ Note that permanent location is a code, and location may be an authval. [% END %] + + [% MACRO jsinclude BLOCK %] [% INCLUDE 'catalog-strings.inc' %] [% Asset.js("js/catalog.js") | $raw %] @@ -1315,6 +1351,7 @@ Note that permanent location is a code, and location may be an authval. [% INCLUDE 'datatables.inc' %] [% Asset.js("lib/jquery/plugins/jquery.dataTables.columnFilter.js") | $raw %] [% INCLUDE 'columns_settings.inc' %] + [% INCLUDE 'js-date-format.inc' %] [% Asset.js("js/browser.js") | $raw %] [% Asset.js("js/table_filters.js") | $raw %]