Skip to content

Commit

Permalink
馃悰 Fixed adding recommendation when oembed fails (#19861)
Browse files Browse the repository at this point in the history
refs https://linear.app/tryghost/issue/ENG-750

- when adding a recommendation, we fetch the recommended site's metadata
- before this change, if the metadata fetch failed for some reason, we'd show an error and block the recommendation from being added
- after this change, we use fallback values if the metadata fails to fetch, instead of blocking the recommendation from being added. We use the site domain as the title and leave the rest empty (no favicon, no description)
- this change also means we are not checking whether a site exists or not for the publisher anymore. It鈥檚 then up to the publisher to make sure they don鈥檛 enter broken URLs
  • Loading branch information
sagzy committed Mar 14, 2024
1 parent 04c9bf0 commit 7a40ab5
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 1 deletion.
Expand Up @@ -2217,6 +2217,39 @@ Object {
}
`;
exports[`Recommendations Admin API check Returns nullified values if site fails to fetch 1: [body] 1`] = `
Object {
"recommendations": Array [
Object {
"created_at": null,
"description": null,
"excerpt": null,
"favicon": null,
"featured_image": null,
"id": null,
"one_click_subscribe": false,
"title": null,
"updated_at": null,
"url": "https://dogpictures.com/",
},
],
}
`;
exports[`Recommendations Admin API check Returns nullified values if site fails to fetch 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": "214",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Accept-Version, Origin, Accept-Encoding",
"x-cache-invalidate": "/*",
"x-powered-by": "Express",
}
`;
exports[`Recommendations Admin API delete Can delete recommendation 1: [body] 1`] = `Object {}`;
exports[`Recommendations Admin API delete Can delete recommendation 2: [headers] 1`] = `
Expand Down
27 changes: 27 additions & 0 deletions ghost/core/test/e2e-api/admin/recommendations.test.js
Expand Up @@ -703,6 +703,33 @@ describe('Recommendations Admin API', function () {
assert.equal(body.recommendations[0].favicon, 'https://dogpictures.com/favicon.ico');
assert.equal(body.recommendations[0].one_click_subscribe, true);
});

it('Returns nullified values if site fails to fetch', async function () {
nock('https://dogpictures.com')
.get('/')
.reply(404);

const {body} = await agent.post('recommendations/check/')
.body({
recommendations: [{
url: 'https://dogpictures.com'
}]
})
.expectStatus(200)
.matchHeaderSnapshot({
'content-version': anyContentVersion,
etag: anyEtag
})
.matchBodySnapshot({});

assert.equal(body.recommendations[0].title, null);
assert.equal(body.recommendations[0].url, 'https://dogpictures.com/');
assert.equal(body.recommendations[0].description, null);
assert.equal(body.recommendations[0].excerpt, null);
assert.equal(body.recommendations[0].featured_image, null);
assert.equal(body.recommendations[0].favicon, null);
assert.equal(body.recommendations[0].one_click_subscribe, false);
});
});

describe('delete', function () {
Expand Down
1 change: 1 addition & 0 deletions ghost/recommendations/src/RecommendationMetadataService.ts
Expand Up @@ -114,6 +114,7 @@ export class RecommendationMetadataService {

// Use the oembed service to fetch metadata
const oembed = await this.#oembedService.fetchOembedDataFromUrl(url.toString(), 'mention');

return {
title: oembed?.metadata?.title || null,
excerpt: oembed?.metadata?.description || null,
Expand Down
17 changes: 16 additions & 1 deletion ghost/recommendations/src/RecommendationService.ts
Expand Up @@ -150,7 +150,22 @@ export class RecommendationService {
return existing.plain;
}

const metadata = await this.recommendationMetadataService.fetch(url);
let metadata;
try {
metadata = await this.recommendationMetadataService.fetch(url);
} catch (e) {
logging.error('[Recommendations] Failed to fetch metadata for url ' + url, e);

return {
url: url,
title: undefined,
excerpt: undefined,
featuredImage: undefined,
favicon: undefined,
oneClickSubscribe: false
};
}

return {
url: url,
title: metadata.title ?? undefined,
Expand Down
14 changes: 14 additions & 0 deletions ghost/recommendations/test/RecommendationService.test.ts
Expand Up @@ -280,6 +280,20 @@ describe('RecommendationService', function () {
url: new URL('http://localhost/newone')
});
});

it('Returns undefined recommendation metadata if metadata fails to fetch', async function () {
fetchMetadataStub.rejects(new Error('Metadata failed to fetch'));

const response = await service.checkRecommendation(new URL('http://localhost/newone'));
assert.deepEqual(response, {
title: undefined,
excerpt: undefined,
featuredImage: undefined,
favicon: undefined,
oneClickSubscribe: false,
url: new URL('http://localhost/newone')
});
});
});

describe('updateRecommendationsEnabledSetting', function () {
Expand Down

0 comments on commit 7a40ab5

Please sign in to comment.