0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-03-11 02:12:21 -05:00

Added uniqueness validation for the recommendation URL (#18163)

closes https://github.com/TryGhost/Product/issues/3818

- in Admin, when adding a recommendation, the URL is compared against all existing ones. If the URL is already recommended, the publisher is shown an error: "A recommendation with this URL already exists.". Protocol, www, query parameters and hash fragments are ignored during the URL comparison.
- on the backend, there is another uniqueness validation for the recommendation URL. This check is redundant when adding a recommendation from Admin, but helps to keep data integrity when recommendations are added through other paths (e.g. via the API)
This commit is contained in:
Sag 2023-09-15 15:14:47 +02:00 committed by GitHub
parent caab89ff4d
commit 6e68c43f78
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 268 additions and 88 deletions

View file

@ -30,7 +30,7 @@
"build": "tsc && vite build", "build": "tsc && vite build",
"lint": "yarn run lint:js", "lint": "yarn run lint:js",
"lint:js": "eslint --ext .js,.ts,.cjs,.tsx --cache src test", "lint:js": "eslint --ext .js,.ts,.cjs,.tsx --cache src test",
"test:unit": "yarn build", "test:unit": "yarn build && vitest run",
"test:e2e": "NODE_OPTIONS='--experimental-specifier-resolution=node --no-warnings' VITE_TEST=true playwright test", "test:e2e": "NODE_OPTIONS='--experimental-specifier-resolution=node --no-warnings' VITE_TEST=true playwright test",
"test:slowmo": "TIMEOUT=100000 PLAYWRIGHT_SLOWMO=100 yarn test:e2e --headed", "test:slowmo": "TIMEOUT=100000 PLAYWRIGHT_SLOWMO=100 yarn test:e2e --headed",
"test:e2e:full": "ALL_BROWSERS=1 yarn test:e2e", "test:e2e:full": "ALL_BROWSERS=1 yarn test:e2e",

View file

@ -6,7 +6,8 @@ import React from 'react';
import URLTextField from '../../../../admin-x-ds/global/form/URLTextField'; import URLTextField from '../../../../admin-x-ds/global/form/URLTextField';
import useForm from '../../../../hooks/useForm'; import useForm from '../../../../hooks/useForm';
import useRouting from '../../../../hooks/useRouting'; import useRouting from '../../../../hooks/useRouting';
import {EditOrAddRecommendation} from '../../../../api/recommendations'; import {EditOrAddRecommendation, useBrowseRecommendations} from '../../../../api/recommendations';
import {arePathsEqual, trimSearchAndHash} from '../../../../utils/url';
import {showToast} from '../../../../admin-x-ds/global/Toast'; import {showToast} from '../../../../admin-x-ds/global/Toast';
import {toast} from 'react-hot-toast'; import {toast} from 'react-hot-toast';
import {useExternalGhostSite} from '../../../../api/external-ghost-site'; import {useExternalGhostSite} from '../../../../api/external-ghost-site';
@ -22,6 +23,7 @@ const AddRecommendationModal: React.FC<AddRecommendationModalProps> = ({recommen
const {updateRoute} = useRouting(); const {updateRoute} = useRouting();
const {query: queryOembed} = useGetOembed(); const {query: queryOembed} = useGetOembed();
const {query: queryExternalGhostSite} = useExternalGhostSite(); const {query: queryExternalGhostSite} = useExternalGhostSite();
const {data: {recommendations} = {}} = useBrowseRecommendations();
const {formState, updateForm, handleSave, errors, validate, saveState, clearError} = useForm({ const {formState, updateForm, handleSave, errors, validate, saveState, clearError} = useForm({
initialState: recommendation ?? { initialState: recommendation ?? {
@ -34,28 +36,19 @@ const AddRecommendationModal: React.FC<AddRecommendationModalProps> = ({recommen
one_click_subscribe: false one_click_subscribe: false
}, },
onSave: async () => { onSave: async () => {
let validatedUrl: URL | null = null; let validatedUrl: URL;
try { validatedUrl = new URL(formState.url);
validatedUrl = new URL(formState.url); validatedUrl = trimSearchAndHash(validatedUrl);
} catch (e) {
// Ignore
}
// First check if it s a Ghost site or not // First check if it s a Ghost site or not
let externalGhostSite = validatedUrl && validatedUrl.protocol === 'https:' ? (await queryExternalGhostSite('https://' + validatedUrl.host)) : null; let externalGhostSite = validatedUrl.protocol === 'https:' ? (await queryExternalGhostSite('https://' + validatedUrl.host)) : null;
let defaultTitle = formState.title;
if (!defaultTitle) { // Use the hostname as fallback title
if (validatedUrl) { const defaultTitle = validatedUrl.hostname.replace('www.', '');
defaultTitle = validatedUrl.hostname.replace('www.', '');
} else {
// Ignore
defaultTitle = formState.url;
}
}
const updatedRecommendation = { const updatedRecommendation = {
...formState, ...formState,
title: defaultTitle url: validatedUrl.toString()
}; };
if (externalGhostSite) { if (externalGhostSite) {
@ -65,7 +58,6 @@ const AddRecommendationModal: React.FC<AddRecommendationModalProps> = ({recommen
updatedRecommendation.featured_image = externalGhostSite.site.cover_image?.toString() ?? formState.featured_image ?? null; updatedRecommendation.featured_image = externalGhostSite.site.cover_image?.toString() ?? formState.featured_image ?? null;
updatedRecommendation.favicon = externalGhostSite.site.icon?.toString() ?? externalGhostSite.site.logo?.toString() ?? formState.favicon ?? null; updatedRecommendation.favicon = externalGhostSite.site.icon?.toString() ?? externalGhostSite.site.logo?.toString() ?? formState.favicon ?? null;
updatedRecommendation.one_click_subscribe = externalGhostSite.site.allow_self_signup; updatedRecommendation.one_click_subscribe = externalGhostSite.site.allow_self_signup;
updatedRecommendation.url = externalGhostSite.site.url.toString();
} else { } else {
// For non-Ghost sites, we use the Oemebd API to fetch metadata // For non-Ghost sites, we use the Oemebd API to fetch metadata
const oembed = await queryOembed({ const oembed = await queryOembed({
@ -95,10 +87,15 @@ const AddRecommendationModal: React.FC<AddRecommendationModalProps> = ({recommen
// Check domain includes a dot // Check domain includes a dot
if (!u.hostname.includes('.')) { if (!u.hostname.includes('.')) {
newErrors.url = 'Please enter a valid URL'; newErrors.url = 'Please enter a valid URL.';
}
// Check that it doesn't exist already
if (recommendations?.find(r => arePathsEqual(r.url, u.toString()))) {
newErrors.url = 'A recommendation with this URL already exists.';
} }
} catch (e) { } catch (e) {
newErrors.url = 'Please enter a valid URL'; newErrors.url = 'Please enter a valid URL.';
} }
return newErrors; return newErrors;
@ -133,18 +130,11 @@ const AddRecommendationModal: React.FC<AddRecommendationModalProps> = ({recommen
toast.remove(); toast.remove();
try { try {
if (await handleSave({force: true})) { await handleSave({force: true});
// Already handled
} else {
showToast({
type: 'pageError',
message: 'One or more fields have errors, please double check that you\'ve filled all mandatory fields.'
});
}
} catch (e) { } catch (e) {
showToast({ showToast({
type: 'pageError', type: 'pageError',
message: 'Something went wrong while checking this URL, please try again' message: 'Something went wrong while checking this URL, please try again.'
}); });
} }
}} }}

View file

@ -101,12 +101,12 @@ const AddRecommendationModalConfirm: React.FC<AddRecommendationModalProps> = ({r
} }
dismissAllToasts(); dismissAllToasts();
if (await handleSave({force: true})) { try {
// Already handled await handleSave({force: true});
} else { } catch (e) {
showToast({ showToast({
type: 'pageError', type: 'pageError',
message: 'One or more fields have errors, please double check that you\'ve filled all mandatory fields.' message: 'Something went wrong when adding this recommendation, please try again.'
}); });
} }
}} }}

View file

@ -0,0 +1,38 @@
export function trimSearch(url: URL) {
url.search = '';
return url;
}
export function trimHash(url: URL) {
url.hash = '';
return url;
}
export function trimSearchAndHash(url: URL) {
url.search = '';
url.hash = '';
return url;
}
/* Compare two URLs based on their hostname and pathname.
* Query params, hash fragements, protocol and www are ignored.
*
* Example:
* - https://a.com, http://a.com, https://www.a.com, https://a.com?param1=value, https://a.com/#segment-1 are all considered equal
* - but, https://a.com/path-1 and https://a.com/path-2 are not
*/
export function arePathsEqual(urlStr1: string, urlStr2: string) {
let url1, url2;
try {
url1 = new URL(urlStr1);
url2 = new URL(urlStr2);
} catch (e) {
return false;
}
return (
url1.hostname.replace('www.', '') === url2.hostname.replace('www.', '') &&
url1.pathname === url2.pathname
);
}

View file

@ -0,0 +1,94 @@
import * as assert from 'assert/strict';
import {arePathsEqual, trimHash, trimSearch, trimSearchAndHash} from '../../../src/utils/url';
describe('trimSearch', function () {
it('removes the query parameters from a URL', function () {
const url = 'https://example.com/?foo=bar&baz=qux';
const parsedUrl = new URL(url);
assert.equal(trimSearch(parsedUrl).toString(), 'https://example.com/');
});
});
describe('trimHash', function () {
it('removes the hash fragment from a URL', function () {
const url = 'https://example.com/path#section-1';
const parsedUrl = new URL(url);
assert.equal(trimHash(parsedUrl).toString(), 'https://example.com/path');
});
});
describe('trimSearchAndHash', function () {
it('removes the hash fragment from a URL', function () {
const url = 'https://example.com/path#section-1?foo=bar&baz=qux';
const parsedUrl = new URL(url);
assert.equal(trimSearchAndHash(parsedUrl).toString(), 'https://example.com/path');
});
});
describe('arePathsEqual', function () {
it('returns false if one of the param is not a URL', function () {
const url1 = 'foo';
const url2 = 'https://example.com';
assert.equal(arePathsEqual(url1, url2), false);
});
it('returns false if hostnames are different', function () {
const url1 = 'https://a.com';
const url2 = 'https://b.com';
assert.equal(arePathsEqual(url1, url2), false);
});
it('returns false if top level domains are different', function () {
const url1 = 'https://a.io';
const url2 = 'https://a.com';
assert.equal(arePathsEqual(url1, url2), false);
});
it('returns false if sub domains are different', function () {
const url1 = 'https://sub.a.com';
const url2 = 'https://subdiff.a.com';
assert.equal(arePathsEqual(url1, url2), false);
});
it('returns false if paths are different', function () {
const url1 = 'https://a.com/path-1';
const url2 = 'https://a.com/path-2';
assert.equal(arePathsEqual(url1, url2), false);
});
it('returns true even if protocols are different', function () {
const url1 = 'http://a.com';
const url2 = 'https://a.com';
assert.equal(arePathsEqual(url1, url2), true);
});
it('returns true even if www is used in one of the urls', function () {
const url1 = 'https://www.a.com';
const url2 = 'https://a.com';
assert.equal(arePathsEqual(url1, url2), true);
});
it('returns true even if query parameters are different', function () {
const url1 = 'http://a.com?foo=bar';
const url2 = 'http://a.com';
assert.equal(arePathsEqual(url1, url2), true);
});
it('returns true even if hash segments are different', function () {
const url1 = 'http://a.com#segment-1';
const url2 = 'http://a.com';
assert.equal(arePathsEqual(url1, url2), true);
});
});

View file

@ -74,8 +74,7 @@ export default (function viteConfig() {
test: { test: {
globals: true, // required for @testing-library/jest-dom extensions globals: true, // required for @testing-library/jest-dom extensions
environment: 'jsdom', environment: 'jsdom',
setupFiles: './test/test-setup.js', include: ['./test/unit/**/*'],
include: ['./test/unit/*'],
testTimeout: process.env.TIMEOUT ? parseInt(process.env.TIMEOUT) : 10000, testTimeout: process.env.TIMEOUT ? parseInt(process.env.TIMEOUT) : 10000,
...(process.env.CI && { // https://github.com/vitest-dev/vitest/issues/1674 ...(process.env.CI && { // https://github.com/vitest-dev/vitest/issues/1674
minThreads: 1, minThreads: 1,

View file

@ -115,12 +115,12 @@ Object {
"recommendations": Array [ "recommendations": Array [
Object { Object {
"created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"excerpt": "Dogs are cute", "excerpt": null,
"favicon": "https://dogpictures.com/favicon.ico", "favicon": null,
"featured_image": "https://dogpictures.com/dog.jpg", "featured_image": null,
"id": StringMatching /\\[a-f0-9\\]\\{24\\}/, "id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
"one_click_subscribe": true, "one_click_subscribe": false,
"reason": "Because dogs are cute", "reason": null,
"title": "Dog Pictures", "title": "Dog Pictures",
"updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, "updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"url": "https://dogpictures.com/", "url": "https://dogpictures.com/",
@ -133,7 +133,7 @@ exports[`Recommendations Admin API Can browse 2: [headers] 1`] = `
Object { Object {
"access-control-allow-origin": "http://127.0.0.1:2369", "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", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "463", "content-length": "372",
"content-type": "application/json; charset=utf-8", "content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
@ -169,7 +169,7 @@ Object {
"reason": "Because cats are cute", "reason": "Because cats are cute",
"title": "Cat Pictures", "title": "Cat Pictures",
"updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, "updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"url": "https://catpictures.com/", "url": "https://dogpictures.com/",
}, },
], ],
} }
@ -198,22 +198,10 @@ Object {
"page": 1, "page": 1,
"pages": 2, "pages": 2,
"prev": null, "prev": null,
"total": 16, "total": 15,
}, },
}, },
"recommendations": Array [ "recommendations": Array [
Object {
"created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"excerpt": "Dogs are cute",
"favicon": "https://dogpictures.com/favicon.ico",
"featured_image": "https://dogpictures.com/dog.jpg",
"id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
"one_click_subscribe": true,
"reason": "Because dogs are cute",
"title": "Dog Pictures",
"updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"url": "https://dogpictures.com/",
},
Object { Object {
"created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"excerpt": null, "excerpt": null,
@ -322,6 +310,18 @@ Object {
"updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, "updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"url": "https://recommendation6.com/", "url": "https://recommendation6.com/",
}, },
Object {
"created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"excerpt": null,
"favicon": null,
"featured_image": null,
"id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
"one_click_subscribe": false,
"reason": "Reason 5",
"title": "Recommendation 5",
"updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"url": "https://recommendation5.com/",
},
], ],
} }
`; `;
@ -330,7 +330,7 @@ exports[`Recommendations Admin API Can request pages 2: [headers] 1`] = `
Object { Object {
"access-control-allow-origin": "http://127.0.0.1:2369", "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", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "2979", "content-length": "2902",
"content-type": "application/json; charset=utf-8", "content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
@ -348,22 +348,10 @@ Object {
"page": 2, "page": 2,
"pages": 2, "pages": 2,
"prev": 1, "prev": 1,
"total": 16, "total": 15,
}, },
}, },
"recommendations": Array [ "recommendations": Array [
Object {
"created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"excerpt": null,
"favicon": null,
"featured_image": null,
"id": StringMatching /\\[a-f0-9\\]\\{24\\}/,
"one_click_subscribe": false,
"reason": "Reason 5",
"title": "Recommendation 5",
"updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"url": "https://recommendation5.com/",
},
Object { Object {
"created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"excerpt": null, "excerpt": null,
@ -432,7 +420,7 @@ exports[`Recommendations Admin API Can request pages 4: [headers] 1`] = `
Object { Object {
"access-control-allow-origin": "http://127.0.0.1:2369", "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", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "1775", "content-length": "1497",
"content-type": "application/json; charset=utf-8", "content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
@ -507,7 +495,7 @@ exports[`Recommendations Admin API Uses default limit of 5 1: [headers] 1`] = `
Object { Object {
"access-control-allow-origin": "http://127.0.0.1:2369", "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", "cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "1585", "content-length": "109",
"content-type": "application/json; charset=utf-8", "content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/, "content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/, "etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,

View file

@ -4,6 +4,17 @@ const assert = require('assert/strict');
const recommendationsService = require('../../../core/server/services/recommendations'); const recommendationsService = require('../../../core/server/services/recommendations');
const {Recommendation} = require('@tryghost/recommendations'); const {Recommendation} = require('@tryghost/recommendations');
async function addDummyRecommendation(agent) {
await agent.post('recommendations/').body({
recommendations: [{
title: 'Dog Pictures',
url: 'https://dogpictures.com'
}]
});
const id = (await recommendationsService.repository.getAll())[0].id;
return id;
}
describe('Recommendations Admin API', function () { describe('Recommendations Admin API', function () {
let agent; let agent;
@ -11,15 +22,14 @@ describe('Recommendations Admin API', function () {
agent = await agentProvider.getAdminAPIAgent(); agent = await agentProvider.getAdminAPIAgent();
await fixtureManager.init('posts'); await fixtureManager.init('posts');
await agent.loginAsOwner(); await agent.loginAsOwner();
});
// Clear placeholders afterEach(async function () {
for (const recommendation of (await recommendationsService.repository.getAll())) { for (const recommendation of (await recommendationsService.repository.getAll())) {
recommendation.delete(); recommendation.delete();
await recommendationsService.repository.save(recommendation); await recommendationsService.repository.save(recommendation);
} }
});
afterEach(function () {
mockManager.restore(); mockManager.restore();
}); });
@ -94,13 +104,32 @@ describe('Recommendations Admin API', function () {
assert.equal(body.recommendations[0].one_click_subscribe, true); assert.equal(body.recommendations[0].one_click_subscribe, true);
}); });
it('Cannot add the same recommendation twice', async function () {
await agent.post('recommendations/')
.body({
recommendations: [{
title: 'Dog Pictures',
url: 'https://dogpictures.com'
}]
});
await agent.post('recommendations/')
.body({
recommendations: [{
title: 'Dog Pictures 2',
url: 'https://dogpictures.com'
}]
})
.expectStatus(422);
});
it('Can edit recommendation', async function () { it('Can edit recommendation', async function () {
const id = (await recommendationsService.repository.getAll())[0].id; const id = await addDummyRecommendation(agent);
const {body} = await agent.put(`recommendations/${id}/`) const {body} = await agent.put(`recommendations/${id}/`)
.body({ .body({
recommendations: [{ recommendations: [{
title: 'Cat Pictures', title: 'Cat Pictures',
url: 'https://catpictures.com', url: 'https://dogpictures.com',
reason: 'Because cats are cute', reason: 'Because cats are cute',
excerpt: 'Cats are cute', excerpt: 'Cats are cute',
featured_image: 'https://catpictures.com/cat.jpg', featured_image: 'https://catpictures.com/cat.jpg',
@ -126,7 +155,7 @@ describe('Recommendations Admin API', function () {
// Check everything is set correctly // Check everything is set correctly
assert.equal(body.recommendations[0].id, id); assert.equal(body.recommendations[0].id, id);
assert.equal(body.recommendations[0].title, 'Cat Pictures'); assert.equal(body.recommendations[0].title, 'Cat Pictures');
assert.equal(body.recommendations[0].url, 'https://catpictures.com/'); assert.equal(body.recommendations[0].url, 'https://dogpictures.com/');
assert.equal(body.recommendations[0].reason, 'Because cats are cute'); assert.equal(body.recommendations[0].reason, 'Because cats are cute');
assert.equal(body.recommendations[0].excerpt, 'Cats are cute'); assert.equal(body.recommendations[0].excerpt, 'Cats are cute');
assert.equal(body.recommendations[0].featured_image, 'https://catpictures.com/cat.jpg'); assert.equal(body.recommendations[0].featured_image, 'https://catpictures.com/cat.jpg');
@ -135,16 +164,17 @@ describe('Recommendations Admin API', function () {
}); });
it('Cannot use invalid protocols when editing', async function () { it('Cannot use invalid protocols when editing', async function () {
const id = (await recommendationsService.repository.getAll())[0].id; const id = await addDummyRecommendation(agent);
await agent.put(`recommendations/${id}/`) await agent.put(`recommendations/${id}/`)
.body({ .body({
recommendations: [{ recommendations: [{
title: 'Cat Pictures', title: 'Cat Pictures',
url: 'https://catpictures.com', url: 'https://dogpictures.com',
reason: 'Because cats are cute', reason: 'Because cats are cute',
excerpt: 'Cats are cute', excerpt: 'Cats are cute',
featured_image: 'ftp://catpictures.com/cat.jpg', featured_image: 'ftp://dogpictures.com/dog.jpg',
favicon: 'ftp://catpictures.com/favicon.ico', favicon: 'ftp://dogpictures.com/favicon.ico',
one_click_subscribe: false one_click_subscribe: false
}] }]
}) })
@ -163,7 +193,7 @@ describe('Recommendations Admin API', function () {
}); });
it('Can delete recommendation', async function () { it('Can delete recommendation', async function () {
const id = (await recommendationsService.repository.getAll())[0].id; const id = await addDummyRecommendation(agent);
await agent.delete(`recommendations/${id}/`) await agent.delete(`recommendations/${id}/`)
.expectStatus(204) .expectStatus(204)
.matchHeaderSnapshot({ .matchHeaderSnapshot({
@ -174,6 +204,8 @@ describe('Recommendations Admin API', function () {
}); });
it('Can browse', async function () { it('Can browse', async function () {
await addDummyRecommendation(agent);
await agent.get('recommendations/') await agent.get('recommendations/')
.expectStatus(200) .expectStatus(200)
.matchHeaderSnapshot({ .matchHeaderSnapshot({
@ -227,7 +259,7 @@ describe('Recommendations Admin API', function () {
assert.equal(page1.meta.pagination.pages, 2); assert.equal(page1.meta.pagination.pages, 2);
assert.equal(page1.meta.pagination.next, 2); assert.equal(page1.meta.pagination.next, 2);
assert.equal(page1.meta.pagination.prev, null); assert.equal(page1.meta.pagination.prev, null);
assert.equal(page1.meta.pagination.total, 16); assert.equal(page1.meta.pagination.total, 15);
const {body: page2} = await agent.get('recommendations/?page=2&limit=10') const {body: page2} = await agent.get('recommendations/?page=2&limit=10')
.expectStatus(200) .expectStatus(200)
@ -236,7 +268,7 @@ describe('Recommendations Admin API', function () {
etag: anyEtag etag: anyEtag
}) })
.matchBodySnapshot({ .matchBodySnapshot({
recommendations: new Array(6).fill({ recommendations: new Array(5).fill({
id: anyObjectId, id: anyObjectId,
created_at: anyISODateTime, created_at: anyISODateTime,
updated_at: anyISODateTime updated_at: anyISODateTime
@ -248,7 +280,7 @@ describe('Recommendations Admin API', function () {
assert.equal(page2.meta.pagination.pages, 2); assert.equal(page2.meta.pagination.pages, 2);
assert.equal(page2.meta.pagination.next, null); assert.equal(page2.meta.pagination.next, null);
assert.equal(page2.meta.pagination.prev, 1); assert.equal(page2.meta.pagination.prev, 1);
assert.equal(page2.meta.pagination.total, 16); assert.equal(page2.meta.pagination.total, 15);
}); });
it('Uses default limit of 5', async function () { it('Uses default limit of 5', async function () {

View file

@ -7,10 +7,19 @@ type Sentry = {
captureException(err: unknown): void; captureException(err: unknown): void;
} }
type RecommendationFindOneData<T> = {
id?: T;
url?: string;
};
type RecommendationModelClass<T> = ModelClass<T> & {
findOne: (data: RecommendationFindOneData<T>, options?: { require?: boolean }) => Promise<ModelInstance<T> | null>;
};
export class BookshelfRecommendationRepository extends BookshelfRepository<string, Recommendation> implements RecommendationRepository { export class BookshelfRecommendationRepository extends BookshelfRepository<string, Recommendation> implements RecommendationRepository {
sentry?: Sentry; sentry?: Sentry;
constructor(Model: ModelClass<string>, deps: {sentry?: Sentry} = {}) { constructor(Model: RecommendationModelClass<string>, deps: {sentry?: Sentry} = {}) {
super(Model); super(Model);
this.sentry = deps.sentry; this.sentry = deps.sentry;
} }
@ -65,4 +74,9 @@ export class BookshelfRecommendationRepository extends BookshelfRepository<strin
updatedAt: 'updated_at' updatedAt: 'updated_at'
} as Record<keyof Recommendation, string>; } as Record<keyof Recommendation, string>;
} }
async getByUrl(url: URL): Promise<Recommendation | null> {
const model = await (this.Model as RecommendationModelClass<string>).findOne({url: url.toString()}, {require: false});
return model ? this.modelToEntity(model) : null;
}
} }

View file

@ -6,4 +6,10 @@ export class InMemoryRecommendationRepository extends InMemoryRepository<string,
toPrimitive(entity: Recommendation): object { toPrimitive(entity: Recommendation): object {
return entity; return entity;
} }
getByUrl(url: URL): Promise<Recommendation | null> {
return this.getAll().then((recommendations) => {
return recommendations.find(recommendation => recommendation.url.toString() === url.toString()) || null;
});
}
} }

View file

@ -100,10 +100,18 @@ export class Recommendation {
this.reason = null; this.reason = null;
} }
this.url = this.cleanURL(this.url);
this.createdAt.setMilliseconds(0); this.createdAt.setMilliseconds(0);
this.updatedAt?.setMilliseconds(0); this.updatedAt?.setMilliseconds(0);
} }
cleanURL(url: URL) {
url.search = '';
url.hash = '';
return url;
};
static create(data: RecommendationCreateData) { static create(data: RecommendationCreateData) {
const id = data.id ?? ObjectId().toString(); const id = data.id ?? ObjectId().toString();
@ -123,6 +131,7 @@ export class Recommendation {
this.validate(d); this.validate(d);
const recommendation = new Recommendation(d); const recommendation = new Recommendation(d);
recommendation.clean(); recommendation.clean();
return recommendation; return recommendation;
} }

View file

@ -4,6 +4,7 @@ import {Recommendation} from './Recommendation';
export interface RecommendationRepository { export interface RecommendationRepository {
save(entity: Recommendation): Promise<void>; save(entity: Recommendation): Promise<void>;
getById(id: string): Promise<Recommendation | null>; getById(id: string): Promise<Recommendation | null>;
getByUrl(url: URL): Promise<Recommendation | null>;
getAll({filter, order}?: {filter?: string, order?: OrderOption<Recommendation>}): Promise<Recommendation[]>; getAll({filter, order}?: {filter?: string, order?: OrderOption<Recommendation>}): Promise<Recommendation[]>;
getPage({filter, order, page, limit}: { getPage({filter, order, page, limit}: {
filter?: string; filter?: string;

View file

@ -76,6 +76,15 @@ export class RecommendationService {
async addRecommendation(addRecommendation: AddRecommendation) { async addRecommendation(addRecommendation: AddRecommendation) {
const recommendation = Recommendation.create(addRecommendation); const recommendation = Recommendation.create(addRecommendation);
// If a recommendation with this URL already exists, throw an error
const existing = await this.repository.getByUrl(recommendation.url);
if (existing) {
throw new errors.ValidationError({
message: 'A recommendation with this URL already exists.'
});
}
await this.repository.save(recommendation); await this.repository.save(recommendation);
const recommendations = await this.listRecommendations(); const recommendations = await this.listRecommendations();