TryGhost/Ghost
1
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2024-12-25 11:55:03 +03:00
Code Issues Projects Releases Wiki Activity

Switched AMP to be 'off' by default in all new Ghost instances (#13907)

Browse Source 
refs https://github.com/TryGhost/Team/issues/1189

Support for AMP is slowly in decline, and makes developing new cards trickier,
since AMP no longer has an effect of SEO we're going to disable it by default
as a first step toward moving away from it.

Co-authored-by: Thibaut Patel <thibaut@ghost.org>
This commit is contained in:
Fabien 'egg' O'Carroll 2022-01-14 18:55:48 +02:00 committed by GitHub
parent 8df29f6574
commit 10c214c148
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 191 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
core/server/data/schema/default-settings.json
View File

@ -397,7 +397,7 @@
}, },
"amp": { "amp": {
"amp": { "amp": {
"defaultValue": "true", "defaultValue": "false",
"validations": { "validations": {
"isIn": [["true", "false"]] "isIn": [["true", "false"]]
}, },

2
test/e2e-api/admin/settings.test.js
View File

@ -35,7 +35,7 @@ describe('Settings API', function () {
localUtils.API.checkResponse(jsonResponse, 'settings'); localUtils.API.checkResponse(jsonResponse, 'settings');
JSON.parse(_.find(jsonResponse.settings, {key: 'unsplash'}).value).should.eql(true); JSON.parse(_.find(jsonResponse.settings, {key: 'unsplash'}).value).should.eql(true);
JSON.parse(_.find(jsonResponse.settings, {key: 'amp'}).value).should.eql(true); JSON.parse(_.find(jsonResponse.settings, {key: 'amp'}).value).should.eql(false);
should.not.exist(_.find(jsonResponse.settings, {key: 'permalinks'})); should.not.exist(_.find(jsonResponse.settings, {key: 'permalinks'}));
should.not.exist(_.find(jsonResponse.settings, {key: 'ghost_head'})); should.not.exist(_.find(jsonResponse.settings, {key: 'ghost_head'}));
should.not.exist(_.find(jsonResponse.settings, {key: 'ghost_foot'})); should.not.exist(_.find(jsonResponse.settings, {key: 'ghost_foot'}));

19
test/e2e-frontend/advanced_url_config.test.js
View File

@ -1,8 +1,10 @@
const should = require('should'); const should = require('should');
const sinon = require('sinon');
const supertest = require('supertest'); const supertest = require('supertest');
const testUtils = require('../utils'); const testUtils = require('../utils');
const configUtils = require('../utils/configUtils'); const configUtils = require('../utils/configUtils');
const urlUtils = require('../utils/urlUtils'); const urlUtils = require('../utils/urlUtils');
const settings = require('../../core/shared/settings-cache');
let request; let request;
@ -31,6 +33,10 @@ describe('Advanced URL Configurations', function () {
urlUtils.restore(); urlUtils.restore();
}); });
afterEach(function () {
sinon.restore();
});
it('http://localhost should 404', async function () { it('http://localhost should 404', async function () {
await request.get('/') await request.get('/')
.expect(404); .expect(404);
@ -67,11 +73,22 @@ describe('Advanced URL Configurations', function () {
.expect(404); .expect(404);
}); });
it('/blog/welcome/amp/ should 200', async function () { it('/blog/welcome/amp/ should 200 if amp is enabled', async function () {
sinon.stub(settings, 'get').callsFake(function (key, ...rest) {
if (key === 'amp') {
return true;
}
return settings.get.wrappedMethod.call(settings, key, ...rest);
});
await request.get('/blog/welcome/amp/') await request.get('/blog/welcome/amp/')
.expect(200); .expect(200);
}); });
it('/blog/welcome/amp/ should 301', async function () {
await request.get('/blog/welcome/amp/')
.expect(301);
});
it('/welcome/amp/ should 404', async function () { it('/welcome/amp/ should 404', async function () {
await request.get('/welcome/amp/') await request.get('/welcome/amp/')
.expect(404); .expect(404);

79
test/e2e-frontend/default_routes.test.js
View File

@ -190,52 +190,63 @@ describe('Default Frontend routing', function () {
}); });
describe('AMP post', function () { describe('AMP post', function () {
it('should respond with html for valid url', async function () { describe('AMP Enabled', function () {
await request.get('/welcome/amp/') beforeEach(function () {
.expect('Content-Type', /html/) sinon.stub(settingsCache, 'get').callsFake(function (key, options) {
.expect('Cache-Control', testUtils.cacheRules.public) if (key === 'amp' && !options) {
.expect(200) return true;
.expect(assertCorrectFrontendHeaders) }
.expect((res) => { return origCache.get(key, options);
const $ = cheerio.load(res.text);
$('.post-title').text().should.equal('Start here for a quick overview of everything you need to know');
$('.content .post').length.should.equal(1);
$('.powered').text().should.equal(' Published with Ghost');
$('body.amp-template').length.should.equal(1);
$('article.post').length.should.equal(1);
$('style[amp-custom]').length.should.equal(1);
// This asserts we should have some content (and not [object Promise] !)
$('.post-content p').length.should.be.greaterThan(0);
res.text.should.containEql(':root {--ghost-accent-color: #FF1A75;}');
res.text.should.not.containEql('__GHOST_URL__');
}); });
}); });
it('should respond with html for valid url', async function () {
await request.get('/welcome/amp/')
.expect('Content-Type', /html/)
.expect('Cache-Control', testUtils.cacheRules.public)
.expect(200)
.expect(assertCorrectFrontendHeaders)
.expect((res) => {
const $ = cheerio.load(res.text);
it('should not work with date permalinks', async function () { $('.post-title').text().should.equal('Start here for a quick overview of everything you need to know');
// get today's date
const date = moment().format('YYYY/MM/DD');
await request.get('/' + date + '/welcome/amp/') $('.content .post').length.should.equal(1);
.expect('Cache-Control', testUtils.cacheRules.private) $('.powered').text().should.equal(' Published with Ghost');
.expect(404) $('body.amp-template').length.should.equal(1);
.expect(/Page not found/) $('article.post').length.should.equal(1);
.expect(assertCorrectFrontendHeaders);
$('style[amp-custom]').length.should.equal(1);
// This asserts we should have some content (and not [object Promise] !)
$('.post-content p').length.should.be.greaterThan(0);
res.text.should.containEql(':root {--ghost-accent-color: #FF1A75;}');
res.text.should.not.containEql('__GHOST_URL__');
});
});
it('should not work with date permalinks', async function () {
// get today's date
const date = moment().format('YYYY/MM/DD');
await request.get('/' + date + '/welcome/amp/')
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(404)
.expect(/Page not found/)
.expect(assertCorrectFrontendHeaders);
});
}); });
describe('AMP Disabled', function () { describe('AMP Disabled', function () {
it('/amp/ should redirect to regular post, including any query params', async function () { beforeEach(function () {
sinon.stub(settingsCache, 'get').callsFake(function (key, options) { sinon.stub(settingsCache, 'get').callsFake(function (key, options) {
if (key === 'amp' && !options) { if (key === 'amp' && !options) {
return false; return false;
} }
return origCache.get(key, options); return origCache.get(key, options);
}); });
});
it('/amp/ should redirect to regular post, including any query params', async function () {
await request.get('/welcome/amp/?q=a') await request.get('/welcome/amp/?q=a')
.expect('Location', '/welcome/?q=a') .expect('Location', '/welcome/?q=a')
.expect(301) .expect(301)

46
test/regression/mock-express-style/api-vs-frontend/canary.test.js
View File

@ -66,20 +66,40 @@ describe('Integration - Web - Site canary', function () {
}); });
}); });
it('serve amp', function () { describe('AMP enabled', function () {
const req = { it('serve amp', function () {
secure: true, localUtils.defaultMocks(sinon, {amp: true});
method: 'GET', const req = {
url: '/html-ipsum/amp/', secure: true,
host: 'example.com' method: 'GET',
}; url: '/html-ipsum/amp/',
host: 'example.com'
};
return localUtils.mockExpress.invoke(app, req) return localUtils.mockExpress.invoke(app, req)
.then(function (response) { .then(function (response) {
response.statusCode.should.eql(200); response.statusCode.should.eql(200);
response.template.should.match(/amp\.hbs/); response.template.should.match(/amp\.hbs/);
response.body.should.match(/<h1>HTML Ipsum Presents<\/h1>/); response.body.should.match(/<h1>HTML Ipsum Presents<\/h1>/);
}); });
});
});
describe('AMP disabled', function () {
it('serve amp', function () {
localUtils.defaultMocks(sinon, {amp: false});
const req = {
secure: true,
method: 'GET',
url: '/html-ipsum/amp/',
host: 'example.com'
};
return localUtils.mockExpress.invoke(app, req)
.then(function (response) {
response.statusCode.should.eql(301);
});
});
}); });
it('post not found', function () { it('post not found', function () {

46
test/regression/mock-express-style/api-vs-frontend/v2.test.js
View File

@ -66,20 +66,40 @@ describe('Integration - Web - Site v2', function () {
}); });
}); });
it('serve amp', function () { describe('AMP enabled', function () {
const req = { it('serve amp', function () {
secure: true, localUtils.defaultMocks(sinon, {amp: true});
method: 'GET', const req = {
url: '/html-ipsum/amp/', secure: true,
host: 'example.com' method: 'GET',
}; url: '/html-ipsum/amp/',
host: 'example.com'
};
return localUtils.mockExpress.invoke(app, req) return localUtils.mockExpress.invoke(app, req)
.then(function (response) { .then(function (response) {
response.statusCode.should.eql(200); response.statusCode.should.eql(200);
response.template.should.match(/amp\.hbs/); response.template.should.match(/amp\.hbs/);
response.body.should.match(/<h1>HTML Ipsum Presents<\/h1>/); response.body.should.match(/<h1>HTML Ipsum Presents<\/h1>/);
}); });
});
});
describe('AMP disabled', function () {
it('serve amp', function () {
localUtils.defaultMocks(sinon, {amp: false});
const req = {
secure: true,
method: 'GET',
url: '/html-ipsum/amp/',
host: 'example.com'
};
return localUtils.mockExpress.invoke(app, req)
.then(function (response) {
response.statusCode.should.eql(301);
});
});
}); });
it('post not found', function () { it('post not found', function () {

46
test/regression/mock-express-style/api-vs-frontend/v3.test.js
View File

@ -66,20 +66,40 @@ describe('Integration - Web - Site v3', function () {
}); });
}); });
it('serve amp', function () { describe('AMP enabled', function () {
const req = { it('serve amp', function () {
secure: true, localUtils.defaultMocks(sinon, {amp: true});
method: 'GET', const req = {
url: '/html-ipsum/amp/', secure: true,
host: 'example.com' method: 'GET',
}; url: '/html-ipsum/amp/',
host: 'example.com'
};
return localUtils.mockExpress.invoke(app, req) return localUtils.mockExpress.invoke(app, req)
.then(function (response) { .then(function (response) {
response.statusCode.should.eql(200); response.statusCode.should.eql(200);
response.template.should.match(/amp\.hbs/); response.template.should.match(/amp\.hbs/);
response.body.should.match(/<h1>HTML Ipsum Presents<\/h1>/); response.body.should.match(/<h1>HTML Ipsum Presents<\/h1>/);
}); });
});
});
describe('AMP disabled', function () {
it('serve amp', function () {
localUtils.defaultMocks(sinon, {amp: false});
const req = {
secure: true,
method: 'GET',
url: '/html-ipsum/amp/',
host: 'example.com'
};
return localUtils.mockExpress.invoke(app, req)
.then(function (response) {
response.statusCode.should.eql(301);
});
});
}); });
it('post not found', function () { it('post not found', function () {

34
test/regression/site/frontend.test.js
View File

@ -11,6 +11,7 @@ const _ = require('lodash');
const testUtils = require('../../utils'); const testUtils = require('../../utils');
const configUtils = require('../../utils/configUtils'); const configUtils = require('../../utils/configUtils');
const config = require('../../../core/shared/config'); const config = require('../../../core/shared/config');
const settingsCache = require('../../../core/shared/settings-cache');
let request; let request;
describe('Frontend Routing', function () { describe('Frontend Routing', function () {
@ -218,14 +219,31 @@ describe('Frontend Routing', function () {
}); });
describe('amp', function () { describe('amp', function () {
it('should 404 for amp parameter', function (done) { describe('amp enabled', function (){
// NOTE: only post pages are supported so the router doesn't have a way to distinguish if beforeEach(function () {
// the request was done after AMP 'Page' or 'Post' sinon.stub(settingsCache, 'get').withArgs('amp').returns(true);
request.get('/static-page-test/amp/') });
.expect('Cache-Control', testUtils.cacheRules.private) it('should 404 for amp parameter', function (done) {
.expect(404) // NOTE: only post pages are supported so the router doesn't have a way to distinguish if
.expect(/Post not found/) // the request was done after AMP 'Page' or 'Post'
.end(doEnd(done)); request.get('/static-page-test/amp/')
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(404)
.expect(/Post not found/)
.end(doEnd(done));
});
});
describe('amp disabled', function (){
beforeEach(function () {
sinon.stub(settingsCache, 'get').withArgs('amp').returns(false);
});
it('should 301 for amp parameter', function (done) {
// NOTE: only post pages are supported so the router doesn't have a way to distinguish if
// the request was done after AMP 'Page' or 'Post'
request.get('/static-page-test/amp/')
.expect(301)
.end(doEnd(done));
});
}); });
}); });
}); });

2
test/unit/server/data/schema/integrity.test.js
View File

@ -37,7 +37,7 @@ describe('DB version integrity', function () {
// Only these variables should need updating // Only these variables should need updating
const currentSchemaHash = 'e649797a5de92d417744f6f2623c79cf'; const currentSchemaHash = 'e649797a5de92d417744f6f2623c79cf';
const currentFixturesHash = '07d4b0c4cf159b34344a6b5e88c74e9f'; const currentFixturesHash = '07d4b0c4cf159b34344a6b5e88c74e9f';
const currentSettingsHash = 'b06316ebbf62381158e1c46c97c0b77a'; const currentSettingsHash = 'd73b63e33153c9256bca42ebfd376779';
const currentRoutesHash = '3d180d52c663d173a6be791ef411ed01'; const currentRoutesHash = '3d180d52c663d173a6be791ef411ed01';
// If this test is failing, then it is likely a change has been made that requires a DB version bump, // If this test is failing, then it is likely a change has been made that requires a DB version bump,