From 693171cb0544c900cb964b0b6015ca528aa776f2 Mon Sep 17 00:00:00 2001 From: Soreine Date: Tue, 3 May 2016 12:37:26 +0200 Subject: [PATCH 1/3] Adds move and remove article modifier. --- lib/models/summary.js | 32 ++++++++++ lib/modifiers/summary/index.js | 3 +- lib/modifiers/summary/insertArticle.js | 33 ++-------- lib/modifiers/summary/moveArticle.js | 81 +++++++++++++++++++++++++ lib/modifiers/summary/removeArticle.js | 36 +++++++++++ lib/modifiers/summary/unshiftArticle.js | 2 +- 6 files changed, 158 insertions(+), 29 deletions(-) create mode 100644 lib/modifiers/summary/moveArticle.js create mode 100644 lib/modifiers/summary/removeArticle.js diff --git a/lib/models/summary.js b/lib/models/summary.js index ec7e05f41..a25d9ecd3 100644 --- a/lib/models/summary.js +++ b/lib/models/summary.js @@ -129,6 +129,27 @@ Summary.prototype.getPrevArticle = function(current) { return prev; }; +/** + Return the parent article, or parent part of an article + + @param {String|Article} current + @return {Article|Part|Null} +*/ +Summary.prototype.getParent = function (level) { + // Coerce to level + level = is.string(level)? level : level.getLevel(); + + // Get parent level + var parentLevel = getParentLevel(level); + if (!parentLevel) { + return null; + } + + // Get parent of the position + var parentArticle = this.getByLevel(parentLevel); + return parentArticle || null; +}; + /** Render summary as text @@ -188,4 +209,15 @@ Summary.createFromParts = function createFromParts(file, parts) { }); }; +/** + Returns parent level of a level + + @param {String} level + @return {String} +*/ +function getParentLevel(level) { + var parts = level.split('.'); + return parts.slice(0, -1).join('.'); +} + module.exports = Summary; diff --git a/lib/modifiers/summary/index.js b/lib/modifiers/summary/index.js index 855d7cc50..4498287e3 100644 --- a/lib/modifiers/summary/index.js +++ b/lib/modifiers/summary/index.js @@ -1,6 +1,7 @@ - module.exports = { insertArticle: require('./insertArticle'), + moveArticle: require('./moveArticle'), + removeArticle: require('./removeArticle'), unshiftArticle: require('./unshiftArticle'), editPartTitle: require('./editPartTitle'), diff --git a/lib/modifiers/summary/insertArticle.js b/lib/modifiers/summary/insertArticle.js index ae920c287..d97943e34 100644 --- a/lib/modifiers/summary/insertArticle.js +++ b/lib/modifiers/summary/insertArticle.js @@ -3,18 +3,6 @@ var SummaryArticle = require('../../models/summaryArticle'); var editArticle = require('./editArticle'); var indexArticleLevels = require('./indexArticleLevels'); - -/** - Get level of parent of an article - - @param {String} level - @return {String} -*/ -function getParentLevel(level) { - var parts = level.split('.'); - return parts.slice(0, -1).join('.'); -} - /** Insert an article in a summary at a specific position @@ -27,21 +15,13 @@ function insertArticle(summary, level, article) { article = SummaryArticle(article); level = is.string(level)? level : level.getLevel(); - var parentLevel = getParentLevel(level); - - if (!parentLevel) { - // todo: insert new part - return summary; - } - - // Get parent of the position - var parentArticle = summary.getByLevel(parentLevel); - if (!parentLevel) { + var parent = summary.getParent(level); + if (!parent) { return summary; } // Find the index to insert at - var articles = parentArticle.getArticles(); + var articles = parent.getArticles(); var index = articles.findIndex(function(art) { return art.getLevel() === level; }); @@ -53,11 +33,10 @@ function insertArticle(summary, level, article) { articles = articles.insert(index, article); // Reindex the level from here - parentArticle = parentArticle.set('articles', articles); - parentArticle = indexArticleLevels(parentArticle); - - return editArticle(summary, parentLevel, parentArticle); + parent = parent.set('articles', articles); + parent = indexArticleLevels(parent); + return editArticle(summary, parent.getLevel(), parent); } module.exports = insertArticle; diff --git a/lib/modifiers/summary/moveArticle.js b/lib/modifiers/summary/moveArticle.js new file mode 100644 index 000000000..83904eca2 --- /dev/null +++ b/lib/modifiers/summary/moveArticle.js @@ -0,0 +1,81 @@ +var is = require('is'); +var removeArticle = require('./removeArticle'); +var insertArticle = require('./insertArticle'); + +/** + Remove an article from a level, and insert it after another. + + @param {Summary} summary + @param {String|SummaryArticle} from: level to remove + @param {String|SummaryArticle} to: level to insert after + @return {Summary} +*/ +function moveArticle(summary, from, to) { + // Coerce to level + var fromLevel = is.string(from)? from : from.getLevel(); + var toLevel = is.string(to)? to : to.getLevel(); + + var article = summary.getByLevel(fromLevel); + + // Remove + var removed = removeArticle(summary, from); + + // Adjust toLevel if removing impacted it + toLevel = arrayToLevel( + shiftLevel(levelToArray(fromLevel), + levelToArray(toLevel))); + // Re-insert + return insertArticle(removed, to, article); +} + +/** + @param {Array} removedLevel + @param {Array} level The level to udpate + @return {Array} + */ +function shiftLevel(removedLevel, level) { + if (level.length === 0) { + // `removedLevel` is under level, so no effect + return level; + } else if (removedLevel.length === 0) { + // Either `level` is a child of `removedLevel`... or they are equal + // This is undefined behavior. + return level; + } + + var removedRoot = removedLevel[0]; + var root = level[0]; + var removedRest = removedLevel.unshift(); + var rest = level.unshift(); + + if (removedRoot < root) { + // It will shift levels at this point. The rest is unchanged. + return Array.prototype.concat(root - 1, rest); + } else if (removedRoot === root) { + // Look deeper + return Array.prototype.concat(root, shiftLevel(removedRest, rest)); + } else { + // No impact + return level; + } +} + +/** + @param {String} + @return {Array} + */ +function levelToArray(l) { + return l.split('.').map(function (char) { + return parseInt(char, 10); + }); +} + +/** + @param {Array} + @return {String} + */ +function arrayToLevel(a) { + return a.join('.'); +} + +module.exports = moveArticle; diff --git a/lib/modifiers/summary/removeArticle.js b/lib/modifiers/summary/removeArticle.js new file mode 100644 index 000000000..4cb848a09 --- /dev/null +++ b/lib/modifiers/summary/removeArticle.js @@ -0,0 +1,36 @@ +var is = require('is'); +var editArticle = require('./editArticle'); +var indexArticleLevels = require('./indexArticleLevels'); + +/** + Remove an article from a level. + + @param {Summary} summary + @param {String|SummaryArticle} level: level to remove + @return {Summary} +*/ +function removeArticle(summary, level) { + // Coerce to level + level = is.string(level)? level : level.getLevel(); + + var parent = summary.getParent(summary, level); + + // Find the index to remove + var index = articles.findIndex(function(art) { + return art.getLevel() === level; + }); + if (!index) { + return summary; + } + + // Remove from children + var articles = parent.getArticles().remove(index); + parent = parent.set('articles', articles); + + // Reindex the level from here + parent = indexArticleLevels(parent); + + return editArticle(summary, parent.getLevel(), parent); +} + +module.exports = removeArticle; diff --git a/lib/modifiers/summary/unshiftArticle.js b/lib/modifiers/summary/unshiftArticle.js index 3f2ae4df0..d1ebc0581 100644 --- a/lib/modifiers/summary/unshiftArticle.js +++ b/lib/modifiers/summary/unshiftArticle.js @@ -4,7 +4,7 @@ var SummaryPart = require('../../models/summaryPart'); var indexLevels = require('./indexLevels'); /** - Insert an article at the + Insert an article at the beginning of summary @param {Summary} summary @param {Article} article From 4ebe28faaebb8752107ef6cfa6253f7b6f8a2a86 Mon Sep 17 00:00:00 2001 From: Soreine Date: Tue, 3 May 2016 13:11:32 +0200 Subject: [PATCH 2/3] Update insert specification --- lib/modifiers/summary/insertArticle.js | 5 +++-- lib/modifiers/summary/moveArticle.js | 27 +++++++++++++------------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/lib/modifiers/summary/insertArticle.js b/lib/modifiers/summary/insertArticle.js index d97943e34..0a010b497 100644 --- a/lib/modifiers/summary/insertArticle.js +++ b/lib/modifiers/summary/insertArticle.js @@ -4,10 +4,11 @@ var editArticle = require('./editArticle'); var indexArticleLevels = require('./indexArticleLevels'); /** - Insert an article in a summary at a specific position + Returns a new Summary with the article at the given level, with + subsequent article shifted. @param {Summary} summary - @param {String|Article} level: level to insert after + @param {String|Article} level: level to insert at @param {Article} article @return {Summary} */ diff --git a/lib/modifiers/summary/moveArticle.js b/lib/modifiers/summary/moveArticle.js index 83904eca2..9819245ce 100644 --- a/lib/modifiers/summary/moveArticle.js +++ b/lib/modifiers/summary/moveArticle.js @@ -3,29 +3,30 @@ var removeArticle = require('./removeArticle'); var insertArticle = require('./insertArticle'); /** - Remove an article from a level, and insert it after another. + Returns a new summary, with the given article removed from its + origin level, and placed at the given target level. @param {Summary} summary - @param {String|SummaryArticle} from: level to remove - @param {String|SummaryArticle} to: level to insert after + @param {String|SummaryArticle} origin: level to remove + @param {String|SummaryArticle} target: the level where the article will be found @return {Summary} */ -function moveArticle(summary, from, to) { +function moveArticle(summary, origin, target) { // Coerce to level - var fromLevel = is.string(from)? from : from.getLevel(); - var toLevel = is.string(to)? to : to.getLevel(); + var originLevel = is.string(origin)? origin : origin.getLevel(); + var targetLevel = is.string(target)? target : target.getLevel(); - var article = summary.getByLevel(fromLevel); + var article = summary.getByLevel(originLevel); // Remove - var removed = removeArticle(summary, from); + var removed = removeArticle(summary, origin); - // Adjust toLevel if removing impacted it - toLevel = arrayToLevel( - shiftLevel(levelToArray(fromLevel), - levelToArray(toLevel))); + // Adjust targetLevel if removing impacted it + targetLevel = arrayToLevel( + shiftLevel(levelToArray(originLevel), + levelToArray(targetLevel))); // Re-insert - return insertArticle(removed, to, article); + return insertArticle(removed, target, article); } /** From 3fc90554f5e7e6ff2fb4f023319799e1e8f81454 Mon Sep 17 00:00:00 2001 From: Soreine Date: Tue, 3 May 2016 16:02:58 +0200 Subject: [PATCH 3/3] Add tests, and fixes. --- lib/models/summary.js | 2 +- .../summary/__tests__/mergeAtLevel.js | 45 ++++++++++++ .../summary/__tests__/moveArticle.js | 68 +++++++++++++++++++ lib/modifiers/summary/editArticleTitle.js | 4 +- lib/modifiers/summary/editPartTitle.js | 1 - lib/modifiers/summary/insertArticle.js | 22 +++--- .../{editArticle.js => mergeAtLevel.js} | 37 +++++----- lib/modifiers/summary/moveArticle.js | 4 +- lib/modifiers/summary/removeArticle.js | 11 +-- 9 files changed, 159 insertions(+), 35 deletions(-) create mode 100644 lib/modifiers/summary/__tests__/mergeAtLevel.js create mode 100644 lib/modifiers/summary/__tests__/moveArticle.js rename lib/modifiers/summary/{editArticle.js => mergeAtLevel.js} (60%) diff --git a/lib/models/summary.js b/lib/models/summary.js index a25d9ecd3..8a4afc7fd 100644 --- a/lib/models/summary.js +++ b/lib/models/summary.js @@ -56,7 +56,7 @@ Summary.prototype.getArticle = function(iter, partIter) { Return a part/article by its level @param {String} level - @return {Article} + @return {Article|Part} */ Summary.prototype.getByLevel = function(level) { function iterByLevel(article) { diff --git a/lib/modifiers/summary/__tests__/mergeAtLevel.js b/lib/modifiers/summary/__tests__/mergeAtLevel.js new file mode 100644 index 000000000..e2635ec44 --- /dev/null +++ b/lib/modifiers/summary/__tests__/mergeAtLevel.js @@ -0,0 +1,45 @@ +var Immutable = require('immutable'); +var Summary = require('../../../models/summary'); +var File = require('../../../models/file'); + +describe('mergeAtLevel', function() { + var mergeAtLevel = require('../mergeAtLevel'); + var summary = Summary.createFromParts(File(), [ + { + articles: [ + { + title: '1.1', + path: '1.1' + }, + { + title: '1.2', + path: '1.2' + } + ] + }, + { + title: 'Part I', + articles: [] + } + ]); + + it('should edit a part', function() { + var beforeChildren = summary.getByLevel('1').getArticles(); + var newSummary = mergeAtLevel(summary, '1', {title: 'Part O'}); + var edited = newSummary.getByLevel('1'); + + expect(edited.getTitle()).toBe('Part O'); + // Same children + expect(Immutable.is(beforeChildren, edited.getArticles())).toBe(true); + }); + + it('should edit a part', function() { + var beforePath = summary.getByLevel('1.2').getPath(); + var newSummary = mergeAtLevel(summary, '1.2', {title: 'Renamed article'}); + var edited = newSummary.getByLevel('1.2'); + + expect(edited.getTitle()).toBe('Renamed article'); + // Same children + expect(Immutable.is(beforePath, edited.getPath())).toBe(true); + }); +}); diff --git a/lib/modifiers/summary/__tests__/moveArticle.js b/lib/modifiers/summary/__tests__/moveArticle.js new file mode 100644 index 000000000..9a101f6c9 --- /dev/null +++ b/lib/modifiers/summary/__tests__/moveArticle.js @@ -0,0 +1,68 @@ +var Immutable = require('immutable'); +var Summary = require('../../../models/summary'); +var File = require('../../../models/file'); + +describe('moveArticle', function() { + var moveArticle = require('../moveArticle'); + var summary = Summary.createFromParts(File(), [ + { + articles: [ + { + title: '1.1', + path: '1.1' + }, + { + title: '1.2', + path: '1.2' + } + ] + }, + { + title: 'Part I', + articles: [ + { + title: '2.1', + path: '2.1', + articles: [ + { + title: '2.1.1', + path: '2.1.1' + }, + { + title: '2.1.2', + path: '2.1.2' + } + ] + }, + { + title: '2.2', + path: '2.2' + } + ] + } + ]); + + it('should move an article at in place', function() { + var newSummary = moveArticle(summary, '2.1', '2.1'); + + expect(Immutable.is(summary, newSummary)).toBe(true); + }); + + it('should move an article to an previous level', function() { + var newSummary = moveArticle(summary, '2.2', '2.1'); + var moved = newSummary.getByLevel('2.1'); + var other = newSummary.getByLevel('2.2'); + + expect(moved.getTitle()).toBe('2.2'); + expect(other.getTitle()).toBe('2.1'); + }); + + it('should move an article to a next level', function() { + var newSummary = moveArticle(summary, '2.1', '2.2'); + var moved = newSummary.getByLevel('2.1'); + var other = newSummary.getByLevel('2.2'); + + expect(moved.getTitle()).toBe('2.2'); + expect(other.getTitle()).toBe('2.1'); + }); +}); diff --git a/lib/modifiers/summary/editArticleTitle.js b/lib/modifiers/summary/editArticleTitle.js index bd9b6f26e..4edee832a 100644 --- a/lib/modifiers/summary/editArticleTitle.js +++ b/lib/modifiers/summary/editArticleTitle.js @@ -1,4 +1,4 @@ -var editArticle = require('./editArticle'); +var mergeAtLevel = require('./mergeAtLevel'); /** Edit title of an article @@ -9,7 +9,7 @@ var editArticle = require('./editArticle'); @return {Summary} */ function editArticleTitle(summary, level, newTitle) { - return editArticle(summary, level, { + return mergeAtLevel(summary, level, { title: newTitle }); } diff --git a/lib/modifiers/summary/editPartTitle.js b/lib/modifiers/summary/editPartTitle.js index 472399b78..b79ac1e48 100644 --- a/lib/modifiers/summary/editPartTitle.js +++ b/lib/modifiers/summary/editPartTitle.js @@ -1,4 +1,3 @@ - /** Edit title of a part in the summary diff --git a/lib/modifiers/summary/insertArticle.js b/lib/modifiers/summary/insertArticle.js index 0a010b497..849f39e64 100644 --- a/lib/modifiers/summary/insertArticle.js +++ b/lib/modifiers/summary/insertArticle.js @@ -1,6 +1,6 @@ var is = require('is'); var SummaryArticle = require('../../models/summaryArticle'); -var editArticle = require('./editArticle'); +var mergeAtLevel = require('./mergeAtLevel'); var indexArticleLevels = require('./indexArticleLevels'); /** @@ -23,12 +23,7 @@ function insertArticle(summary, level, article) { // Find the index to insert at var articles = parent.getArticles(); - var index = articles.findIndex(function(art) { - return art.getLevel() === level; - }); - if (!index) { - return summary; - } + var index = getLeafIndex(level); // Insert the article at the right index articles = articles.insert(index, article); @@ -37,7 +32,18 @@ function insertArticle(summary, level, article) { parent = parent.set('articles', articles); parent = indexArticleLevels(parent); - return editArticle(summary, parent.getLevel(), parent); + return mergeAtLevel(summary, parent.getLevel(), parent); +} + +/** + @param {String} + @return {Number} The index of this level within its parent's children + */ +function getLeafIndex(level) { + var arr = level.split('.').map(function (char) { + return parseInt(char, 10); + }); + return arr[arr.length - 1] - 1; } module.exports = insertArticle; diff --git a/lib/modifiers/summary/editArticle.js b/lib/modifiers/summary/mergeAtLevel.js similarity index 60% rename from lib/modifiers/summary/editArticle.js rename to lib/modifiers/summary/mergeAtLevel.js index 162539884..9a95ffc3d 100644 --- a/lib/modifiers/summary/editArticle.js +++ b/lib/modifiers/summary/mergeAtLevel.js @@ -11,16 +11,17 @@ function editArticleInList(articles, level, newArticle) { return articles.map(function(article) { var articleLevel = article.getLevel(); - if (articleLevel == level) { + if (articleLevel === level) { + // it is the article to edit return article.merge(newArticle); - } - - if (level.indexOf(articleLevel) === 0) { + } else if (level.indexOf(articleLevel) === 0) { + // it is a parent var articles = editArticleInList(article.getArticles(), level, newArticle); return article.set('articles', articles); + } else { + // This is not the article you are looking for + return article; } - - return article; }); } @@ -35,36 +36,40 @@ function editArticleInList(articles, level, newArticle) { */ function editArticleInPart(part, level, newArticle) { var articles = part.getArticles(); - articles = editArticleInList(articles); + articles = editArticleInList(articles, level, newArticle); return part.set('articles', articles); } /** - Edit an article in a summary + Edit an article, or a part, in a summary. Does a shallow merge. @param {Summary} summary @param {String} level - @param {Article} newArticle + @param {Article|Part} newValue @return {Summary} */ -function editArticle(summary, level, newArticle) { - var parts = summary.getParts(); - +function mergeAtLevel(summary, level, newValue) { var levelParts = level.split('.'); - var partIndex = Number(levelParts[0]); + var partIndex = Number(levelParts[0]) -1; + var parts = summary.getParts(); var part = parts.get(partIndex); if (!part) { return summary; } - part = editArticleInPart(part, level, newArticle); - parts = parts.set(partIndex, part); + var isEditingPart = levelParts.length < 2; + if (isEditingPart) { + part = part.merge(newValue); + } else { + part = editArticleInPart(part, level, newValue); + } + parts = parts.set(partIndex, part); return summary.set('parts', parts); } -module.exports = editArticle; +module.exports = mergeAtLevel; diff --git a/lib/modifiers/summary/moveArticle.js b/lib/modifiers/summary/moveArticle.js index 9819245ce..06d82ca9f 100644 --- a/lib/modifiers/summary/moveArticle.js +++ b/lib/modifiers/summary/moveArticle.js @@ -46,8 +46,8 @@ function shiftLevel(removedLevel, level) { var removedRoot = removedLevel[0]; var root = level[0]; - var removedRest = removedLevel.unshift(); - var rest = level.unshift(); + var removedRest = removedLevel.slice(1); + var rest = level.slice(1); if (removedRoot < root) { // It will shift levels at this point. The rest is unchanged. diff --git a/lib/modifiers/summary/removeArticle.js b/lib/modifiers/summary/removeArticle.js index 4cb848a09..8a30d0ae5 100644 --- a/lib/modifiers/summary/removeArticle.js +++ b/lib/modifiers/summary/removeArticle.js @@ -1,5 +1,5 @@ var is = require('is'); -var editArticle = require('./editArticle'); +var mergeAtLevel = require('./mergeAtLevel'); var indexArticleLevels = require('./indexArticleLevels'); /** @@ -13,24 +13,25 @@ function removeArticle(summary, level) { // Coerce to level level = is.string(level)? level : level.getLevel(); - var parent = summary.getParent(summary, level); + var parent = summary.getParent(level); + var articles = parent.getArticles(); // Find the index to remove var index = articles.findIndex(function(art) { return art.getLevel() === level; }); - if (!index) { + if (index === -1) { return summary; } // Remove from children - var articles = parent.getArticles().remove(index); + articles = articles.remove(index); parent = parent.set('articles', articles); // Reindex the level from here parent = indexArticleLevels(parent); - return editArticle(summary, parent.getLevel(), parent); + return mergeAtLevel(summary, parent.getLevel(), parent); } module.exports = removeArticle;