feat(app): determine destination columns via states

packages/app/lib/apps/automatic-dev-flow.js

@@ -1,6 +1,6 @@
-const IN_PROGRESS = 'In Progress';
-const NEEDS_REVIEW = 'Needs Review';
-const DONE = 'Done';
+const IN_PROGRESS = 'IN_PROGRESS';
+const IN_REVIEW = 'IN_REVIEW';
+const CLOSED = 'CLOSED';
 
 
 /**
@@ -24,7 +24,9 @@ module.exports = function(webhookEvents, githubIssues, columns) {
       issue
     } = context.payload;
 
-    await githubIssues.moveIssue(context, issue || pull_request, DONE);
+    const column = columns.getByState(CLOSED);
+
+    await githubIssues.moveIssue(context, issue || pull_request, column);
   });
 
   webhookEvents.on('pull_request.ready_for_review', async (context) => {
@@ -33,9 +35,11 @@ module.exports = function(webhookEvents, githubIssues, columns) {
       pull_request
     } = context.payload;
 
+    const column = columns.getByState(IN_REVIEW);
+
     await Promise.all([
-      githubIssues.moveIssue(context, pull_request, NEEDS_REVIEW),
-      githubIssues.moveReferencedIssues(context, pull_request, NEEDS_REVIEW)
+      githubIssues.moveIssue(context, pull_request, column),
+      githubIssues.moveReferencedIssues(context, pull_request, column)
     ]);
   });
 
@@ -48,11 +52,13 @@ module.exports = function(webhookEvents, githubIssues, columns) {
       pull_request
     } = context.payload;
 
-    const newState = isDraft(pull_request) ? IN_PROGRESS : NEEDS_REVIEW;
+    const newState = isDraft(pull_request) ? IN_PROGRESS : IN_REVIEW;
+
+    const column = columns.getByState(newState);
 
     await Promise.all([
-      githubIssues.moveIssue(context, pull_request, newState),
-      githubIssues.moveReferencedIssues(context, pull_request, newState)
+      githubIssues.moveIssue(context, pull_request, column),
+      githubIssues.moveReferencedIssues(context, pull_request, column)
     ]);
   });
 
@@ -90,7 +96,9 @@ module.exports = function(webhookEvents, githubIssues, columns) {
 
     const issue_number = match[1];
 
-    return githubIssues.findAndMoveIssue(context, issue_number, IN_PROGRESS, assignee);
+    const column = columns.getByState(IN_PROGRESS);
+
+    return githubIssues.findAndMoveIssue(context, issue_number, column, assignee);
   });
 
 };

packages/app/lib/apps/board-api-routes.js

@@ -20,13 +20,14 @@ const {
  * @param {AuthRoutes} authRoutes
  * @param {GithubIssues} githubIssues
  * @param {Search} search
+ * @param {Columns} columns
  */
 module.exports = async (
   config, store,
   router, logger,
   githubClient, authRoutes,
   userAccess, githubIssues,
-  search
+  search, columns
 ) => {
 
   const log = logger.child({
@@ -129,19 +130,18 @@ module.exports = async (
 
   }
 
-  function moveIssue(context, issue, params) {
+  function moveIssue(context, issue, column, position) {
 
     const {
       id
     } = issue;
 
     const {
       before,
-      after,
-      column
-    } = params;
+      after
+    } = position;
 
-    store.updateOrder(id, before, after, column);
+    store.updateOrder(id, before, after, column.name);
 
     // we move the issue via GitHub and rely on the automatic-dev-flow
     // to pick up the update (and react to it)
@@ -222,11 +222,23 @@ module.exports = async (
 
     const {
       id,
-      ...params
+      column: columnName,
+      before,
+      after
     } = body;
 
     const issue = store.getIssueById(id);
 
+    if (!issue) {
+      return res.status(404).json({});
+    }
+
+    const column = columns.getByName(columnName);
+
+    if (!column) {
+      return res.status(404).json({});
+    }
+
     const repo = repoAndOwner(issue);
 
     const canWrite = await userAccess.canWrite(login, repo);
@@ -250,7 +262,7 @@ module.exports = async (
     };
 
     return (
-      moveIssue(context, issue, params)
+      moveIssue(context, issue, column, { before, after })
         .then(() => {
           res.type('json').json({});
         })

packages/app/lib/apps/github-issues/GithubIssues.js

@@ -36,12 +36,10 @@ function GithubIssues(logger, config, columns) {
 
   }
 
-  function getStateUpdate(issue, newState) {
+  function getStateUpdate(issue, newColumn) {
 
     let update = {};
 
-    const newColumn = columns.getByName(newState);
-
     const issueState = newColumn.closed ? 'closed' : 'open';
 
     if (issue.state !== issueState) {
@@ -88,12 +86,12 @@ function GithubIssues(logger, config, columns) {
       });
   }
 
-  function findAndMoveIssue(context, number, newState, newAssignee) {
+  function findAndMoveIssue(context, number, newColumn, newAssignee) {
     return findIssue(context, number)
-      .then((issue) => issue && moveIssue(context, issue, newState, newAssignee));
+      .then((issue) => issue && moveIssue(context, issue, newColumn, newAssignee));
   }
 
-  async function moveReferencedIssues(context, issue, newState, newAssignee) {
+  async function moveReferencedIssues(context, issue, newColumn, newAssignee) {
 
     // TODO(nikku): do that lazily, i.e. react to PR label changes?
     // would slower the movement but support manual moving-issue with PR
@@ -124,19 +122,19 @@ function GithubIssues(logger, config, columns) {
         number
       } = link;
 
-      return findAndMoveIssue(context, number, newState, newAssignee);
+      return findAndMoveIssue(context, number, newColumn, newAssignee);
     }));
   }
 
-  function moveIssue(context, issue, newState, newAssignee) {
+  function moveIssue(context, issue, newColumn, newAssignee) {
 
     const {
       number: issue_number
     } = issue;
 
     const update = {
       ...getAssigneeUpdate(issue, newAssignee),
-      ...getStateUpdate(issue, newState)
+      ...getStateUpdate(issue, newColumn)
     };
 
     if (!hasKeys(update)) {

packages/app/lib/columns.js

@@ -108,7 +108,7 @@ function createColumnGetter(columns) {
       return issue.labels.find(l => l.name === columnLabel);
     });
 
-    return (column || defaultColumn).name;
+    return (column || defaultColumn);
   };
 }
 
@@ -118,6 +118,7 @@ function groupByStates(columns) {
     const defaultName = StateToNames[state];
 
     const column = (
+      columns.find(c => c.states && c.states.includes(state)) ||
       columns.find(c => c.name === defaultName)
     );
 

packages/app/lib/store.js

@@ -74,7 +74,7 @@ class Store {
       links
     } = this.updateLinks(issue);
 
-    const column = newColumn || this.getIssueColumn(issue);
+    const column = newColumn || this.getIssueColumn(issue).name;
 
     // automatically compute desired order unless
     // the order is provided by the user

packages/app/test/apps.test.js

@@ -27,20 +27,22 @@ describe('apps', () => {
 
   describe('githubIssues', function() {
 
-    let app, githubIssues;
+    let app, githubIssues, columns;
 
     beforeEach(async function() {
       app = await bootstrap({
         modules: [ GithubIssues ]
       });
 
       githubIssues = await app.get('githubIssues');
+
+      columns = await app.get('columns');
     });
 
 
     describe('state update', function() {
 
-      it('should compute Inbox -> Done', function() {
+      it('should compute Inbox -> Done', async function() {
 
         // given
         const issue = {
@@ -52,8 +54,10 @@ describe('apps', () => {
           state: 'open'
         };
 
+        const newColumn = columns.getByState('CLOSED');
+
         // when
-        const update = githubIssues.getStateUpdate(issue, 'Done');
+        const update = githubIssues.getStateUpdate(issue, newColumn);
 
         // then
         expect(update).to.eql({
@@ -75,8 +79,10 @@ describe('apps', () => {
           state: 'open'
         };
 
+        const newColumn = columns.getByState('DEFAULT');
+
         // when
-        const update = githubIssues.getStateUpdate(issue, 'Inbox');
+        const update = githubIssues.getStateUpdate(issue, newColumn);
 
         // then
         expect(update).to.eql({});
@@ -99,8 +105,10 @@ describe('apps', () => {
           state: 'open'
         };
 
+        const newColumn = columns.getByState('CLOSED');
+
         // when
-        const update = githubIssues.getStateUpdate(issue, 'Done');
+        const update = githubIssues.getStateUpdate(issue, newColumn);
 
         // then
         expect(update).to.eql({
@@ -126,8 +134,10 @@ describe('apps', () => {
           state: 'open'
         };
 
+        const newColumn = columns.getByState('DEFAULT');
+
         // when
-        const update = githubIssues.getStateUpdate(issue, 'Inbox');
+        const update = githubIssues.getStateUpdate(issue, newColumn);
 
         // then
         expect(update).to.eql({
@@ -155,8 +165,10 @@ describe('apps', () => {
           state: 'closed'
         };
 
+        const newColumn = columns.getByState('CLOSED');
+
         // when
-        const update = githubIssues.getStateUpdate(issue, 'Done');
+        const update = githubIssues.getStateUpdate(issue, newColumn);
 
         // then
         expect(update).to.eql({

packages/app/test/columns.test.js

@@ -146,7 +146,7 @@ describe('columns', function() {
 
       const column = columns.getIssueColumn(issue);
 
-      expect(column).to.eql(expectedColumn);
+      expect(column.name).to.eql(expectedColumn);
     }
 
   });
@@ -192,6 +192,50 @@ describe('columns', function() {
 
     });
 
+
+    describe('should map explicit', function() {
+
+      const columns = new Columns([
+        { name: 'Default', label: null, states: [ 'DEFAULT' ] },
+        { name: 'External Contribution', label: null, states: [ 'EXTERNAL_CONTRIBUTION' ] },
+        { name: 'Doing', label: 'in progress', states: [ 'IN_PROGRESS' ] },
+        { name: 'Reviewing', label: 'needs review', states: [ 'IN_REVIEW' ] },
+        { name: 'Closed', label: null, closed: true, states: [ 'CLOSED' ] }
+      ]);
+
+
+      it('<Default> to DEFAULT', function() {
+        expectStateColumn('DEFAULT', 'Default');
+      });
+
+
+      it('<External Contribution> to EXTERNAL_CONTRIBUTION', function() {
+        expectStateColumn('EXTERNAL_CONTRIBUTION', 'External Contribution');
+      });
+
+
+      it('<Reviewing> to IN_REVIEW', function() {
+        expectStateColumn('IN_REVIEW', 'Reviewing');
+      });
+
+
+      it('<Closed> to CLOSED', function() {
+        expectStateColumn('CLOSED', 'Closed');
+      });
+
+
+      // helpers ////////////////////
+
+      function expectStateColumn(state, expectedColumn) {
+
+        const column = columns.getByState(state);
+
+        expect(column).to.exist;
+        expect(column.name).to.eql(expectedColumn);
+      }
+
+    });
+
   });
 
 });