From c428445f2483af3303a0c36b3b0102c4290d8325 Mon Sep 17 00:00:00 2001 From: Lebbeous Fogle-Weekley Date: Thu, 6 Dec 2012 19:05:59 -0500 Subject: [PATCH] Tracked down two bugs to get the right proximity into the hold copy map The first took a little while because I was learning the lay of the land and needed Mike's help. SELECTs in action.hold_copy_calculated_proximity() didn't quite filter their result rows hard enough. The second was more subtle. After fixing the first bug, all proximities reaching the copy map after targeting were zero. This came down to one place where we mistook a parameter that really means 'copy location' to mean 'hold pickup lib', and wound up measuring the length of a path whose two points were both hold pickup lib (so always zero). Finally tracked down and repaired. Signed-off-by: Lebbeous Fogle-Weekley --- .../lib/OpenILS/Application/Storage/Publisher/asset.pm | 18 +++++++++++++----- Open-ILS/src/sql/Pg/090.schema.action.sql | 8 ++++++-- .../src/sql/Pg/upgrade/XXXX.schema.org_prox_adjust.sql | 8 ++++++-- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/asset.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/asset.pm index b4e3fbd8e2..90ca204c6b 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/asset.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/asset.pm @@ -396,7 +396,7 @@ sub copy_proximity { my $client = shift; my $cp = shift; - my $org = shift; + my $org = shift; # hold pickup lib my $hold = shift; return unless ($cp && $org); @@ -410,13 +410,21 @@ sub copy_proximity { ); return $row->{prox} if $row; - $log->debug("Calculating copy proximity with: action.hold_copy_calculated_proximity($hold,$cp,$org)", DEBUG); + # There was a bug here before. + # action.hold_copy_calculated_proximity() was called with a + # third argument, $org. Wrong. a.hccp() interprets its third + # argument as an optional override of copy circ lib. $org + # here is hold pickup lib. This had the effect of basically + # measuring the distance between a hold's pickup lib and + # itself, which is always zero, so all proximities landing in + # the hold copy map were zero. + + $log->debug("Calculating copy proximity with: action.hold_copy_calculated_proximity($hold,$cp)", DEBUG); $row = action::hold_request->db_Main->selectrow_hashref( - 'SELECT action.hold_copy_calculated_proximity(?,?,?) AS prox', + 'SELECT action.hold_copy_calculated_proximity(?,?) AS prox', {}, "$hold", - "$cp", - "$org" + "$cp" ); return $row->{prox} if $row; diff --git a/Open-ILS/src/sql/Pg/090.schema.action.sql b/Open-ILS/src/sql/Pg/090.schema.action.sql index 9d5630c359..2bbb098386 100644 --- a/Open-ILS/src/sql/Pg/090.schema.action.sql +++ b/Open-ILS/src/sql/Pg/090.schema.action.sql @@ -1010,7 +1010,9 @@ BEGIN LEFT JOIN actor.org_unit_ancestors_distance(acl.owning_lib) acl_ol ON (acn_ol.id = adj.copy_location) LEFT JOIN actor.org_unit_ancestors_distance(ahr.pickup_lib) ahr_pl ON (ahr_pl.id = adj.hold_pickup_lib) LEFT JOIN actor.org_unit_ancestors_distance(ahr.request_lib) ahr_rl ON (ahr_rl.id = adj.hold_request_lib) - WHERE (adj.circ_mod IS NULL OR adj.circ_mod = acp.circ_modifier) AND absolute_adjustment + WHERE (adj.circ_mod IS NULL OR adj.circ_mod = acp.circ_modifier) AND + absolute_adjustment AND + COALESCE(acp_cl.id, acn_ol.id, acl_ol.id, ahr_pl.id, ahr_rl.id) IS NOT NULL ORDER BY COALESCE(acp_cl.distance,999) + COALESCE(acn_ol.distance,999) @@ -1033,7 +1035,9 @@ BEGIN LEFT JOIN actor.org_unit_ancestors_distance(acl.owning_lib) acl_ol ON (acn_ol.id = adj.copy_location) LEFT JOIN actor.org_unit_ancestors_distance(ahr.pickup_lib) ahr_pl ON (ahr_pl.id = adj.hold_pickup_lib) LEFT JOIN actor.org_unit_ancestors_distance(ahr.request_lib) ahr_rl ON (ahr_rl.id = adj.hold_request_lib) - WHERE (adj.circ_mod IS NULL OR adj.circ_mod = acp.circ_modifier) AND NOT absolute_adjustment + WHERE (adj.circ_mod IS NULL OR adj.circ_mod = acp.circ_modifier) AND + NOT absolute_adjustment AND + COALESCE(acp_cl.id, acn_ol.id, acl_ol.id, ahr_pl.id, ahr_rl.id) IS NOT NULL LOOP baseline_prox := baseline_prox + aoupa.prox_adjustment; END LOOP; diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.org_prox_adjust.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.org_prox_adjust.sql index d36d6027e3..d9eb082a5b 100644 --- a/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.org_prox_adjust.sql +++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.schema.org_prox_adjust.sql @@ -69,7 +69,9 @@ BEGIN LEFT JOIN actor.org_unit_ancestors_distance(acl.owning_lib) acl_ol ON (acn_ol.id = adj.copy_location) LEFT JOIN actor.org_unit_ancestors_distance(ahr.pickup_lib) ahr_pl ON (ahr_pl.id = adj.hold_pickup_lib) LEFT JOIN actor.org_unit_ancestors_distance(ahr.request_lib) ahr_rl ON (ahr_rl.id = adj.hold_request_lib) - WHERE (adj.circ_mod IS NULL OR adj.circ_mod = acp.circ_modifier) AND absolute_adjustment + WHERE (adj.circ_mod IS NULL OR adj.circ_mod = acp.circ_modifier) AND + absolute_adjustment AND + COALESCE(acp_cl.id, acn_ol.id, acl_ol.id, ahr_pl.id, ahr_rl.id) IS NOT NULL ORDER BY COALESCE(acp_cl.distance,999) + COALESCE(acn_ol.distance,999) @@ -92,7 +94,9 @@ BEGIN LEFT JOIN actor.org_unit_ancestors_distance(acl.owning_lib) acl_ol ON (acn_ol.id = adj.copy_location) LEFT JOIN actor.org_unit_ancestors_distance(ahr.pickup_lib) ahr_pl ON (ahr_pl.id = adj.hold_pickup_lib) LEFT JOIN actor.org_unit_ancestors_distance(ahr.request_lib) ahr_rl ON (ahr_rl.id = adj.hold_request_lib) - WHERE (adj.circ_mod IS NULL OR adj.circ_mod = acp.circ_modifier) AND NOT absolute_adjustment + WHERE (adj.circ_mod IS NULL OR adj.circ_mod = acp.circ_modifier) AND + NOT absolute_adjustment AND + COALESCE(acp_cl.id, acn_ol.id, acl_ol.id, ahr_pl.id, ahr_rl.id) IS NOT NULL LOOP baseline_prox := baseline_prox + aoupa.prox_adjustment; END LOOP; -- 2.11.0