diff --git a/CHANGELOG.md b/CHANGELOG.md index e4baabc461..f7fbc90352 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,20 +13,13 @@ and this project adheres to - ♿(frontend) improve accessibility: - #1354 - ♿ improve accessibility by adding landmark roles to layout #1394 + - ✨ add document visible in list and openable via enter key #1365 + - ♿ add pdf outline property to enable bookmarks display #1368 ### Fixed - 🐛(backend) duplicate sub docs as root for reader users - -### Changed - -- ♿(frontend) improve accessibility: - - ✨ add document visible in list and openable via enter key #1365 - -### Changed - -- ♿(frontend) improve accessibility: - - ♿ add pdf outline property to enable bookmarks display #1368 +- 🐛(frontend) fix 404 page when reload 403 page #1402 ## [3.7.0] - 2025-09-12 diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-member-create.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-member-create.spec.ts index 4533910643..12d1b83bc5 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-member-create.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-member-create.spec.ts @@ -7,6 +7,7 @@ import { randomName, verifyDocName, } from './utils-common'; +import { connectOtherUserToDoc, updateRoleUser } from './utils-share'; import { createRootSubPage } from './utils-sub-pages'; test.describe('Document create member', () => { @@ -209,10 +210,6 @@ test.describe('Document create member', () => { await expect(userInvitation).toBeHidden(); }); -}); - -test.describe('Document create member: Multiple login', () => { - test.use({ storageState: { cookies: [], origins: [] } }); test('It creates a member from a request coming from a 403 page', async ({ page, @@ -220,9 +217,6 @@ test.describe('Document create member: Multiple login', () => { }) => { test.slow(); - await page.goto('/'); - await keyCloakSignIn(page, browserName); - const [docTitle] = await createDoc( page, 'Member access request', @@ -232,55 +226,37 @@ test.describe('Document create member: Multiple login', () => { await verifyDocName(page, docTitle); - const urlDoc = page.url(); - await page - .getByRole('button', { - name: 'Logout', - }) - .click(); - - const otherBrowser = BROWSERS.find((b) => b !== browserName); - - await keyCloakSignIn(page, otherBrowser!); + .locator('.ProseMirror') + .locator('.bn-block-outer') + .last() + .fill('Hello World'); - await expect(page.getByTestId('header-logo-link')).toBeVisible(); + const urlDoc = page.url(); - await page.goto(urlDoc); + // Other user will request access + const { otherPage, otherBrowserName, cleanup } = + await connectOtherUserToDoc(browserName, urlDoc); await expect( - page.getByText('Insufficient access rights to view the document.'), + otherPage.getByText('Insufficient access rights to view the document.'), ).toBeVisible({ timeout: 10000, }); - await page.getByRole('button', { name: 'Request access' }).click(); + await otherPage.getByRole('button', { name: 'Request access' }).click(); await expect( - page.getByText('Your access request for this document is pending.'), + otherPage.getByText('Your access request for this document is pending.'), ).toBeVisible(); - await page - .getByRole('button', { - name: 'Logout', - }) - .click(); - - await page.goto('/'); - await keyCloakSignIn(page, browserName); - - await expect(page.getByTestId('header-logo-link')).toBeVisible({ - timeout: 10000, - }); - - await page.goto(urlDoc); - + // First user approves the request await page.getByRole('button', { name: 'Share' }).click(); await expect(page.getByText('Access Requests')).toBeVisible(); - await expect(page.getByText(`E2E ${otherBrowser}`)).toBeVisible(); + await expect(page.getByText(`E2E ${otherBrowserName}`)).toBeVisible(); - const emailRequest = `user.test@${otherBrowser}.test`; + const emailRequest = `user.test@${otherBrowserName}.test`; await expect(page.getByText(emailRequest)).toBeVisible(); const container = page.getByTestId( `doc-share-access-request-row-${emailRequest}`, @@ -291,8 +267,26 @@ test.describe('Document create member: Multiple login', () => { await expect(page.getByText('Access Requests')).toBeHidden(); await expect(page.getByText('Share with 2 users')).toBeVisible(); - await expect(page.getByText(`E2E ${otherBrowser}`)).toBeVisible(); + await expect(page.getByText(`E2E ${otherBrowserName}`)).toBeVisible(); + + // Other user verifies he has access + await otherPage.reload(); + await verifyDocName(otherPage, docTitle); + await expect(otherPage.getByText('Hello World')).toBeVisible(); + + // Revoke access + await updateRoleUser(page, 'Remove access', emailRequest); + await expect( + otherPage.getByText('Insufficient access rights to view the document.'), + ).toBeVisible(); + + // Cleanup: other user logout + await cleanup(); }); +}); + +test.describe('Document create member: Multiple login', () => { + test.use({ storageState: { cookies: [], origins: [] } }); test('It cannot request member access on child doc on a 403 page', async ({ page, diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-routing.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-routing.spec.ts index 2c084eec7a..5800a66b37 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-routing.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-routing.spec.ts @@ -93,6 +93,12 @@ test.describe('Doc Routing', () => { await expect(page.getByText('Log in to access the document.')).toBeVisible({ timeout: 10000, }); + + await expect(page.locator('meta[name="robots"]')).toHaveAttribute( + 'content', + 'noindex', + ); + await expect(page).toHaveTitle(/401 Unauthorized - Docs/); }); }); diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-visibility.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-visibility.spec.ts index 37c8c5f2a2..003c9c0919 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-visibility.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-visibility.spec.ts @@ -7,6 +7,7 @@ import { keyCloakSignIn, verifyDocName, } from './utils-common'; +import { addNewMember, connectOtherUserToDoc } from './utils-share'; import { createRootSubPage } from './utils-sub-pages'; test.describe('Doc Visibility', () => { @@ -146,47 +147,31 @@ test.describe('Doc Visibility: Restricted', () => { await verifyDocName(page, docTitle); - await page.getByRole('button', { name: 'Share' }).click(); - - const inputSearch = page.getByRole('combobox', { - name: 'Quick search input', - }); - - const otherBrowser = BROWSERS.find((b) => b !== browserName); - if (!otherBrowser) { - throw new Error('No alternative browser found'); - } - const username = `user.test@${otherBrowser}.test`; - await inputSearch.fill(username); - await page.getByRole('option', { name: username }).click(); - - // Choose a role - const container = page.getByTestId('doc-share-add-member-list'); - await container.getByLabel('doc-role-dropdown').click(); - await page.getByLabel('Reader').click(); - - await page.getByRole('button', { name: 'Invite' }).click(); - - await page.locator('.c__modal__backdrop').click({ - position: { x: 0, y: 0 }, - }); + await page + .locator('.ProseMirror') + .locator('.bn-block-outer') + .last() + .fill('Hello World'); const urlDoc = page.url(); - await page - .getByRole('button', { - name: 'Logout', - }) - .click(); + const { otherBrowserName, otherPage } = await connectOtherUserToDoc( + browserName, + urlDoc, + ); - await keyCloakSignIn(page, otherBrowser); + await expect( + otherPage.getByText('Insufficient access rights to view the document.'), + ).toBeVisible({ + timeout: 10000, + }); - await expect(page.getByTestId('header-logo-link')).toBeVisible(); + await page.getByRole('button', { name: 'Share' }).click(); - await page.goto(urlDoc); + await addNewMember(page, 0, 'Reader', otherBrowserName); - await verifyDocName(page, docTitle); - await expect(page.getByLabel('Share button')).toBeVisible(); + await otherPage.reload(); + await expect(otherPage.getByText('Hello World')).toBeVisible(); }); }); diff --git a/src/frontend/apps/e2e/__tests__/app-impress/utils-common.ts b/src/frontend/apps/e2e/__tests__/app-impress/utils-common.ts index 25cb5b353f..4c82d8c7ac 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/utils-common.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/utils-common.ts @@ -1,6 +1,7 @@ import { Page, expect } from '@playwright/test'; -export const BROWSERS = ['chromium', 'webkit', 'firefox']; +export type BrowserName = 'chromium' | 'firefox' | 'webkit'; +export const BROWSERS: BrowserName[] = ['chromium', 'webkit', 'firefox']; export const CONFIG = { AI_FEATURE_ENABLED: true, diff --git a/src/frontend/apps/e2e/__tests__/app-impress/utils-share.ts b/src/frontend/apps/e2e/__tests__/app-impress/utils-share.ts index 32bf0d82d9..5fe90a4816 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/utils-share.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/utils-share.ts @@ -1,4 +1,11 @@ -import { Page, expect } from '@playwright/test'; +import { Page, chromium, expect } from '@playwright/test'; + +import { + BROWSERS, + BrowserName, + keyCloakSignIn, + verifyDocName, +} from './utils-common'; export type Role = 'Administrator' | 'Owner' | 'Member' | 'Editor' | 'Reader'; export type LinkReach = 'Private' | 'Connected' | 'Public'; @@ -61,6 +68,70 @@ export const updateShareLink = async ( } }; +export const updateRoleUser = async ( + page: Page, + role: Role | 'Remove access', + email: string, +) => { + const list = page.getByTestId('doc-share-quick-search'); + + const currentUser = list.getByTestId(`doc-share-member-row-${email}`); + const currentUserRole = currentUser.getByLabel('doc-role-dropdown'); + await currentUserRole.click(); + await page.getByLabel(role).click(); + await list.click(); +}; + +/** + * Connects another user to a document. + * Useful to test real-time collaboration features. + * @param browserName The name of the browser to use. + * @param docUrl The URL of the document to connect to. + * @param docTitle The title of the document (optional). + * @returns An object containing the other browser, context, and page. + */ +export const connectOtherUserToDoc = async ( + browserName: BrowserName, + docUrl: string, + docTitle?: string, +) => { + const otherBrowserName = BROWSERS.find((b) => b !== browserName); + if (!otherBrowserName) { + throw new Error('No alternative browser found'); + } + + const otherBrowser = await chromium.launch({ headless: true }); + const otherContext = await otherBrowser.newContext({ + locale: 'en-US', + timezoneId: 'Europe/Paris', + permissions: [], + storageState: { + cookies: [], + origins: [], + }, + }); + const otherPage = await otherContext.newPage(); + await otherPage.goto(docUrl); + + await otherPage.getByRole('button', { name: 'Login' }).click({ + timeout: 15000, + }); + + await keyCloakSignIn(otherPage, otherBrowserName, false); + + if (docTitle) { + await verifyDocName(otherPage, docTitle); + } + + const cleanup = async () => { + await otherPage.close(); + await otherContext.close(); + await otherBrowser.close(); + }; + + return { otherBrowser, otherContext, otherPage, otherBrowserName, cleanup }; +}; + export const mockedInvitations = async (page: Page, json?: object) => { let result = [ { diff --git a/src/frontend/apps/impress/src/pages/docs/[id]/403.tsx b/src/frontend/apps/impress/src/features/docs/doc-management/components/DocPage403.tsx similarity index 69% rename from src/frontend/apps/impress/src/pages/docs/[id]/403.tsx rename to src/frontend/apps/impress/src/features/docs/doc-management/components/DocPage403.tsx index 41fe571b1e..c479b3650d 100644 --- a/src/frontend/apps/impress/src/pages/docs/[id]/403.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-management/components/DocPage403.tsx @@ -1,50 +1,23 @@ import { Button } from '@openfun/cunningham-react'; import Head from 'next/head'; import Image from 'next/image'; -import { useRouter } from 'next/router'; import { useTranslation } from 'react-i18next'; import styled from 'styled-components'; import img403 from '@/assets/icons/icon-403.png'; import { Box, Icon, Loading, StyledLink, Text } from '@/components'; -import { DEFAULT_QUERY_RETRY } from '@/core'; -import { KEY_DOC, useDoc } from '@/docs/doc-management'; import { ButtonAccessRequest } from '@/docs/doc-share'; import { useDocAccessRequests } from '@/docs/doc-share/api/useDocAccessRequest'; -import { MainLayout } from '@/layouts'; -import { NextPageWithLayout } from '@/types/next'; const StyledButton = styled(Button)` width: fit-content; `; -export function DocLayout() { - const { - query: { id }, - } = useRouter(); - - if (typeof id !== 'string') { - return null; - } - - return ( - <> - - - - - - - - - ); -} - interface DocProps { id: string; } -const DocPage403 = ({ id }: DocProps) => { +export const DocPage403 = ({ id }: DocProps) => { const { t } = useTranslation(); const { data: requests, @@ -54,39 +27,19 @@ const DocPage403 = ({ id }: DocProps) => { docId: id, page: 1, }); - const { replace } = useRouter(); const hasRequested = !!requests?.results.find( (request) => request.document === id, ); - const { error: docError, isLoading: isLoadingDoc } = useDoc( - { id }, - { - staleTime: 0, - queryKey: [KEY_DOC, { id }], - retry: (failureCount, error) => { - if (error.status == 403) { - return false; - } else { - return failureCount < DEFAULT_QUERY_RETRY; - } - }, - }, - ); - - if (!isLoadingDoc && docError?.status !== 403) { - void replace(`/docs/${id}`); - return ; - } - - if (isLoadingDoc || isLoadingRequest) { + if (isLoadingRequest) { return ; } return ( <> + {t('Access Denied - Error 403')} - {t('Docs')} @@ -152,13 +105,3 @@ const DocPage403 = ({ id }: DocProps) => { ); }; - -const Page: NextPageWithLayout = () => { - return null; -}; - -Page.getLayout = function getLayout() { - return ; -}; - -export default Page; diff --git a/src/frontend/apps/impress/src/features/docs/doc-management/components/index.ts b/src/frontend/apps/impress/src/features/docs/doc-management/components/index.ts index 50da09e9fc..a3a6a6cef4 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-management/components/index.ts +++ b/src/frontend/apps/impress/src/features/docs/doc-management/components/index.ts @@ -1,2 +1,3 @@ +export * from './DocPage403'; export * from './ModalRemoveDoc'; export * from './SimpleDocItem'; diff --git a/src/frontend/apps/impress/src/features/docs/doc-management/stores/useProviderStore.tsx b/src/frontend/apps/impress/src/features/docs/doc-management/stores/useProviderStore.tsx index ae7ed15170..d6779f21ed 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-management/stores/useProviderStore.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-management/stores/useProviderStore.tsx @@ -13,11 +13,14 @@ export interface UseCollaborationStore { destroyProvider: () => void; provider: HocuspocusProvider | undefined; isConnected: boolean; + hasLostConnection: boolean; + resetLostConnection: () => void; } const defaultValues = { provider: undefined, isConnected: false, + hasLostConnection: false, }; export const useProviderStore = create((set, get) => ({ @@ -36,8 +39,15 @@ export const useProviderStore = create((set, get) => ({ name: storeId, document: doc, onStatus: ({ status }) => { - set({ - isConnected: status === WebSocketStatus.Connected, + set((state) => { + const nextConnected = status === WebSocketStatus.Connected; + return { + isConnected: nextConnected, + hasLostConnection: + state.isConnected && !nextConnected + ? true + : state.hasLostConnection, + }; }); }, }); @@ -56,4 +66,5 @@ export const useProviderStore = create((set, get) => ({ set(defaultValues); }, + resetLostConnection: () => set({ hasLostConnection: false }), })); diff --git a/src/frontend/apps/impress/src/pages/401.tsx b/src/frontend/apps/impress/src/pages/401.tsx index f6cab4064b..ac9eb50cf2 100644 --- a/src/frontend/apps/impress/src/pages/401.tsx +++ b/src/frontend/apps/impress/src/pages/401.tsx @@ -1,4 +1,5 @@ import { Button } from '@openfun/cunningham-react'; +import Head from 'next/head'; import Image from 'next/image'; import { useRouter } from 'next/router'; import { ReactElement, useEffect } from 'react'; @@ -22,32 +23,43 @@ const Page: NextPageWithLayout = () => { }, [authenticated, replace]); return ( - - {t('Image - - - - {t('Log in to access the document.')} - - - + <> + + + {`${t('401 Unauthorized')} - ${t('Docs')}`} + + + + {t('Image + + + + {t('Log in to access the document.')} + + + + - + ); }; diff --git a/src/frontend/apps/impress/src/pages/docs/[id]/index.tsx b/src/frontend/apps/impress/src/pages/docs/[id]/index.tsx index 73ed0a925e..86ce81e155 100644 --- a/src/frontend/apps/impress/src/pages/docs/[id]/index.tsx +++ b/src/frontend/apps/impress/src/pages/docs/[id]/index.tsx @@ -10,10 +10,12 @@ import { DEFAULT_QUERY_RETRY } from '@/core'; import { DocEditor } from '@/docs/doc-editor'; import { Doc, + DocPage403, KEY_DOC, useCollaboration, useDoc, useDocStore, + useProviderStore, } from '@/docs/doc-management/'; import { KEY_AUTH, setAuthUrl, useAuth } from '@/features/auth'; import { getDocChildren, subPageToTree } from '@/features/docs/doc-tree/'; @@ -56,6 +58,7 @@ interface DocProps { } const DocPage = ({ id }: DocProps) => { + const { hasLostConnection, resetLostConnection } = useProviderStore(); const { data: docQuery, isError, @@ -86,6 +89,14 @@ const DocPage = ({ id }: DocProps) => { const { t } = useTranslation(); const { authenticated } = useAuth(); + // Invalidate when provider store reports a lost connection + useEffect(() => { + if (hasLostConnection && doc?.id) { + queryClient.invalidateQueries({ queryKey: [KEY_DOC, { id: doc.id }] }); + resetLostConnection(); + } + }, [hasLostConnection, doc?.id, queryClient, resetLostConnection]); + useEffect(() => { if (!docQuery || isFetching) { return; @@ -118,7 +129,7 @@ const DocPage = ({ id }: DocProps) => { }, [addTask, doc?.id, queryClient]); if (isError && error) { - if ([403, 404, 401].includes(error.status)) { + if ([404, 401].includes(error.status)) { let replacePath = `/${error.status}`; if (error.status === 401) { @@ -129,8 +140,6 @@ const DocPage = ({ id }: DocProps) => { }); } setAuthUrl(); - } else if (error.status === 403) { - replacePath = `/docs/${id}/403`; } void replace(replacePath); @@ -138,6 +147,10 @@ const DocPage = ({ id }: DocProps) => { return ; } + if (error.status === 403) { + return ; + } + return (