Bugzilla – Attachment 134309 Details for
Bug 29346
Real-time holds queue update
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 29346: Refactor loop code into a subroutine
Bug-29346-Refactor-loop-code-into-a-subroutine.patch (text/plain), 6.12 KB, created by
Tomás Cohen Arazi (tcohen)
on 2022-04-28 22:55:08 UTC
(
hide
)
Description:
Bug 29346: Refactor loop code into a subroutine
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2022-04-28 22:55:08 UTC
Size:
6.12 KB
patch
obsolete
>From 77299c59d0e7c84807c559f39a4ccb5d2f083557 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Thu, 24 Feb 2022 08:01:53 -0300 >Subject: [PATCH] Bug 29346: Refactor loop code into a subroutine > >The CreateQueue() method deletes the holds queue data, fetches some >configuration (branches to use, transport cost matrix) and then loops >through a list of biblionumbers, generating the tmp_holdsqueue and >hold_fill_targets rows for the specified biblio. > >This patch simply moves that last bit that is run inside the biblios >loop into a separate sub. > >The update_queue_for_biblio sub is designed so it does the exact same >thing it did inside the loop, but also gets added the capability of >querying those parameters if not passed, and it also gets a 'delete' >parameter so it deletes the biblio-specific holds queue rows before >starting to work. > >This way, it can be reused to write a background job for real-time holds >queue update :-D > >To test: >1. Run: > $ kshell > k$ prove t/db_dependent/HoldsQueue.t >=> SUCCESS: Tests pass! >2. Apply this patch >3. Repeat 1 >=> SUCCESS: Tests still pass! Behavior is kept! >4. Sign off :-D > >Sponsored-by: Montgomery County Public Libraries >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > C4/HoldsQueue.pm | 115 +++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 97 insertions(+), 18 deletions(-) > >diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm >index b7cf0c7027..7073760881 100644 >--- a/C4/HoldsQueue.pm >+++ b/C4/HoldsQueue.pm >@@ -44,6 +44,8 @@ BEGIN { > TransportCostMatrix > UpdateTransportCostMatrix > GetPendingHoldRequestsForBib >+ load_branches_to_pull_from >+ update_queue_for_biblio > ); > } > >@@ -204,27 +206,19 @@ sub CreateQueue { > my $bibs_with_pending_requests = GetBibsWithPendingHoldRequests(); > > foreach my $biblionumber (@$bibs_with_pending_requests) { >+ > $total_bibs++; >- my $hold_requests = GetPendingHoldRequestsForBib($biblionumber); >- my $available_items = GetItemsAvailableToFillHoldRequestsForBib($biblionumber, $branches_to_use); >- $total_requests += scalar(@$hold_requests); >- $total_available_items += scalar(@$available_items); > >- my $item_map = MapItemsToHoldRequests($hold_requests, $available_items, $branches_to_use, $transport_cost_matrix); >- $item_map or next; >- my $item_map_size = scalar(keys %$item_map) >- or next; >+ my $result = update_queue_for_biblio( >+ { biblio_id => $biblionumber, >+ branches_to_use => $branches_to_use, >+ transport_cost_matrix => $transport_cost_matrix, >+ } >+ ); > >- $num_items_mapped += $item_map_size; >- CreatePicklistFromItemMap($item_map); >- AddToHoldTargetMap($item_map); >- if (($item_map_size < scalar(@$hold_requests )) and >- ($item_map_size < scalar(@$available_items))) { >- # DOUBLE CHECK, but this is probably OK - unfilled item-level requests >- # FIXME >- #warn "unfilled requests for $biblionumber"; >- #warn Dumper($hold_requests), Dumper($available_items), Dumper($item_map); >- } >+ $total_requests += $result->{requests}; >+ $total_available_items += $result->{available_items}; >+ $num_items_mapped += $result->{mapped_items}; > } > } > >@@ -855,5 +849,90 @@ sub least_cost_branch { > # return $branch[0] if @branch == 1; > } > >+=head3 update_queue_for_biblio >+ >+ my $result = update_queue_for_biblio( >+ { >+ biblio_id => $biblio_id, >+ [ branches_to_use => $branches_to_use, >+ transport_cost_matrix => $transport_cost_matrix, >+ delete => $delete, ] >+ } >+ ); >+ >+Given a I<biblio_id>, this method calculates and sets the holds queue entries >+for the biblio's holds, and the hold fill targets (items). >+ >+=head4 Return value >+ >+It return a hashref containing: >+ >+=over >+ >+=item I<requests>: the pending holds count for the biblio. >+ >+=item I<available_items> the count of items that are available to fill holds for the biblio. >+ >+=item I<mapped_items> the total items that got mapped. >+ >+=back >+ >+=head4 Optional parameters >+ >+=over >+ >+=item I<branches_to_use> a list of branchcodes to be used to restrict which items can be used. >+ >+=item I<transport_cost_matrix> is the output of C<TransportCostMatrix>. >+ >+=item I<delete> tells the method to delete prior entries on the related tables for the biblio_id. >+ >+=back >+ >+Note: All the optional parameters will be calculated in the method if omitted. They >+are allowed to be passed to avoid calculating them many times inside loops. >+ >+=cut >+ >+sub update_queue_for_biblio { >+ my ($args) = @_; >+ >+ my $biblio_id = $args->{biblio_id}; >+ >+ my $branches_to_use = $args->{branches_to_use} // load_branches_to_pull_from( C4::Context->preference('UseTransportCostMatrix') ); >+ my $transport_cost_matrix; >+ >+ if ( !exists $args->{transport_cost_matrix} >+ && C4::Context->preference('UseTransportCostMatrix') ) { >+ $transport_cost_matrix = TransportCostMatrix(); >+ } else { >+ $transport_cost_matrix = $args->{transport_cost_matrix}; >+ } >+ >+ if ( $args->{delete} ) { >+ my $dbh = C4::Context->dbh; >+ >+ $dbh->do("DELETE FROM tmp_holdsqueue WHERE biblionumber=$biblio_id"); >+ $dbh->do("DELETE FROM hold_fill_targets WHERE biblionumber=$biblio_id"); >+ } >+ >+ my $hold_requests = GetPendingHoldRequestsForBib($biblio_id); >+ my $available_items = GetItemsAvailableToFillHoldRequestsForBib( $biblio_id, $branches_to_use ); >+ >+ my $result = { >+ requests => scalar( @{$hold_requests} ), >+ available_items => scalar( @{$available_items} ), >+ }; >+ >+ my $item_map = MapItemsToHoldRequests( $hold_requests, $available_items, $branches_to_use, $transport_cost_matrix ); >+ $result->{mapped_items} = scalar( keys %{$item_map} ); >+ >+ if ($item_map) { >+ CreatePicklistFromItemMap($item_map); >+ AddToHoldTargetMap($item_map); >+ } >+ >+ return $result; >+} > > 1; >-- >2.34.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 29346
:
131088
|
131089
|
131811
|
131812
|
131813
|
131815
|
132078
|
132079
|
132080
|
132081
|
132082
|
132083
|
133183
|
133184
|
133185
|
133186
|
133187
|
133188
|
133189
|
133190
|
133191
|
133205
|
133991
|
133992
|
133993
|
133994
|
133995
|
133996
|
133997
|
133998
|
133999
|
134000
|
134001
|
134010
|
134225
|
134227
|
134309
|
134310
|
134311
|
134312
|
134313
|
134314
|
134315
|
134316
|
134317
|
134318
|
134319
|
134320
|
134321
|
134322
|
134328
|
134329
|
134330
|
134331
|
134332
|
134333
|
134334
|
134335
|
134336
|
134337
|
134338
|
134339
|
134340
|
134341
|
134342
|
134343