From 8515bdf587f088b7f57cad3ed99b12a0b27c38fa Mon Sep 17 00:00:00 2001 From: Sag Date: Wed, 20 Sep 2023 18:53:10 +0200 Subject: [PATCH] Updated uniqueness validation for the Recommendations URL (#18253) closes https://github.com/TryGhost/Product/issues/3818 - instead of fetching all recommendations and matching URLs on the frontend, we now query the database directly to find an existing Recommendation by URL. When comparing URLs, we don't take into account the protocol, www, query parameters nor hash fragments --- .../src/api/recommendations.ts | 25 ++++++- .../AddRecommendationModal.tsx | 23 +++--- apps/admin-x-settings/src/utils/errors.ts | 6 ++ apps/admin-x-settings/src/utils/url.ts | 23 ------ .../test/unit/utils/url.test.ts | 67 +----------------- .../recommendations.test.js.snap | 70 +++++++++++++++++++ .../e2e-api/admin/recommendations.test.js | 45 ++++++++++-- .../src/BookshelfRecommendationRepository.ts | 6 +- .../src/InMemoryRecommendationRepository.ts | 7 +- .../src/RecommendationRepository.ts | 2 +- .../src/RecommendationService.ts | 2 +- 11 files changed, 162 insertions(+), 114 deletions(-) diff --git a/apps/admin-x-settings/src/api/recommendations.ts b/apps/admin-x-settings/src/api/recommendations.ts index 0571d418b9..8c09f809bd 100644 --- a/apps/admin-x-settings/src/api/recommendations.ts +++ b/apps/admin-x-settings/src/api/recommendations.ts @@ -1,4 +1,4 @@ -import {Meta, createMutation, createPaginatedQuery} from '../utils/apiRequests'; +import {Meta, apiUrl, createMutation, createPaginatedQuery, useFetchApi} from '../utils/apiRequests'; export type Recommendation = { id: string @@ -69,3 +69,26 @@ export const useAddRecommendation = createMutation { + const fetchApi = useFetchApi(); + const path = '/recommendations/'; + + return { + async query(url: URL): Promise { + const urlFilter = `url:~'${url.host.replace('www.', '')}${url.pathname.replace(/\/$/, '')}'`; + const endpoint = apiUrl(path, {filter: urlFilter, limit: '1'}); + try { + const result = await fetchApi(endpoint, { + method: 'GET', + timeout: 5000 + }); + return result as RecommendationResponseType; + } catch (e) { + // eslint-disable-next-line no-console + console.error(e); + return null; + } + } + }; +}; diff --git a/apps/admin-x-settings/src/components/settings/site/recommendations/AddRecommendationModal.tsx b/apps/admin-x-settings/src/components/settings/site/recommendations/AddRecommendationModal.tsx index 0cd615986c..3370f0d6a6 100644 --- a/apps/admin-x-settings/src/components/settings/site/recommendations/AddRecommendationModal.tsx +++ b/apps/admin-x-settings/src/components/settings/site/recommendations/AddRecommendationModal.tsx @@ -6,11 +6,12 @@ import React from 'react'; import URLTextField from '../../../../admin-x-ds/global/form/URLTextField'; import useForm from '../../../../hooks/useForm'; import useRouting from '../../../../hooks/useRouting'; -import {EditOrAddRecommendation, useBrowseRecommendations} from '../../../../api/recommendations'; +import {AlreadyExistsError} from '../../../../utils/errors'; +import {EditOrAddRecommendation, RecommendationResponseType, useGetRecommendationByUrl} from '../../../../api/recommendations'; import {RoutingModalProps} from '../../../providers/RoutingProvider'; -import {arePathsEqual, trimSearchAndHash} from '../../../../utils/url'; import {showToast} from '../../../../admin-x-ds/global/Toast'; import {toast} from 'react-hot-toast'; +import {trimSearchAndHash} from '../../../../utils/url'; import {useExternalGhostSite} from '../../../../api/external-ghost-site'; import {useGetOembed} from '../../../../api/oembed'; @@ -24,7 +25,7 @@ const AddRecommendationModal: React.FC 0) { + throw new AlreadyExistsError('A recommendation with this URL already exists.'); + } + + // Check if it s a Ghost site or not let externalGhostSite = validatedUrl.protocol === 'https:' ? (await queryExternalGhostSite('https://' + validatedUrl.host)) : null; // Use the hostname as fallback title @@ -90,11 +97,6 @@ const AddRecommendationModal: React.FC arePathsEqual(r.url, u.toString()))) { - newErrors.url = 'A recommendation with this URL already exists.'; - } } catch (e) { newErrors.url = 'Please enter a valid URL.'; } @@ -133,9 +135,10 @@ const AddRecommendationModal: React.FC; } - async getByUrl(url: URL): Promise { - const model = await (this.Model as RecommendationModelClass).findOne({url: url.toString()}, {require: false}); - return model ? this.modelToEntity(model) : null; + async getByUrl(url: URL): Promise { + const urlFilter = `url:~'${url.host.replace('www.', '')}${url.pathname.replace(/\/$/, '')}'`; + return this.getPage({filter: urlFilter, page: 1, limit: 1}); } } diff --git a/ghost/recommendations/src/InMemoryRecommendationRepository.ts b/ghost/recommendations/src/InMemoryRecommendationRepository.ts index 95835e1ff4..ba66f8c11b 100644 --- a/ghost/recommendations/src/InMemoryRecommendationRepository.ts +++ b/ghost/recommendations/src/InMemoryRecommendationRepository.ts @@ -7,9 +7,8 @@ export class InMemoryRecommendationRepository extends InMemoryRepository { - return this.getAll().then((recommendations) => { - return recommendations.find(recommendation => recommendation.url.toString() === url.toString()) || null; - }); + getByUrl(url: URL): Promise { + const urlFilter = `url:~'${url.host.replace('www.', '')}${url.pathname.replace(/\/$/, '')}'`; + return this.getPage({filter: urlFilter, page: 1, limit: 1}); } } diff --git a/ghost/recommendations/src/RecommendationRepository.ts b/ghost/recommendations/src/RecommendationRepository.ts index a569a7f3aa..ff5276aefe 100644 --- a/ghost/recommendations/src/RecommendationRepository.ts +++ b/ghost/recommendations/src/RecommendationRepository.ts @@ -4,7 +4,7 @@ import {Recommendation} from './Recommendation'; export interface RecommendationRepository { save(entity: Recommendation): Promise; getById(id: string): Promise; - getByUrl(url: URL): Promise; + getByUrl(url: URL): Promise; getAll({filter, order}?: {filter?: string, order?: OrderOption}): Promise; getPage({filter, order, page, limit}: { filter?: string; diff --git a/ghost/recommendations/src/RecommendationService.ts b/ghost/recommendations/src/RecommendationService.ts index fdbfbd82c9..9c6ef60699 100644 --- a/ghost/recommendations/src/RecommendationService.ts +++ b/ghost/recommendations/src/RecommendationService.ts @@ -135,7 +135,7 @@ export class RecommendationService { // If a recommendation with this URL already exists, throw an error const existing = await this.repository.getByUrl(recommendation.url); - if (existing) { + if (existing && existing.length > 0) { throw new errors.ValidationError({ message: 'A recommendation with this URL already exists.' });