Skip to content

Commit

Permalink
848 hierarchical categories (#2734)
Browse files Browse the repository at this point in the history
* Hierarchical categories can be assigned

The code still needs some cleanup, but the new test passes without
breaking the old ones.

* more tests, cleanup

added tests for category list and front-matter processor to include
multiple categories; cleaned up Post.setCategories() code

* -whitespace

* --no-fat-arrow

CodeClimate didn't like 'em...

* Added test for dupe parents

ensure we do not create a duplicate post/category xref for duplicate
parent categories
  • Loading branch information
danieljsummers authored and NoahDragon committed Sep 5, 2017
1 parent 658cc0f commit d27b704
Show file tree
Hide file tree
Showing 4 changed files with 185 additions and 61 deletions.
89 changes: 51 additions & 38 deletions lib/models/post.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,63 +133,76 @@ module.exports = function(ctx) {
});

Post.method('setCategories', function(cats) {
cats = removeEmptyTag(cats);
// Remove empty categories, preserving hierarchies
cats = cats.filter(function(cat) {
return Array.isArray(cat) ? true : cat != null && cat !== '';
}).map(function(cat) {
return Array.isArray(cat) ? removeEmptyTag(cat) : cat + '';
});

var PostCategory = ctx.model('PostCategory');
var Category = ctx.model('Category');
var id = this._id;
var arr = [];
var allIds = [];
var existed = PostCategory.find({post_id: id}, {lean: true}).map(pickID);

// Don't use "Promise.map". It doesn't run in series.
// MUST USE "Promise.each".
return Promise.each(cats, function(cat, i) {
// Find the category by name
var data = Category.findOne({
name: cat,
parent: i ? arr[i - 1] : {$exists: false}
}, {lean: true});

if (data) {
arr.push(data._id);
return data;
}

// Insert the category if not exist
var obj = {name: cat};
if (i) obj.parent = arr[i - 1];

return Category.insert(obj).catch(function(err) {
// Try to find the category again. Throw the error if not found
var hasHierarchy = cats.filter(Array.isArray).length > 0;

// Add a hierarchy of categories
var addHierarchy = function(catHierarchy) {
var parentIds = [];
if (!Array.isArray(catHierarchy)) catHierarchy = [catHierarchy];
// Don't use "Promise.map". It doesn't run in series.
// MUST USE "Promise.each".
return Promise.each(catHierarchy, function(cat, i) {
// Find the category by name
var data = Category.findOne({
name: cat,
parent: i ? arr[i - 1] : {$exists: false}
parent: i ? parentIds[i - 1] : {$exists: false}
}, {lean: true});

if (data) return data;
throw err;
}).then(function(data) {
arr.push(data._id);
return data;
if (data) {
allIds.push(data._id);
parentIds.push(data._id);
return data;
}

// Insert the category if not exist
var obj = {name: cat};
if (i) obj.parent = parentIds[i - 1];

return Category.insert(obj).catch(function(err) {
// Try to find the category again. Throw the error if not found
var data = Category.findOne({
name: cat,
parent: i ? parentIds[i - 1] : {$exists: false}
}, {lean: true});

if (data) return data;
throw err;
}).then(function(data) {
allIds.push(data._id);
parentIds.push(data._id);
return data;
});
});
}).map(function() {
// Get the index from the second argument
// and get the category id from arr.
var cat = arr[arguments[1]];
};

return (hasHierarchy ? Promise.each(cats, addHierarchy) : Promise.resolve(addHierarchy(cats))
).then(function() {
return allIds;
}).map(function(catId) {
// Find the reference
var ref = PostCategory.findOne({post_id: id, category_id: cat}, {lean: true});
var ref = PostCategory.findOne({post_id: id, category_id: catId}, {lean: true});
if (ref) return ref;

// Insert the reference if not exist
return PostCategory.insert({
post_id: id,
category_id: cat
category_id: catId
});
}).then(function(cats) {
}).then(function(postCats) {
// Remove old categories
var deleted = _.difference(existed, cats.map(pickID));
return deleted;
return _.difference(existed, postCats.map(pickID));
}).map(function(cat) {
return PostCategory.removeById(cat);
});
Expand Down
94 changes: 71 additions & 23 deletions test/scripts/helpers/list_categories.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@ describe('list_categories', () => {
{source: 'foo', slug: 'foo'},
{source: 'bar', slug: 'bar'},
{source: 'baz', slug: 'baz'},
{source: 'boo', slug: 'boo'}
{source: 'boo', slug: 'boo'},
{source: 'bat', slug: 'bat'}
])).then(posts => Promise.each([
['baz'],
['baz', 'bar'],
['foo'],
['baz']
['baz'],
['bat', ['baz', 'bar']]
], (cats, i) => posts[i].setCategories(cats))).then(() => {
hexo.locals.invalidate();
ctx.site = hexo.locals.toObject();
Expand All @@ -37,10 +39,13 @@ describe('list_categories', () => {
result.should.eql([
'<ul class="category-list">',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/baz/">baz</a><span class="category-list-count">3</span>',
'<a class="category-list-link" href="/categories/bat/">bat</a><span class="category-list-count">1</span>',
'</li>',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/baz/">baz</a><span class="category-list-count">4</span>',
'<ul class="category-list-child">',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/baz/bar/">bar</a><span class="category-list-count">1</span>',
'<a class="category-list-link" href="/categories/baz/bar/">bar</a><span class="category-list-count">2</span>',
'</li>',
'</ul>',
'</li>',
Expand All @@ -59,7 +64,10 @@ describe('list_categories', () => {
result.should.eql([
'<ul class="category-list">',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/baz/">baz</a><span class="category-list-count">3</span>',
'<a class="category-list-link" href="/categories/bat/">bat</a><span class="category-list-count">1</span>',
'</li>',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/baz/">baz</a><span class="category-list-count">4</span>',
'</li>',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/foo/">foo</a><span class="category-list-count">1</span>',
Expand All @@ -74,8 +82,9 @@ describe('list_categories', () => {
});

result.should.eql([
'<a class="category-link" href="/categories/baz/">baz<span class="category-count">3</span></a>',
'<a class="category-link" href="/categories/baz/bar/">bar<span class="category-count">1</span></a>',
'<a class="category-link" href="/categories/bat/">bat<span class="category-count">1</span></a>',
'<a class="category-link" href="/categories/baz/">baz<span class="category-count">4</span></a>',
'<a class="category-link" href="/categories/baz/bar/">bar<span class="category-count">2</span></a>',
'<a class="category-link" href="/categories/foo/">foo<span class="category-count">1</span></a>'
].join(', '));
});
Expand All @@ -87,6 +96,9 @@ describe('list_categories', () => {

result.should.eql([
'<ul class="category-list">',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/bat/">bat</a>',
'</li>',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/baz/">baz</a>',
'<ul class="category-list-child">',
Expand All @@ -110,10 +122,13 @@ describe('list_categories', () => {
result.should.eql([
'<ul class="test-list">',
'<li class="test-list-item">',
'<a class="test-list-link" href="/categories/baz/">baz</a><span class="test-list-count">3</span>',
'<a class="test-list-link" href="/categories/bat/">bat</a><span class="test-list-count">1</span>',
'</li>',
'<li class="test-list-item">',
'<a class="test-list-link" href="/categories/baz/">baz</a><span class="test-list-count">4</span>',
'<ul class="test-list-child">',
'<li class="test-list-item">',
'<a class="test-list-link" href="/categories/baz/bar/">bar</a><span class="test-list-count">1</span>',
'<a class="test-list-link" href="/categories/baz/bar/">bar</a><span class="test-list-count">2</span>',
'</li>',
'</ul>',
'</li>',
Expand All @@ -132,7 +147,10 @@ describe('list_categories', () => {
result.should.eql([
'<ul class="category-list">',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/baz/">baz</a><span class="category-list-count">3</span>',
'<a class="category-list-link" href="/categories/bat/">bat</a><span class="category-list-count">1</span>',
'</li>',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/baz/">baz</a><span class="category-list-count">4</span>',
'</li>',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/foo/">foo</a><span class="category-list-count">1</span>',
Expand All @@ -152,10 +170,13 @@ describe('list_categories', () => {
'<a class="category-list-link" href="/categories/foo/">foo</a><span class="category-list-count">1</span>',
'</li>',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/baz/">baz</a><span class="category-list-count">3</span>',
'<a class="category-list-link" href="/categories/bat/">bat</a><span class="category-list-count">1</span>',
'</li>',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/baz/">baz</a><span class="category-list-count">4</span>',
'<ul class="category-list-child">',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/baz/bar/">bar</a><span class="category-list-count">1</span>',
'<a class="category-list-link" href="/categories/baz/bar/">bar</a><span class="category-list-count">2</span>',
'</li>',
'</ul>',
'</li>',
Expand All @@ -174,13 +195,16 @@ describe('list_categories', () => {
'<a class="category-list-link" href="/categories/foo/">foo</a><span class="category-list-count">1</span>',
'</li>',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/baz/">baz</a><span class="category-list-count">3</span>',
'<a class="category-list-link" href="/categories/baz/">baz</a><span class="category-list-count">4</span>',
'<ul class="category-list-child">',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/baz/bar/">bar</a><span class="category-list-count">1</span>',
'<a class="category-list-link" href="/categories/baz/bar/">bar</a><span class="category-list-count">2</span>',
'</li>',
'</ul>',
'</li>',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/bat/">bat</a><span class="category-list-count">1</span>',
'</li>',
'</ul>'
].join(''));
});
Expand All @@ -195,10 +219,13 @@ describe('list_categories', () => {
result.should.eql([
'<ul class="category-list">',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/baz/">BAZ</a><span class="category-list-count">3</span>',
'<a class="category-list-link" href="/categories/bat/">BAT</a><span class="category-list-count">1</span>',
'</li>',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/baz/">BAZ</a><span class="category-list-count">4</span>',
'<ul class="category-list-child">',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/baz/bar/">BAR</a><span class="category-list-count">1</span>',
'<a class="category-list-link" href="/categories/baz/bar/">BAR</a><span class="category-list-count">2</span>',
'</li>',
'</ul>',
'</li>',
Expand All @@ -209,15 +236,30 @@ describe('list_categories', () => {
].join(''));
});

it('separator', () => {
it('separator (blank)', () => {
var result = listCategories({
style: false,
separator: ''
});

result.should.eql([
'<a class="category-link" href="/categories/baz/">baz<span class="category-count">3</span></a>',
'<a class="category-link" href="/categories/baz/bar/">bar<span class="category-count">1</span></a>',
'<a class="category-link" href="/categories/bat/">bat<span class="category-count">1</span></a>',
'<a class="category-link" href="/categories/baz/">baz<span class="category-count">4</span></a>',
'<a class="category-link" href="/categories/baz/bar/">bar<span class="category-count">2</span></a>',
'<a class="category-link" href="/categories/foo/">foo<span class="category-count">1</span></a>'
].join(''));
});

it('separator (non-blank)', () => {
var result = listCategories({
style: false,
separator: '|'
});

result.should.eql([
'<a class="category-link" href="/categories/bat/">bat<span class="category-count">1</span></a>|',
'<a class="category-link" href="/categories/baz/">baz<span class="category-count">4</span></a>|',
'<a class="category-link" href="/categories/baz/bar/">bar<span class="category-count">2</span></a>|',
'<a class="category-link" href="/categories/foo/">foo<span class="category-count">1</span></a>'
].join(''));
});
Expand All @@ -229,11 +271,14 @@ describe('list_categories', () => {

result.should.eql([
'<ul class="category-list">',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/bat/">bat</a><span class="category-list-count">1</span>',
'</li>',
'<li class="category-list-item has-children">',
'<a class="category-list-link" href="/categories/baz/">baz</a><span class="category-list-count">3</span>',
'<a class="category-list-link" href="/categories/baz/">baz</a><span class="category-list-count">4</span>',
'<ul class="category-list-child">',
'<li class="category-list-item">',
'<a class="category-list-link" href="/categories/baz/bar/">bar</a><span class="category-list-count">1</span>',
'<a class="category-list-link" href="/categories/baz/bar/">bar</a><span class="category-list-count">2</span>',
'</li>',
'</ul>',
'</li>',
Expand All @@ -252,10 +297,13 @@ describe('list_categories', () => {
result.should.eql([
'<ul class="category-list">',
'<li class="category-list-item">',
'<a class="category-list-link current" href="/categories/baz/">baz</a><span class="category-list-count">3</span>',
'<a class="category-list-link" href="/categories/bat/">bat</a><span class="category-list-count">1</span>',
'</li>',
'<li class="category-list-item">',
'<a class="category-list-link current" href="/categories/baz/">baz</a><span class="category-list-count">4</span>',
'<ul class="category-list-child">',
'<li class="category-list-item">',
'<a class="category-list-link current" href="/categories/baz/bar/">bar</a><span class="category-list-count">1</span>',
'<a class="category-list-link current" href="/categories/baz/bar/">bar</a><span class="category-list-count">2</span>',
'</li>',
'</ul>',
'</li>',
Expand Down
38 changes: 38 additions & 0 deletions test/scripts/models/post.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,44 @@ describe('Post', () => {
}).finally(() => Post.removeById(id));
});

it('setCategories() - multiple hierarchies', () => Post.insert({
source: 'foo.md',
slug: 'bar'
}).then(post => post.setCategories([['foo', '', 'bar'], '', 'baz'])
.thenReturn(Post.findById(post._id))).then(post => {
var cats = post.categories.toArray();

// There should have been 3 categories set; blanks eliminated
cats.should.have.lengthOf(3);

// Category 1 should be foo, no parent
cats[0].name.should.eql('foo');
should.not.exist(cats[0].parent);

// Category 2 should be bar, foo as parent
cats[1].name.should.eql('bar');
cats[1].parent.should.eql(cats[0]._id);

// Category 3 should be baz, no parent
cats[2].name.should.eql('baz');
should.not.exist(cats[2].parent);

return Post.removeById(post._id);
}));

it('setCategories() - multiple hierarchies (dedupes repeated parent)', () => Post.insert({
source: 'foo.md',
slug: 'bar'
}).then(post => post.setCategories([['foo', 'bar'], ['foo', 'baz']])
.thenReturn(Post.findById(post._id))).then(post => {
var cats = post.categories.toArray();

// There should have been 3 categories set (foo is dupe)
cats.should.have.lengthOf(3);

return Post.removeById(post._id);
}));

it('remove PostTag references when a post is removed', () => Post.insert({
source: 'foo.md',
slug: 'bar'
Expand Down
Loading

0 comments on commit d27b704

Please sign in to comment.