From e235239fdaf409edd18a0ee50bb1b697da8f3510 Mon Sep 17 00:00:00 2001 From: Kevin Day Date: Fri, 8 May 2020 18:30:08 -0500 Subject: [PATCH] Implement fetching on change and dimplify internal request service design I accidentally left an incomplete @todo that was intended to be implemented in the relevant card. Simplify the implementation and update the tests while I am at it. --- app/controllers/internalRequestController.js | 61 ++++++++++-- app/services/productsService.js | 70 +++++++++++--- .../remoteProductsByProductIdService.js | 66 ------------- .../modals/pushInternalRequestModal.html | 8 +- tests/mock/service/mockProductsService.js | 42 ++++++-- .../mockRemoteProductsByProductIdService.js | 36 ------- .../internalRequestControllerTest.js | 73 ++++++++++++-- .../services/internalRequestsServiceTest.js | 2 +- tests/unit/services/productsServiceTest.js | 28 +++++- .../remoteProductsByProductIdServiceTest.js | 96 ------------------- 10 files changed, 231 insertions(+), 251 deletions(-) delete mode 100644 app/services/remoteProductsByProductIdService.js delete mode 100644 tests/mock/service/mockRemoteProductsByProductIdService.js delete mode 100644 tests/unit/services/remoteProductsByProductIdServiceTest.js diff --git a/app/controllers/internalRequestController.js b/app/controllers/internalRequestController.js index 9ff4ea0..156a7be 100644 --- a/app/controllers/internalRequestController.js +++ b/app/controllers/internalRequestController.js @@ -1,4 +1,4 @@ -app.controller('InternalRequestController', function ($controller, $scope, ApiResponseActions, InternalRequestRepo, InternalRequestsService, ProductRepo, RemoteProductsByProductIdService, ProductsService) { +app.controller('InternalRequestController', function ($controller, $scope, ApiResponseActions, InternalRequestRepo, InternalRequestsService, ProductRepo, ProductsService, WsApi) { angular.extend(this, $controller('AbstractController', { $scope: $scope @@ -12,6 +12,10 @@ app.controller('InternalRequestController', function ($controller, $scope, ApiRe $scope.featureRequestToPush = {}; + $scope.products = []; + $scope.remoteProducts = {}; + $scope.remoteProductsLoading = false; + $scope.resetInternalRequestForms = function () { InternalRequestRepo.clearValidationResults(); @@ -27,11 +31,40 @@ app.controller('InternalRequestController', function ($controller, $scope, ApiRe $scope.resetInternalRequestForms(); if ($scope.isManager() || $scope.isAdmin()) { - $scope.remoteProductsByProduct = RemoteProductsByProductIdService.remoteProducts(); - $scope.products = ProductsService.getProducts(); + $scope.selectRemoteProducts = function () { + if ($scope.remoteProductsLoading === false) { + $scope.remoteProducts = {}; + + if (angular.isDefined($scope.featureRequestToPush.productId) && $scope.featureRequestToPush.productId !== null) { + var productId = $scope.featureRequestToPush.productId; + var remoteProducts = ProductsService.getRemoteProducts(); + + if (angular.isDefined(remoteProducts[productId])) { + angular.extend($scope.remoteProducts, remoteProducts[productId]); + } else { + $scope.refreshRemoteProducts(productId); + } + } + } + }; + + $scope.refreshRemoteProducts = function (productId) { + if ($scope.remoteProductsLoading === false) { + $scope.remoteProductsLoading = true; + $scope.remoteProducts = {}; + + ProductsService.refreshRemoteProducts(productId).then(null, null, function (res) { + remoteProducts = ProductsService.getRemoteProducts(); - $scope.refreshProductRemoteProducts = function () { - RemoteProductsByProductIdService.refreshRemoteProductsByProductId($scope.featureRequestToPush.productId); + if (angular.isDefined(remoteProducts[productId])) { + angular.extend($scope.remoteProducts, remoteProducts[productId]); + } + + $scope.remoteProductsLoading = false; + }).catch(function() { + $scope.remoteProductsLoading = false; + }); + } }; $scope.createInternalRequest = function () { @@ -123,13 +156,21 @@ app.controller('InternalRequestController', function ($controller, $scope, ApiRe }); }; - ProductRepo.listen([ApiResponseActions.CREATE, ApiResponseActions.DELETE, ApiResponseActions.UPDATE], function () { - var products = ProductRepo.getAll(); + ProductsService.ready.then(null, null, function () { + $scope.products = ProductsService.getProducts(); + }); - $scope.products.length = 0; + WsApi.listen(apiMapping.Product.listen).then(null, null, function (res) { + var productId = angular.isDefined($scope.featureRequestToPush.productId) ? $scope.featureRequestToPush.productId : null; - for (var i in products) { - $scope.products.push(products[i]); + ProductsService.refreshProducts(); + + if ($scope.remoteProductsLoading === false && productId !== null) { + var apiRes = angular.fromJson(res.body); + + if (apiRes.meta.status === 'SUCCESS') { + $scope.refreshRemoteProducts(productId); + } } }); } diff --git a/app/services/productsService.js b/app/services/productsService.js index ed48929..167f04a 100644 --- a/app/services/productsService.js +++ b/app/services/productsService.js @@ -1,39 +1,81 @@ app.service('ProductsService', function ($q, ProductRepo, WsApi) { - var productsService = this; + var service = this; - var products = {}; + var products = []; + var remoteProducts = {}; var defer = $q.defer(); var process = function (response) { var apiRes = angular.fromJson(response.body); + if (apiRes.meta.status === 'SUCCESS') { + products.length = 0; angular.extend(products, apiRes.payload['ArrayList']); - defer.resolve(); - ProductRepo.reset(); + defer.notify(); } else { console.error(apiRes.meta); throw "Unable to retrieve products"; } }; + var processRemoteProduct = function (response, productId, deferRefresh) { + var apiRes = angular.fromJson(response.body); + + if (apiRes.meta.status === 'SUCCESS') { + if (angular.isDefined(remoteProducts[productId])) { + for (var key in remoteProducts[productId]) { + remoteProducts[productId][key] = undefined; + } + } else { + remoteProducts[productId] = {}; + } + + angular.extend(remoteProducts[productId], apiRes.payload.HashMap); + deferRefresh.notify(); + } else { + throw "Unable to retrieve remote products for product " + productId; + } + }; + WsApi.listen(apiMapping.Product.listen).then(null, null, function (response) { process(response); }); - productsService.refreshProducts = function () { + service.refreshProducts = function () { WsApi.fetch(apiMapping.Product.all).then(function (response) { process(response); + + for (var productId in remoteProducts) { + service.refreshRemoteProducts(productId); + } }); }; - productsService.getProducts = function () { - return products; + service.refreshRemoteProducts = function (productId) { + var deferRefresh = $q.defer(); + var options = { + pathValues: { + productId: productId + } + }; + + WsApi.fetch(apiMapping.RemoteProducts.byProduct, options).then(function (response) { + processRemoteProduct(response, productId, deferRefresh); + }); + + return deferRefresh.promise; + }; + + service.getRemoteProducts = function () { + return remoteProducts; }; - productsService.getProductRemoteProducts = function (productId) { - var remoteProducts = {}; + service.getProducts = function () { + return products; + }; + service.getRemoteProductInfo = function (productId) { for (var i in products) { if (products[i].id == productId) { return products[i].remoteProducts; @@ -41,20 +83,20 @@ app.service('ProductsService', function ($q, ProductRepo, WsApi) { } }; - productsService.getById = function (id) { + service.getById = function (id) { return $q(function (resolve, reject) { - productsService.ready.then(function () { + service.ready.then(function () { for (var i in products) { if (products[i].id == id) { resolve(products[i]); } } }); - }.bind(productsService)); + }.bind(service)); }; - productsService.refreshProducts(); + service.refreshProducts(); - productsService.ready = defer.promise; + service.ready = defer.promise; }); diff --git a/app/services/remoteProductsByProductIdService.js b/app/services/remoteProductsByProductIdService.js deleted file mode 100644 index a17a434..0000000 --- a/app/services/remoteProductsByProductIdService.js +++ /dev/null @@ -1,66 +0,0 @@ -app.service('RemoteProductsByProductIdService', function ($q, WsApi) { - var service = this; - - var productId; - var remoteProducts = {}; - - var process = function (response) { - var apiRes = angular.fromJson(response.body); - - if (apiRes.meta.status === 'SUCCESS') { - if (productId) { - angular.extend(remoteProducts, apiRes.payload.HashMap); - } - } else { - console.error(apiRes.meta); - throw "Unable to retrieve remote products for product"; - } - }; - - WsApi.listen(apiMapping.Product.listen).then(null, null, function (response) { - if (productId === undefined) { - return; - } - - // @todo: walk through list of projects and only perform this if project for projectId is present. - - //service.fetchRemoteProductsByProductId(); - }); - - service.refreshRemoteProductsByProductId = function (id) { - if (productId == id) { - return; - } - - if (remoteProducts) { - for (var key in remoteProducts) { - remoteProducts[key] = undefined; - } - } - - if (!id) { - return; - } - - productId = id; - - var options = { - pathValues: { - productId: id - } - }; - - WsApi.fetch(apiMapping.RemoteProducts.byProduct, options).then(function (response) { - process(response); - }); - }; - - service.productId = function () { - return productId; - }; - - service.remoteProducts = function () { - return remoteProducts; - }; - -}); diff --git a/app/views/modals/pushInternalRequestModal.html b/app/views/modals/pushInternalRequestModal.html index cdd3f59..41ac52a 100644 --- a/app/views/modals/pushInternalRequestModal.html +++ b/app/views/modals/pushInternalRequestModal.html @@ -7,7 +7,7 @@

Push Internal Request (Feature Request)

- +
@@ -33,20 +33,20 @@

Push Internal Request (Feature Request)

disabled="'true'" form="internalRequest.push" validations="internalRequestForms.validations" - results="internalRequestForms.getResults()" + results="internalRequestForms.getResults()"e autocomplete="off">
-
-
diff --git a/tests/mock/service/mockProductsService.js b/tests/mock/service/mockProductsService.js index a9fd57a..9f4053e 100644 --- a/tests/mock/service/mockProductsService.js +++ b/tests/mock/service/mockProductsService.js @@ -1,26 +1,50 @@ -var mockProductsService = function ($q, ProductRepo, WsApi) { +var mockProductsService = function ($q, $timeout) { var service = mockService($q); var products = {}; + var remoteProducts = {}; service.getById = function (id) { - // @todo - return {}; + return $q(function (resolve, reject) { + for (var i in dataProductRepo1) { + if (dataProductRepo1[i].id === scopeId) { + resolve(dataProductRepo1[i]); + } + } + reject(undefined); + }); } - service.getProductRemoteProducts = function (id) { - // @todo - return {}; - }; - service.getProducts = function () { return products; }; + service.getRemoteProductInfo = function (productId) { + for (var i in products) { + if (products[i].id == productId) { + return products[i]; + } + } + }; + + service.getRemoteProducts = function () { + return remoteProducts; + }; + service.refreshProducts = function () { - // @todo + return messagePromise(); }; + service.refreshRemoteProducts = function (productId) { + if (dataFeatureRequest1.id == productId) { + return notifyPromise($timeout, $q.defer(), dataFeatureRequest1); + } + + return rejectPromise($q.defer()); + }; + + service.ready = $q.defer().promise; + return service; }; diff --git a/tests/mock/service/mockRemoteProductsByProductIdService.js b/tests/mock/service/mockRemoteProductsByProductIdService.js deleted file mode 100644 index d35b3cd..0000000 --- a/tests/mock/service/mockRemoteProductsByProductIdService.js +++ /dev/null @@ -1,36 +0,0 @@ -var mockRemoteProductsByProductIdService = function ($q, WsApi) { - var service = mockService($q); - - var productId; - var remoteProducts = {}; - - service.refreshRemoteProductsByProductId = function (id) { - if (productId == id) { - return; - } - - if (remoteProducts) { - for (var key in remoteProducts) { - remoteProducts[key] = undefined; - } - } - - if (!id) { - return; - } - - productId = id; - }; - - service.productId = function () { - return productId; - }; - - service.remoteProducts = function () { - return remoteProducts; - }; - - return service; -}; - -angular.module("mock.remoteProductsByProductIdService", []).service("RemoteProductsByProductIdService", mockRemoteProductsByProductIdService); diff --git a/tests/unit/controllers/internalRequestControllerTest.js b/tests/unit/controllers/internalRequestControllerTest.js index 526ddc1..d1aac7f 100644 --- a/tests/unit/controllers/internalRequestControllerTest.js +++ b/tests/unit/controllers/internalRequestControllerTest.js @@ -1,11 +1,12 @@ describe("controller: InternalRequestController", function () { - var $q, $scope, $templateCache, MockedInternalRequest, InternalRequestRepo, InternalRequestsService, ProductRepo, ProductsService, RemoteProductsByProductIdService, WsApi, controller; + var $q, $scope, $templateCache, $timeout, MockedInternalRequest, InternalRequestRepo, InternalRequestsService, ProductRepo, ProductsService, WsApi, controller; var initializeVariables = function () { - inject(function (_$compile_, _$q_, _$templateCache_, _InternalRequestRepo_, _InternalRequestsService_, _ProductRepo_, _ProductsService_, _RemoteProductsByProductIdService_, _RemoteProductManagerRepo_, _WsApi_) { + inject(function (_$compile_, _$q_, _$templateCache_, _$timeout_, _InternalRequestRepo_, _InternalRequestsService_, _ProductRepo_, _ProductsService_, _RemoteProductManagerRepo_, _WsApi_) { $compile = _$compile_; $q = _$q_; $templateCache = _$templateCache_; + $timeout = _$timeout_; MockedInternalRequest = new mockInternalRequest($q); @@ -13,7 +14,6 @@ describe("controller: InternalRequestController", function () { InternalRequestsService = _InternalRequestsService_; ProductRepo = _ProductRepo_; ProductsService = _ProductsService_; - RemoteProductsByProductIdService = _RemoteProductsByProductIdService_; WsApi = _WsApi_; }); @@ -33,7 +33,6 @@ describe("controller: InternalRequestController", function () { ModalService: _ModalService_, ProductRepo: ProductRepo, ProductsService: ProductsService, - RemoteProductsByProductIdService: RemoteProductsByProductIdService, UserService: _UserService_ }); @@ -57,7 +56,7 @@ describe("controller: InternalRequestController", function () { module("mock.internalRequestRepo"); module("mock.internalRequestsService"); module("mock.remoteProductManagerRepo"); - module("mock.remoteProductsByProductIdService"); + module("mock.productsService"); module("mock.user", function ($provide) { var User = function () { return MockedUser; @@ -98,9 +97,10 @@ describe("controller: InternalRequestController", function () { "editInternalRequest", "pushFeatureRequest", "pushInternalRequest", - "refreshProductRemoteProducts", + "refreshRemoteProducts", "resetCreateInternalRequest", "resetInternalRequestForms", + "selectRemoteProducts", "updateInternalRequest" ]; @@ -250,15 +250,36 @@ describe("controller: InternalRequestController", function () { expect($scope.openModal).toHaveBeenCalled(); }); - it("refreshProductRemoteProducts call refreshRemoteProductsByProductId()", function () { + it("refreshRemoteProducts call refreshRemoteProducts() and toggle remoteProductsLoading", function () { + var featureRequest = new mockFeatureRequest($q); + $scope.remoteProductsLoading = false; + $scope.remoteProducts = { removeThis: "" }; + + spyOn(ProductsService, "refreshRemoteProducts").and.callThrough(); + + $scope.refreshRemoteProducts(featureRequest.productId); + + expect($scope.remoteProductsLoading).toBe(true); + + $timeout.flush(); + + expect(ProductsService.refreshRemoteProducts).toHaveBeenCalledWith(featureRequest.productId); + expect($scope.remoteProducts.removeThis).not.toBeDefined(); + expect($scope.remoteProductsLoading).toBe(false); + }); + + it("refreshRemoteProducts should do nothing when remoteProductsLoading is true", function () { var featureRequest = new mockFeatureRequest($q); $scope.featureRequestToPush = featureRequest; + $scope.remoteProductsLoading = true; + $scope.remoteProducts = { removeThis: "" }; - spyOn(RemoteProductsByProductIdService, "refreshRemoteProductsByProductId"); + spyOn(ProductsService, "refreshRemoteProducts"); - $scope.refreshProductRemoteProducts(); + $scope.refreshRemoteProducts(); - expect(RemoteProductsByProductIdService.refreshRemoteProductsByProductId).toHaveBeenCalled(); + expect(ProductsService.refreshRemoteProducts).not.toHaveBeenCalled(); + expect($scope.remoteProducts.removeThis).toBeDefined(); }); it("resetCreateInternalRequest call resetInternalRequestForms() and clear out the properties", function () { @@ -308,6 +329,38 @@ describe("controller: InternalRequestController", function () { expect($scope.closeModal).toHaveBeenCalled(); }); + it("selectRemoteProducts select a remote product, set loading boolean, and refresh if needed", function () { + var featureRequest = new mockFeatureRequest($q); + + $scope.remoteProducts = { removeThis: "" }; + + $scope.featureRequestToPush = featureRequest; + $scope.remoteProductsLoading = false; + + spyOn($scope, "refreshRemoteProducts"); + + $scope.selectRemoteProducts(); + + expect($scope.refreshRemoteProducts).toHaveBeenCalled(); + expect($scope.remoteProducts.removeThis).not.toBeDefined(); + }); + + it("selectRemoteProducts does nothing when remoteProductsLoading is true", function () { + var featureRequest = new mockFeatureRequest($q); + + $scope.remoteProducts = { removeThis: "" }; + + $scope.featureRequestToPush = featureRequest; + $scope.remoteProductsLoading = true; + + spyOn($scope, "refreshRemoteProducts"); + + $scope.selectRemoteProducts(); + + expect($scope.refreshRemoteProducts).not.toHaveBeenCalled(); + expect($scope.remoteProducts.removeThis).toBeDefined(); + }); + it("updateInternalRequest call dirty and save on the InternalRequest, and then call cancelEditInternalRequest", function () { var internalRequest = new mockInternalRequest($q); diff --git a/tests/unit/services/internalRequestsServiceTest.js b/tests/unit/services/internalRequestsServiceTest.js index d963b64..f1ef659 100644 --- a/tests/unit/services/internalRequestsServiceTest.js +++ b/tests/unit/services/internalRequestsServiceTest.js @@ -2,7 +2,7 @@ describe("service: InternalRequestsService", function () { var $q, $rootScope, $scope, InternalRequestRepo, WsApi, service; var initializeVariables = function (settings) { - inject(function (_$q_, _$rootScope_, _InternalRequestRepo_, _WsApi_) { + inject(function (_$q_, _$rootScope_, _$timeout_, _InternalRequestRepo_, _WsApi_) { $q = _$q_; $rootScope = _$rootScope_; diff --git a/tests/unit/services/productsServiceTest.js b/tests/unit/services/productsServiceTest.js index bd41b4b..2c83b7b 100644 --- a/tests/unit/services/productsServiceTest.js +++ b/tests/unit/services/productsServiceTest.js @@ -43,9 +43,10 @@ describe("service: ProductsService", function () { describe("Is the service method", function () { var methods = [ "getById", - "getProductRemoteProducts", "getProducts", - "refreshProducts" + "getRemoteProductInfo", + "refreshProducts", + "refreshRemoteProducts" ]; var serviceMethodExists = function (method) { @@ -82,16 +83,33 @@ describe("service: ProductsService", function () { //expect(response).toBe(); }); - it("getProductRemoteProducts should return all remote products for a given product", function () { + it("getRemoteProductInfo should return all remote product info for a given product", function () { var response; - var id = 1; - // @todo + // @todo //response = service.getById(id); //$scope.$digest(); //expect(response.id).toBe(id); }); + + it("refreshProducts should refresh products", function () { + var response; + + // @todo + //response = service.refreshProducts(); + //$scope.$digest(); + + //expect(response.id).toBe(id); + }); + + it("refreshRemoteProducts should refresh remote products", function () { + var response; + + // @todo + //response = service.refreshRemoteProducts(id); + //$scope.$digest(); + }); }); }); diff --git a/tests/unit/services/remoteProductsByProductIdServiceTest.js b/tests/unit/services/remoteProductsByProductIdServiceTest.js deleted file mode 100644 index 38cc308..0000000 --- a/tests/unit/services/remoteProductsByProductIdServiceTest.js +++ /dev/null @@ -1,96 +0,0 @@ -describe("service: RemoteProductsByProductIdService", function () { - var $q, $rootScope, $scope, WsApi, service; - - var initializeVariables = function (settings) { - inject(function (_$q_, _$rootScope_, _WsApi_) { - $q = _$q_; - $rootScope = _$rootScope_; - - WsApi = _WsApi_; - }); - }; - - var initializeService = function (settings) { - inject(function ($injector, _RemoteProductsByProductIdService_) { - $scope = $rootScope.$new(); - - service = _RemoteProductsByProductIdService_; - - // ensure that the isReady() is called. - if (!$scope.$$phase) { - $scope.$digest(); - } - }); - }; - - beforeEach(function () { - module("core"); - module("app"); - module("mock.product"); - module("mock.wsApi"); - - initializeVariables(); - initializeService(); - }); - - describe("Is the service", function () { - it("defined", function () { - expect(service).toBeDefined(); - }); - }); - - describe("Is the service method", function () { - var methods = [ - "productId", - "refreshRemoteProductsByProductId", - "remoteProducts" - ]; - - var serviceMethodExists = function (method) { - return function() { - expect(service[method]).toBeDefined(); - expect(typeof service[method]).toEqual("function"); - }; - }; - - for (var i in methods) { - it(methods[i] + " defined", serviceMethodExists(methods[i])); - } - }); - - describe("Do the service method", function () { - it("productId should return a product id, when assigned and null when unassigned", function () { - var response; - var id = 1; - - response = service.productId(); - expect(response).not.toBeDefined(); - - service.refreshRemoteProductsByProductId(id); - - response = service.productId(); - expect(response).toBe(1); - }); - - it("refreshRemoteProductsByProductId should assign a product id", function () { - var response; - var id = 2; - - service.refreshRemoteProductsByProductId(id); - expect(service.productId()).toBe(id); - }); - - it("remoteProducts should return remote products for the given id", function () { - var response; - var id = 1; - - service.refreshRemoteProductsByProductId(id); - - // @todo: - //response = service.remoteProducts(); - //expect(response).toBe(1); - }); - - }); - -});