From 1e6f4ba3408f23f30f6e0d575bf17dca37af2f06 Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Thu, 21 Mar 2019 19:08:38 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20Admin=20API=20v2=20wasn'?= =?UTF-8?q?t=20returning=20preview=20url?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit no issue - Admin API v2 returned /404/, see comment in code base: /** * CASE: admin api should serve preview urls * * @NOTE * The url service has no clue of the draft/scheduled concept. It only generates urls for published resources. * Adding a hardcoded fallback into the url service feels wrong IMO. * * Imagine the site won't be part of core and core does not serve urls anymore. * Core needs to offer a preview API, which returns draft posts. * That means the url is no longer /p/:uuid, it's e.g. GET /api/v2/content/preview/:uuid/. * /p/ is a concept of the site, not of core. * * The site is not aware of existing drafts. It won't be able to get the uuid. * * Needs further discussion. */ --- .../utils/serializers/output/utils/mapper.js | 2 +- .../v2/utils/serializers/output/utils/url.js | 33 +++++++++++++++++-- core/test/acceptance/old/admin/pages_spec.js | 2 +- core/test/acceptance/old/admin/posts_spec.js | 3 +- .../serializers/output/utils/url_spec.js | 2 +- 5 files changed, 35 insertions(+), 7 deletions(-) diff --git a/core/server/api/v2/utils/serializers/output/utils/mapper.js b/core/server/api/v2/utils/serializers/output/utils/mapper.js index 2d0075fd19..77aab97d44 100644 --- a/core/server/api/v2/utils/serializers/output/utils/mapper.js +++ b/core/server/api/v2/utils/serializers/output/utils/mapper.js @@ -32,7 +32,7 @@ const mapPost = (model, frame) => { const jsonModel = model.toJSON(extendedOptions); - url.forPost(model.id, jsonModel, frame.options); + url.forPost(model.id, jsonModel, frame); if (utils.isContentAPI(frame)) { date.forPost(jsonModel); diff --git a/core/server/api/v2/utils/serializers/output/utils/url.js b/core/server/api/v2/utils/serializers/output/utils/url.js index 5aed4c56c4..ba12e2bd2a 100644 --- a/core/server/api/v2/utils/serializers/output/utils/url.js +++ b/core/server/api/v2/utils/serializers/output/utils/url.js @@ -1,9 +1,36 @@ const _ = require('lodash'); const urlService = require('../../../../../../services/url'); +const localUtils = require('../../../index'); -const forPost = (id, attrs, options) => { +const forPost = (id, attrs, frame) => { attrs.url = urlService.getUrlByResourceId(id, {absolute: true}); + /** + * CASE: admin api should serve preview urls + * + * @NOTE + * The url service has no clue of the draft/scheduled concept. It only generates urls for published resources. + * Adding a hardcoded fallback into the url service feels wrong IMO. + * + * Imagine the site won't be part of core and core does not serve urls anymore. + * Core needs to offer a preview API, which returns draft posts. + * That means the url is no longer /p/:uuid, it's e.g. GET /api/v2/content/preview/:uuid/. + * /p/ is a concept of the site, not of core. + * + * The site is not aware of existing drafts. It won't be able to get the uuid. + * + * Needs further discussion. + */ + if (!localUtils.isContentAPI(frame)) { + if (attrs.status !== 'published' && attrs.url.match(/\/404\//)) { + attrs.url = urlService + .utils + .urlFor({ + relativeUrl: urlService.utils.urlJoin('/p', attrs.uuid, '/') + }, null, true); + } + } + if (attrs.feature_image) { attrs.feature_image = urlService.utils.urlFor('image', {image: attrs.feature_image}, true); } @@ -25,7 +52,7 @@ const forPost = (id, attrs, options) => { assetsOnly: true }; - if (options.absolute_urls) { + if (frame.options.absolute_urls) { urlOptions.assetsOnly = false; } @@ -37,7 +64,7 @@ const forPost = (id, attrs, options) => { ).html(); } - if (options.columns && !options.columns.includes('url')) { + if (frame.options.columns && !frame.options.columns.includes('url')) { delete attrs.url; } diff --git a/core/test/acceptance/old/admin/pages_spec.js b/core/test/acceptance/old/admin/pages_spec.js index fd498643c2..fa783e37f0 100644 --- a/core/test/acceptance/old/admin/pages_spec.js +++ b/core/test/acceptance/old/admin/pages_spec.js @@ -50,7 +50,7 @@ describe('Pages API', function () { _.isBoolean(jsonResponse.pages[0].featured).should.eql(true); // Absolute urls by default - jsonResponse.pages[0].url.should.eql(`${config.get('url')}/404/`); + jsonResponse.pages[0].url.should.match(new RegExp(`${config.get('url')}/p/[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}`)); jsonResponse.pages[1].url.should.eql(`${config.get('url')}/static-page-test/`); done(); diff --git a/core/test/acceptance/old/admin/posts_spec.js b/core/test/acceptance/old/admin/posts_spec.js index 3d256116f5..7904f67aad 100644 --- a/core/test/acceptance/old/admin/posts_spec.js +++ b/core/test/acceptance/old/admin/posts_spec.js @@ -53,7 +53,7 @@ describe('Posts API', function () { jsonResponse.posts[12].slug.should.eql('html-ipsum'); // Absolute urls by default - jsonResponse.posts[0].url.should.eql(`${config.get('url')}/404/`); + jsonResponse.posts[0].url.should.match(new RegExp(`${config.get('url')}/p/[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}`)); jsonResponse.posts[2].url.should.eql(`${config.get('url')}/welcome/`); jsonResponse.posts[11].feature_image.should.eql(`${config.get('url')}/content/images/2018/hey.jpg`); @@ -278,6 +278,7 @@ describe('Posts API', function () { .then((res) => { res.body.posts.length.should.eql(1); localUtils.API.checkResponse(res.body.posts[0], 'post'); + res.body.posts[0].url.should.match(new RegExp(`${config.get('url')}/p/[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}`)); should.not.exist(res.headers['x-cache-invalidate']); return models.Post.findOne({ diff --git a/core/test/unit/api/v2/utils/serializers/output/utils/url_spec.js b/core/test/unit/api/v2/utils/serializers/output/utils/url_spec.js index 16a61fe2b8..cff6fa630f 100644 --- a/core/test/unit/api/v2/utils/serializers/output/utils/url_spec.js +++ b/core/test/unit/api/v2/utils/serializers/output/utils/url_spec.js @@ -30,7 +30,7 @@ describe('Unit: v2/utils/serializers/output/utils/url', () => { feature_image: 'value', })); - urlUtil.forPost(post.id, post, {}); + urlUtil.forPost(post.id, post, {options: {}}); post.hasOwnProperty('url').should.be.true();