From 8bc57fe58b364f0dc5dc43b3efa82adebdf8f9a0 Mon Sep 17 00:00:00 2001
From: Bill Erickson <berickxx@gmail.com>
Date: Thu, 16 Jul 2020 12:22:52 -0400
Subject: [PATCH] LP1865564 Holds grid avoid dupes (AngularJS)

Address a couple if cases where the record holds grid in the AngularJS
staff catalog would make multiple network calls to fetch holds data.
In some cases, these calls would result displaying duplicate holds.

1. Avoid fetching holds when the pickup lib selector fires its on change
if the value provided matches the pickup lib we are already using.

2. Avoid reseting and reloading the grid during an active grid load.
Instead wait for the current load to complete before launching the next
load action.

Patch also includes a minor sanity check in the holdings code to avoid
console errors caused during pickup lib change.

Signed-off-by: Bill Erickson <berickxx@gmail.com>
Signed-off-by: Michele Morgan <mmorgan@noblenet.org>
Signed-off-by: Galen Charlton <gmc@equinoxinitiative.org>
---
 .../js/ui/default/staff/cat/catalog/app.js    | 47 ++++++++++++++++---
 .../ui/default/staff/cat/services/holdings.js |  1 +
 2 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/Open-ILS/web/js/ui/default/staff/cat/catalog/app.js b/Open-ILS/web/js/ui/default/staff/cat/catalog/app.js
index c3b6b1f6bf..809ffeaa94 100644
--- a/Open-ILS/web/js/ui/default/staff/cat/catalog/app.js
+++ b/Open-ILS/web/js/ui/default/staff/cat/catalog/app.js
@@ -1778,6 +1778,7 @@ function($scope , $routeParams , $location , $window , $q , egCore , egHolds , e
     var provider = egGridDataProvider.instance({});
     var holds = []; // current list of holds
     var hold_count = 0;
+    var hold_grid_load_promise;
 
     $scope.hold_grid_data_provider = provider;
     $scope.grid_actions = egHoldGridActions;
@@ -1787,8 +1788,16 @@ function($scope , $routeParams , $location , $window , $q , egCore , egHolds , e
     provider.get = function(offset, count) {
         if ($scope.record_tab != 'holds') return $q.when();
 
+        if (hold_grid_load_promise) {
+            // Active load in progress.
+            console.debug('Exiting concurrent hold fetch');
+            return hold_grid_load_promise;
+        }
+
         // see if we have the requested range cached
         if (holds[offset]) {
+            console.debug(
+                'Serving holds from cache with pickup lib', $scope.pickup_ou.id());
             return provider.arrayNotifier(holds, offset, count);
         }
 
@@ -1828,12 +1837,16 @@ function($scope , $routeParams , $location , $window , $q , egCore , egHolds , e
             });
         }
 
+        console.debug(
+            'Fetching holds from network with PU lib', $scope.pickup_ou.id());
+
         egProgressDialog.open({max : 1, value : 0});
         var first = true;
-        return egHolds.fetch_wide_holds(
+        hold_grid_load_promise = egHolds.fetch_wide_holds(
             restrictions,
             order_by
         ).then(function () {
+                hold_grid_load_promise = null;
                 return provider.arrayNotifier(holds, offset, count);
             },
             null,
@@ -1852,8 +1865,12 @@ function($scope , $routeParams , $location , $window , $q , egCore , egHolds , e
                     holds.push(new_item);
                 }
             }
-        ).finally(egProgressDialog.close);
+        ).finally(function() {
+            hold_grid_load_promise = null;
+            egProgressDialog.close();
+        });
 
+        return hold_grid_load_promise;
     }
 
     $scope.detail_view = function(action, user_data, items) {
@@ -1869,10 +1886,28 @@ function($scope , $routeParams , $location , $window , $q , egCore , egHolds , e
     // refresh the list of record holds when the pickup lib is changed.
     $scope.pickup_ou = egCore.org.get(egCore.auth.user().ws_ou());
     $scope.pickup_ou_changed = function(org) {
-        $scope.pickup_ou = org;
-        holds = []
-        hold_count = 0;
-        provider.refresh();
+        if ($scope.pickup_ou && $scope.pickup_ou.id() == org.id()) {
+            // This fires on every component render, even though the
+            // value we already have may match.  Avoid duplicate lookups.
+            return;
+        }
+
+        var promise = hold_grid_load_promise || $q.when();
+
+        // Avoid refreshing the grid if it's currently loading data.
+        promise.finally(function() {
+
+            // Previous grid data load complete.  Timeout gives the
+            // grid a chance to mark itself as load-completed, which
+            // happens after the data load promise is done.
+            setTimeout(function() {
+                console.debug('Refreshing holds after PU lib change to ', org.id());
+                $scope.pickup_ou = org;
+                holds = []
+                hold_count = 0;
+                provider.refresh();
+            });
+        })
     }
 
     function map_prefix_to_subhash (h,pf) {
diff --git a/Open-ILS/web/js/ui/default/staff/cat/services/holdings.js b/Open-ILS/web/js/ui/default/staff/cat/services/holdings.js
index aeae4b9751..d76e7bbaa2 100644
--- a/Open-ILS/web/js/ui/default/staff/cat/services/holdings.js
+++ b/Open-ILS/web/js/ui/default/staff/cat/services/holdings.js
@@ -191,6 +191,7 @@ function(egCore , $q) {
                     function(circ) {
                         var cp = svc.copies.filter(function(c) { 
                             return c.id == circ.target_copy() })[0];
+                        if (!cp) { return; } // can disappear during reloads.
                         cp._circ = egCore.idl.toHash(circ, true);
                         cp._circ_lib = circ.circ_lib();
                         cp._duration = circ.duration();
-- 
2.43.2