Added recommend back URL (#18382)

refs https://github.com/TryGhost/Product/issues/3958

- Disabled automatic network retries for external site lookups (=> timed
out to 5s in every situation because it returned 404 when a site doesn't
implement the Ghost api)
- Disabled representing a modal when it is already present on hash
changes
- Added support for search params in modals
- Handle `?url` search param in the addRecommendationModal
This commit is contained in:
Simon Backx 2023-09-28 12:54:16 +02:00 committed by GitHub
parent 95ec7b5016
commit 05215734af
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 89 additions and 28 deletions

View File

@ -14,7 +14,7 @@ export default defineConfig({
/* Retry on CI only */ /* Retry on CI only */
retries: process.env.CI ? 2 : 0, retries: process.env.CI ? 2 : 0,
/* Hardcode to use all cores in CI */ /* Hardcode to use all cores in CI */
workers: process.env.CI ? '100%' : undefined, workers: process.env.CI ? '100%' : (process.env.PLAYWRIGHT_SLOWMO ? 1 : undefined),
/* Reporter to use. See https://playwright.dev/docs/test-reporters */ /* Reporter to use. See https://playwright.dev/docs/test-reporters */
reporter: 'html', reporter: 'html',
/* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */

View File

@ -3,7 +3,7 @@ import TextField, {TextFieldProps} from './TextField';
import validator from 'validator'; import validator from 'validator';
import {useFocusContext} from '../../providers/DesignSystemProvider'; import {useFocusContext} from '../../providers/DesignSystemProvider';
const formatUrl = (value: string, baseUrl?: string) => { export const formatUrl = (value: string, baseUrl?: string) => {
let url = value.trim(); let url = value.trim();
if (!url) { if (!url) {

View File

@ -31,7 +31,8 @@ export const useExternalGhostSite = () => {
const result = await fetchApi(url, { const result = await fetchApi(url, {
method: 'GET', method: 'GET',
credentials: 'omit', // Allow CORS wildcard, credentials: 'omit', // Allow CORS wildcard,
timeout: 5000 timeout: 5000,
retry: false
}); });
// We need to validate all data types here for extra safety // We need to validate all data types here for extra safety

View File

@ -30,7 +30,8 @@ export const RouteContext = createContext<RoutingContextData>({
export type RoutingModalProps = { export type RoutingModalProps = {
pathName: string; pathName: string;
params?: Record<string, string> params?: Record<string, string>,
searchParams?: URLSearchParams
} }
const modalPaths: {[key: string]: ModalName} = { const modalPaths: {[key: string]: ModalName} = {
@ -85,6 +86,7 @@ const handleNavigation = (currentRoute: string | undefined) => {
let url = new URL(hash, domain); let url = new URL(hash, domain);
const pathName = getHashPath(url.pathname); const pathName = getHashPath(url.pathname);
const searchParams = url.searchParams;
if (pathName) { if (pathName) {
const [, currentModalName] = Object.entries(modalPaths).find(([modalPath]) => matchRoute(currentRoute || '', modalPath)) || []; const [, currentModalName] = Object.entries(modalPaths).find(([modalPath]) => matchRoute(currentRoute || '', modalPath)) || [];
@ -93,9 +95,9 @@ const handleNavigation = (currentRoute: string | undefined) => {
return { return {
pathName, pathName,
changingModal: modalName && modalName !== currentModalName, changingModal: modalName && modalName !== currentModalName,
modal: (path && modalName) ? modal: (path && modalName) ? // we should consider adding '&& modalName !== currentModalName' here, but this breaks tests
import('./routing/modals').then(({default: modals}) => { import('./routing/modals').then(({default: modals}) => {
NiceModal.show(modals[modalName] as ModalComponent, {pathName, params: matchRoute(pathName, path)}); NiceModal.show(modals[modalName] as ModalComponent, {pathName, params: matchRoute(pathName, path), searchParams});
}) : }) :
undefined undefined
}; };

View File

@ -8,8 +8,10 @@ import useForm from '../../../../hooks/useForm';
import useRouting from '../../../../hooks/useRouting'; import useRouting from '../../../../hooks/useRouting';
import {AlreadyExistsError} from '../../../../utils/errors'; import {AlreadyExistsError} from '../../../../utils/errors';
import {EditOrAddRecommendation, RecommendationResponseType, useGetRecommendationByUrl} from '../../../../api/recommendations'; import {EditOrAddRecommendation, RecommendationResponseType, useGetRecommendationByUrl} from '../../../../api/recommendations';
import {LoadingIndicator} from '../../../../admin-x-ds/global/LoadingIndicator';
import {RoutingModalProps} from '../../../providers/RoutingProvider'; import {RoutingModalProps} from '../../../providers/RoutingProvider';
import {dismissAllToasts, showToast} from '../../../../admin-x-ds/global/Toast'; import {dismissAllToasts, showToast} from '../../../../admin-x-ds/global/Toast';
import {formatUrl} from '../../../../admin-x-ds/global/form/URLTextField';
import {trimSearchAndHash} from '../../../../utils/url'; import {trimSearchAndHash} from '../../../../utils/url';
import {useExternalGhostSite} from '../../../../api/external-ghost-site'; import {useExternalGhostSite} from '../../../../api/external-ghost-site';
import {useGetOembed} from '../../../../api/oembed'; import {useGetOembed} from '../../../../api/oembed';
@ -19,7 +21,11 @@ interface AddRecommendationModalProps {
animate?: boolean animate?: boolean
} }
const AddRecommendationModal: React.FC<RoutingModalProps & AddRecommendationModalProps> = ({recommendation, animate}) => { const doFormatUrl = (url: string) => {
return formatUrl(url).save;
};
const AddRecommendationModal: React.FC<RoutingModalProps & AddRecommendationModalProps> = ({searchParams, recommendation, animate}) => {
const [enterPressed, setEnterPressed] = useState(false); const [enterPressed, setEnterPressed] = useState(false);
const modal = useModal(); const modal = useModal();
const {updateRoute} = useRouting(); const {updateRoute} = useRouting();
@ -27,10 +33,18 @@ const AddRecommendationModal: React.FC<RoutingModalProps & AddRecommendationModa
const {query: queryExternalGhostSite} = useExternalGhostSite(); const {query: queryExternalGhostSite} = useExternalGhostSite();
const {query: getRecommendationByUrl} = useGetRecommendationByUrl(); const {query: getRecommendationByUrl} = useGetRecommendationByUrl();
// Handle a URL that was passed via the URL
const initialUrl = recommendation ? '' : (searchParams?.get('url') ?? '');
const {save: initialUrlCleaned} = initialUrl ? formatUrl(initialUrl) : {save: ''};
// Show loading view when we had an initial URL
const didInitialSubmit = React.useRef(false);
const [showLoadingView, setShowLoadingView] = React.useState(!!initialUrlCleaned);
const {formState, updateForm, handleSave, errors, saveState, clearError} = useForm({ const {formState, updateForm, handleSave, errors, saveState, clearError} = useForm({
initialState: recommendation ?? { initialState: recommendation ?? {
title: '', title: '',
url: '', url: initialUrlCleaned,
reason: '', reason: '',
excerpt: null, excerpt: null,
featured_image: null, featured_image: null,
@ -89,6 +103,10 @@ const AddRecommendationModal: React.FC<RoutingModalProps & AddRecommendationModa
// Switch modal without changing the route (the second modal is not reachable by URL) // Switch modal without changing the route (the second modal is not reachable by URL)
modal.remove(); modal.remove();
// todo: we should change the URL, but this also keeps adding a new modal -> infinite loop
// updateRoute('recommendations/add?url=' + encodeURIComponent(updatedRecommendation.url));
NiceModal.show(AddRecommendationModalConfirm, { NiceModal.show(AddRecommendationModalConfirm, {
animate: false, animate: false,
recommendation: updatedRecommendation recommendation: updatedRecommendation
@ -108,11 +126,16 @@ const AddRecommendationModal: React.FC<RoutingModalProps & AddRecommendationModa
newErrors.url = 'Please enter a valid URL.'; newErrors.url = 'Please enter a valid URL.';
} }
// If we have errors: close direct submit view
if (showLoadingView) {
setShowLoadingView(Object.keys(newErrors).length === 0);
}
return newErrors; return newErrors;
} }
}); });
const saveForm = async () => { const onOk = React.useCallback(async () => {
if (saveState === 'saving') { if (saveState === 'saving') {
// Already saving // Already saving
return; return;
@ -120,7 +143,9 @@ const AddRecommendationModal: React.FC<RoutingModalProps & AddRecommendationModa
dismissAllToasts(); dismissAllToasts();
try { try {
await handleSave({force: true}); if (await handleSave({force: true})) {
return;
}
} catch (e) { } catch (e) {
const message = e instanceof AlreadyExistsError ? e.message : 'Something went wrong while checking this URL, please try again.'; const message = e instanceof AlreadyExistsError ? e.message : 'Something went wrong while checking this URL, please try again.';
showToast({ showToast({
@ -128,22 +153,44 @@ const AddRecommendationModal: React.FC<RoutingModalProps & AddRecommendationModa
message message
}); });
} }
};
// If we have errors: close direct submit view
if (showLoadingView) {
setShowLoadingView(false);
}
}, [handleSave, saveState, showLoadingView, setShowLoadingView]);
// Make sure we submit initially when opening in loading view state
React.useEffect(() => {
if (showLoadingView && !didInitialSubmit.current) {
didInitialSubmit.current = true;
onOk();
}
}, [showLoadingView, onOk]);
useEffect(() => { useEffect(() => {
if (enterPressed) { if (enterPressed) {
saveForm(); onOk();
setEnterPressed(false); // Reset for future use setEnterPressed(false); // Reset for future use
} }
// eslint-disable-next-line react-hooks/exhaustive-deps // eslint-disable-next-line react-hooks/exhaustive-deps
}, [formState]); }, [formState]);
const formatUrl = (url: string) => { if (showLoadingView) {
if (!url.startsWith('http://') && !url.startsWith('https://')) { return <Modal
url = `https://${url}`; afterClose={() => {
// Closed without saving: reset route
updateRoute('recommendations');
}}
animate={animate ?? true}
backDropClick={false}
cancelLabel=''
okLabel=''
size='sm'
>
<LoadingIndicator />
</Modal>;
} }
return url;
};
return <Modal return <Modal
afterClose={() => { afterClose={() => {
@ -158,7 +205,7 @@ const AddRecommendationModal: React.FC<RoutingModalProps & AddRecommendationModa
size='sm' size='sm'
testId='add-recommendation-modal' testId='add-recommendation-modal'
title='Add recommendation' title='Add recommendation'
onOk={saveForm} onOk={onOk}
> >
<p className="mt-4">You can recommend any site your audience will find valuable, not just those published on Ghost.</p> <p className="mt-4">You can recommend any site your audience will find valuable, not just those published on Ghost.</p>
<Form <Form
@ -172,7 +219,7 @@ const AddRecommendationModal: React.FC<RoutingModalProps & AddRecommendationModa
placeholder='https://www.example.com' placeholder='https://www.example.com'
title='URL' title='URL'
value={formState.url} value={formState.url}
onBlur={() => updateForm(state => ({...state, url: formatUrl(formState.url)}))} onBlur={() => updateForm(state => ({...state, url: doFormatUrl(formState.url)}))}
onChange={(e) => { onChange={(e) => {
clearError?.('url'); clearError?.('url');
updateForm(state => ({...state, url: e.target.value})); updateForm(state => ({...state, url: e.target.value}));
@ -180,7 +227,7 @@ const AddRecommendationModal: React.FC<RoutingModalProps & AddRecommendationModa
onKeyDown={(e) => { onKeyDown={(e) => {
if (e.key === 'Enter') { if (e.key === 'Enter') {
e.preventDefault(); e.preventDefault();
updateForm(state => ({...state, url: formatUrl(formState.url)})); updateForm(state => ({...state, url: doFormatUrl(formState.url)}));
setEnterPressed(true); setEnterPressed(true);
} }
}} }}

View File

@ -27,6 +27,7 @@ interface RequestOptions {
}; };
credentials?: 'include' | 'omit' | 'same-origin'; credentials?: 'include' | 'omit' | 'same-origin';
timeout?: number; timeout?: number;
retry?: boolean;
} }
export const useFetchApi = () => { export const useFetchApi = () => {
@ -34,7 +35,7 @@ export const useFetchApi = () => {
const sentryDSN = useSentryDSN(); const sentryDSN = useSentryDSN();
// eslint-disable-next-line @typescript-eslint/no-explicit-any // eslint-disable-next-line @typescript-eslint/no-explicit-any
return async <ResponseData = any>(endpoint: string | URL, options: RequestOptions = {}) => { return async <ResponseData = any>(endpoint: string | URL, options: RequestOptions = {}): Promise<ResponseData> => {
// By default, we set the Content-Type header to application/json // By default, we set the Content-Type header to application/json
const defaultHeaders: Record<string, string> = { const defaultHeaders: Record<string, string> = {
'app-pragma': 'no-cache', 'app-pragma': 'no-cache',
@ -56,6 +57,7 @@ export const useFetchApi = () => {
// 1. Server Unreachable error from the browser (code 0 or TypeError), typically from short internet blips // 1. Server Unreachable error from the browser (code 0 or TypeError), typically from short internet blips
// 2. Maintenance error from Ghost, upgrade in progress so API is temporarily unavailable // 2. Maintenance error from Ghost, upgrade in progress so API is temporarily unavailable
let attempts = 0; let attempts = 0;
let shouldRetry = options.retry === true || options.retry === undefined;
let retryingMs = 0; let retryingMs = 0;
const startTime = Date.now(); const startTime = Date.now();
const maxRetryingMs = 15_000; const maxRetryingMs = 15_000;
@ -75,7 +77,7 @@ export const useFetchApi = () => {
return data; return data;
}; };
while (true) { while (attempts === 0 || shouldRetry) {
try { try {
const response = await fetch(endpoint, { const response = await fetch(endpoint, {
headers: { headers: {
@ -97,7 +99,7 @@ export const useFetchApi = () => {
} catch (error) { } catch (error) {
retryingMs = Date.now() - startTime; retryingMs = Date.now() - startTime;
if (import.meta.env.MODE !== 'development' && retryableErrors.some(errorClass => error instanceof errorClass) && retryingMs <= maxRetryingMs) { if (shouldRetry && (import.meta.env.MODE !== 'development' && retryableErrors.some(errorClass => error instanceof errorClass) && retryingMs <= maxRetryingMs)) {
await new Promise((resolve) => { await new Promise((resolve) => {
setTimeout(resolve, retryPeriods[attempts] || retryPeriods[retryPeriods.length - 1]); setTimeout(resolve, retryPeriods[attempts] || retryPeriods[retryPeriods.length - 1]);
}); });
@ -122,6 +124,11 @@ export const useFetchApi = () => {
throw newError; throw newError;
}; };
} }
// Used for type checking
// this can't happen, but TS isn't smart enough to undeerstand that the loop will never exit without an error or return
// because of shouldRetry + attemps usage combination
return undefined as never;
}; };
}; };

View File

@ -366,7 +366,7 @@ exports[`Incoming Recommendation Emails Sends an email if we receive a recommend
<table border=\\"0\\" cellpadding=\\"0\\" cellspacing=\\"0\\" style=\\"border-collapse: separate; mso-table-lspace: 0pt; mso-table-rspace: 0pt; width: auto;\\"> <table border=\\"0\\" cellpadding=\\"0\\" cellspacing=\\"0\\" style=\\"border-collapse: separate; mso-table-lspace: 0pt; mso-table-rspace: 0pt; width: auto;\\">
<tbody> <tbody>
<tr> <tr>
<td style=\\"font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; font-size: 16px; vertical-align: top; background-color: #FF1A75; border-radius: 5px; text-align: center;\\"> <a href=\\"http://127.0.0.1:2369/ghost/#/settings-x/recommendations\\" target=\\"_blank\\" style=\\"display: inline-block; color: #ffffff; background-color: #FF1A75; border: solid 1px #FF1A75; border-radius: 5px; box-sizing: border-box; cursor: pointer; text-decoration: none; font-size: 16px; font-weight: normal; margin: 0; padding: 9px 22px 10px; border-color: #FF1A75;\\">Recommend back</a></td> <td style=\\"font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; font-size: 16px; vertical-align: top; background-color: #FF1A75; border-radius: 5px; text-align: center;\\"> <a href=\\"http://127.0.0.1:2369/ghost/#/settings-x/recommendations/add?url=https%3A%2F%2Fwww.otherghostsite.com%2F\\" target=\\"_blank\\" style=\\"display: inline-block; color: #ffffff; background-color: #FF1A75; border: solid 1px #FF1A75; border-radius: 5px; box-sizing: border-box; cursor: pointer; text-decoration: none; font-size: 16px; font-weight: normal; margin: 0; padding: 9px 22px 10px; border-color: #FF1A75;\\">Recommend back</a></td>
</tr> </tr>
</tbody> </tbody>
</table> </table>
@ -563,7 +563,7 @@ exports[`Incoming Recommendation Emails Sends an email if we receive a recommend
<table border=\\"0\\" cellpadding=\\"0\\" cellspacing=\\"0\\" style=\\"border-collapse: separate; mso-table-lspace: 0pt; mso-table-rspace: 0pt; width: auto;\\"> <table border=\\"0\\" cellpadding=\\"0\\" cellspacing=\\"0\\" style=\\"border-collapse: separate; mso-table-lspace: 0pt; mso-table-rspace: 0pt; width: auto;\\">
<tbody> <tbody>
<tr> <tr>
<td style=\\"font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; font-size: 16px; vertical-align: top; background-color: #FF1A75; border-radius: 5px; text-align: center;\\"> <a href=\\"http://127.0.0.1:2369/ghost/#/settings-x/recommendations\\" target=\\"_blank\\" style=\\"display: inline-block; color: #ffffff; background-color: #FF1A75; border: solid 1px #FF1A75; border-radius: 5px; box-sizing: border-box; cursor: pointer; text-decoration: none; font-size: 16px; font-weight: normal; margin: 0; padding: 9px 22px 10px; border-color: #FF1A75;\\">Recommend back</a></td> <td style=\\"font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; font-size: 16px; vertical-align: top; background-color: #FF1A75; border-radius: 5px; text-align: center;\\"> <a href=\\"http://127.0.0.1:2369/ghost/#/settings-x/recommendations/add?url=https%3A%2F%2Fwww.otherghostsite.com%2F\\" target=\\"_blank\\" style=\\"display: inline-block; color: #ffffff; background-color: #FF1A75; border: solid 1px #FF1A75; border-radius: 5px; box-sizing: border-box; cursor: pointer; text-decoration: none; font-size: 16px; font-weight: normal; margin: 0; padding: 9px 22px 10px; border-color: #FF1A75;\\">Recommend back</a></td>
</tr> </tr>
</tbody> </tbody>
</table> </table>

View File

@ -98,7 +98,7 @@ export class IncomingRecommendationService {
// Check if we are also recommending this URL // Check if we are also recommending this URL
const existing = await this.#recommendationService.countRecommendations({ const existing = await this.#recommendationService.countRecommendations({
filter: `url:~^'${url}'` filter: `url:~'${url}'`
}); });
const recommendingBack = existing > 0; const recommendingBack = existing > 0;

View File

@ -494,6 +494,10 @@ class StaffServiceEmails {
} }
return array.slice(0,limit); return array.slice(0,limit);
}); });
this.Handlebars.registerHelper('encodeURIComponent', function (string) {
return encodeURIComponent(string);
});
} }
async renderHTML(templateName, data) { async renderHTML(templateName, data) {

View File

@ -59,7 +59,7 @@
{{#if recommendation.recommendingBack}} {{#if recommendation.recommendingBack}}
<td style="font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; font-size: 16px; vertical-align: top; background-color: {{accentColor}}; border-radius: 5px; text-align: center;"> <a href="{{siteUrl}}ghost/#/settings-x/recommendations" target="_blank" style="display: inline-block; color: #ffffff; background-color: {{accentColor}}; border: solid 1px {{accentColor}}; border-radius: 5px; box-sizing: border-box; cursor: pointer; text-decoration: none; font-size: 16px; font-weight: normal; margin: 0; padding: 9px 22px 10px; border-color: {{accentColor}};">View recommendations</a></td> <td style="font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; font-size: 16px; vertical-align: top; background-color: {{accentColor}}; border-radius: 5px; text-align: center;"> <a href="{{siteUrl}}ghost/#/settings-x/recommendations" target="_blank" style="display: inline-block; color: #ffffff; background-color: {{accentColor}}; border: solid 1px {{accentColor}}; border-radius: 5px; box-sizing: border-box; cursor: pointer; text-decoration: none; font-size: 16px; font-weight: normal; margin: 0; padding: 9px 22px 10px; border-color: {{accentColor}};">View recommendations</a></td>
{{else}} {{else}}
<td style="font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; font-size: 16px; vertical-align: top; background-color: {{accentColor}}; border-radius: 5px; text-align: center;"> <a href="{{siteUrl}}ghost/#/settings-x/recommendations" target="_blank" style="display: inline-block; color: #ffffff; background-color: {{accentColor}}; border: solid 1px {{accentColor}}; border-radius: 5px; box-sizing: border-box; cursor: pointer; text-decoration: none; font-size: 16px; font-weight: normal; margin: 0; padding: 9px 22px 10px; border-color: {{accentColor}};">Recommend back</a></td> <td style="font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, Helvetica, Arial, sans-serif, 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol'; font-size: 16px; vertical-align: top; background-color: {{accentColor}}; border-radius: 5px; text-align: center;"> <a href="{{siteUrl}}ghost/#/settings-x/recommendations/add?url={{ encodeURIComponent recommendation.url }}" target="_blank" style="display: inline-block; color: #ffffff; background-color: {{accentColor}}; border: solid 1px {{accentColor}}; border-radius: 5px; box-sizing: border-box; cursor: pointer; text-decoration: none; font-size: 16px; font-weight: normal; margin: 0; padding: 9px 22px 10px; border-color: {{accentColor}};">Recommend back</a></td>
{{/if}} {{/if}}
</tr> </tr>
</tbody> </tbody>