🐛 Fixed newsletters' header_image saved as absolute url (#14690)

refs https://github.com/TryGhost/Team/issues/1579

- When writing to the database, the header_image is tranformed to the transformReady path
- When reading from the database, the transformReady path is transformed to an absolute path
- Includes a test when adding a newsletter

Migration:
- Updates all newsletter who have a header_image to make sure it is saved in transform ready format
- Down operation is required to work with the old model logic and transforms it back to an absolute format

core/server/data/migrations/versions/4.47/2022-05-04-10-03-transform-newsletter-header-image.js

@@ -0,0 +1,26 @@
+const logging = require('@tryghost/logging');
+const urlUtils = require('../../../../../shared/url-utils');
+const {createTransactionalMigration} = require('../../utils');
+
+module.exports = createTransactionalMigration(
+    async function up(knex) {
+        const newsletters = await knex.select('id', 'header_image').from('newsletters').whereNotNull('header_image');
+
+        logging.info(`Transforming header_image to transformReady format for ${newsletters.length} newsletters.`);
+
+        // eslint-disable-next-line no-restricted-syntax
+        for (const newsletter of newsletters) {
+            await knex('newsletters').update('header_image', urlUtils.toTransformReady(newsletter.header_image)).where('id', newsletter.id);
+        }
+    },
+    async function down(knex) {
+        const newsletters = await knex.select('id', 'header_image').from('newsletters').whereNotNull('header_image');
+
+        logging.info(`Transforming header_image to absolute format for ${newsletters.length} newsletters.`);
+
+        // eslint-disable-next-line no-restricted-syntax
+        for (const newsletter of newsletters) {
+            await knex('newsletters').update('header_image', urlUtils.transformReadyToAbsolute(newsletter.header_image)).where('id', newsletter.id);
+        }
+    }
+);

core/server/models/newsletter.js

@@ -1,6 +1,7 @@
 const ghostBookshelf = require('./base');
 const ObjectID = require('bson-objectid');
 const uuid = require('uuid');
+const urlUtils = require('../../shared/url-utils');
 
 const Newsletter = ghostBookshelf.Model.extend({
     tableName: 'newsletters',
@@ -74,6 +75,28 @@ const Newsletter = ghostBookshelf.Model.extend({
         }
 
         return query;
+    },
+
+    formatOnWrite(attrs) {
+        ['header_image'].forEach((attr) => {
+            if (attrs[attr]) {
+                attrs[attr] = urlUtils.toTransformReady(attrs[attr]);
+            }
+        });
+
+        return attrs;
+    },
+
+    parse() {
+        const attrs = ghostBookshelf.Model.prototype.parse.apply(this, arguments);
+
+        ['header_image'].forEach((attr) => {
+            if (attrs[attr]) {
+                attrs[attr] = urlUtils.transformReadyToAbsolute(attrs[attr]);
+            }
+        });
+
+        return attrs;
     }
 }, {
     orderDefaultRaw: function () {

test/e2e-api/admin/__snapshots__/newsletters.test.js.snap

@@ -215,7 +215,7 @@ Object {
       "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
       "description": null,
       "footer_content": null,
-      "header_image": null,
+      "header_image": "http://127.0.0.1:2369/content/images/2022/05/cover-image.jpg",
       "id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
       "name": "My test newsletter",
       "sender_email": null,
@@ -244,7 +244,7 @@ exports[`Newsletters API Can add a newsletter 2: [headers] 1`] = `
 Object {
   "access-control-allow-origin": "http://127.0.0.1:2369",
   "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
-  "content-length": "652",
+  "content-length": "710",
   "content-type": "application/json; charset=utf-8",
   "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
   "location": StringMatching /https\\?:\\\\/\\\\/\\.\\*\\?\\\\/newsletters\\\\/\\[a-f0-9\\]\\{24\\}\\\\//,

test/e2e-api/admin/newsletters.test.js

@@ -4,6 +4,10 @@ const {agentProvider, mockManager, fixtureManager, matchers} = require('../../ut
 const {anyEtag, anyObjectId, anyUuid, anyISODateTime, anyLocationFor} = matchers;
 const configUtils = require('../../utils/configUtils');
 const uuid = require('uuid');
+const urlUtils = require('../../../core/shared/url-utils');
+const db = require('../../../core/server/data/db');
+const knex = db.knex;
+require('should');
 
 const assert = require('assert');
 
@@ -98,6 +102,10 @@ describe('Newsletters API', function () {
     });
 
     it('Can add a newsletter', async function () {
+        const siteUrl = urlUtils.getSiteUrl();
+        const relativePath = 'content/images/2022/05/cover-image.jpg';
+        const absolutePath = siteUrl + relativePath;
+        const transformReadyPath = '__GHOST_URL__/' + relativePath;
         const newsletter = {
             uuid: uuid.v4(),
             name: 'My test newsletter',
@@ -111,20 +119,31 @@ describe('Newsletters API', function () {
             show_header_icon: true,
             show_header_title: true,
             show_badge: true,
-            sort_order: 0
+            sort_order: 0,
+            header_image: absolutePath
         };
 
-        await agent
+        const {body: body2} = await agent
             .post(`newsletters/`)
             .body({newsletters: [newsletter]})
             .expectStatus(201)
             .matchBodySnapshot({
                 newsletters: [newsletterSnapshot]
             })
+            .expect(({body}) => {
+                // Should still be absolute
+                body.newsletters[0].header_image.should.equal(absolutePath);
+            })
             .matchHeaderSnapshot({
                 etag: anyEtag,
                 location: anyLocationFor('newsletters')
             });
+
+        const id = body2.newsletters[0].id;
+
+        // Check with a database query if the header_image is saved correctly with a 'transformReady' path
+        const [header_image] = await knex('newsletters').where('id', id).pluck('header_image');
+        header_image.should.equal(transformReadyPath);
     });
 
     it('Can add multiple newsletters', async function () {
@@ -341,7 +360,6 @@ describe('Newsletters API', function () {
     });
 
     it('Can add a newsletter - with custom sender_email and subscribe existing members', async function () {
-        const db = require('../../../core/server/data/db');
         if (DatabaseInfo.isSQLite(db.knex)) {
             return;
         }