From 849e78b114d772f03185626efd74dbe30e7e7b42 Mon Sep 17 00:00:00 2001 From: Lebbeous Fogle-Weekley Date: Wed, 3 Apr 2013 13:58:07 -0400 Subject: [PATCH] Fix various Traditional and holds-go-home best-hold sort orders approx is a more correct determinant to give the behavior sites are used to. hprox can cause copies to be too eager to go home when there are holds with that copy's circ lib as its request lib (if that's what you want, then you do pick or create a sort-order with hprox near the top). Address a problem in the copy_has_not_been_home CTE. This expression was always meant to provide a TRUE or FALSE value as its lone result, but would return NULL in cases where copies had no transit history. Also use pickup_lib, not request_lib, as the determinant of nearness-to-home. request_lib was used with the thinking that an item's "owning" patrons should have their wishes favored at holds-go-home time, even if where they wanted to send the copy was not actually home, but that's neither necessarily desired nor very intuitive. Signed-off-by: Lebbeous Fogle-Weekley --- .../perlmods/lib/OpenILS/Application/Circ/Holds.pm | 4 ++ .../Application/Storage/Publisher/action.pm | 32 ++++++----- Open-ILS/src/sql/Pg/950.data.seed-values.sql | 8 +-- ...XXX.data.best-hold-order-traditional-approx.sql | 67 ++++++++++++++++++++++ 4 files changed, 92 insertions(+), 19 deletions(-) create mode 100644 Open-ILS/src/sql/Pg/upgrade/XXXX.data.best-hold-order-traditional-approx.sql diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Holds.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Holds.pm index 9ba8272fa6..fc43e1d89a 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Holds.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Holds.pm @@ -3007,6 +3007,10 @@ sub find_nearest_permitted_hold { my $fifo = $U->ou_ancestor_setting_value($user->ws_ou, 'circ.holds_fifo'); + # the nearest_hold API call now needs this + $copy->call_number($editor->retrieve_asset_call_number($copy->call_number)) + unless ref $copy->call_number; + # search for what should be the best holds for this copy to fulfill my $best_holds = $U->storagereq( "open-ils.storage.action.hold_request.nearest_hold.atomic", diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm index c0c953b1f5..d7bc4b957c 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm @@ -17,10 +17,12 @@ use OpenILS::Application::Circ::CircCommon; use OpenILS::Application::AppUtils; my $U = "OpenILS::Application::AppUtils"; -# used in build_hold_sort_clause() +# Used in build_hold_sort_clause(). See the hash %order_by_sprintf_args in +# that sub to confirm what gets used to replace the formatters, and see +# nearest_hold() for the main body of the SQL query that these go into. my %HOLD_SORT_ORDER_BY = ( pprox => 'p.prox', - hprox => 'actor.org_unit_proximity(%d, h.request_lib)', # $cp->circ_lib + hprox => 'actor.org_unit_proximity(%d, h.pickup_lib)', # $cp->call_number->owning_lib aprox => 'COALESCE(hm.proximity, p.prox)', approx => 'action.hold_copy_calculated_proximity(h.id, %d, %d)', # $cp,$here priority => 'pgt.hold_priority', @@ -30,17 +32,17 @@ my %HOLD_SORT_ORDER_BY = ( htime => q! CASE WHEN copy_has_not_been_home.result - THEN actor.org_unit_proximity(%d, acn.owning_lib) + THEN actor.org_unit_proximity(%d, h.pickup_lib) ELSE 999 END - !, + !, # $cp->call_number->owning_lib shtime => q! CASE WHEN copy_has_not_been_home_even_to_idle.result - THEN actor.org_unit_proximity(%d, acn.owning_lib) + THEN actor.org_unit_proximity(%d, h.pickup_lib) ELSE 999 END - !, + !, # $cp->call_number->owning_lib ); @@ -349,10 +351,10 @@ sub build_hold_sort_clause { my ($columns, $cp, $here) = @_; my %order_by_sprintf_args = ( - hprox => [$cp->circ_lib], + hprox => [$cp->call_number->owning_lib], approx => [$cp->id, $here], - htime => [$cp->circ_lib], - shtime => [$cp->circ_lib] + htime => [$cp->call_number->owning_lib], + shtime => [$cp->call_number->owning_lib] ); my @clauses; @@ -402,22 +404,21 @@ sub build_hold_sort_clause { circ.xact_start >= NOW() - go_home_interval.value ) IS NULL AND ( -- part 2 - SELECT atc.dest <> %d FROM action.transit_copy atc - JOIN go_home_interval ON (true) - WHERE + SELECT COALESCE(atc.dest <> %d, TRUE) + FROM go_home_interval + LEFT JOIN action.transit_copy atc ON ( atc.id = ( SELECT MAX(id) FROM action.transit_copy atc_inner WHERE atc_inner.target_copy = %d AND atc_inner.source_send_time < NOW() - go_home_interval.value ) + ) ) AS result ) !, $cp->id, $cp->circ_lib, $cp->circ_lib, $cp->id); $joins .= q! JOIN copy_has_not_been_home ON (true) - JOIN asset.copy acp ON (hm.target_copy = acp.id) - JOIN asset.call_number acn ON (acn.id = acp.call_number) !; } @@ -469,7 +470,8 @@ sub nearest_hold { my $self = shift; my $client = shift; my $here = shift; # just the ID - my $cp = shift; # now an object, formerly just the ID + my $cp = shift; # now an object with call_number fleshed, + # formerly just copy ID my $limit = int(shift()) || 10; my $age = shift() || '0 seconds'; my $fifo = shift(); diff --git a/Open-ILS/src/sql/Pg/950.data.seed-values.sql b/Open-ILS/src/sql/Pg/950.data.seed-values.sql index d693e6ce37..a8ddf6a09f 100644 --- a/Open-ILS/src/sql/Pg/950.data.seed-values.sql +++ b/Open-ILS/src/sql/Pg/950.data.seed-values.sql @@ -12489,15 +12489,15 @@ INSERT INTO config.org_unit_setting_type ( INSERT INTO config.best_hold_order ( name, - pprox, aprox, priority, cut, depth, rtime, htime, hprox + approx, pprox, aprox, priority, cut, depth, rtime ) VALUES ( 'Traditional', - 1, 2, 3, 4, 5, 6, 7, 8 + 1, 2, 3, 4, 5, 6, 7 ); INSERT INTO config.best_hold_order ( name, - hprox, pprox, aprox, priority, cut, depth, rtime, htime + hprox, approx, pprox, aprox, priority, cut, depth, rtime ) VALUES ( 'Traditional with Holds-always-go-home', 1, 2, 3, 4, 5, 6, 7, 8 @@ -12505,7 +12505,7 @@ INSERT INTO config.best_hold_order ( INSERT INTO config.best_hold_order ( name, - htime, hprox, pprox, aprox, priority, cut, depth, rtime + htime, approx, pprox, aprox, priority, cut, depth, rtime ) VALUES ( 'Traditional with Holds-go-home', 1, 2, 3, 4, 5, 6, 7, 8 diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.data.best-hold-order-traditional-approx.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.data.best-hold-order-traditional-approx.sql new file mode 100644 index 0000000000..ddd525d551 --- /dev/null +++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.data.best-hold-order-traditional-approx.sql @@ -0,0 +1,67 @@ +BEGIN; + +-- INSERT INTO config.upgrade_log (version) VALUES ('XXXX'); + +UPDATE config.best_hold_order +SET + approx = 1, + pprox = 2, + aprox = 3, + priority = 4, + cut = 5, + depth = 6, + rtime = 7, + hprox = NULL, + htime = NULL +WHERE name = 'Traditional' AND + pprox = 1 AND + aprox = 2 AND + priority = 3 AND + cut = 4 AND + depth = 5 AND + rtime = 6 ; + +UPDATE config.best_hold_order +SET + hprox = 1, + approx = 2, + pprox = 3, + aprox = 4, + priority = 5, + cut = 6, + depth = 7, + rtime = 8, + htime = NULL +WHERE name = 'Traditional with Holds-always-go-home' AND + hprox = 1 AND + pprox = 2 AND + aprox = 3 AND + priority = 4 AND + cut = 5 AND + depth = 6 AND + rtime = 7 AND + htime = 8; + +UPDATE config.best_hold_order +SET + htime = 1, + approx = 2, + pprox = 3, + aprox = 4, + priority = 5, + cut = 6, + depth = 7, + rtime = 8, + hprox = NULL +WHERE name = 'Traditional with Holds-go-home' AND + htime = 1 AND + hprox = 2 AND + pprox = 3 AND + aprox = 4 AND + priority = 5 AND + cut = 6 AND + depth = 7 AND + rtime = 8 ; + + +COMMIT; -- 2.11.0