From 0ec789d66d5aa7a4d92c6e517d72c4a96688bfc1 Mon Sep 17 00:00:00 2001 From: Dan Wells Date: Wed, 23 May 2018 14:39:49 -0400 Subject: [PATCH] LP#1715697 Refactor empty volume adding 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 Signed-off-by: Mike Rylander --- .../js/ui/default/staff/cat/catalog/app.js | 43 +++----- .../ui/default/staff/cat/services/holdings.js | 22 +--- .../js/ui/default/staff/cat/volcopy/app.js | 100 +++++++++--------- 3 files changed, 65 insertions(+), 100 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 30cea7bf8d..f9ba1e7fcc 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 @@ -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') }); 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 ec387ec0f2..b4fb738524 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 @@ -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() { diff --git a/Open-ILS/web/js/ui/default/staff/cat/volcopy/app.js b/Open-ILS/web/js/ui/default/staff/cat/volcopy/app.js index 32222cf1f9..82e888dab2 100644 --- a/Open-ILS/web/js/ui/default/staff/cat/volcopy/app.js +++ b/Open-ILS/web/js/ui/default/staff/cat/volcopy/app.js @@ -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) } } ); -- 2.43.2