From 5f614c05fd3b0691fa5aa0779dbfcb1d0d5b94ec 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 and run the database update 1) Configuration: `BundleNotLoanValue` should have been set by the database update and point to a newly added AV value. 2) 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. 3) 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. 4) Error cases * Try adding an item that already belongs to a bundle to another bundle: Note an error is displayed in the modal form. 5) The bundles feature can be disabled by unsetting the `BundleNotLoanValue` system preference. Signed-off-by: Katrin Fischer Signed-off-by: Kyle M Hall Signed-off-by: Tomas Cohen Arazi --- Koha/Item.pm | 158 +++++++++++ Koha/REST/V1/Items.pm | 121 ++++++++ admin/columns_settings.yml | 22 ++ api/v1/swagger/definitions/bundle_link.yaml | 14 + api/v1/swagger/definitions/item.yaml | 1 + api/v1/swagger/paths/items.yaml | 157 +++++++++++ api/v1/swagger/swagger.yaml | 6 + catalogue/detail.pl | 14 + .../prog/css/src/staff-global.scss | 4 + .../admin/preferences/cataloguing.pref | 4 + .../prog/en/modules/catalogue/detail.tt | 258 +++++++++++++++++- 11 files changed, 757 insertions(+), 2 deletions(-) create mode 100644 api/v1/swagger/definitions/bundle_link.yaml diff --git a/Koha/Item.pm b/Koha/Item.pm index 38b266655b..bb587a1caa 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -20,6 +20,7 @@ package Koha::Item; use Modern::Perl; use List::MoreUtils qw( any ); +use Try::Tiny qw( catch try ); use Koha::Database; use Koha::DateUtils qw( dt_from_string output_pref ); @@ -1465,6 +1466,163 @@ 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) = @_; + + my $bundle_items_rs = $self->_result->item_bundles_item; + return unless $bundle_items_rs; + return Koha::Item->_new_from_dbic($bundle_items_rs->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 d3c5d7e177..2e5478770f 100644 --- a/Koha/REST/V1/Items.pm +++ b/Koha/REST/V1/Items.pm @@ -148,4 +148,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/admin/columns_settings.yml b/admin/columns_settings.yml index 7e68c4d0b5..06300ccc26 100644 --- a/admin/columns_settings.yml +++ b/admin/columns_settings.yml @@ -525,6 +525,28 @@ modules: - columnname: checkin_on + bundle_tables: + columns: + - + columnname: title + cannot_be_toggled: 1 + - + columnname: author + - + columnname: collection_code + - + columnname: item_type + - + columnname: callnumber + - + columnname: external_id + - + columnname: status + - + columnname: bundle_actions + cannot_be_toggled: 1 + cannot_be_modified: 1 + cataloguing: addbooks: reservoir-table: diff --git a/api/v1/swagger/definitions/bundle_link.yaml b/api/v1/swagger/definitions/bundle_link.yaml new file mode 100644 index 0000000000..572be83d8e --- /dev/null +++ b/api/v1/swagger/definitions/bundle_link.yaml @@ -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.yaml b/api/v1/swagger/definitions/item.yaml index 8b00c96c88..b525c16484 100644 --- a/api/v1/swagger/definitions/item.yaml +++ b/api/v1/swagger/definitions/item.yaml @@ -7,6 +7,7 @@ properties: biblio_id: type: integer description: Internal identifier for the parent bibliographic record + biblio: {} external_id: type: - string diff --git a/api/v1/swagger/paths/items.yaml b/api/v1/swagger/paths/items.yaml index 66fc1ee6a8..38399e9908 100644 --- a/api/v1/swagger/paths/items.yaml +++ b/api/v1/swagger/paths/items.yaml @@ -93,6 +93,163 @@ x-koha-authorization: permissions: catalogue: "1" +"/items/{item_id}/bundled_items": + post: + x-mojo-to: Items#add_to_bundle + operationId: addToBundle + tags: + - items + summary: Add item to bundle + parameters: + - $ref: "../swagger.yaml#/parameters/item_id_pp" + - name: body + in: body + description: A JSON object containing information about the new bundle link + required: true + schema: + $ref: "../swagger.yaml#/definitions/bundle_link" + consumes: + - application/json + produces: + - application/json + responses: + "201": + description: A successfully created bundle link + schema: + items: + $ref: "../swagger.yaml#/definitions/item" + "400": + description: Bad parameter + schema: + $ref: "../swagger.yaml#/definitions/error" + "401": + description: Authentication required + schema: + $ref: "../swagger.yaml#/definitions/error" + "403": + description: Access forbidden + schema: + $ref: "../swagger.yaml#/definitions/error" + "404": + description: Resource not found + schema: + $ref: "../swagger.yaml#/definitions/error" + "409": + description: Conflict in creating resource + schema: + $ref: "../swagger.yaml#/definitions/error" + "500": + description: Internal server error + schema: + $ref: "../swagger.yaml#/definitions/error" + "503": + description: Under maintenance + schema: + $ref: "../swagger.yaml#/definitions/error" + x-koha-authorization: + permissions: + catalogue: 1 + get: + x-mojo-to: Items#bundled_items + operationId: bundledItems + tags: + - items + summary: List bundled items + parameters: + - $ref: "../swagger.yaml#/parameters/item_id_pp" + - name: external_id + in: query + description: Search on the item's barcode + required: false + type: string + - $ref: "../swagger.yaml#/parameters/match" + - $ref: "../swagger.yaml#/parameters/order_by" + - $ref: "../swagger.yaml#/parameters/page" + - $ref: "../swagger.yaml#/parameters/per_page" + - $ref: "../swagger.yaml#/parameters/q_param" + - $ref: "../swagger.yaml#/parameters/q_body" + - $ref: "../swagger.yaml#/parameters/q_header" + consumes: + - application/json + produces: + - application/json + responses: + "200": + description: A list of item + schema: + type: array + items: + $ref: "../swagger.yaml#/definitions/item" + "401": + description: Authentication required + schema: + $ref: "../swagger.yaml#/definitions/error" + "403": + description: Access forbidden + schema: + $ref: "../swagger.yaml#/definitions/error" + "500": + description: Internal server error + schema: + $ref: "../swagger.yaml#/definitions/error" + "503": + description: Under maintenance + schema: + $ref: "../swagger.yaml#/definitions/error" + x-koha-authorization: + permissions: + catalogue: "1" + x-koha-embed: + - biblio + - checkout +"/items/{item_id}/bundled_items/{bundled_item_id}": + delete: + x-mojo-to: Items#remove_from_bundle + operationId: removeFromBundle + tags: + - items + summary: Remove item from bundle + parameters: + - $ref: "../swagger.yaml#/parameters/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: "../swagger.yaml#/definitions/error" + "401": + description: Authentication required + schema: + $ref: "../swagger.yaml#/definitions/error" + "403": + description: Access forbidden + schema: + $ref: "../swagger.yaml#/definitions/error" + "404": + description: Resource not found + schema: + $ref: "../swagger.yaml#/definitions/error" + "500": + description: Internal server error + schema: + $ref: "../swagger.yaml#/definitions/error" + "503": + description: Under maintenance + schema: + $ref: "../swagger.yaml#/definitions/error" + x-koha-authorization: + permissions: + catalogue: 1 "/items/{item_id}/pickup_locations": get: x-mojo-to: Items#pickup_locations diff --git a/api/v1/swagger/swagger.yaml b/api/v1/swagger/swagger.yaml index dbb0c20881..7e3a8d8afc 100644 --- a/api/v1/swagger/swagger.yaml +++ b/api/v1/swagger/swagger.yaml @@ -10,6 +10,8 @@ definitions: $ref: ./definitions/allows_renewal.yaml basket: $ref: ./definitions/basket.yaml + bundle_link: + $ref: ./definitions/bundle_link.yaml cashup: $ref: ./definitions/cashup.yaml checkout: @@ -169,6 +171,10 @@ paths: $ref: ./paths/items.yaml#/~1items "/items/{item_id}": $ref: "./paths/items.yaml#/~1items~1{item_id}" + "/items/{item_id}/bundled_items": + $ref: ./paths/items.yaml#/~1items~1{item_id}~1bundled_items + "/items/{item_id}/bundled_items/{bundled_item_id}": + $ref: ./paths/items.yaml#/~1items~1{item_id}~1bundled_items~1{bundled_item_id} "/items/{item_id}/pickup_locations": $ref: "./paths/items.yaml#/~1items~1{item_id}~1pickup_locations" /libraries: diff --git a/catalogue/detail.pl b/catalogue/detail.pl index 34447566b5..b3deb4305a 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -206,6 +206,11 @@ if (@hostitems){ my $dat = &GetBiblioData($biblionumber); +#is biblio a collection and are bundles enabled +my $leader = $record->leader(); +$dat->{bundlesEnabled} = ( ( substr( $leader, 7, 1 ) eq 'c' ) + && C4::Context->preference('BundleNotLoanValue') ) ? 1 : 0; + #coping with subscriptions my $subscriptionsnumber = CountSubscriptionFromBiblionumber($biblionumber); my @subscriptions = SearchSubscriptions({ biblionumber => $biblionumber, orderby => 'title' }); @@ -451,6 +456,15 @@ foreach my $item (@items) { } } + 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 0afb6c768a..cac83f13d1 100644 --- a/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss +++ b/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss @@ -2355,6 +2355,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 6f66761c37..63e2241c3e 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 @@ -165,6 +165,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 eeecfd1669..9e280654bc 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt @@ -368,12 +368,12 @@ [% IF ( analyze ) %]Used in[% END %] [% IF ( ShowCourseReserves ) %]Course reserves[% END %] [% IF ( SpineLabelShowPrintOnBibDetails ) %]Spine label[% END %] - [% IF ( CAN_user_editcatalogue_edit_items ) %] [% END %] + [% IF ( CAN_user_editcatalogue_edit_items ) %] [% END %] [% FOREACH item IN items %] - + [% IF (StaffDetailItemSelection) %] @@ -545,6 +545,11 @@ Note that permanent location is a code, and location may be an authval. [% IF ( item.restricted ) %] ([% 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.issues || 0 | html %] @@ -631,6 +636,9 @@ Note that permanent location is a code, and location may be an authval. Edit [% END %] [% END %] + [% IF bundlesEnabled %] + + [% END %] [% END %] @@ -1222,6 +1230,37 @@ Note that permanent location is a code, and location may be an authval. + [% IF bundlesEnabled %] + + [% END %] + [% MACRO jsinclude BLOCK %] [% INCLUDE 'catalog-strings.inc' %] [% Asset.js("js/catalog.js") | $raw %] @@ -1528,6 +1567,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 %]