feat(app): reduce issue re-ordering noise

* keep issue order, if possible: if an issue is already in
  the right order compared to the linked issues, do not
  move it

* if an issue updates and stays within its column, do not
  move it to the top

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

@@ -132,21 +132,16 @@ module.exports = async (
 
   function moveIssue(context, issue, column, position) {
 
-    const {
-      id
-    } = issue;
-
     const {
       before,
       after
     } = position;
 
-    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)
 
     return Promise.all([
+      store.updateIssueOrder(issue, before, after, column.name),
       githubIssues.moveIssue(context, issue, column),
       githubIssues.moveReferencedIssues(context, issue, column)
     ]);

packages/app/lib/store.js

@@ -24,6 +24,7 @@ class Store {
 
     this.issues = [];
     this.issueOrder = {};
+    this.issueColumn = {};
 
     this.updates = new Updates();
     this.links = new Links();
@@ -35,7 +36,7 @@ class Store {
     this.boardCache = null;
   }
 
-  getIssueColumn(issue) {
+  computeIssueColumn(issue) {
     return this.columns.getIssueColumn(issue);
   }
 
@@ -74,16 +75,16 @@ class Store {
       links
     } = this.updateLinks(issue);
 
-    const column = newColumn || this.getIssueColumn(issue).name;
+    const column = newColumn || this.computeIssueColumn(issue).name;
 
     // automatically compute desired order unless
     // the order is provided by the user
     //
     // this ensures the board is automatically pre-sorted
     // as links are being created and removed on GitHub
     const order = newOrder || this.computeLinkedOrder(
-      issue, column,
-      this.getOrder(id),
+      issue,
+      column,
       links
     );
 
@@ -110,13 +111,29 @@ class Store {
     return issue;
   }
 
-  async updateOrder(issue, before, after, column) {
-    const order = this.computeOrder(before, after);
+  updateIssueOrder(issue, before, after, newColumn) {
 
-    await this.updateIssue(this.getIssueById(issue), column, order);
+    const {
+      id,
+      key
+    } = issue;
+
+    const newOrder = this.computeOrder(before, after, this.getIssueOrder(id));
+
+    this.log.debug({
+      issue: key,
+      newOrder,
+      newColumn
+    }, 'update issue order');
+
+    return this.updateIssue(issue, newColumn, newOrder);
   }
 
-  computeLinkedOrder(issue, column, order, links) {
+  computeLinkedOrder(issue, column, links) {
+
+    const {
+      id
+    } = issue;
 
     const beforeTypes = {
       REQUIRED_BY: 1,
@@ -155,18 +172,21 @@ class Store {
       }
     }
 
+    const currentOrder = this.getIssueOrder(id);
+    const currentColumn = this.getIssueColumn(id);
+
     if (!before && !after) {
 
       // keep order if issue stays within column
-      if (column === issue.column) {
-        return order;
+      if (column === currentColumn) {
+        return currentOrder;
       }
 
       // insert on top of column
       before = this.issues[0];
     }
 
-    return this.computeOrder(before && before.id, after && after.id);
+    return this.computeOrder(before && before.id, after && after.id, currentOrder);
   }
 
   updateLinks(issue) {
@@ -260,11 +280,13 @@ class Store {
       id,
       key,
       order,
+      column,
       labels
     } = issue;
 
-    // update order
-    this.setOrder(id, order);
+    // update column and order
+    this.setIssueOrder(id, order);
+    this.setIssueColumn(id, column);
 
     // attach column label meta-data
     issue = {
@@ -361,6 +383,9 @@ class Store {
     delete this.issuesByKey[key];
     delete this.linkedCache[id];
 
+    delete this.issueOrder[id];
+    delete this.issueColumn[id];
+
     this.boardCache = null;
 
     this.issues = this.issues.filter(issue => issue.id !== id);
@@ -387,32 +412,52 @@ class Store {
     return this.issues;
   }
 
-  computeOrder(beforeId, afterId) {
+  computeOrder(beforeId, afterId, currentOrder) {
 
     const beforeOrder = beforeId && this.issueOrder[beforeId];
     const afterOrder = afterId && this.issueOrder[afterId];
 
     if (beforeOrder && afterOrder) {
-      return (beforeOrder + afterOrder) / 2;
+      return (
+        typeof currentOrder === 'number' && currentOrder < beforeOrder && currentOrder > afterOrder
+          ? currentOrder
+          : (beforeOrder + afterOrder) / 2
+      );
     }
 
     if (beforeOrder) {
-      return beforeOrder - 99999.89912;
+      return (
+        typeof currentOrder === 'number' && currentOrder < beforeOrder
+          ? currentOrder
+          : beforeOrder - 99999.89912
+      );
     }
 
     if (afterOrder) {
-      return afterOrder + 99999.89912;
+      return (
+        typeof currentOrder === 'number' && currentOrder > afterOrder
+          ? currentOrder
+          : afterOrder + 99999.89912
+      );
     }
 
     // a good start :)
     return 779999.89912;
   }
 
-  setOrder(issueId, order) {
+  setIssueColumn(issueId, column) {
+    this.issueColumn[String(issueId)] = column;
+  }
+
+  getIssueColumn(issueId) {
+    return this.issueColumn[String(issueId)];
+  }
+
+  setIssueOrder(issueId, order) {
     this.issueOrder[String(issueId)] = order;
   }
 
-  getOrder(issueId) {
+  getIssueOrder(issueId) {
     return this.issueOrder[String(issueId)];
   }
 
@@ -456,13 +501,15 @@ class Store {
       issues,
       lastSync,
       issueOrder,
+      issueColumn,
       links
     } = this;
 
     return JSON.stringify({
       issues,
       lastSync,
       issueOrder,
+      issueColumn,
       links: links.asJSON()
     });
   }
@@ -476,12 +523,14 @@ class Store {
       issues,
       lastSync,
       issueOrder,
+      issueColumn,
       links
     } = JSON.parse(json);
 
     this.issues = issues || [];
     this.lastSync = lastSync;
     this.issueOrder = issueOrder || {};
+    this.issueColumn = issueColumn || {};
 
     if (links) {
       this.links.loadJSON(links);

packages/app/test/store.test.js

@@ -346,8 +346,8 @@ describe('store', function() {
 
         if (last) {
 
-          const lastOrder = store.getOrder(last.id);
-          const currentOrder = store.getOrder(current.id);
+          const lastOrder = store.getIssueOrder(last.id);
+          const currentOrder = store.getIssueOrder(current.id);
 
           if (lastOrder > currentOrder) {
             throw new Error(
@@ -539,6 +539,43 @@ describe('store', function() {
       expectOrder(store, [ issue_B, issue_C, issue_A ]);
     });
 
+
+    it('should keep unliked order without column change', async function() {
+
+      // given
+      const store = createStore();
+
+      // when
+      const issue_A = await store.updateIssue(createIssue({
+        state: 'closed'
+      }));
+
+      const issue_B = await store.updateIssue(createIssue({
+        state: 'closed'
+      }));
+
+      const issue_C = await store.updateIssue(createIssue({
+        state: 'closed'
+      }));
+
+      const issue_D = await store.updateIssue(createIssue({
+        state: 'open'
+      }));
+
+      // when
+      const updated_issue_D = await store.updateIssueOrder(issue_D, issue_A.id, issue_B.id, 'Done');
+
+      const final_issue_D = await store.updateIssue({
+        ...issue_D,
+        state: 'closed'
+      });
+
+      // then
+      expect(final_issue_D.order).to.eql(updated_issue_D.order);
+
+      expectOrder(store, [ issue_C, issue_B, issue_D, issue_A ]);
+    });
+
   });
 
 });