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 <lebbeous@esilibrary.com>

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 b4e3fbd..90ca204 100644 (file)
--- 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 9d5630c..2bbb098 100644 (file)
--- 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 d36d602..d9eb082 100644 (file)
--- 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;