From b3350022692bd64d207436e80df1a8cd20cb9b6c Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Wed, 11 Jul 2018 00:14:35 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20Disqus=20comments=20on?= =?UTF-8?q?=20post=20preview=20pages?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes #9727 - this is a short term fix to proof that Disqus comments on preview pages no longer appear on other threads - this is not a full solution to the problem - the private API still returns /404/, which is right now inconsistent, but not critical in any way - the url helper will now output the post preview url if you serve a draft/scheduled post - this should register unique page urls at Disqus and ensure uniquness for threads - i still don't understand why the cross posting happens at all, because we also pass an unique identifier to Disqus (the post ID) - it could be that comments, which are added on the preview page, won't appear on the published urls, because the published url !== preview url. I wasn't able to figure this out via testing or reading their docs --- core/server/data/meta/url.js | 24 ++++++++++++- core/test/unit/data/meta/url_spec.js | 42 +++++++++++++++++++++++ core/test/unit/services/url/utils_spec.js | 10 ++++++ 3 files changed, 75 insertions(+), 1 deletion(-) diff --git a/core/server/data/meta/url.js b/core/server/data/meta/url.js index a9a1d18384..0362e4d798 100644 --- a/core/server/data/meta/url.js +++ b/core/server/data/meta/url.js @@ -12,7 +12,29 @@ function sanitizeAmpUrl(url) { } function getUrl(data, absolute) { - if (schema.isPost(data) || schema.isTag(data) || schema.isUser(data)) { + if (schema.isPost(data)) { + /** + * @NOTE + * + * We return the post preview url if you are making use of the `{{url}}` helper and the post is not published. + * If we don't do it, we can break Disqus a bit. See https://github.com/TryGhost/Ghost/issues/9727. + * + * This short term fix needs a better solution than this, because this is inconsistent with our private API. The + * private API would still return /404/ for drafts. The public API doesn't serve any drafts - nothing we have to + * worry about. We first would like to see if this resolves the Disqus bug when commenting on preview pages. + * + * A long term solution should be part of the final version of Dynamic Routing. + */ + if (data.status !== 'published' && urlService.getUrlByResourceId(data.id) === '/404/') { + return urlService + .utils + .urlFor({relativeUrl: urlService.utils.urlJoin('/p', data.uuid, '/'), secure: data.secure}, null, absolute); + } + + return urlService.getUrlByResourceId(data.id, {secure: data.secure, absolute: absolute, withSubdirectory: true}); + } + + if (schema.isTag(data) || schema.isUser(data)) { return urlService.getUrlByResourceId(data.id, {secure: data.secure, absolute: absolute, withSubdirectory: true}); } diff --git a/core/test/unit/data/meta/url_spec.js b/core/test/unit/data/meta/url_spec.js index 7acad5abdd..ce84f80673 100644 --- a/core/test/unit/data/meta/url_spec.js +++ b/core/test/unit/data/meta/url_spec.js @@ -24,6 +24,48 @@ describe('getUrl', function () { getUrl(post).should.eql('post url'); }); + describe('preview url: drafts/scheduled posts', function () { + it('not absolute, not secure', function () { + const post = testUtils.DataGenerator.forKnex.createPost({status: 'draft'}); + urlService.getUrlByResourceId.withArgs(post.id).returns('/404/'); + urlService.utils.urlFor.withArgs({relativeUrl: '/p/' + post.uuid + '/', secure: undefined}, null, undefined).returns('relative'); + let url = getUrl(post); + + urlService.getUrlByResourceId.calledOnce.should.be.true(); + urlService.utils.urlFor.withArgs({relativeUrl: '/p/' + post.uuid + '/', secure: undefined}, null, undefined) + .calledOnce.should.be.true(); + + url.should.eql('relative'); + }); + + it('absolute, not secure', function () { + const post = testUtils.DataGenerator.forKnex.createPost({status: 'draft'}); + urlService.getUrlByResourceId.withArgs(post.id).returns('/404/'); + urlService.utils.urlFor.withArgs({relativeUrl: '/p/' + post.uuid + '/', secure: undefined}, null, true).returns('absolute'); + let url = getUrl(post, true); + + urlService.getUrlByResourceId.calledOnce.should.be.true(); + urlService.utils.urlFor.withArgs({relativeUrl: '/p/' + post.uuid + '/', secure: undefined}, null, true) + .calledOnce.should.be.true(); + + url.should.eql('absolute'); + }); + + it('absolute, secure', function () { + const post = testUtils.DataGenerator.forKnex.createPost({status: 'draft'}); + post.secure = true; + urlService.getUrlByResourceId.withArgs(post.id).returns('/404/'); + urlService.utils.urlFor.withArgs({relativeUrl: '/p/' + post.uuid + '/', secure: true}, null, true).returns('absolute secure'); + let url = getUrl(post, true); + + urlService.getUrlByResourceId.calledOnce.should.be.true(); + urlService.utils.urlFor.withArgs({relativeUrl: '/p/' + post.uuid + '/', secure: true}, null, true) + .calledOnce.should.be.true(); + + url.should.eql('absolute secure'); + }); + }); + it('should return absolute url for a post', function () { const post = testUtils.DataGenerator.forKnex.createPost(); diff --git a/core/test/unit/services/url/utils_spec.js b/core/test/unit/services/url/utils_spec.js index a6e6581308..efa3e71424 100644 --- a/core/test/unit/services/url/utils_spec.js +++ b/core/test/unit/services/url/utils_spec.js @@ -210,6 +210,16 @@ describe('Url', function () { configUtils.set({url: 'http://my-ghost-blog.com/blog'}); urlService.utils.urlFor(testContext).should.equal('/blog/about/'); urlService.utils.urlFor(testContext, true).should.equal('http://my-ghost-blog.com/blog/about/'); + + testContext.secure = true; + urlService.utils.urlFor(testContext, true).should.equal('https://my-ghost-blog.com/blog/about/'); + + testContext.secure = false; + urlService.utils.urlFor(testContext, true).should.equal('http://my-ghost-blog.com/blog/about/'); + + testContext.secure = false; + configUtils.set({url: 'https://my-ghost-blog.com'}); + urlService.utils.urlFor(testContext, true).should.equal('https://my-ghost-blog.com/about/'); }); it('should deduplicate subdirectories in paths', function () {