Moved raw order for tag filtering to correct place (#10166)

refs #10105 - ordering !== filtering
TryGhost · Nov 15, 2018 · 2e81852 · 2e81852
1 parent 95ba6a5
commit 2e81852
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 15 deletions.
diff --git a/core/server/models/base/index.js b/core/server/models/base/index.js
@@ -654,7 +654,7 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
         // @TODO: we can't use order raw when running migrations (see https://github.com/tgriesser/knex/issues/2763)
         if (this.orderDefaultRaw && !options.migrating) {
             itemCollection.query((qb) => {
-                qb.orderByRaw(this.orderDefaultRaw());
+                qb.orderByRaw(this.orderDefaultRaw(options));
             });
         }
 
@@ -722,7 +722,7 @@ ghostBookshelf.Model = ghostBookshelf.Model.extend({
         if (options.order) {
             options.order = this.parseOrderOption(options.order, options.withRelated);
         } else if (this.orderDefaultRaw) {
-            options.orderRaw = this.orderDefaultRaw();
+            options.orderRaw = this.orderDefaultRaw(options);
         } else if (this.orderDefaultOptions) {
             options.order = this.orderDefaultOptions();
         }

diff --git a/core/server/models/plugins/filter.js b/core/server/models/plugins/filter.js
@@ -124,15 +124,6 @@ filter = function filter(Bookshelf) {
                     .query('leftOuterJoin', 'posts_tags', 'posts_tags.post_id', '=', 'posts.id')
                     .query('leftOuterJoin', 'tags', 'posts_tags.tag_id', '=', 'tags.id');
 
-                // The order override should ONLY happen if we are doing an "IN" query
-                // TODO move the order handling to the query building that is currently inside pagination
-                // TODO make the order handling in pagination handle orderByRaw
-                // TODO extend this handling to all joins
-                if (gql.json.findStatement(this._filters.statements, {prop: /^tags/, op: 'IN'})) {
-                    // TODO make this count the number of MATCHING tags, not just the number of tags
-                    this.query('orderByRaw', 'count(tags.id) DESC');
-                }
-
                 // We need to add a group by to counter the double left outer join
                 // TODO improve on the group by handling
                 options.groups = options.groups || [];

diff --git a/core/server/models/post.js b/core/server/models/post.js
@@ -530,14 +530,21 @@ Post = ghostBookshelf.Model.extend({
         };
     },
 
-    orderDefaultRaw: function () {
-        return '' +
+    orderDefaultRaw: function (options) {
+        let order = '' +
             'CASE WHEN posts.status = \'scheduled\' THEN 1 ' +
             'WHEN posts.status = \'draft\' THEN 2 ' +
             'ELSE 3 END ASC,' +
             'CASE WHEN posts.status != \'draft\' THEN posts.published_at END DESC,' +
             'posts.updated_at DESC,' +
             'posts.id DESC';
+
+        // CASE: if the filter contains an `IN` operator, we should return the posts first, which match both tags
+        if (options.filter && options.filter.match(/(tags|tag):\s?\[.*\]/)) {
+            order = `count(tags.id) DESC, ${order}`;
+        }
+
+        return order;
     },
 
     /**

diff --git a/core/test/unit/models/post_spec.js b/core/test/unit/models/post_spec.js
@@ -46,12 +46,12 @@ describe('Unit: models/post', function () {
             });
 
             return models.Post.findPage({
-                filter: 'tags: [photo, video] + id: -' + testUtils.filterData.data.posts[3].id,
+                filter: 'tags:[photo, video]+id:-' + testUtils.filterData.data.posts[3].id,
                 limit: 3,
                 withRelated: ['tags']
             }).then(() => {
                 queries.length.should.eql(2);
-                queries[0].sql.should.eql('select count(distinct posts.id) as aggregate from `posts` left outer join `posts_tags` on `posts_tags`.`post_id` = `posts`.`id` left outer join `tags` on `posts_tags`.`tag_id` = `tags`.`id` where (`posts`.`page` = ? and `posts`.`status` = ?) and (`tags`.`slug` in (?, ?) and `posts`.`id` != ?) order by count(tags.id) DESC');
+                queries[0].sql.should.eql('select count(distinct posts.id) as aggregate from `posts` left outer join `posts_tags` on `posts_tags`.`post_id` = `posts`.`id` left outer join `tags` on `posts_tags`.`tag_id` = `tags`.`id` where (`posts`.`page` = ? and `posts`.`status` = ?) and (`tags`.`slug` in (?, ?) and `posts`.`id` != ?)');
                 queries[0].bindings.should.eql([
                     false,
                     'published',