diff --git a/packages/app/lib/apps/board-api-routes.js b/packages/app/lib/apps/board-api-routes.js index 503fdc8c..c83f1f40 100644 --- a/packages/app/lib/apps/board-api-routes.js +++ b/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) ]); diff --git a/packages/app/lib/store.js b/packages/app/lib/store.js index b3a2a6d7..27d5955b 100644 --- a/packages/app/lib/store.js +++ b/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,7 +75,7 @@ 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 @@ -82,8 +83,8 @@ class Store { // 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,6 +501,7 @@ class Store { issues, lastSync, issueOrder, + issueColumn, links } = this; @@ -463,6 +509,7 @@ class Store { 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); diff --git a/packages/app/test/store.test.js b/packages/app/test/store.test.js index 0649dbcf..4f52bb8a 100644 --- a/packages/app/test/store.test.js +++ b/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 ]); + }); + }); });