Merge pull request #2062 from CodeNow/revert-commit-selector

ASAP-revert-commit-selector

Author: Nathan219

client/directives/components/editRepoCommit/EditRepoCommitController.js

@@ -16,8 +16,7 @@ function EditRepoCommitController(
   errs,
   loading,
   ModalService,
-  updateInstanceWithNewAcvData,
-  github
+  updateInstanceWithNewAcvData
 ) {
     var ERCC = this;
     ERCC.isLatestCommitDeployed = true;
@@ -30,14 +29,15 @@ function EditRepoCommitController(
 
     $scope.$watch('ERCC.acv', function (newAcv, oldAcv) {
       if (newAcv) {
+        var branch = ERCC.acv.githubRepo.newBranch(ERCC.acv.attrs.branch, {warn: false});
         $q.all({
           activeCommit: fetchCommitData.activeCommit(ERCC.acv),
-          branchCommits: github.branchOrPRCommits(ERCC.acv)
+          branchCommits: promisify(branch.commits, 'fetch')()
         })
           .then(function(commits) {
             ERCC.activeCommit = commits.activeCommit;
-            ERCC.latestBranchCommit = keypather.get(commits, 'branchCommits[0]');
-            ERCC.isLatestCommitDeployed = ERCC.activeCommit.attrs.sha === ERCC.latestBranchCommit.sha;
+            ERCC.latestBranchCommit = keypather.get(commits, 'branchCommits.models[0]');
+            ERCC.isLatestCommitDeployed = ERCC.activeCommit.attrs.sha === ERCC.latestBranchCommit.attrs.sha;
           });
       }
     });
@@ -107,12 +107,13 @@ function EditRepoCommitController(
     };
 
     ERCC.updateInstance = function() {
+      var branch = ERCC.acv.githubRepo.newBranch(ERCC.acv.attrs.branch, {warn: false});
       loading('updatingInstance', true);
-      $q.when(github.branchOrPRCommits(ERCC.acv))
+      promisify(branch.commits, 'fetch')()
         .then(function(commits) {
-          ERCC.latestBranchCommit = commits[0];
+          ERCC.latestBranchCommit = keypather.get(commits, 'models[0]');
           repoObject.commit = ERCC.latestBranchCommit;
-          if (ERCC.activeCommit.attrs.sha !== ERCC.latestBranchCommit.sha) {
+          if (ERCC.activeCommit.attrs.sha !== ERCC.latestBranchCommit.attrs.sha) {
             return updateInstanceWithNewAcvData(ERCC.instance, ERCC.acv, repoObject)
               .then(function(instance) {
                 loading('updatingInstance', false);

client/directives/components/lists/branchCommitSelector/branchCommitSelectorController.js

@@ -13,19 +13,18 @@ function BranchCommitSelectorController(
   BCSC.eventTracking = eventTracking;
 
   BCSC.onCommitFetch = function (commits) {
-    if (!commits.length) { return; }
-    BCSC.data.commits = commits;
+    if (!commits.models.length) { return; }
     if (BCSC.data.commit) {
-      BCSC.data.commit = commits.find(function (otherCommits) {
+      BCSC.data.commit = commits.models.find(function (otherCommits) {
         return otherCommits === BCSC.data.commit;
-      }) || commits[0];
-      BCSC.isLatestCommitDeployed = commits[0] === BCSC.data.commit;
+      }) || commits.models[0];
+      BCSC.isLatestCommitDeployed = commits.models[0] === BCSC.data.commit;
     }
   };
 
   BCSC.isLatestCommit = function (setToLatestCommit) {
     if (arguments.length) {
-      BCSC.data.commit = keypather.get(BCSC.data.branch, 'commits[0]');
+      BCSC.data.commit = keypather.get(BCSC.data.branch, 'commits.models[0]');
       BCSC.data.useLatest = setToLatestCommit;
       if (setToLatestCommit) {
         $scope.$emit('commit::selected', BCSC.data.commit);
@@ -44,7 +43,7 @@ function BranchCommitSelectorController(
 
   BCSC.deployLatestCommit = function () {
     if (BCSC.isAutoDeployOn() && !BCSC.isLatestCommitDeployed) {
-      BCSC.data.commit = keypather.get(BCSC.data.branch, 'commits[0]');
+      BCSC.data.commit = keypather.get(BCSC.data.branch, 'commits.models[0]');
       BCSC.updateInstance();
     }
   };

client/directives/components/lists/branchCommitSelector/branchCommitSelectorDirective.js

@@ -11,8 +11,7 @@ require('app')
  */
 function branchCommitSelector(
   errs,
-  promisify,
-  github
+  promisify
 ) {
   return {
     restrict: 'A',
@@ -29,8 +28,7 @@ function branchCommitSelector(
       $scope.$watch('BCSC.data.branch', function (branch) {
         if (branch) {
           $scope.fetchingCommits = true;
-          var acv = $scope.BCSC.data.acv;
-          return github.branchOrPRCommits(acv)
+          return promisify(branch.commits, 'fetch')()
             .then($scope.BCSC.onCommitFetch)
             .catch(errs.handler)
             .finally(function () {

client/directives/components/lists/branchCommitSelector/branchCommitSelectorView.jade

@@ -63,18 +63,18 @@ ul.list.list-servers
     ng-include = "'spinner'"
   )
   li.list-item(
-    ng-attr-title = "{{commit.commit.message}}"
+    ng-attr-title = "{{commit.attrs.commit.message}}"
     ng-disabled = "BCSC.isAutoDeployOn() || BCSC.isLastestCommit()"
     ng-class = "{\
       'active': BCSC.data.commit === commit,\
       'disabled': BCSC.isAutoDeployOn() || BCSC.isLastestCommit()\
     }"
     ng-click = "BCSC.selectCommit(commit, $index === 0)"
     ng-if = "!fetchingCommits"
-    ng-repeat = "commit in BCSC.data.commits | orderBy: '-commit.committer.date'"
-  ) {{commit.commit.message}}
+    ng-repeat = "commit in BCSC.data.branch.commits.models | orderBy: '-attrs.commit.committer.date'"
+  ) {{commit.attrs.commit.message}}
     .row.row-author.clearfix(
-      ng-attr-title = "{{commit.author.login}} authored {{commit.commit.committer.date | timeFrom}}"
+      ng-attr-title = "{{commit.attrs.author.login}} authored {{commit.attrs.commit.committer.date | timeFrom}}"
     )
       //- if invite flows...
       .btn-user.text-overflow(
@@ -88,19 +88,19 @@ ul.list.list-servers
       )
       span.small(
         ng-if = "$root.featureFlags.inviteFlows"
-      ) —{{commit.commit.committer.date | timeFrom}}
+      ) —{{commit.attrs.commit.committer.date | timeFrom}}
 
       //- else
       span.small(
         ng-if = "!$root.featureFlags.inviteFlows"
-      ) {{commit.author.login}}—{{commit.commit.committer.date | timeFrom}}
+      ) {{commit.attrs.author.login}}—{{commit.attrs.commit.committer.date | timeFrom}}
 
       a.small.link.monospace(
         ng-click = "$event.stopPropagation()"
-        ng-href = "{{commit.html_url}}"
+        ng-href = "{{commit.attrs.html_url}}"
         target = "_blank"
         title = "View on GitHub"
-      ) {{::commit.sha | limitTo:7}}
+      ) {{::commit.attrs.sha | limitTo:7}}
     svg.iconnables.icons-fat-check(
       ng-if = "!BCSC.isLatestCommit()"
     )

client/services/githubService.js

@@ -33,21 +33,6 @@ function github(
       });
   }
   return {
-    branchOrPRCommits: function (acv) {
-      var pullRequest = acv.attrs.pullRequest;
-      var repo = acv.attrs.lowerRepo;
-      var githubUrl;
-      if (pullRequest) {
-        githubUrl = '/repos/' + repo + '/pulls/' + pullRequest + '/commits?per_page=100';
-      } else {
-        githubUrl = '/repos/' + repo + '/commits?sha=' + acv.attrs.branch + '&per_page=100';
-      }
-      var ghRequest = {
-        method: 'get',
-        url: githubAPIUrl + githubUrl
-      };
-      return makeGhRequest(ghRequest);
-    },
     forkRepo: function (repoOwner, repoName, targetOrg, isPersonalAccount) {
       var ghRequest = {
         method: 'post',

test/unit/controllers/branchCommitSelectorController.unit.js

@@ -16,7 +16,12 @@ describe('branchCommitSelectorController'.bold.underline.blue, function () {
     ctx.commit = {
       name: 'This is a commit message!'
     };
-    ctx.commits = [ctx.commit];
+    ctx.commits = {
+      fetch: sinon.stub().returns({
+        models: [ctx.commit]
+      }),
+      models: [ctx.commit]
+    };
     ctx.branch = {
       attrs: {
         name: 'default'
@@ -84,15 +89,15 @@ describe('branchCommitSelectorController'.bold.underline.blue, function () {
       $scope.$digest();
       branchCommitSelectorController.onCommitFetch(ctx.commits);
       $scope.$digest();
-      expect(branchCommitSelectorController.data.commit, 'data.commit').to.equal(ctx.commits[0]);
+      expect(branchCommitSelectorController.data.commit, 'data.commit').to.equal(ctx.commits.models[0]);
       $rootScope.$destroy();
     });
 
     it('should leave the commit if it can find the current one in the list', function () {
       var newCommit = {
         sadsa: 'asdasd'
       };
-      ctx.commits.push(newCommit);
+      ctx.commits.models.push(newCommit);
       branchCommitSelectorController.data = {
         useLatest: true,
         branch: ctx.branch,
@@ -115,7 +120,7 @@ describe('branchCommitSelectorController'.bold.underline.blue, function () {
       branchCommitSelectorController.isLatestCommit(false);
       expect(branchCommitSelectorController.isLatestCommit(), 'useLatest').to.be.false;
       expect(branchCommitSelectorController.data.useLatest, 'data.useLatest').to.be.false;
-      expect(branchCommitSelectorController.data.commit, 'data.commit').to.equal(ctx.commits[0]);
+      expect(branchCommitSelectorController.data.commit, 'data.commit').to.equal(ctx.commits.models[0]);
       $rootScope.$destroy();
     });
     it('useLatest emits commit::selected on set true, not false', function () {
@@ -134,7 +139,7 @@ describe('branchCommitSelectorController'.bold.underline.blue, function () {
       sinon.assert.calledOnce(commitSelectedSpy);
 
       expect(branchCommitSelectorController.data.useLatest, 'data.useLatest').to.be.true;
-      expect(branchCommitSelectorController.data.commit, 'data.commit').to.equal(ctx.commits[0]);
+      expect(branchCommitSelectorController.data.commit, 'data.commit').to.equal(ctx.commits.models[0]);
       $rootScope.$destroy();
     });
 

test/unit/directives/branchCommitSelectorDirective.unit.js

@@ -40,12 +40,6 @@ describe('branchCommitSelectorDirective'.bold.underline.blue, function () {
           link: angular.noop
         };
       });
-      $provide.factory('github', function ($q) {
-        ctx.github = {
-          branchOrPRCommits: sinon.stub().returns($q.when(ctx.commits))
-        };
-        return ctx.github;
-      });
     });
 
     angular.mock.inject(function (
@@ -87,7 +81,7 @@ describe('branchCommitSelectorDirective'.bold.underline.blue, function () {
       //Should fetch once the branch is set
       $scope.$digest();
       expect($elScope.BCSC.data.branch, 'data.branch').to.equal(ctx.branch);
-      sinon.assert.called(ctx.github.branchOrPRCommits);
+      sinon.assert.called(ctx.branch.commits.fetch);
       expect($elScope.fetchingCommits, 'fetchingCommits').to.be.false;
       $rootScope.$destroy();
     });

test/unit/directives/editRepoCommitController.unit.js

@@ -11,26 +11,31 @@ describe('EditRepoCommitController'.bold.underline.blue, function() {
   var $q;
   var editRepoCommitController;
   var ERCC;
+  var getCommitStub;
   var commits;
-  var newCommits;
   var acvMock;
-  var branchCommitsStub;
+  var newBranchStub;
 
   function setup() {
     ctx = {};
-    ctx.commitData = apiMocks.commit.bitcoinRepoCommit1
     ctx.branch = {attrs: apiMocks.branches.bitcoinRepoBranches[0]};
-    ctx.commit = {attrs: ctx.commitData};
-    branchCommitsStub = sinon.stub().returns([ctx.commitData]);
+    ctx.commit = {attrs: apiMocks.commit.bitcoinRepoCommit1};
+    getCommitStub = sinon.stub().returns({models: [ctx.commit]});
+    newBranchStub = sinon.stub().returns({
+      commits: {
+        fetch: getCommitStub
+      }
+    })
     acvMock = {
       attrs: {
         repo: 'foo/bar',
         commit: 'commitSha'
       },
-      githubRepo: {}
+      githubRepo: {
+        newBranch: newBranchStub
+      }
     };
     commits = {models: [apiMocks.commit.bitcoinRepoCommit2, ctx.commit]};
-    newCommits = [apiMocks.commit.bitcoinRepoCommit2, ctx.commitData]
     angular.mock.module('app', function ($provide) {
       $provide.factory('fetchCommitData', function ($q) {
         ctx.fetchCommitData = {
@@ -55,12 +60,6 @@ describe('EditRepoCommitController'.bold.underline.blue, function() {
         });
         return promisifyMock;
       });
-      $provide.factory('github', function ($q) {
-        ctx.github = {
-          branchOrPRCommits: branchCommitsStub
-        };
-        return ctx.github;
-      });
     });
 
     angular.mock.inject(function (
@@ -121,7 +120,7 @@ describe('EditRepoCommitController'.bold.underline.blue, function() {
 
       expect($scope.model.attrs.commit).to.equal(sha);
     });
-
+    
     it('should trigger update on change of locked attribute', function () {
       expect($scope.ERCC.autoDeploy()).to.not.be.ok;
 
@@ -160,14 +159,15 @@ describe('EditRepoCommitController'.bold.underline.blue, function() {
 
   describe('updating the instance when new commits are made', function() {
     beforeEach(function() {
-      branchCommitsStub.returns(newCommits);
+      getCommitStub.returns(commits);
       $scope.ERCC.acv = {
         attrs: {
           repo: 'foo/bar',
           commit: 'commitSha'
         },
-        githubRepo: {}
-      };
+        githubRepo: {
+          newBranch: newBranchStub
+      }};
       $scope.$digest();
     });
 
@@ -176,11 +176,11 @@ describe('EditRepoCommitController'.bold.underline.blue, function() {
     });
 
     it('should update the instance when triggered by user', function () {
-      branchCommitsStub.reset();
+      getCommitStub.reset();
       $scope.ERCC.updateInstance();
       $scope.$digest();
-      expect($scope.ERCC.latestBranchCommit.sha).to.equal(newCommits[0].sha);
-      sinon.assert.calledOnce(branchCommitsStub);
+      expect($scope.ERCC.latestBranchCommit.attrs.sha).to.equal(commits.models[0].attrs.sha);
+      sinon.assert.calledOnce(getCommitStub);
       sinon.assert.calledOnce(updateInstanceWithNewAcvDataStub);
     });
   });