LP#1715697 Refactor empty volume adding
authorDan Wells <dbw2@calvin.edu>
Wed, 23 May 2018 18:39:49 +0000 (14:39 -0400)
committerDan Wells <dbw2@calvin.edu>
Wed, 18 Jul 2018 13:40:39 +0000 (09:40 -0400)
The new ability to add empty volumes was causing the existing
ability to add new volume/copy combos to not work as expected.
More specifically, added volume/copy combos would not generate
in the selected org_unit, but always in the ws_ou.

To correct this, this change refactors/reverts significant portions of
920f585052ef809ea6ca1e447d416ada871b467c.  Reasons include:

- Existing code distinguishs 'adds' from 'edits' via two wrappers,
spawnHoldingsAdd and spawnHoldingsEdit.  With this commit, empty volume
adding now extends the 'add' function rather than the 'edit' one, as
this seems more intuitive.

- The previous change had extended both the catalog app and another
similar directive which is only used in a merging context.  Since the
merge context had no ability to add anything, and the new code was not
wired up to the interface, this has simply been removed (for now).

- The volcopy app is set up around the concept of passed in
'prototype' vol/copy objects of varying degrees of completeness.  It
then loops over these to generate the interface.  The previous code
extended this setup with a loop over a potential 'owners' array to
generate empty volumes, but this unrelated loop within a loop seemed
counterintuitive (and was the source of the original bug).  This change
has been removed, and empty volume creation now hews more closely to
the original model.

While this commit appears large, when viewed in the context of the
pre-920f58505 code, it is quite limited in scope.

Signed-off-by: Dan Wells <dbw2@calvin.edu>
Signed-off-by: Mike Rylander <mrylander@gmail.com>

Open-ILS/web/js/ui/default/staff/cat/catalog/app.js
Open-ILS/web/js/ui/default/staff/cat/services/holdings.js
Open-ILS/web/js/ui/default/staff/cat/volcopy/app.js

index 30cea7b..f9ba1e7 100644 (file)
@@ -1120,15 +1120,6 @@ function($scope , $routeParams , $location , $window , $q , egCore , egHolds , e
         $scope.holdings_cb_changed(item.checkbox,item.checked);
     }
 
-    function gatherSelectedOwners () {
-        var owner_list = [];
-        angular.forEach(
-            $scope.holdingsGridControls.selectedItems(),
-            function (item) { owner_list.push(item.owner_id) }
-        );
-        return owner_list;
-    }
-
     function gatherSelectedHoldingsIds () {
         var cp_id_list = [];
         angular.forEach(
@@ -1242,7 +1233,7 @@ function($scope , $routeParams , $location , $window , $q , egCore , egHolds , e
     $scope.selectedHoldingsVolCopyDelete = function () { $scope.selectedHoldingsDelete(true,true) }
     $scope.selectedHoldingsEmptyVolCopyDelete = function () { $scope.selectedHoldingsDelete(true,false) }
 
-    spawnHoldingsAdd = function (vols,copies){
+    spawnHoldingsAdd = function (vols,copies,only_add_vol){
         var raw = [];
         if (copies) { // just a copy on existing volumes
             angular.forEach(gatherSelectedVolumeIds(), function (v) {
@@ -1255,7 +1246,7 @@ function($scope , $routeParams , $location , $window , $q , egCore , egHolds , e
                     function (item) {
                         raw.push({
                             owner : item.owner_id,
-                            label : item.call_number.label
+                            label : ((item.call_number) ? item.call_number.label : null)
                         });
                     });
             } else {
@@ -1274,7 +1265,8 @@ function($scope , $routeParams , $location , $window , $q , egCore , egHolds , e
                 record_id: $scope.record_id,
                 raw: raw,
                 hide_vols : false,
-                hide_copies : false
+                hide_copies : ((only_add_vol) ? true : false),
+                only_add_vol : only_add_vol
             }
         ).then(function(key) {
             if (key) {
@@ -1285,31 +1277,22 @@ function($scope , $routeParams , $location , $window , $q , egCore , egHolds , e
             }
         });
     }
-    $scope.selectedHoldingsVolCopyAdd = function () { spawnHoldingsAdd(true,false) }
-    $scope.selectedHoldingsCopyAdd = function () { spawnHoldingsAdd(false,true) }
-
-    spawnHoldingsEdit = function (hide_vols,hide_copies,add_vol){
-
-        var raw;
-        if (add_vol) {
-            raw = [{callnumber:null}];
-        } else {
-            raw = gatherSelectedEmptyVolumeIds().map(
-                function(v){ return { callnumber : v } }
-            );
-        }
+    $scope.selectedHoldingsVolCopyAdd = function () { spawnHoldingsAdd(true,false,false) }
+    $scope.selectedHoldingsCopyAdd = function () { spawnHoldingsAdd(false,true,false) }
+    $scope.selectedHoldingsVolAdd = function () { spawnHoldingsAdd(true,false,true) }
 
+    spawnHoldingsEdit = function (hide_vols,hide_copies){
         egCore.net.request(
             'open-ils.actor',
             'open-ils.actor.anon_cache.set_value',
             null, 'edit-these-copies', {
                 record_id: $scope.record_id,
                 copies: gatherSelectedHoldingsIds(),
-                raw: raw,
+                raw: gatherSelectedEmptyVolumeIds().map(
+                    function(v){ return { callnumber : v } }
+                ),
                 hide_vols : hide_vols,
-                hide_copies : hide_copies,
-                only_add_vol : ((add_vol) ? true : false),
-                owners : gatherSelectedOwners()
+                hide_copies : hide_copies
             }
         ).then(function(key) {
             if (key) {
@@ -1324,8 +1307,6 @@ function($scope , $routeParams , $location , $window , $q , egCore , egHolds , e
     $scope.selectedHoldingsVolEdit = function () { spawnHoldingsEdit(false,true) }
     $scope.selectedHoldingsCopyEdit = function () { spawnHoldingsEdit(true,false) }
 
-    $scope.selectedHoldingsVolAdd = function () { spawnHoldingsEdit(false,true,true) }
-
     $scope.selectedHoldingsItemStatus = function (){
         var url = egCore.env.basePath + 'cat/item/search/' + gatherSelectedHoldingsIds().join(',')
         $timeout(function() { $window.open(url, '_blank') });
index ec387ec..b4fb738 100644 (file)
@@ -360,15 +360,6 @@ function(egCore , $q) {
                     }
                 });
 
-                function gatherSelectedOwners () {
-                    var owner_list = [];
-                    angular.forEach(
-                        $scope.holdingsGridControls.selectedItems(),
-                        function (item) { owner_list.push(item.owner_id) }
-                    );
-                    return owner_list;
-                }
-
                 function gatherHoldingsIds () {
                     var cp_id_list = [];
                     angular.forEach(
@@ -378,7 +369,7 @@ function(egCore , $q) {
                     return cp_id_list;
                 }
 
-                var spawn_volume_editor = function (copies_too,add_vol) {
+                var spawn_volume_editor = function (copies_too) {
                     egCore.net.request(
                         'open-ils.actor',
                         'open-ils.actor.anon_cache.set_value',
@@ -386,9 +377,7 @@ function(egCore , $q) {
                             record_id: $scope.recordId,
                             copies: gatherHoldingsIds(),
                             hide_vols : false,
-                            hide_copies : ((copies_too) ? false : true),
-                            only_add_vol : ((add_vol) ? true : false),
-                            owners : gatherSelectedOwners()
+                            hide_copies : ((copies_too) ? false : true)
                         }
                     ).then(function(key) {
                         if (key) {
@@ -410,14 +399,11 @@ function(egCore , $q) {
                         }
                     });
                 }
-                $scope.add_emtpy_volumes = function() {
-                    spawn_volume_editor(false,true);
-                }
                 $scope.edit_volumes = function() {
-                    spawn_volume_editor(false,false);
+                    spawn_volume_editor(false);
                 }
                 $scope.edit_copies = function() {
-                    spawn_volume_editor(true,false);
+                    spawn_volume_editor(true);
                 }
 
                 function load_holdings() {
index 32222cf..82e888d 100644 (file)
@@ -1367,8 +1367,6 @@ function($scope , $q , $window , $routeParams , $location , $timeout , egCore ,
         ).then(function (data) {
 
             if (data) {
-                var owners = [ egCore.auth.user().ws_ou() ];
-
                 if (data.hide_vols && !$scope.defaults.always_volumes) $scope.show_vols = false;
                 if (data.hide_copies) {
                     $scope.show_copies = false;
@@ -1376,9 +1374,6 @@ function($scope , $q , $window , $routeParams , $location , $timeout , egCore ,
                 }
                 if (data.only_add_vol) {
                     $scope.only_add_vol = true;
-                    $scope.only_vols = true;
-                    $scope.show_copies = false;
-                    owners = data.owners;
                 }
 
                 $scope.record_id = data.record_id;
@@ -1422,56 +1417,59 @@ function($scope , $q , $window , $routeParams , $location , $timeout , egCore ,
                                     itemSvc.addCopy(cp)
                                 });
                             } else {
-                                angular.forEach(owners, function(owner) {
-                                    var cn = new egCore.idl.acn();
-                                    cn.id( --itemSvc.new_cn_id );
-                                    cn.isnew( true );
-                                    cn.prefix( $scope.defaults.prefix || -1 );
-                                    cn.suffix( $scope.defaults.suffix || -1 );
-                                    cn.owning_lib( owner );
-                                    cn.record( $scope.record_id );
-                                    egCore.org.settings(
-                                        ['cat.default_classification_scheme'],
-                                        cn.owning_lib()
-                                    ).then(function (val) {
-                                        cn.label_class(
-                                            $scope.defaults.classification ||
-                                            val['cat.default_classification_scheme'] ||
-                                            1
-                                        );
-                                        if (proto.label) {
-                                            cn.label( proto.label );
-                                        } else {
-                                            egCore.net.request(
-                                                'open-ils.cat',
-                                                'open-ils.cat.biblio.record.marc_cn.retrieve',
-                                                $scope.record_id,
-                                                cn.label_class()
-                                            ).then(function(cn_array) {
-                                                if (cn_array.length > 0) {
-                                                    for (var field in cn_array[0]) {
-                                                        cn.label( cn_array[0][field] );
-                                                        break;
-                                                    }
+                                var cn = new egCore.idl.acn();
+                                cn.id( --itemSvc.new_cn_id );
+                                cn.isnew( true );
+                                cn.prefix( $scope.defaults.prefix || -1 );
+                                cn.suffix( $scope.defaults.suffix || -1 );
+                                cn.owning_lib( proto.owner || egCore.auth.user().ws_ou() );
+                                cn.record( $scope.record_id );
+                                egCore.org.settings(
+                                    ['cat.default_classification_scheme'],
+                                    cn.owning_lib()
+                                ).then(function (val) {
+                                    cn.label_class(
+                                        $scope.defaults.classification ||
+                                        val['cat.default_classification_scheme'] ||
+                                        1
+                                    );
+                                    if (proto.label) {
+                                        cn.label( proto.label );
+                                    } else {
+                                        egCore.net.request(
+                                            'open-ils.cat',
+                                            'open-ils.cat.biblio.record.marc_cn.retrieve',
+                                            $scope.record_id,
+                                            cn.label_class()
+                                        ).then(function(cn_array) {
+                                            if (cn_array.length > 0) {
+                                                for (var field in cn_array[0]) {
+                                                    cn.label( cn_array[0][field] );
+                                                    break;
                                                 }
-                                            });
-                                        }
-                                    });
+                                            }
+                                        });
+                                    }
+                                });
 
-                                    var cp = new itemSvc.generateNewCopy(
-                                        cn,
-                                        proto.owner || egCore.auth.user().ws_ou(),
-                                        $scope.is_fast_add,
-                                        true
-                                    );
+                                // If we are adding an empty vol,
+                                // this is ultimately just a placeholder copy
+                                // which gets removed before saving.
+                                // TODO: consider ways to remove this
+                                // requirement
+                                var cp = new itemSvc.generateNewCopy(
+                                    cn,
+                                    proto.owner || egCore.auth.user().ws_ou(),
+                                    $scope.is_fast_add,
+                                    true
+                                );
 
-                                    if (proto.barcode) {
-                                        cp.barcode( proto.barcode );
-                                        cp.empty_barcode = false;
-                                    }
+                                if (proto.barcode) {
+                                    cp.barcode( proto.barcode );
+                                    cp.empty_barcode = false;
+                                }
 
-                                    itemSvc.addCopy(cp)
-                                });
+                                itemSvc.addCopy(cp)
                             }
                         }
                     );