From 6488b096b17fe6ddeba95b88fa783e8e22b90a81 Mon Sep 17 00:00:00 2001 From: Mark Polak Date: Thu, 6 Apr 2017 17:29:16 +0200 Subject: [PATCH] Refactoring of dataset group selector --- src/main/app.js | 35 +++--- src/main/datasets/datasetgroup-service.js | 107 +++++++++-------- .../datasetgroupselector-directive.js | 44 +++---- src/main/manifest.webapp | 4 +- src/test/mocks/datasetgroup-service_mock.js | 15 +-- .../dataset/datasetgroup-factory_spec.js | 39 ------ .../dataset/datasetgroup-service_spec.js | 112 ------------------ .../datasetgroupselector-directive_spec.js | 17 ++- .../dataset/datasetview-directive_spec.js | 7 ++ 9 files changed, 115 insertions(+), 265 deletions(-) delete mode 100644 src/test/specs/dataset/datasetgroup-factory_spec.js diff --git a/src/main/app.js b/src/main/app.js index 13b5ee2..349a3a1 100644 --- a/src/main/app.js +++ b/src/main/app.js @@ -30,7 +30,7 @@ angular.module('PEPFAR.approvals') function appController(periodService, $scope, currentUser, mechanismsService, approvalLevelsService, toastr, AppManifest, systemSettings, $translate, userApprovalLevels$, - organisationunitsService, $log, rx, workflowService) { + organisationunitsService, $log, rx, workflowService, dataSetGroupService) { var self = this; var vm = this; var organisationUnit$ = new rx.ReplaySubject(1); @@ -458,26 +458,27 @@ function appController(periodService, $scope, currentUser, mechanismsService, } //When the dataset group is changed update the filter types and the datasets - $scope.$on('DATASETGROUP.changed', function (event, dataSets) { - // Grab the period types from the Workflow - workflowService.setCurrentWorkflow(dataSets.get()[0].workflow); + dataSetGroupService.currentDataSetGroup$ + .subscribe(function (dataSets) { + // Grab the period types from the Workflow + workflowService.setCurrentWorkflow(dataSets.get()[0].workflow); - $scope.details.dataSets = dataSets.get(); - mechanismsService.categories = dataSets.getCategoryIds(); - mechanismsService.dataSetIds = dataSets.getIds(); - mechanismsService.dataSets = dataSets.get(); + $scope.details.dataSets = dataSets.get(); + mechanismsService.categories = dataSets.getCategoryIds(); + mechanismsService.dataSetIds = dataSets.getIds(); + mechanismsService.dataSets = dataSets.get(); - // Reset the selection - $scope.details.currentSelection = []; + // Reset the selection + $scope.details.currentSelection = []; - // When the details are available load the mechanisms for the table - if (self.hasTableDetails()) { - self.showData = false; - self.deSelect(); - } + // When the details are available load the mechanisms for the table + if (self.hasTableDetails()) { + self.showData = false; + self.deSelect(); + } - self.updateViewButton(); - }); + self.updateViewButton(); + }); function setOrganisationUnit() { if (organisationunitsService.currentOrganisationUnit.name === 'Global') { diff --git a/src/main/datasets/datasetgroup-service.js b/src/main/datasets/datasetgroup-service.js index fbf69c5..9c70299 100644 --- a/src/main/datasets/datasetgroup-service.js +++ b/src/main/datasets/datasetgroup-service.js @@ -1,13 +1,38 @@ -function dataSetGroupService($q, periodService, Restangular, workflowService, errorHandler) { - var service = this; +function dataSetGroupService($q, periodService, Restangular, workflowService, rx, errorHandler) { var dataSetGroups = {}; var dataSetGroupNames = []; + var dataSetGroups$ = new rx.ReplaySubject(1); + var currentDataSetGroup$ = new rx.ReplaySubject(1); - this.getGroups = function () { - return dataSetGroups; + workflowService.workflows$ + .flatMap(function (workflows) { + return rx.Observable.fromPromise( + loadDataSetsForWorkflows(workflows) + .then(matchDataSetsToWorkflows) + .then(setWorkflowsOntoService) + ); + }) + .subscribe( + function () {}, + function (error) { + errorHandler.error(error.message); + } + ); + + return { + setCurrentDataSetGroup: setCurrentDataSetGroup, + dataSetGroups$: dataSetGroups$, + currentDataSetGroup$: currentDataSetGroup$, + getDataSetsForGroup: getDataSetsForGroup }; - this.filterDataSetsForUser = function (workflows) { + function setCurrentDataSetGroup(selectedDataSetGroup) { + var dataSetGroup = dataSetGroupFactory(getDataSetsForGroup(selectedDataSetGroup)); + + currentDataSetGroup$.onNext(dataSetGroup); + } + + function filterDataSetsForUser(workflows) { function onlyWorkflowsWithDataSets(workflow) { return Array.isArray(workflow.dataSets) && workflow.dataSets.length > 0; } @@ -31,18 +56,16 @@ function dataSetGroupService($q, periodService, Restangular, workflowService, er if (!(dataSetGroups && dataSetGroupNames[0] && dataSetGroups[dataSetGroupNames[0]])) { errorHandler.warning('No dataset groups were found that your account can access. This could be the result of your account not having access to these datasets.', true); + } else { + dataSetGroups$.onNext(dataSetGroupNames); } - }; - - this.getDataSetGroupNames = function () { - return dataSetGroupNames; - }; + } - this.getDataSetsForGroup = function (dataSetGroupName) { + function getDataSetsForGroup(dataSetGroupName) { if (dataSetGroups[dataSetGroupName]) { return dataSetGroups[dataSetGroupName].dataSets; } - }; + } function pickId(value) { return value.id; @@ -103,46 +126,32 @@ function dataSetGroupService($q, periodService, Restangular, workflowService, er if (!Array.isArray(workflows)) { return $q.reject('Could not properly load the Workflows from the api.'); } - service.filterDataSetsForUser(workflows); - } - workflowService.workflows$ - .subscribe( - function (workflows) { - loadDataSetsForWorkflows(workflows) - .then(matchDataSetsToWorkflows) - .then(setWorkflowsOntoService); - }, - function (error) { - errorHandler.error(error.message); - } - ); + filterDataSetsForUser(workflows); + } } -function dataSetGroupFactory() { - return function (dataSets) { - return { - get: function () { - return dataSets; - }, - getIds: function () { - return _.pluck(dataSets, 'id'); - }, - getPeriodTypes: function () { - return _.uniq(_.pluck(dataSets, 'periodType')); - }, - getCategoryIds: function () { - var categoriesFromCategoryCombos; - - categoriesFromCategoryCombos = _.pluck(_.pluck(dataSets, 'categoryCombo'), 'categories'); - categoriesFromCategoryCombos = _.flatten(categoriesFromCategoryCombos); - categoriesFromCategoryCombos = _.pluck(categoriesFromCategoryCombos, 'id'); - - return _.uniq(categoriesFromCategoryCombos); - } - }; +function dataSetGroupFactory(dataSets) { + return { + get: function () { + return dataSets; + }, + getIds: function () { + return _.pluck(dataSets, 'id'); + }, + getPeriodTypes: function () { + return _.uniq(_.pluck(dataSets, 'periodType')); + }, + getCategoryIds: function () { + var categoriesFromCategoryCombos; + + categoriesFromCategoryCombos = _.pluck(_.pluck(dataSets, 'categoryCombo'), 'categories'); + categoriesFromCategoryCombos = _.flatten(categoriesFromCategoryCombos); + categoriesFromCategoryCombos = _.pluck(categoriesFromCategoryCombos, 'id'); + + return _.uniq(categoriesFromCategoryCombos); + } }; } -angular.module('PEPFAR.approvals').service('dataSetGroupService', dataSetGroupService); -angular.module('PEPFAR.approvals').factory('dataSetGroupFactory', dataSetGroupFactory); +angular.module('PEPFAR.approvals').factory('dataSetGroupService', dataSetGroupService); diff --git a/src/main/datasets/datasetgroupselector-directive.js b/src/main/datasets/datasetgroupselector-directive.js index 9ad303c..7557da1 100644 --- a/src/main/datasets/datasetgroupselector-directive.js +++ b/src/main/datasets/datasetgroupselector-directive.js @@ -1,4 +1,4 @@ -function dataSetGroupSelectorDirective(dataSetGroupService, dataSetGroupFactory) { +function dataSetGroupSelectorDirective(dataSetGroupService) { return { restrict: 'E', replace: true, @@ -10,35 +10,25 @@ function dataSetGroupSelectorDirective(dataSetGroupService, dataSetGroupFactory) selectedDataSetGroup: undefined }; - function updateDataSetGroups(datasetGroups) { - if (angular.isArray(datasetGroups)) { - scope.dataset.groups = datasetGroups; - scope.dataset.selectedDataSetGroup = scope.dataset.groups[0]; - } - } - - scope.$watch(function () { - return dataSetGroupService.getDataSetGroupNames(); - }, function (newVal, oldVal) { - if (newVal !== oldVal) { - updateDataSetGroups(newVal); - } - }); - - scope.$watch(function () { - return scope.dataset.selectedDataSetGroup; - }, function (newVal, oldVal) { - if (newVal !== oldVal) { - scope.$emit( - 'DATASETGROUP.changed', - dataSetGroupFactory(dataSetGroupService.getDataSetsForGroup(scope.dataset.selectedDataSetGroup)) - ); - } - }); - scope.onChange = function ($item) { + // Set scope to keep dropdown in sync with dropdown scope.dataset.selectedDataSetGroup = $item; + dataSetGroupService.setCurrentDataSetGroup($item); }; + + dataSetGroupService + .dataSetGroups$ + .take(1) + .map(function (datasetGroups) { + if (angular.isArray(datasetGroups)) { + scope.dataset.groups = datasetGroups; + + // Fire onChange to set to the first group + scope.onChange(scope.dataset.groups[0]); + } + }) + .safeApply(scope) + .subscribe(); } }; } diff --git a/src/main/manifest.webapp b/src/main/manifest.webapp index 2a33616..327918d 100644 --- a/src/main/manifest.webapp +++ b/src/main/manifest.webapp @@ -1,5 +1,5 @@ { - "version": "1.3.1", + "version": "1.4.0", "name": "Data Approval", "description": "Approvals app for PEPFAR", "icons": { @@ -11,7 +11,7 @@ "company": "DHIS2 Core Team", "email": "markpo@ifi.uio.no" }, - "launch_path": "index.html?v=1.3.1", + "launch_path": "index.html?v=1.4.0", "default_locale": "en", "activities": { "dhis": { diff --git a/src/test/mocks/datasetgroup-service_mock.js b/src/test/mocks/datasetgroup-service_mock.js index bcba150..376fec0 100644 --- a/src/test/mocks/datasetgroup-service_mock.js +++ b/src/test/mocks/datasetgroup-service_mock.js @@ -1,11 +1,8 @@ -function dataSetGroupServiceMock() { +function dataSetGroupServiceMock(rx) { return { - datasetGroups: [ 'MER', 'EA' ], - getDataSetGroupNames: function () { - return this.datasetGroups; - }, - getDataSetsForGroup: function () { - return []; - } - } + setCurrentDataSetGroup: function () {}, + dataSetGroups$: rx.Observable.just(['MER', 'EA']) + // currentDataSetGroup$: currentDataSetGroup$, + // getDataSetsForGroup: getDataSetsForGroup + }; } diff --git a/src/test/specs/dataset/datasetgroup-factory_spec.js b/src/test/specs/dataset/datasetgroup-factory_spec.js deleted file mode 100644 index 52296a7..0000000 --- a/src/test/specs/dataset/datasetgroup-factory_spec.js +++ /dev/null @@ -1,39 +0,0 @@ -describe('Dataset group factory', function () { - var dataSetGroupFactory; - var dataSetGroup; - - beforeEach(module('PEPFAR.approvals')); - beforeEach(inject(function (_dataSetGroupFactory_) { - dataSetGroupFactory = _dataSetGroupFactory_; - - dataSetGroup = dataSetGroupFactory(fixtures.get('dataSets')); - })); - - it('should return a function', function () { - expect(dataSetGroupFactory).to.be.a('function'); - }); - - it('should return an object', function () { - expect(dataSetGroupFactory()).to.be.a('object'); - }); - - it('should accept an array of datasets', function () { - dataSetGroupFactory(fixtures.get('dataSets')); - }); - - it('should return an array of dataSets when calling get', function () { - expect(dataSetGroup.get()).to.deep.equal(fixtures.get('dataSets')); - }); - - it('should return the period types when calling getPeriodTypes', function () { - expect(dataSetGroup.getPeriodTypes()).to.deep.equal(['Monthly', 'Yearly']); - }); - - it('should return the ids for the datasets in the group', function () { - expect(dataSetGroup.getIds()).to.deep.equal(['Zqg76KonUx1', 'cIGsv0OBVi8', 'lr508Rm7mHS', 'xY5nwObRyi7', 'vX0MoAE7JfL']); - }); - - it('should return the unique categories', function () { - expect(dataSetGroup.getCategoryIds()).to.deep.equal(['SH885jaRe0o', 'GLevLNI9wkl']); - }); -}); diff --git a/src/test/specs/dataset/datasetgroup-service_spec.js b/src/test/specs/dataset/datasetgroup-service_spec.js index 211e992..48dc35d 100644 --- a/src/test/specs/dataset/datasetgroup-service_spec.js +++ b/src/test/specs/dataset/datasetgroup-service_spec.js @@ -99,118 +99,6 @@ describe('Datasetgroup service', function () { expect(service).to.be.a('object'); }); - it('should have a method to get available groups', function () { - expect(service.getGroups).to.be.a('function'); - }); - - describe('getGroups', function () { - it('should return an array', function () { - expect(service.getGroups()).to.deep.equal({}); - }); - - xit('should filter the datasetgroups after they have been loaded', function () { - sinon.spy(service, 'filterDataSetsForUser'); - - $httpBackend.flush(); - - expect(service.filterDataSetsForUser).to.have.callCount(1); - }); - - xit('should return the filtered data sets groups when loaded', function () { - var expectedDataSetGroups = { - MER: { - name: 'MER', - dataSets: [{ - name: 'DSD: DS 1', - shortName: 'DSD: DS 1', - id: 'iP8irTNtByO' - }] - }, - EA: { - name: 'EA', - dataSets: [{ - name: 'EA: DataSet 1', - shortName: 'EA: DS1', - id: 'eLRAaV32xH5' - }] - } - }; - $httpBackend.expectGET(eaUrl) - .respond(200, { dataSets: [ - { name : 'EA: DataSet 1', shortName : 'EA: DS1', id : 'eLRAaV32xH5' }, - ] }); - - $httpBackend.flush(); - - expect(service.getGroups()).to.deep.equal(expectedDataSetGroups); - }); - }); - - describe('filterDataSetsForUser', function () { - it('should exist as a method', function () { - expect(service.filterDataSetsForUser).to.be.a('function'); - }); - - xit('should filter the dataset list based on the ones that are accessible', function () { - var dataFromResult = JSON.parse(fixtures.get('dataSetGroups')); - var filteredUserGroups = { - EA: { - name: "EA", - dataSets: [ - { - name: 'EA: DataSet 1', - shortName: 'EA: DS1', - id: 'eLRAaV32xH5' - }, - { - name: 'EA: DataSet 2', - shortName: 'EA: DS 2', - id: 'A4ivU53utt2' - } - ] - } - }; - $httpBackend.expectGET(merUrl) - .respond(200, { dataSets: [ - ] }); - - $httpBackend.flush(); - - service.filterDataSetsForUser([dataFromResult[1]]); - - $httpBackend.flush(); - - expect(service.getGroups()).to.deep.equal(filteredUserGroups); - }); - }); - - describe('getDataSetGroupNames', function () { - it('should exist as a method', function () { - expect(service.getDataSetGroupNames).to.be.a('function'); - }); - - xit('should return the data set group names', function () { - $httpBackend.flush(); - - expect(service.getDataSetGroupNames()).to.deep.equal([ 'EA', 'MER' ]); - }); - }); - - describe('getDataSets', function () { - beforeEach(function () { - $httpBackend.flush(); - }); - - xit('should return the datasets based for this the key', function () { - var dataSets = service.getDataSetsForGroup('MER'); - - expect(dataSets.length).to.equal(1); - expect(dataSets[0].id).to.equal('iP8irTNtByO'); - expect(dataSets[0].name).to.equal('DSD: DS 1'); - expect(dataSets[0].workflow).not.to.be.undefined; - }); - }); - xit('after loading the datasets it should call the periodService', function () { $httpBackend.flush(); diff --git a/src/test/specs/dataset/datasetgroupselector-directive_spec.js b/src/test/specs/dataset/datasetgroupselector-directive_spec.js index c18e035..219764e 100644 --- a/src/test/specs/dataset/datasetgroupselector-directive_spec.js +++ b/src/test/specs/dataset/datasetgroupselector-directive_spec.js @@ -1,11 +1,15 @@ describe('Dataset group selector directive', function () { var element; var scope; - var dataSetGroupService = dataSetGroupServiceMock(); + var dataSetGroupService; beforeEach(module('datasets/datasetgroupselector.html')); - beforeEach(module('PEPFAR.approvals', { - dataSetGroupService: dataSetGroupService + beforeEach(module('PEPFAR.approvals', function ($provide) { + $provide.factory('dataSetGroupService', function (rx) { + dataSetGroupService = dataSetGroupServiceMock(rx); + + return dataSetGroupService; + }); })); beforeEach(inject(function ($rootScope, $compile) { scope = $rootScope.$new(); @@ -34,13 +38,6 @@ describe('Dataset group selector directive', function () { selectElement = element.children().first(); }); - it('should have no options when there are no dataset groups', function () { - dataSetGroupService.datasetGroups = []; - scope.$apply(); - - expect(selectElement.find('.ui-select-choices-group .ui-select-choices-row').length).to.equal(0); - }); - it('should have options when they are set onto the scope', function () { expect(selectElement.find('.ui-select-choices-group .ui-select-choices-row').length).to.equal(2); }); diff --git a/src/test/specs/dataset/datasetview-directive_spec.js b/src/test/specs/dataset/datasetview-directive_spec.js index 9d96f0e..5924fb5 100644 --- a/src/test/specs/dataset/datasetview-directive_spec.js +++ b/src/test/specs/dataset/datasetview-directive_spec.js @@ -3,6 +3,13 @@ describe('Dataset view directive', function () { beforeEach(module('datasets/datasetsview.html')); beforeEach(module('PEPFAR.approvals', function ($provide) { + $provide.factory('dataSetGroupService', function (rx) { + return { + dataSetGroups$: rx.Observable.just([ + { name: 'MER Results' } + ]), + } + }); $provide.factory('workflowService', function () { return { currentWorkflow$: {