Added sentiment ordering and include for posts (#15657)

fixes https://github.com/TryGhost/Team/issues/2090 - This changes how sentiment is exposed in the API. Now it is exposed as a `sentiment` relation, directly on the model (no longer in counts). Internally we still use `count.sentiment`. - Content API users (and themes) can include the 'sentiment' relation and order by sentiment. - Updated Admin to use sentiment instead of count.sentiment
TryGhost · Oct 19, 2022 · 6380b82 · 6380b82
1 parent 3db8fb5
commit 6380b82
Show file tree

Hide file tree

Showing 12 changed files with 70 additions and 23 deletions.
diff --git a/ghost/admin/app/components/posts/analytics.hbs b/ghost/admin/app/components/posts/analytics.hbs
@@ -72,7 +72,7 @@
         {{#if this.post.showAudienceFeedback }}
           <tabs.tab>
             <h3><span class="hide-when-small">More </span><div class="visible-when-small">like</div><span class="hide-when-small"> this</span></h3>
-            <p>{{format-number this.post.count.positive_feedback}} <strong>{{this.post.count.sentiment}}%</strong></p>
+            <p>{{format-number this.post.count.positive_feedback}} <strong>{{this.post.sentiment}}%</strong></p>
           </tabs.tab>
 
           <tabs.tabPanel>More like this</tabs.tabPanel>

diff --git a/ghost/admin/app/models/post.js b/ghost/admin/app/models/post.js
@@ -79,6 +79,7 @@ export default Model.extend(Comparable, ValidationEngine, {
     validationType: 'post',
 
     count: attr(),
+    sentiment: attr(),
     createdAtUTC: attr('moment-utc'),
     excerpt: attr('string'),
     customExcerpt: attr('string'),
@@ -195,8 +196,8 @@ export default Model.extend(Comparable, ValidationEngine, {
             && this.email && this.email.status === 'failed';
     }),
 
-    showAudienceFeedback: computed('count', function () {
-        return this.feature.get('audienceFeedback') && this.count.sentiment !== undefined;
+    showAudienceFeedback: computed('sentiment', function () {
+        return this.feature.get('audienceFeedback') && this.sentiment !== undefined;
     }),
 
     showEmailOpenAnalytics: computed('hasBeenEmailed', 'isSent', 'isPublished', function () {

diff --git a/ghost/core/core/server/api/endpoints/posts-public.js b/ghost/core/core/server/api/endpoints/posts-public.js
@@ -1,7 +1,7 @@
 const models = require('../../models');
 const tpl = require('@tryghost/tpl');
 const errors = require('@tryghost/errors');
-const allowedIncludes = ['tags', 'authors', 'tiers'];
+const allowedIncludes = ['tags', 'authors', 'tiers', 'sentiment'];
 
 const messages = {
     postNotFound: 'Post not found.'

diff --git a/ghost/core/core/server/api/endpoints/posts.js b/ghost/core/core/server/api/endpoints/posts.js
@@ -11,7 +11,8 @@ const allowedIncludes = [
     'newsletter',
     'count.signups',
     'count.paid_conversions',
-    'count.clicks'
+    'count.clicks',
+    'sentiment'
 ];
 const unsafeAttrs = ['status', 'authors', 'visibility'];
 

diff --git a/ghost/core/core/server/api/endpoints/utils/serializers/input/posts.js b/ghost/core/core/server/api/endpoints/utils/serializers/input/posts.js
@@ -16,7 +16,26 @@ function removeSourceFormats(frame) {
     }
 }
 
+/**
+ * Map names of relations to the internal names
+ */
+function mapWithRelated(frame) {
+    if (frame.options.withRelated) {
+        // Map sentiment to count.sentiment
+        if (labs.isSet('audienceFeedback')) {
+            frame.options.withRelated = frame.options.withRelated.map((relation) => {
+                return relation === 'sentiment' ? 'count.sentiment' : relation;
+            });
+        }
+        return;
+    }
+}
+
 function defaultRelations(frame) {
+    // Apply same mapping as content API
+    mapWithRelated(frame);
+
+    // Addditional defaults for admin API
     if (frame.options.withRelated) {
         return;
     }
@@ -111,6 +130,7 @@ module.exports = {
 
             setDefaultOrder(frame);
             forceVisibilityColumn(frame);
+            mapWithRelated(frame);
         }
 
         if (!localUtils.isContentAPI(frame)) {

diff --git a/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/posts.js b/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/posts.js
@@ -119,8 +119,21 @@ module.exports = async (model, frame, options = {}) => {
         );
     }
 
-    if (jsonModel.count && !jsonModel.count.sentiment) {
-        jsonModel.count.sentiment = 0;
+    // The sentiment has been loaded as a count relation in count.sentiment. But externally in the API we use just 'sentiment' instead of count.sentiment
+    // This part moves count.sentiment to just 'sentiment' when it has been loaded
+    if (frame.options.withRelated && frame.options.withRelated.includes('count.sentiment')) {
+        if (!jsonModel.count) {
+            jsonModel.sentiment = 0;
+        } else {
+            jsonModel.sentiment = jsonModel.count.sentiment ?? 0;
+
+            // Delete it from the original location
+            delete jsonModel.count.sentiment;
+
+            if (Object.keys(jsonModel.count).length === 0) {
+                delete jsonModel.count;
+            }
+        }
     }
 
     if (jsonModel.count && !jsonModel.count.positive_feedback) {

diff --git a/ghost/core/core/server/models/post.js b/ghost/core/core/server/models/post.js
@@ -236,6 +236,17 @@ Post = ghostBookshelf.Model.extend({
     },
 
     orderRawQuery: function orderRawQuery(field, direction, withRelated) {
+        if (field === 'sentiment') {
+            if (withRelated.includes('count.sentiment')) {
+                // Internally sentiment can be included via the count.sentiment relation. We can do a quick optimisation of the query in that case.
+                return {
+                    orderByRaw: `count__sentiment ${direction}`
+                };
+            }
+            return {
+                orderByRaw: `(select AVG(score) from \`members_feedback\` where posts.id = members_feedback.post_id) ${direction}`
+            };
+        }
         if (field === 'email.open_rate' && withRelated && withRelated.indexOf('email') > -1) {
             return {
                 // *1.0 is needed on one of the columns to prevent sqlite from
@@ -1377,7 +1388,7 @@ Post = ghostBookshelf.Model.extend({
             },
             sentiment(modelOrCollection) {
                 modelOrCollection.query('columns', 'posts.*', (qb) => {
-                    qb.select(qb.client.raw('ROUND(AVG(score) * 100)'))
+                    qb.select(qb.client.raw('COALESCE(ROUND(AVG(score) * 100), 0)'))
                         .from('members_feedback')
                         .whereRaw('posts.id = members_feedback.post_id')
                         .as('count__sentiment');

diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/posts.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/posts.test.js.snap
@@ -23,7 +23,6 @@ Object {
         "clicks": 0,
         "conversions": 0,
         "positive_feedback": 0,
-        "sentiment": 0,
       },
       "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
       "custom_excerpt": null,
@@ -49,6 +48,7 @@ Object {
       "primary_author": Any<Object>,
       "primary_tag": Any<Object>,
       "published_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
+      "sentiment": 0,
       "slug": "scheduled-post",
       "status": "scheduled",
       "tags": Any<Array>,
@@ -72,7 +72,6 @@ Object {
         "clicks": 0,
         "conversions": 0,
         "positive_feedback": 0,
-        "sentiment": 0,
       },
       "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
       "custom_excerpt": null,
@@ -100,6 +99,7 @@ Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac tu
       "primary_author": Any<Object>,
       "primary_tag": Any<Object>,
       "published_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
+      "sentiment": 0,
       "slug": "unfinished",
       "status": "draft",
       "tags": Any<Array>,
@@ -152,7 +152,6 @@ Object {
         "clicks": 0,
         "conversions": 0,
         "positive_feedback": 0,
-        "sentiment": 0,
       },
       "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
       "custom_excerpt": null,
@@ -182,6 +181,7 @@ Object {
       "primary_tag": Any<Object>,
       "published_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
       "reading_time": 0,
+      "sentiment": 0,
       "slug": "scheduled-post",
       "status": "scheduled",
       "tags": Any<Array>,
@@ -205,7 +205,6 @@ Object {
         "clicks": 0,
         "conversions": 0,
         "positive_feedback": 0,
-        "sentiment": 0,
       },
       "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
       "custom_excerpt": null,
@@ -255,6 +254,7 @@ Header Level 3
       "primary_tag": Any<Object>,
       "published_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
       "reading_time": 1,
+      "sentiment": 0,
       "slug": "unfinished",
       "status": "draft",
       "tags": Any<Array>,
@@ -297,7 +297,6 @@ Object {
         "clicks": 0,
         "conversions": 0,
         "positive_feedback": 0,
-        "sentiment": 0,
       },
       "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
       "custom_excerpt": null,
@@ -326,6 +325,7 @@ Object {
       "primary_tag": Any<Object>,
       "published_at": null,
       "reading_time": 0,
+      "sentiment": 0,
       "slug": "lexical-test",
       "status": "draft",
       "tags": Any<Array>,
@@ -369,7 +369,6 @@ Object {
         "clicks": 0,
         "conversions": 0,
         "positive_feedback": 0,
-        "sentiment": 0,
       },
       "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
       "custom_excerpt": null,
@@ -398,6 +397,7 @@ Object {
       "primary_tag": Any<Object>,
       "published_at": null,
       "reading_time": 0,
+      "sentiment": 0,
       "slug": "mobiledoc-test",
       "status": "draft",
       "tags": Any<Array>,
@@ -542,7 +542,6 @@ Object {
         "clicks": 0,
         "conversions": 0,
         "positive_feedback": 0,
-        "sentiment": 0,
       },
       "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
       "custom_excerpt": null,
@@ -571,6 +570,7 @@ Object {
       "primary_tag": Any<Object>,
       "published_at": null,
       "reading_time": 0,
+      "sentiment": 0,
       "slug": "lexical-update-test",
       "status": "draft",
       "tags": Any<Array>,
@@ -614,7 +614,6 @@ Object {
         "clicks": 0,
         "conversions": 0,
         "positive_feedback": 0,
-        "sentiment": 0,
       },
       "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
       "custom_excerpt": null,
@@ -643,6 +642,7 @@ Object {
       "primary_tag": Any<Object>,
       "published_at": null,
       "reading_time": 0,
+      "sentiment": 0,
       "slug": "lexical-update-test",
       "status": "draft",
       "tags": Any<Array>,
@@ -686,7 +686,6 @@ Object {
         "clicks": 0,
         "conversions": 0,
         "positive_feedback": 0,
-        "sentiment": 0,
       },
       "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
       "custom_excerpt": null,
@@ -715,6 +714,7 @@ Object {
       "primary_tag": Any<Object>,
       "published_at": null,
       "reading_time": 0,
+      "sentiment": 0,
       "slug": "mobiledoc-update-test",
       "status": "draft",
       "tags": Any<Array>,
@@ -758,7 +758,6 @@ Object {
         "clicks": 0,
         "conversions": 0,
         "positive_feedback": 0,
-        "sentiment": 0,
       },
       "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
       "custom_excerpt": null,
@@ -787,6 +786,7 @@ Object {
       "primary_tag": Any<Object>,
       "published_at": null,
       "reading_time": 0,
+      "sentiment": 0,
       "slug": "mobiledoc-update-test",
       "status": "draft",
       "tags": Any<Array>,

diff --git a/ghost/core/test/e2e-api/admin/posts-legacy.test.js b/ghost/core/test/e2e-api/admin/posts-legacy.test.js
@@ -112,7 +112,7 @@ describe('Posts API', function () {
             jsonResponse.posts[0],
             'post',
             null,
-            ['authors', 'primary_author', 'email', 'tiers', 'newsletter', 'count']
+            ['authors', 'primary_author', 'email', 'tiers', 'newsletter', 'count', 'sentiment']
         );
 
         localUtils.API.checkResponse(jsonResponse.meta.pagination, 'pagination');
@@ -233,7 +233,7 @@ describe('Posts API', function () {
         should.exist(jsonResponse);
         should.exist(jsonResponse.posts);
 
-        localUtils.API.checkResponse(jsonResponse.posts[0], 'post', null, ['count']);
+        localUtils.API.checkResponse(jsonResponse.posts[0], 'post', null, ['count', 'sentiment']);
 
         jsonResponse.posts[0].authors[0].should.be.an.Object();
         localUtils.API.checkResponse(jsonResponse.posts[0].authors[0], 'user');

diff --git a/ghost/core/test/e2e-api/admin/utils.js b/ghost/core/test/e2e-api/admin/utils.js
@@ -86,7 +86,8 @@ const expectedProperties = {
         'email_only',
         'tiers',
         'newsletter',
-        'count'
+        'count',
+        'sentiment'
     ],
 
     page: [

diff --git a/ghost/core/test/e2e-webhooks/__snapshots__/pages.test.js.snap b/ghost/core/test/e2e-webhooks/__snapshots__/pages.test.js.snap
@@ -205,7 +205,6 @@ Object {
       "count": Object {
         "paid_conversions": 0,
         "positive_feedback": 0,
-        "sentiment": 0,
         "signups": 0,
       },
       "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,

diff --git a/ghost/core/test/regression/api/admin/utils.js b/ghost/core/test/regression/api/admin/utils.js
@@ -66,7 +66,8 @@ const expectedProperties = {
         'email_only',
         'tiers',
         'newsletter',
-        'count'
+        'count',
+        'sentiment'
     ],
     user: [
         'id',