From d3307aa23a30070e1a21b1b2f5ab3317844998db Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 10 May 2023 15:18:15 +0200 Subject: [PATCH] Bug 35717: Add link between suggestions and orders Signed-off-by: Nick Clemens --- Koha/Acquisition/Order.pm | 36 ++++++--- acqui/addorder.pl | 12 +++ acqui/basket.pl | 6 -- acqui/orderreceive.pl | 1 - acqui/parcel.pl | 6 +- api/v1/swagger/definitions/order.yaml | 3 + api/v1/swagger/definitions/suggestion.yaml | 6 ++ api/v1/swagger/paths/acquisitions_orders.yaml | 6 +- .../prog/en/modules/acqui/basket.tt | 8 +- .../prog/en/modules/acqui/orderreceive.tt | 38 +++++----- .../prog/en/modules/acqui/parcel.tt | 52 ++++++------- t/db_dependent/Koha/Acquisition/Order.t | 74 +++++++++++++++---- 12 files changed, 165 insertions(+), 83 deletions(-) diff --git a/Koha/Acquisition/Order.pm b/Koha/Acquisition/Order.pm index d869b9764da..2d92e13dd7c 100644 --- a/Koha/Acquisition/Order.pm +++ b/Koha/Acquisition/Order.pm @@ -146,15 +146,17 @@ sub cancel { } # If ordered from a suggestion, revert the suggestion status to ACCEPTED - my $suggestion = Koha::Suggestions->find({ biblionumber => $self->biblionumber, status => "ORDERED" }); - if ( $suggestion and $suggestion->id ) { - ModSuggestion( - { - suggestionid => $suggestion->id, - biblionumber => $self->biblionumber, - STATUS => 'ACCEPTED', - } - ); + my $suggestions = $self->suggestions; + while ( my $suggestion = $suggestions->next ){ + if ( $suggestion && $suggestion->STATUS eq "ORDERED" ) { + ModSuggestion( + { + suggestionid => $suggestion->id, + biblionumber => $self->biblionumber, + STATUS => 'ACCEPTED', + } + ); + } } my $biblio = $self->biblio; @@ -297,6 +299,22 @@ sub subscription { return Koha::Subscription->_new_from_dbic( $subscription_rs ); } +=head3 suggestions + + my $suggestions = $order->suggestions + +Returns the I object for the suggestions associated +to the order. + +=cut + +sub suggestions { + my ( $self ) = @_; + my $rs = $self->_result->suggestions; + return unless $rs; + return Koha::Suggestions->_new_from_dbic( $rs ); +} + =head3 current_item_level_holds my $holds = $order->current_item_level_holds; diff --git a/acqui/addorder.pl b/acqui/addorder.pl index ef759925c59..f0ee2c006b9 100755 --- a/acqui/addorder.pl +++ b/acqui/addorder.pl @@ -369,6 +369,18 @@ if ( $op eq 'cud-order' ) { $order->populate_with_prices_for_ordering(); $order->store; + # change suggestion status if applicable + if ( my $suggestionid = $input->param('suggestionid') ) { + ModSuggestion( + { + suggestionid => $suggestionid, + biblionumber => $orderinfo->{biblionumber}, + ordernumber => $order->ordernumber, + STATUS => 'ORDERED', + } + ); + } + # Log the order creation if ( C4::Context->preference("AcquisitionLog") ) { my $infos = {}; diff --git a/acqui/basket.pl b/acqui/basket.pl index 1a6ef3ca478..84c88714511 100755 --- a/acqui/basket.pl +++ b/acqui/basket.pl @@ -27,7 +27,6 @@ use CGI qw ( -utf8 ); use C4::Acquisition qw( GetBasket CanUserManageBasket GetBasketAsCSV NewBasket NewBasketgroup ModBasket ReopenBasket ModBasketUsers GetBasketgroup GetBasketgroups GetBasketUsers GetOrders GetOrder get_rounded_price ); use C4::Budgets qw( GetBudgetHierarchy GetBudget CanUserUseBudget ); use C4::Contract qw( GetContract ); -use C4::Suggestions qw( GetSuggestion GetSuggestionInfoFromBiblionumber GetSuggestionInfo ); use Koha::Biblios; use Koha::Acquisition::Baskets; use Koha::Acquisition::Booksellers; @@ -519,11 +518,6 @@ sub get_order_infos { $line{deleted_biblio} = Koha::Old::Biblios->find( $order->{deleted_biblionumber} ); } - my $suggestion = GetSuggestionInfoFromBiblionumber($line{biblionumber}); - $line{suggestionid} = $$suggestion{suggestionid}; - $line{surnamesuggestedby} = $$suggestion{surnamesuggestedby}; - $line{firstnamesuggestedby} = $$suggestion{firstnamesuggestedby}; - $line{estimated_delivery_date} = $order->{estimated_delivery_date}; foreach my $key (qw(transferred_from transferred_to)) { diff --git a/acqui/orderreceive.pl b/acqui/orderreceive.pl index 0282020a7b2..d84a2430d6d 100755 --- a/acqui/orderreceive.pl +++ b/acqui/orderreceive.pl @@ -68,7 +68,6 @@ use C4::Output qw( output_html_with_http_headers ); use C4::Budgets qw( GetBudget GetBudgetPeriods GetBudgetPeriod GetBudgetHierarchy CanUserUseBudget ); use C4::Members; use C4::Biblio qw( GetMarcStructure ); -use C4::Suggestions qw( GetSuggestion GetSuggestionInfoFromBiblionumber GetSuggestionInfo ); use Koha::Acquisition::Booksellers; use Koha::Acquisition::Currencies qw( get_active ); diff --git a/acqui/parcel.pl b/acqui/parcel.pl index 7fd16c76d23..f02bb386f7f 100755 --- a/acqui/parcel.pl +++ b/acqui/parcel.pl @@ -61,7 +61,6 @@ use C4::Acquisition qw( CancelReceipt GetInvoice GetInvoiceDetails get_rounded_p use C4::Budgets qw( GetBudget GetBudgetByOrderNumber GetBudgetName ); use CGI qw ( -utf8 ); use C4::Output qw( output_html_with_http_headers ); -use C4::Suggestions qw( GetSuggestion GetSuggestionInfoFromBiblionumber GetSuggestionInfo ); use Koha::Acquisition::Baskets; use Koha::Acquisition::Bookseller; @@ -149,10 +148,7 @@ for my $order ( @orders ) { $total_tax_excluded += get_rounded_price($line{unitprice_tax_excluded}) * $line{quantity}; $total_tax_included += get_rounded_price($line{unitprice_tax_included}) * $line{quantity}; - my $suggestion = GetSuggestionInfoFromBiblionumber($line{biblionumber}); - $line{suggestionid} = $suggestion->{suggestionid}; - $line{surnamesuggestedby} = $suggestion->{surnamesuggestedby}; - $line{firstnamesuggestedby} = $suggestion->{firstnamesuggestedby}; + $line{order_object} = $order_object; if ( $line{parent_ordernumber} != $line{ordernumber} ) { if ( grep { $_->{ordernumber} == $line{parent_ordernumber} } diff --git a/api/v1/swagger/definitions/order.yaml b/api/v1/swagger/definitions/order.yaml index 61aaae480d9..2e1ba3b9a23 100644 --- a/api/v1/swagger/definitions/order.yaml +++ b/api/v1/swagger/definitions/order.yaml @@ -264,4 +264,7 @@ properties: - object - "null" description: Patron that created the order + suggestions: + type: + - array additionalProperties: false diff --git a/api/v1/swagger/definitions/suggestion.yaml b/api/v1/swagger/definitions/suggestion.yaml index daa136ff23b..327feb4301b 100644 --- a/api/v1/swagger/definitions/suggestion.yaml +++ b/api/v1/swagger/definitions/suggestion.yaml @@ -136,6 +136,12 @@ properties: - "null" description: foreign key linking the suggestion to the biblio table after the suggestion has been ordered + order_id: + type: + - integer + - "null" + description: foreign key linking the suggestion to the aqorders table after the + suggestion has been ordered reason: type: - string diff --git a/api/v1/swagger/paths/acquisitions_orders.yaml b/api/v1/swagger/paths/acquisitions_orders.yaml index 804fd9a4b23..b72b770ef97 100644 --- a/api/v1/swagger/paths/acquisitions_orders.yaml +++ b/api/v1/swagger/paths/acquisitions_orders.yaml @@ -57,7 +57,6 @@ - biblio.uncancelled_orders+count - biblio.holds+count - biblio.items+count - - biblio.suggestions.suggester - creator - fund - fund.budget @@ -66,6 +65,8 @@ - items - items+strings - subscription + - suggestions + - suggestions.suggester collectionFormat: csv responses: "200": @@ -184,12 +185,13 @@ - biblio.uncancelled_orders+count - biblio.holds+count - biblio.items+count - - biblio.suggestions.suggester - fund - current_item_level_holds+count - invoice - items - subscription + - suggestion + - suggestion.suggester collectionFormat: csv produces: - application/json diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt index a9f2d15681f..959cf99fb5c 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt @@ -613,10 +613,12 @@ [%- IF ( books_loo.publicationyear ) %], [% books_loo.publicationyear | html -%] [%- ELSIF ( books_loo.copyrightdate ) %] [% books_loo.copyrightdate | html %][% END -%] [%- IF ( books_loo.editionstatement ) %], [% books_loo.editionstatement | html %][% END -%] - [%- IF ( books_loo.suggestionid ) %] + [% SET suggestions = books_loo.order_object.suggestions %] + [% FOREACH suggestion IN suggestions %]
- Suggested by: [% books_loo.surnamesuggestedby | html %][% IF ( books_loo.firstnamesuggestedby ) %], [% books_loo.firstnamesuggestedby | html %] [% END %] - (suggestion #[% books_loo.suggestionid | html %]) + [% SET suggester = suggestion.suggester %] + Suggested by: [% suggester.surname | html %][% IF suggester.firstname %] %], [% suggester.firstname | html %] [% END %] + (suggestion #[% suggestion.suggestionid | html %]) [% END %]

[% IF ( books_loo.order_internalnote ) %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt index 9daac07657f..903430c5704 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt @@ -555,7 +555,7 @@ $(document).ready(function(){ // keep a copy for re-rendering var $funds_tree = $('#bookfund').html(); - var base_query = { "order_id": {"-in": [[% multiple_orders | html %]]}}; + var base_query = { "me.order_id": {"in": [[% multiple_orders | html %]]}}; var pending_orders_url = "/api/v1/acquisitions/orders?only_active=1"; var options = { "ajax": { @@ -563,10 +563,11 @@ }, "embed": [ "basket", - "biblio.suggestions.suggester", + "biblio", "fund.budget", "items+strings", - "creator" + "creator", + "suggestions.suggester", ], 'dom': 'C<"top pager"ilpfB><"#filter_c">tr<"bottom pager"ip>', "columns": [ @@ -696,14 +697,15 @@ params['ordernumber'] = row.order_id; params['booksellerid'] = row.basket.vendor_id; - if(row.biblio.suggestions.length && row.biblio.suggestions[0].reason) { - params["suggestionid"] = row.biblio.suggestions[0].suggestion_id; + if(row.suggestions && row.suggestions[0].reason) { + + params["suggestionid"] = row.suggestions[0].suggestion_id; const options = Array.from(document.querySelectorAll('#reason option')); - if (options.some(option => option.value === row.biblio.suggestions[0].reason)) { - params['reason'] = row.biblio.suggestions[0].reason; + if (options.some(option => option.value === row.suggestions[0].reason)) { + params['reason'] = row.suggestions[0].reason; } else { params['reason'] = 'other'; - params['other_reason'] = row.biblio.suggestions[0].reason; + params['other_reason'] = row.suggestions[0].reason; } } params['datereceived'] = row.date_received; @@ -841,14 +843,14 @@ $("#other_reason").show(); $(this).hide(); } else { - row.biblio.suggestions[0].reason = val; + row.suggestions[0].reason = val; } }); $("#order_edit").on("change", "#select-other_reason", function() { var val = $(this).val(); var row = $("#order_edit").data('row'); - row.biblio.suggestions[0].reason = val; + row.suggestions[0].reason = val; }); $("#order_edit").on("click", "#other_reason a", function() { @@ -1136,31 +1138,31 @@ o.parent().hide(); } }); - if(row.biblio.suggestions.length) { + if(row.suggestions) { $("#suggestion_fieldset").show(); - if(row.biblio.suggestions[0].suggester) { + if(row.suggestions[0].suggester) { $("#biblio_suggestion_suggester").parent().show(); $("#biblio_suggestion_suggester") .html( - [row.biblio.suggestions[0].suggester.surname, row.biblio.suggestions[0].suggester.firstname] + [row.suggestions[0].suggester.surname, row.suggestions[0].suggester.firstname] .filter(function(name){ return name }) - .join(', ')+' ('+SUGGESTION.format(row.biblio.suggestions[0].suggestion_id)+')' + .join(', ')+' ('+SUGGESTION.format(row.suggestions[0].suggestion_id)+')' ); } else { $("#biblio_suggestion_suggester").parent().hide(); } - if(row.biblio.suggestions[0].reason) { + if(row.suggestions[0].reason) { $("#suggestion_reason").show(); const options = Array.from(document.querySelectorAll('#reason option')); - if (options.some(option => option.value === row.biblio.suggestions[0].reason)) { + if (options.some(option => option.value === row.suggestions[0].reason)) { $("#other_reason a").click(); - $("#reason").val(row.biblio.suggestions[0].reason); + $("#reason").val(row.suggestions[0].reason); $("#select-other_reason").val(null); } else { $("#reason").val("other").change(); - $("#select-other_reason").val(row.biblio.suggestions[0].reason); + $("#select-other_reason").val(row.suggestions[0].reason); } } else { diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt index e9f4f5fc7a4..1c28b314956 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt @@ -245,10 +245,13 @@ [%- ELSIF ( order.copyrightdate > 0) -%] [% order.copyrightdate | html %] [% END %] [% END %] - [% IF ( order.suggestionid ) %] -
- Suggested by: [% order.surnamesuggestedby | html %][% IF ( order.firstnamesuggestedby ) %], [% order.firstnamesuggestedby | html %] [% END %] - (suggestion #[% order.suggestionid | html %]) + + [% SET suggestions = order.order_object.suggestions %] + [% FOREACH suggestion IN suggestions %] + [% SET suggester = suggestion.suggester %] +
+ Suggested by: [% suggester.surname | html %][% IF suggester.firstname %] %], [% suggester.firstname | html %] [% END %] + (suggestion #[% suggestion.suggestionid | html %]) [% END %]
[% IF ( order.order_internalnote ) %] @@ -407,10 +410,10 @@ "biblio.uncancelled_orders+count", "biblio.holds+count", "biblio.items+count", - "biblio.suggestions.suggester", "fund", "current_item_level_holds+count", - "items" + "items", + "suggestions.suggester" ], "stateSave": true, // remember state on page reload "columns": [ @@ -471,27 +474,26 @@ result += escape_str(row.biblio.copyright_date); } } - var suggestions = row.biblio.suggestions; - if ( suggestions != null && suggestions.length > 0 ) { - var suggestion = suggestions[0]; - if ( suggestion.suggester != null ) { - var suggester = suggestion.suggester; - var suggested_by = []; - if ( suggester.surname != null ) { - suggested_by.push(escape_str(suggester.surname)); - } - if ( suggester.firstname != null ) { - suggested_by.push(escape_str(suggester.firstname)); - } - - result += "
" + _("Suggested by: ") + - '' - + suggested_by.join(", ") - + " (#" + escape_str(suggestions[0].suggestionid) + ")"; // FIXME: could be changed if we allow matching multiple suggestions + let suggestions = row.suggestions; + suggestions.forEach( (suggestion) => { + if ( suggestion && suggestion.suggested_by != null ) { + var suggester = suggestion.suggester; + var suggested_by = []; + if ( suggester.surname != null ) { + suggested_by.push(escape_str(suggester.surname)); + } + if ( suggester.firstname != null ) { + suggested_by.push(escape_str(suggester.firstname)); } + + result += "
" + _("Suggested by: ") + + '' + + suggested_by.join(", ") + + " (#" + escape_str(suggestion.suggestion_id) + ")"; } + } ); result += '

'; } diff --git a/t/db_dependent/Koha/Acquisition/Order.t b/t/db_dependent/Koha/Acquisition/Order.t index d74afa1ff56..3cc320fb034 100755 --- a/t/db_dependent/Koha/Acquisition/Order.t +++ b/t/db_dependent/Koha/Acquisition/Order.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 13; +use Test::More tests => 14; use Test::Exception; use t::lib::TestBuilder; @@ -210,6 +210,51 @@ subtest 'subscription' => sub { $schema->storage->txn_rollback; }; +subtest 'suggestions() tests' => sub { + plan tests => 4; + + $schema->storage->txn_begin; + my $o = $builder->build_object( + { + class => 'Koha::Acquisition::Orders', + } + ); + + my $order = Koha::Acquisition::Orders->find( $o->ordernumber ); + is( + ref( $order->suggestions ), 'Koha::Suggestions', + '->suggestions should return a Koha::Suggestions object' + ); + is( + $order->suggestions->count, 0, + '->suggestions should return empty set if no linked suggestion' + ); + + $o = $builder->build_object( + { + class => 'Koha::Acquisition::Orders', + } + ); + $o = $builder->build_object( + { + class => 'Koha::Suggestions', + value => { ordernumber => $o->id } + } + ); + + $order = Koha::Acquisition::Orders->find( $o->ordernumber ); + is( + ref( $order->suggestions ), 'Koha::Suggestions', + '->suggestions should return a Koha::Suggestions object' + ); + is( + $order->suggestions->count, 1, + '->suggestions should return linked suggestions' + ); + + $schema->storage->txn_rollback; +}; + subtest 'duplicate_to | add_item' => sub { plan tests => 3; @@ -915,19 +960,6 @@ subtest 'cancel() tests' => sub { $item = $builder->build_sample_item; $biblio_id = $item->biblionumber; - # Add the suggestion - my $suggestion = $builder->build_object( - { - class => 'Koha::Suggestions', - value => { - biblionumber => $biblio_id, - suggesteddate => dt_from_string, - STATUS => 'ORDERED', - archived => 0, - } - } - ); - $order = $builder->build_object( { class => 'Koha::Acquisition::Orders', @@ -940,6 +972,20 @@ subtest 'cancel() tests' => sub { } ); + # Add the suggestion + my $suggestion = $builder->build_object( + { + class => 'Koha::Suggestions', + value => { + biblionumber => $biblio_id, + suggesteddate => dt_from_string, + STATUS => 'ORDERED', + archived => 0, + ordernumber => $order->id, + } + } + ); + $order->cancel({ reason => $reason }) ->discard_changes; -- 2.47.0