From 85a1c694b3a4bb53aca1496d85cbf70107769706 Mon Sep 17 00:00:00 2001 From: Tilman Vatteroth Date: Sun, 3 Jul 2022 20:34:14 +0200 Subject: [PATCH] Add button to create note if not found (#2173) * feat: add CreateNonExistentNote component This component asks the user if they want to create the note that was request, but is not there. Signed-off-by: Philip Molares Signed-off-by: Tilman Vatteroth --- locales/en.json | 26 +++--- src/api/notes/index.ts | 2 +- ...reate-non-existing-note-hint.test.tsx.snap | 72 +++++++++++++++ .../note-loading-boundary.test.tsx.snap | 7 +- .../create-non-existing-note-hint.test.tsx | 89 +++++++++++++++++++ .../create-non-existing-note-hint.tsx | 78 ++++++++++++++++ .../note-loading-boundary.test.tsx | 10 ++- .../note-loading-boundary.tsx | 11 ++- src/components/common/redirect.tsx | 7 +- .../error-pages/common-error-page.tsx | 24 ++--- src/pages/[id].tsx | 4 +- 11 files changed, 297 insertions(+), 33 deletions(-) create mode 100644 src/components/common/note-loading-boundary/__snapshots__/create-non-existing-note-hint.test.tsx.snap create mode 100644 src/components/common/note-loading-boundary/create-non-existing-note-hint.test.tsx create mode 100644 src/components/common/note-loading-boundary/create-non-existing-note-hint.tsx diff --git a/locales/en.json b/locales/en.json index 1aae02e15..04dc0abd2 100644 --- a/locales/en.json +++ b/locales/en.json @@ -41,12 +41,23 @@ } }, "noteLoadingBoundary": { - "errorWhileLoadingContent": "Error while loading note." + "createNote": { + "question": "Do you want to create a note with alias '{{aliasName}}'?", + "create": "Create note", + "creating": "Creating note...", + "error": "Note couldn't be created." + } }, "api": { "note": { - "notFound": "Note not found.", - "accessDenied": "You don't have the needed permission to access this note." + "notFound": { + "title": "Note not found", + "description": "The requested note doesn't exist." + }, + "forbidden": { + "title": "Access denied", + "description": "You don't have the needed permission to access this note" + } } }, "landing": { @@ -495,12 +506,7 @@ "presentation": {}, "readOnly": { "viewCount": "views", - "editNote": "Edit this note", - "loading": "Loading note contents ...", - "error": { - "title": "Error while loading note", - "description": "Probably the requested note does not exist or was deleted." - } + "editNote": "Edit this note" } }, "common": { @@ -564,7 +570,7 @@ }, "errors": { "notFound": { - "title": "404 - page not found", + "title": "Page not found", "description": "The requested page either does not exist or you don't have permission to access it." }, "noteCreationFailed": { diff --git a/src/api/notes/index.ts b/src/api/notes/index.ts index a91c28280..d6c7382ec 100644 --- a/src/api/notes/index.ts +++ b/src/api/notes/index.ts @@ -16,7 +16,7 @@ import { DeleteApiRequestBuilder } from '../common/api-request-builder/delete-ap */ export const getNote = async (noteIdOrAlias: string): Promise => { const response = await new GetApiRequestBuilder('notes/' + noteIdOrAlias) - .withStatusCodeErrorMapping({ 404: 'api.note.notFound', 403: 'api.note.accessDenied' }) + .withStatusCodeErrorMapping({ 404: 'api.note.notFound', 403: 'api.note.forbidden' }) .sendRequest() return response.asParsedJsonObject() } diff --git a/src/components/common/note-loading-boundary/__snapshots__/create-non-existing-note-hint.test.tsx.snap b/src/components/common/note-loading-boundary/__snapshots__/create-non-existing-note-hint.test.tsx.snap new file mode 100644 index 000000000..a2eed506b --- /dev/null +++ b/src/components/common/note-loading-boundary/__snapshots__/create-non-existing-note-hint.test.tsx.snap @@ -0,0 +1,72 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`create non existing note hint redirects when the note has been created 1`] = ` +
+ + Redirecting to + + + /n/mockedPrimaryAlias + + +
+`; + +exports[`create non existing note hint renders a waiting message when button is clicked 1`] = ` +
+ +
+`; + +exports[`create non existing note hint renders an button as initial state 1`] = ` +
+ +
+`; + +exports[`create non existing note hint shows an error message if note couldn't be created 1`] = ` +
+ +
+`; diff --git a/src/components/common/note-loading-boundary/__snapshots__/note-loading-boundary.test.tsx.snap b/src/components/common/note-loading-boundary/__snapshots__/note-loading-boundary.test.tsx.snap index e1754774c..81850aab0 100644 --- a/src/components/common/note-loading-boundary/__snapshots__/note-loading-boundary.test.tsx.snap +++ b/src/components/common/note-loading-boundary/__snapshots__/note-loading-boundary.test.tsx.snap @@ -19,14 +19,17 @@ exports[`Note loading boundary shows an error 1`] = ` titleI18nKey: - noteLoadingBoundary.errorWhileLoadingContent + api.note.notFound.title descriptionI18nKey: - CRAAAAASH + api.note.notFound.description children: + + This is a mock for CreateNonExistingNoteHint + `; diff --git a/src/components/common/note-loading-boundary/create-non-existing-note-hint.test.tsx b/src/components/common/note-loading-boundary/create-non-existing-note-hint.test.tsx new file mode 100644 index 000000000..5fd41bf69 --- /dev/null +++ b/src/components/common/note-loading-boundary/create-non-existing-note-hint.test.tsx @@ -0,0 +1,89 @@ +/* + * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import * as useSingleStringUrlParameterModule from '../../../hooks/common/use-single-string-url-parameter' +import { mockI18n } from '../../markdown-renderer/test-utils/mock-i18n' +import { render, screen } from '@testing-library/react' +import { CreateNonExistingNoteHint } from './create-non-existing-note-hint' +import * as createNoteWithPrimaryAliasModule from '../../../api/notes' +import type { Note, NoteMetadata } from '../../../api/notes/types' +import { Mock } from 'ts-mockery' + +describe('create non existing note hint', () => { + const mockedNoteId = 'mockedNoteId' + + const mockGetNoteIdQueryParameter = () => { + const expectedQueryParameter = 'noteId' + jest.spyOn(useSingleStringUrlParameterModule, 'useSingleStringUrlParameter').mockImplementation((parameter) => { + expect(parameter).toBe(expectedQueryParameter) + return mockedNoteId + }) + } + + const mockCreateNoteWithPrimaryAlias = () => { + jest + .spyOn(createNoteWithPrimaryAliasModule, 'createNoteWithPrimaryAlias') + .mockImplementation((markdown, primaryAlias): Promise => { + expect(markdown).toBe('') + expect(primaryAlias).toBe(mockedNoteId) + const metadata: NoteMetadata = Mock.of({ primaryAddress: 'mockedPrimaryAlias' }) + return Promise.resolve(Mock.of({ metadata })) + }) + } + + const mockFailingCreateNoteWithPrimaryAlias = () => { + jest + .spyOn(createNoteWithPrimaryAliasModule, 'createNoteWithPrimaryAlias') + .mockImplementation((markdown, primaryAlias): Promise => { + expect(markdown).toBe('') + expect(primaryAlias).toBe(mockedNoteId) + return Promise.reject("couldn't create note") + }) + } + + afterEach(() => { + jest.resetAllMocks() + jest.resetModules() + }) + + beforeEach(async () => { + await mockI18n() + mockGetNoteIdQueryParameter() + }) + + it('renders an button as initial state', () => { + mockCreateNoteWithPrimaryAlias() + const view = render() + expect(view.container).toMatchSnapshot() + }) + + it('renders a waiting message when button is clicked', async () => { + mockCreateNoteWithPrimaryAlias() + const view = render() + const button = await screen.findByTestId('createNoteButton') + button.click() + await screen.findByTestId('loadingMessage') + expect(view.container).toMatchSnapshot() + }) + + it('redirects when the note has been created', async () => { + mockCreateNoteWithPrimaryAlias() + const view = render() + const button = await screen.findByTestId('createNoteButton') + button.click() + await screen.findByTestId('redirect') + expect(view.container).toMatchSnapshot() + }) + + it("shows an error message if note couldn't be created", async () => { + mockFailingCreateNoteWithPrimaryAlias() + const view = render() + const button = await screen.findByTestId('createNoteButton') + button.click() + await screen.findByTestId('failedMessage') + expect(view.container).toMatchSnapshot() + }) +}) diff --git a/src/components/common/note-loading-boundary/create-non-existing-note-hint.tsx b/src/components/common/note-loading-boundary/create-non-existing-note-hint.tsx new file mode 100644 index 000000000..daa11046a --- /dev/null +++ b/src/components/common/note-loading-boundary/create-non-existing-note-hint.tsx @@ -0,0 +1,78 @@ +/* + * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import { Trans, useTranslation } from 'react-i18next' +import React, { useCallback } from 'react' +import { Alert, Button } from 'react-bootstrap' +import { useSingleStringUrlParameter } from '../../../hooks/common/use-single-string-url-parameter' +import { createNoteWithPrimaryAlias } from '../../../api/notes' +import { useAsyncFn } from 'react-use' +import { ShowIf } from '../show-if/show-if' +import { Redirect } from '../redirect' +import { ForkAwesomeIcon } from '../fork-awesome/fork-awesome-icon' +import { testId } from '../../../utils/test-id' + +/** + * Shows a button that creates an empty note with the alias from the current window URL. + * When the button was clicked it also shows the progress. + */ +export const CreateNonExistingNoteHint: React.FC = () => { + useTranslation() + const noteIdFromUrl = useSingleStringUrlParameter('noteId', undefined) + + const [returnState, createNote] = useAsyncFn(async () => { + if (noteIdFromUrl === undefined) { + throw new Error('Note id not set') + } + return await createNoteWithPrimaryAlias('', noteIdFromUrl) + }, [noteIdFromUrl]) + + const onClickHandler = useCallback(() => { + void createNote() + }, [createNote]) + + if (noteIdFromUrl === undefined) { + return null + } else if (returnState.value) { + return + } else if (returnState.loading) { + return ( + + + + + ) + } else if (returnState.error !== undefined) { + return ( + + + + + ) + } else { + return ( + + + + +
+ +
+
+ ) + } +} diff --git a/src/components/common/note-loading-boundary/note-loading-boundary.test.tsx b/src/components/common/note-loading-boundary/note-loading-boundary.test.tsx index 6b181cb8f..ed69ad7a6 100644 --- a/src/components/common/note-loading-boundary/note-loading-boundary.test.tsx +++ b/src/components/common/note-loading-boundary/note-loading-boundary.test.tsx @@ -15,6 +15,7 @@ import { Fragment } from 'react' import { mockI18n } from '../../markdown-renderer/test-utils/mock-i18n' import * as CommonErrorPageModule from '../../error-pages/common-error-page' import * as LoadingScreenModule from '../../../components/application-loader/loading-screen/loading-screen' +import * as CreateNonExistingNoteHintModule from './create-non-existing-note-hint' describe('Note loading boundary', () => { const mockedNoteId = 'mockedNoteId' @@ -26,6 +27,13 @@ describe('Note loading boundary', () => { beforeEach(async () => { await mockI18n() + jest.spyOn(CreateNonExistingNoteHintModule, 'CreateNonExistingNoteHint').mockImplementation(() => { + return ( + + This is a mock for CreateNonExistingNoteHint + + ) + }) jest.spyOn(LoadingScreenModule, 'LoadingScreen').mockImplementation(({ errorMessage }) => { return ( @@ -70,7 +78,7 @@ describe('Note loading boundary', () => { jest.spyOn(getNoteModule, 'getNote').mockImplementation((id) => { expect(id).toBe(mockedNoteId) return new Promise((resolve, reject) => { - setTimeout(() => reject(new Error('CRAAAAASH')), 0) + setTimeout(() => reject(new Error('api.note.notFound')), 0) }) }) } diff --git a/src/components/common/note-loading-boundary/note-loading-boundary.tsx b/src/components/common/note-loading-boundary/note-loading-boundary.tsx index 8886af232..c0eece89f 100644 --- a/src/components/common/note-loading-boundary/note-loading-boundary.tsx +++ b/src/components/common/note-loading-boundary/note-loading-boundary.tsx @@ -9,6 +9,8 @@ import React, { Fragment } from 'react' import { useLoadNoteFromServer } from './hooks/use-load-note-from-server' import { LoadingScreen } from '../../application-loader/loading-screen/loading-screen' import { CommonErrorPage } from '../../error-pages/common-error-page' +import { CreateNonExistingNoteHint } from './create-non-existing-note-hint' +import { ShowIf } from '../show-if/show-if' /** * Loads the note identified by the note-id in the URL. @@ -24,10 +26,11 @@ export const NoteLoadingBoundary: React.FC> = ({ chil return } else if (error) { return ( - + + + + + ) } else { return {children} diff --git a/src/components/common/redirect.tsx b/src/components/common/redirect.tsx index 87946fab4..78e550866 100644 --- a/src/components/common/redirect.tsx +++ b/src/components/common/redirect.tsx @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * SPDX-FileCopyrightText: 2022 The HedgeDoc developers (see AUTHORS file) * * SPDX-License-Identifier: AGPL-3.0-only */ @@ -8,6 +8,7 @@ import Link from 'next/link' import React, { useEffect } from 'react' import { useRouter } from 'next/router' import { Logger } from '../../utils/logger' +import { testId } from '../../utils/test-id' export interface RedirectProps { to: string @@ -24,13 +25,13 @@ export const Redirect: React.FC = ({ to }) => { const router = useRouter() useEffect(() => { - router.push(to).catch((error: Error) => { + router?.push(to).catch((error: Error) => { logger.error(`Error while redirecting to ${to}`, error) }) }) return ( - + Redirecting to{' '} {to} diff --git a/src/components/error-pages/common-error-page.tsx b/src/components/error-pages/common-error-page.tsx index 2a29170a6..681154bf6 100644 --- a/src/components/error-pages/common-error-page.tsx +++ b/src/components/error-pages/common-error-page.tsx @@ -4,10 +4,11 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -import React from 'react' import type { PropsWithChildren } from 'react' +import React from 'react' import { LandingLayout } from '../landing-layout/landing-layout' -import { useTranslation } from 'react-i18next' +import { Trans, useTranslation } from 'react-i18next' +import { ShowIf } from '../common/show-if/show-if' export interface CommonErrorPageProps { titleI18nKey: string @@ -26,17 +27,20 @@ export const CommonErrorPage: React.FC> descriptionI18nKey, children }) => { - const { t } = useTranslation() + useTranslation() return ( -
-
-

{t(titleI18nKey)}

-
- {descriptionI18nKey ? t(descriptionI18nKey) : null} - {children} -
+
+

+ +

+ +

+ +

+
+ {children}
) diff --git a/src/pages/[id].tsx b/src/pages/[id].tsx index 808b97d03..7d6c4cd82 100644 --- a/src/pages/[id].tsx +++ b/src/pages/[id].tsx @@ -10,7 +10,7 @@ import type { NextPage } from 'next' import { useAsync } from 'react-use' import { Redirect } from '../components/common/redirect' import { useSingleStringUrlParameter } from '../hooks/common/use-single-string-url-parameter' -import { CommonErrorPage } from '../components/error-pages/common-error-page' +import Custom404 from './404' /** * Redirects the user to the editor if the link is a root level direct link to a version 1 note. @@ -30,7 +30,7 @@ export const DirectLinkFallback: NextPage = () => { }) if (error !== undefined) { - return + return } else if (value !== undefined) { return } else {