From 4a2e524a3c2ae9bed51bbf1fea742acda75e3004 Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Tue, 6 Jan 2015 17:12:54 -0800 Subject: [PATCH] Fixing issues with repo-list not working with multiple repos A ton of extra work to get it working in instanceEdit Also fixing issue with ENV modal sidebar Also fixing Editor sending n amount of file updates (hack fix for now) --- .../styles/scss/modals/modals-sidebar.scss | 1 + .../instance/controllerInstance.js | 4 +- .../instanceEdit/controllerInstanceEdit.js | 28 +---- client/controllers/setup/controllerSetup.js | 3 +- .../activePanel/directiveActivePanel.js | 23 ++-- .../directiveInstanceEditPrimaryActions.js | 74 +++++++++-- .../directives/repoList/directiveRepoList.js | 36 +++--- client/templates/instance/viewInstance.jade | 1 + .../instanceEdit/viewInstanceEdit.jade | 2 + client/templates/setup/viewSetup.jade | 1 + .../differentBitcoinAppCodeVersion.json | 11 ++ test/unit/apiMocks/index.js | 3 +- .../unit/directives/directiveRepoList.unit.js | 15 ++- ...irectiveInstanceEditPrimaryActions.unit.js | 115 ++++++++++++++++-- 14 files changed, 236 insertions(+), 81 deletions(-) create mode 100644 test/unit/apiMocks/appCodeVersions/differentBitcoinAppCodeVersion.json diff --git a/client/assets/styles/scss/modals/modals-sidebar.scss b/client/assets/styles/scss/modals/modals-sidebar.scss index 0bd820115..ae015fe34 100644 --- a/client/assets/styles/scss/modals/modals-sidebar.scss +++ b/client/assets/styles/scss/modals/modals-sidebar.scss @@ -8,6 +8,7 @@ transform: translate3d(0,0,0); transition: transform .15s ease-in; width: 210px; + height: 100%; z-index: $z-modal-sidebar; .show-sidebar & { diff --git a/client/controllers/instance/controllerInstance.js b/client/controllers/instance/controllerInstance.js index fae8c555f..6700ca720 100644 --- a/client/controllers/instance/controllerInstance.js +++ b/client/controllers/instance/controllerInstance.js @@ -18,7 +18,9 @@ function ControllerInstance( fetchUser ) { var dataInstance = $scope.dataInstance = { - data: {}, + data: { + unsavedAcvs: [] + }, actions: {} }; var data = dataInstance.data; diff --git a/client/controllers/instanceEdit/controllerInstanceEdit.js b/client/controllers/instanceEdit/controllerInstanceEdit.js index a8048565b..fe5c61fa0 100644 --- a/client/controllers/instanceEdit/controllerInstanceEdit.js +++ b/client/controllers/instanceEdit/controllerInstanceEdit.js @@ -21,7 +21,9 @@ function ControllerInstanceEdit( ) { var dataInstanceEdit = $scope.dataInstanceEdit = { - data: {}, + data: { + unsavedAcvs: [] + }, actions: {} }; var data = dataInstanceEdit.data; @@ -77,29 +79,6 @@ function ControllerInstanceEdit( .go(); } - // This is to fetch the list of instances. This is separate so the page can load quickly - // since it will have its instance. Only the modals use this list - function fetchInstances(cb) { - new QueryAssist($scope.user, cb) - .wrapFunc('fetchInstances', cb) - .query({ - githubUsername: $stateParams.userName - }) - .cacheFetch(function (instances, cached, cb) { - if (!cached && instances.models.length === 0) { - throw new Error('instance not found'); - } - data.instances = instances; - cb(); - }) - .resolve(function (err, instances, cb) { - if (err) { return $log.error(err); } - data.instances = instances; - cb(); - }) - .go(); - } - // open "Dockerfile" build file by default function setDefaultTabs() { var rootDir = keypather.get($scope, 'build.contextVersions.models[0].rootDir'); @@ -128,7 +107,6 @@ function ControllerInstanceEdit( if (err) { return errs.handler(err); } if (redirect) { return; } setDefaultTabs(); - fetchInstances(angular.noop); }); } diff --git a/client/controllers/setup/controllerSetup.js b/client/controllers/setup/controllerSetup.js index 2dba0c3cf..1288ac218 100755 --- a/client/controllers/setup/controllerSetup.js +++ b/client/controllers/setup/controllerSetup.js @@ -25,7 +25,8 @@ function ControllerSetup( var dataSetup = $scope.dataSetup = { data: { - instanceOpts: {} + instanceOpts: {}, + unsavedAcvs: [] }, actions: {} }; diff --git a/client/directives/activePanel/directiveActivePanel.js b/client/directives/activePanel/directiveActivePanel.js index f71928bab..870f87351 100755 --- a/client/directives/activePanel/directiveActivePanel.js +++ b/client/directives/activePanel/directiveActivePanel.js @@ -100,9 +100,21 @@ function activePanel( } var updateFileDebounce = debounce(updateFile, 333); + + $scope.$watch('openItems.activeHistory.last().state.body', function (newVal, oldVal) { + if (typeof newVal === 'string' && + $scope.openItems.activeHistory.last() && + $scope.openItems.activeHistory.last().id() === $scope.thisFileId) { + if ($scope.update) { + updateFileDebounce(); + } + } + }); + function fetchFile() { var openItems = $scope.openItems; var last = openItems.activeHistory.last(); + $scope.thisFileId = last.id(); if (openItems.isFile(last)) { last.fetch(function () { last.state.reset(); @@ -110,16 +122,9 @@ function activePanel( } } - $scope.$watch('openItems.activeHistory.last().state.body', function (newVal, oldVal) { - if (typeof newVal === 'string' && $scope.openItems.activeHistory.last()) { - if ($scope.update) { - updateFileDebounce(); - } - } - }); - - $scope.$watch('openItems.activeHistory.last().id()', function (newVal, oldVal) { + var openFileWatch = $scope.$watch('openItems.activeHistory.last().id()', function (newVal, oldVal) { if (newVal) { + openFileWatch(); if (!$scope.update) { var file = $scope.openItems.activeHistory.last(); if (!(file.state && (typeof file.state.body === 'string'))) { diff --git a/client/directives/instanceEditPrimaryActions/directiveInstanceEditPrimaryActions.js b/client/directives/instanceEditPrimaryActions/directiveInstanceEditPrimaryActions.js index f6c4360e7..2d1aa580f 100644 --- a/client/directives/instanceEditPrimaryActions/directiveInstanceEditPrimaryActions.js +++ b/client/directives/instanceEditPrimaryActions/directiveInstanceEditPrimaryActions.js @@ -8,6 +8,7 @@ function instanceEditPrimaryActions( QueryAssist, $rootScope, $state, + errs, $stateParams ) { return { @@ -17,7 +18,8 @@ function instanceEditPrimaryActions( user: '=', instance: '=', loading: '=', - openItems: '=' + openItems: '=', + unsavedAcvs: '=' }, link: function ($scope, elem, attrs) { // prevent multiple clicks @@ -48,13 +50,16 @@ function instanceEditPrimaryActions( } }, cb); }, + updateAppCodeVersions, function () { var build = $scope.newBuild; // Catch the update file error $scope.newBuild.build( buildObj, function (err) { - if (err) { throw err; } + if (err) { + return handleError(err); + } var opts = { build: build.id() }; @@ -62,7 +67,9 @@ function instanceEditPrimaryActions( opts.env = $scope.instance.state.env; } $scope.instance.update(opts, function (err) { - if (err) { throw err; } + if (err) { + return handleError(err); + } // will trigger display of completed message if build completes // before reaching next state // $scope.dataInstanceLayout.data.showBuildCompleted = true; @@ -70,9 +77,7 @@ function instanceEditPrimaryActions( }); }); } - ], function (err) { - if (err) { throw err; } - }); + ], handleError); }); }; @@ -97,12 +102,63 @@ function instanceEditPrimaryActions( $scope.newBuild = build; cb(); }) - .resolve(function (err) { - if (err) { throw err; } - }) + .resolve(handleError) .go(); } + function updateAppCodeVersions(cb) { + var modifiedAcvs = $scope.unsavedAcvs.filter(function (obj) { + return obj.unsavedAcv.attrs.commit !== obj.acv.attrs.commit; + }); + if (!modifiedAcvs.length) { + return cb(); + } + + var context = $scope.newBuild.contexts.models[0]; + var contextVersion = $scope.newBuild.contextVersions.models[0]; + var infraCodeVersionId = contextVersion.attrs.infraCodeVersion; + + var appCodeVersionStates = $scope.unsavedAcvs.map(function (obj) { + var acv = obj.unsavedAcv; + return { + repo: acv.attrs.repo, + branch: acv.attrs.branch, + commit: acv.attrs.commit + }; + }); + async.waterfall([ + createContextVersion, + createBuild + ], cb); + + function createContextVersion(cb) { + var body = { + infraCodeVersion: infraCodeVersionId + }; + var newContextVersion = context.createVersion(body, function (err) { + async.each(appCodeVersionStates, function (acvState, cb) { + newContextVersion.appCodeVersions.create(acvState, cb); + }, function (err) { + cb(err, newContextVersion); + }); + }); + } + function createBuild(contextVersion, cb) { + var build = $scope.user.createBuild({ + contextVersions: [contextVersion.id()], + owner: $scope.instance.attrs.owner + }, function (err) { + $scope.newBuild = build; + cb(err, build); + }); + } + } + function handleError(err) { + if (err) { + $scope.loading = false; + errs.handler(err); + } + } } }; } diff --git a/client/directives/repoList/directiveRepoList.js b/client/directives/repoList/directiveRepoList.js index 754a49458..21a0eec12 100644 --- a/client/directives/repoList/directiveRepoList.js +++ b/client/directives/repoList/directiveRepoList.js @@ -17,7 +17,8 @@ function repoList( restrict: 'A', templateUrl: 'viewRepoList', scope: { - loading: '=' + loading: '=', + unsavedAcvs: '=' }, link: function ($scope, elem) { @@ -43,10 +44,12 @@ function repoList( // track all temp acvs generated // for each repo/child-scope - $scope.unsavedAcvs = []; $scope.newUnsavedAcv = function (acv) { var cv = $scope.build.contextVersions.models[0]; - var newAcv = cv.newAppCodeVersion(acv.toJSON(), { + var acvJson = acv.toJSON(); + delete acvJson._id; + delete acvJson.id; + var newAcv = cv.newAppCodeVersion(acvJson, { warn: false }); $scope.unsavedAcvs.push({ @@ -109,25 +112,14 @@ function repoList( // if we find this contextVersion, reuse it. // otherwise create a new one function findOrCreateContextVersion(cb) { - var foundCVs = context.fetchVersions({ - infraCodeVersion: infraCodeVersionId, - appCodeVersions: appCodeVersionStates - }, function (err) { - if (err) { - return cb(err); - } - if (foundCVs.models.length) { - return cb(null, foundCVs.models[0]); - } - var body = { - infraCodeVersion: infraCodeVersionId - }; - var newContextVersion = context.createVersion(body, function (err) { - async.each(appCodeVersionStates, function (acvState, cb) { - newContextVersion.appCodeVersions.create(acvState, cb); - }, function (err) { - cb(err, newContextVersion); - }); + var body = { + infraCodeVersion: infraCodeVersionId + }; + var newContextVersion = context.createVersion(body, function (err) { + async.each(appCodeVersionStates, function (acvState, cb) { + newContextVersion.appCodeVersions.create(acvState, cb); + }, function (err) { + cb(err, newContextVersion); }); }); } diff --git a/client/templates/instance/viewInstance.jade b/client/templates/instance/viewInstance.jade index 2452a8f84..6c43d78bc 100644 --- a/client/templates/instance/viewInstance.jade +++ b/client/templates/instance/viewInstance.jade @@ -39,6 +39,7 @@ section.sidebar.box-sidebar.load section.row.repo-list( repo-list + unsaved-acvs = "dataInstance.data.unsavedAcvs" loading = "dataApp.data.loading" ) diff --git a/client/templates/instanceEdit/viewInstanceEdit.jade b/client/templates/instanceEdit/viewInstanceEdit.jade index 129aa7436..6b6712723 100644 --- a/client/templates/instanceEdit/viewInstanceEdit.jade +++ b/client/templates/instanceEdit/viewInstanceEdit.jade @@ -7,6 +7,7 @@ header.box-header instance-edit-primary-actions( user = "user" instance = "dataInstanceEdit.data.instance" + unsaved-acvs = "dataInstanceEdit.data.unsavedAcvs" loading = "dataApp.data.loading" open-items = "dataInstanceEdit.data.openItems" ) @@ -22,6 +23,7 @@ section.sidebar.box-sidebar section.row.repo-list( repo-list + unsaved-acvs = "dataInstanceEdit.data.unsavedAcvs" ) docker-validation( diff --git a/client/templates/setup/viewSetup.jade b/client/templates/setup/viewSetup.jade index 57d38a0b6..02cab04ef 100755 --- a/client/templates/setup/viewSetup.jade +++ b/client/templates/setup/viewSetup.jade @@ -27,6 +27,7 @@ section.sidebar.box-sidebar section.row.repo-list( repo-list + unsaved-acvs = "dataSetup.data.unsavedAcvs" ) docker-templates( diff --git a/test/unit/apiMocks/appCodeVersions/differentBitcoinAppCodeVersion.json b/test/unit/apiMocks/appCodeVersions/differentBitcoinAppCodeVersion.json new file mode 100644 index 000000000..9c383e3e3 --- /dev/null +++ b/test/unit/apiMocks/appCodeVersions/differentBitcoinAppCodeVersion.json @@ -0,0 +1,11 @@ +{ + "_id": "544ad1004daca7552128f092", + "branch": "master", + "commit": "1f27c310a4bcca758f708358601fa25976d56d94", + "lowerBranch": "master", + "lowerRepo": "cflynn07/bitcoin", + "privateKey": "cflynn07/bitcoin.key", + "publicKey": "cflynn07/bitcoin.key.pub", + "repo": "cflynn07/bitcoin", + "id": "544ad1004daca7552128f09c" +} \ No newline at end of file diff --git a/test/unit/apiMocks/index.js b/test/unit/apiMocks/index.js index b0449e9ac..70354c5f0 100644 --- a/test/unit/apiMocks/index.js +++ b/test/unit/apiMocks/index.js @@ -1,6 +1,7 @@ module.exports = { appCodeVersions: { - bitcoinAppCodeVersion: require('./appCodeVersions/bitcoinAppCodeVersion') + bitcoinAppCodeVersion: require('./appCodeVersions/bitcoinAppCodeVersion'), + differentBitcoinAppCodeVersion: require('./appCodeVersions/differentBitcoinAppCodeVersion') }, builds: { setup: require('./builds/setup') diff --git a/test/unit/directives/directiveRepoList.unit.js b/test/unit/directives/directiveRepoList.unit.js index 6c8db2238..5fae55541 100644 --- a/test/unit/directives/directiveRepoList.unit.js +++ b/test/unit/directives/directiveRepoList.unit.js @@ -47,7 +47,10 @@ describe('directiveRepoList'.bold.underline.blue, function () { $httpBackend.whenGET(host + '/contexts/54398933f5afb6410069bc33/versions/54398934f5afb6410069bc34?') .respond(mocks.contextVersions.setup); - var tpl = directiveTemplate.attribute('repo-list'); + var tpl = directiveTemplate.attribute('repo-list', { + 'unsaved-acvs': 'unsavedAcvs' + }); + $scope.unsavedAcvs = []; element = $compile(tpl)($scope); $scope.$digest(); @@ -97,7 +100,10 @@ describe('directiveRepoList'.bold.underline.blue, function () { .whenGET(compareUrl) .respond(mocks.gh.compare); - var tpl = directiveTemplate.attribute('repo-list'); + var tpl = directiveTemplate.attribute('repo-list', { + 'unsaved-acvs': 'unsavedAcvs' + }); + $scope.unsavedAcvs = []; element = $compile(tpl)($scope); $scope.$digest(); @@ -147,7 +153,10 @@ describe('directiveRepoList'.bold.underline.blue, function () { .whenGET(compareUrl) .respond(mocks.gh.compare); - var tpl = directiveTemplate.attribute('repo-list'); + var tpl = directiveTemplate.attribute('repo-list', { + 'unsaved-acvs': 'unsavedAcvs' + }); + $scope.unsavedAcvs = []; element = $compile(tpl)($scope); $scope.$digest(); diff --git a/test/unit/directives/instance/directiveInstanceEditPrimaryActions.unit.js b/test/unit/directives/instance/directiveInstanceEditPrimaryActions.unit.js index 9162b4885..559e8187e 100644 --- a/test/unit/directives/instance/directiveInstanceEditPrimaryActions.unit.js +++ b/test/unit/directives/instance/directiveInstanceEditPrimaryActions.unit.js @@ -5,6 +5,7 @@ var $rootScope, $state, $compile, $timeout, + errs, $stateParams; var $elScope; var thisUser; @@ -23,7 +24,8 @@ function makeDefaultScope () { isClean: function () { return false; } - } + }, + unsavedAcvs: [] }; } @@ -56,6 +58,7 @@ describe('directiveInstanceEditPrimaryActions'.bold.underline.blue, function() { _$stateParams_, _$rootScope_, _$compile_, + _errs_, _$timeout_, user ) { @@ -65,6 +68,7 @@ describe('directiveInstanceEditPrimaryActions'.bold.underline.blue, function() { thisUser.reset(apiMocks.user); $timeout = _$timeout_; $compile = _$compile_; + errs = _errs_; $rootScope = _$rootScope_; $state = _$state_; $stateParams = _$stateParams_; @@ -88,13 +92,19 @@ describe('directiveInstanceEditPrimaryActions'.bold.underline.blue, function() { user: 'user', instance: 'instance', loading: 'loading', + 'unsaved-acvs': 'unsavedAcvs', 'open-items': 'openItems' }); ctx.element = $compile(ctx.template)($scope); $scope.$digest(); $elScope = ctx.element.isolateScope(); - ctx.mockBuild = angular.copy(apiMocks.builds.setup); + ctx.mockBuild = { + attrs: angular.copy(apiMocks.builds.setup) + }; + ctx.mockBuild2 = { + attrs: angular.copy(apiMocks.instances.build) + }; ctx.instanceUpdateCalled = false; ctx.buildBuildCalled = false; @@ -122,10 +132,24 @@ describe('directiveInstanceEditPrimaryActions'.bold.underline.blue, function() { cb(); } }; + ctx.mockBuild.contexts = { + models: [{ + createVersion: sinon.spy(function(body, cb) { + cb(); + }) + }] + }; ctx.mockBuild.contextVersions = { models: [{ + attrs: apiMocks.contextVersions.running, rootDir: { contents: [ctx.dockerfile] + }, + appCodeVersions: { + create: sinon.spy() + }, + id: function() { + return 213423; } }] }; @@ -248,6 +272,71 @@ describe('directiveInstanceEditPrimaryActions'.bold.underline.blue, function() { expect($elScope.loading).to.equal(true); }); + it('should create a whole slew of new things when an acv is modified', function (done) { + // Set up mocking + + $scope.unsavedAcvs = [{ + acv: { + attrs: apiMocks.appCodeVersions.bitcoinAppCodeVersion + }, + unsavedAcv: { + attrs: apiMocks.appCodeVersions.differentBitcoinAppCodeVersion + } + }]; + ctx.mockBuild.contextVersions.models[0].appCodeVersions.create = sinon.spy(function(acv, cb) { + expect(acv.commit).to.equal(apiMocks.appCodeVersions.differentBitcoinAppCodeVersion.commit); + cb(null, { + attrs: apiMocks.contextVersions.running, + id: function() { return 123; } + }); + }); + ctx.mockBuild.contexts = { + models: [{ + createVersion: sinon.spy(function (body, cb) { + setTimeout(function() { + cb(); + }, 0); + return ctx.mockBuild.contextVersions.models[0]; + }) + }] + }; + thisUser.createBuild = sinon.spy(function(opts, cb) { + setTimeout(function() { + cb(); + }, 0); + return ctx.mockBuild2; + }); + ctx.mockBuild2.build = sinon.spy(function (message, cb) { + expect(message).to.deep.equal({ + message: 'Manual build' + }); + cb(null, ctx.mockBuild2); + }); + ctx.mockBuild2.id = function () { + return this._id; + }; + $rootScope.$digest(); + + ctx.stateMock.go = function (newState) { + expect(newState).to.equal('instance.instance'); + expect(ctx.instanceUpdateCalled).to.be.true; + sinon.assert.called(ctx.mockBuild2.build); + expect(ctx.dockerfileUpdateCalled).to.be.false; + sinon.assert.called(ctx.mockBuild.contextVersions.models[0].appCodeVersions.create); + sinon.assert.called(thisUser.createBuild); + sinon.assert.called(ctx.mockBuild.contexts.models[0].createVersion); + done(); + }; + MockQueryAssist.setMock('fetchBuild', function () { + return ctx.mockBuild; + }); + // Now do it + $scope.$digest(); + $elScope.build(); + $scope.$digest(); + expect($elScope.loading).to.equal(true); + }); + it('should not try to build twice', function (done) { // Set up mocking var fetchBuildCount = 0; @@ -276,12 +365,14 @@ describe('directiveInstanceEditPrimaryActions'.bold.underline.blue, function() { describe('Testing Failures', function () { var inputScope; + var fakeErr; beforeEach(function () { inputScope = makeDefaultScope(); inputScope.openItems.isClean = function () { return true; }; injectSetupCompile(inputScope); + fakeErr = sinon.stub(errs, 'handler'); }); it('should throw an error if there is no user on the scope', function() { @@ -314,9 +405,10 @@ describe('directiveInstanceEditPrimaryActions'.bold.underline.blue, function() { } $scope.$digest(); - expect(doStuff).to.throw(errorMessage); + doStuff(); + sinon.assert.called(fakeErr); // Now do it - expect($elScope.loading).to.equal(true); + expect($elScope.loading).to.equal(false); expect(ctx.instanceUpdateCalled).to.be.false; expect(ctx.buildBuildCalled).to.be.false; expect(ctx.dockerfileUpdateCalled).to.be.false; @@ -338,9 +430,10 @@ describe('directiveInstanceEditPrimaryActions'.bold.underline.blue, function() { } $scope.$digest(); - expect(doStuff).to.throw(errorMessage); + doStuff(); + sinon.assert.called(fakeErr); // Now do it - expect($elScope.loading).to.equal(true); + expect($elScope.loading).to.equal(false); expect(ctx.instanceUpdateCalled).to.be.false; expect(ctx.buildBuildCalled).to.be.true; expect(ctx.dockerfileUpdateCalled).to.be.false; @@ -362,9 +455,10 @@ describe('directiveInstanceEditPrimaryActions'.bold.underline.blue, function() { } $scope.$digest(); - expect(doStuff).to.throw(errorMessage); + doStuff(); + sinon.assert.called(fakeErr); // Now do it - expect($elScope.loading).to.equal(true); + expect($elScope.loading).to.equal(false); expect(ctx.instanceUpdateCalled).to.be.false; expect(ctx.buildBuildCalled).to.be.false; expect(ctx.dockerfileUpdateCalled).to.be.true; @@ -388,9 +482,10 @@ describe('directiveInstanceEditPrimaryActions'.bold.underline.blue, function() { } $scope.$digest(); - expect(doStuff).to.throw(errorMessage); + doStuff(); + sinon.assert.called(fakeErr); // Now do it - expect($elScope.loading).to.equal(true); + expect($elScope.loading).to.equal(false); expect(ctx.instanceUpdateCalled).to.be.true; expect(ctx.buildBuildCalled).to.be.true; expect(ctx.dockerfileUpdateCalled).to.be.false;