Moving the history instance creation after the plugin initialization (to honor the SiteURL config) (#2109)

* Moving the history instance creation after the plugin initialization (to honor the SiteURL config) * Fixing welcome page images urls generation * Fixing share board url generation * Fixing more subpath problems * Adding some tests with subpath * Adding subpath test to welcome page * fix linter error
2022-01-17 20:21:25 +01:00 · 2022-01-17 20:21:25 +01:00 · 02dfb6eceb
commit 02dfb6eceb
parent 1aa60009ea
9 changed files with 324 additions and 66 deletions
--- a/mattermost-plugin/webapp/webpack.config.js
+++ b/mattermost-plugin/webapp/webpack.config.js
@ -118,7 +118,7 @@ module.exports = {
                        options: {
                            name: '[name].[ext]',
                            outputPath: 'static',
-                            publicPath: '/plugins/focalboard/static/',
+                            publicPath: '/static/',
                        },
                    },
                    {
--- a/webapp/src/app.tsx
+++ b/webapp/src/app.tsx
@ -1,6 +1,6 @@
 // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
 // See LICENSE.txt for license information.
-import React, {useEffect} from 'react'
+import React, {useEffect, useMemo} from 'react'
 import {
    Router,
    Redirect,
@ -40,11 +40,19 @@ import {UserSettings} from './userSettings'

 declare let window: IAppWindow

-export const history = createBrowserHistory({basename: Utils.getFrontendBaseURL()})
-
 const UUID_REGEX = new RegExp(/^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$/)

-if (Utils.isDesktop() && Utils.isFocalboardPlugin()) {
+const App = React.memo((): JSX.Element => {
+    const language = useAppSelector<string>(getLanguage)
+    const loggedIn = useAppSelector<boolean|null>(getLoggedIn)
+    const globalError = useAppSelector<string>(getGlobalError)
+    const me = useAppSelector<IUser|null>(getMe)
+    const dispatch = useAppDispatch()
+
+    const browserHistory: ReturnType<typeof createBrowserHistory> = useMemo(() => {
+        const history = createBrowserHistory({basename: Utils.getFrontendBaseURL()})
+
+        if (Utils.isDesktop() && Utils.isFocalboardPlugin()) {
            window.addEventListener('message', (event: MessageEvent) => {
                if (event.origin !== window.location.origin) {
                    return
@ -58,9 +66,8 @@ if (Utils.isDesktop() && Utils.isFocalboardPlugin()) {
                Utils.log(`Navigating Boards to ${pathName}`)
                history.replace(pathName.replace(window.frontendBaseURL, ''))
            })
-}
-
-const browserHistory: typeof history = {
+        }
+        return {
            ...history,
            push: (path: string, state?: unknown) => {
                if (Utils.isDesktop() && Utils.isFocalboardPlugin()) {
@ -77,14 +84,8 @@ const browserHistory: typeof history = {
                    history.push(path, state)
                }
            },
-}
-
-const App = React.memo((): JSX.Element => {
-    const language = useAppSelector<string>(getLanguage)
-    const loggedIn = useAppSelector<boolean|null>(getLoggedIn)
-    const globalError = useAppSelector<string>(getGlobalError)
-    const me = useAppSelector<IUser|null>(getMe)
-    const dispatch = useAppDispatch()
+        }
+    }, [])

    useEffect(() => {
        dispatch(fetchLanguage())
@ -95,7 +96,7 @@ const App = React.memo((): JSX.Element => {
    if (Utils.isFocalboardPlugin()) {
        useEffect(() => {
            if (window.frontendBaseURL) {
-                history.replace(window.location.pathname.replace(window.frontendBaseURL, ''))
+                browserHistory.replace(window.location.pathname.replace(window.frontendBaseURL, ''))
            }
        }, [])
    }
@ -137,9 +138,7 @@ const App = React.memo((): JSX.Element => {
        >
            <DndProvider backend={Utils.isMobile() ? TouchBackend : HTML5Backend}>
                <FlashMessages milliseconds={2000}/>
-                <Router
-                    history={browserHistory}
-                >
+                <Router history={browserHistory}>
                    <div id='frame'>
                        <div id='main'>
                            <NewVersionBanner/>
--- a/webapp/src/components/snapshots/shareBoardComponent.test.tsx.snap
+++ b/webapp/src/components/snapshots/shareBoardComponent.test.tsx.snap
@ -41,11 +41,11 @@ exports[`src/components/shareBoardComponent return shareBoardComponent and click
      >
        <a
          class="shareUrl"
-          href="http://localhost/plugins/focalboard/workspace/1/shared/1/1?r=oneToken"
+          href="http://localhost/workspace/1/shared/1/1?r=oneToken"
          rel="noreferrer"
          target="_blank"
        >
-          http://localhost/plugins/focalboard/workspace/1/shared/1/1?r=oneToken
+          http://localhost/workspace/1/shared/1/1?r=oneToken
        </a>
        <button
          type="button"
@ -112,11 +112,11 @@ exports[`src/components/shareBoardComponent return shareBoardComponent and click
      >
        <a
          class="shareUrl"
-          href="http://localhost/plugins/focalboard/workspace/1/shared/1/1?r=oneToken"
+          href="http://localhost/workspace/1/shared/1/1?r=oneToken"
          rel="noreferrer"
          target="_blank"
        >
-          http://localhost/plugins/focalboard/workspace/1/shared/1/1?r=oneToken
+          http://localhost/workspace/1/shared/1/1?r=oneToken
        </a>
        <button
          type="button"
@ -183,11 +183,11 @@ exports[`src/components/shareBoardComponent return shareBoardComponent and click
      >
        <a
          class="shareUrl"
-          href="http://localhost/plugins/focalboard/workspace/1/shared/1/1?r=oneToken"
+          href="http://localhost/workspace/1/shared/1/1?r=oneToken"
          rel="noreferrer"
          target="_blank"
        >
-          http://localhost/plugins/focalboard/workspace/1/shared/1/1?r=oneToken
+          http://localhost/workspace/1/shared/1/1?r=oneToken
        </a>
        <button
          type="button"
@ -254,11 +254,11 @@ exports[`src/components/shareBoardComponent return shareBoardComponent and click
      >
        <a
          class="shareUrl"
-          href="http://localhost/plugins/focalboard/workspace/1/shared/1/1?r=aToken"
+          href="http://localhost/workspace/1/shared/1/1?r=aToken"
          rel="noreferrer"
          target="_blank"
        >
-          http://localhost/plugins/focalboard/workspace/1/shared/1/1?r=aToken
+          http://localhost/workspace/1/shared/1/1?r=aToken
        </a>
        <button
          type="button"
@ -366,11 +366,82 @@ exports[`src/components/shareBoardComponent should match snapshot with sharing 1
      >
        <a
          class="shareUrl"
-          href="http://localhost/plugins/focalboard/workspace/1/shared/1/1?r=oneToken"
+          href="http://localhost/workspace/1/shared/1/1?r=oneToken"
          rel="noreferrer"
          target="_blank"
        >
-          http://localhost/plugins/focalboard/workspace/1/shared/1/1?r=oneToken
+          http://localhost/workspace/1/shared/1/1?r=oneToken
+        </a>
+        <button
+          type="button"
+        >
+          <span>
+            Copy link
+          </span>
+        </button>
+      </div>
+      <div
+        class="row"
+      >
+        <button
+          type="button"
+        >
+          <span>
+            Regenerate token
+          </span>
+        </button>
+      </div>
+    </div>
+  </div>
+</div>
+`;
+
+exports[`src/components/shareBoardComponent should match snapshot with sharing and subpath 1`] = `
+<div>
+  <div
+    class="Modal bottom"
+  >
+    <div
+      class="toolbar hideOnWidescreen"
+    >
+      <button
+        aria-label="Close"
+        class="Button IconButton"
+        title="Close"
+        type="button"
+      >
+        <i
+          class="CompassIcon icon-close CloseIcon"
+        />
+      </button>
+    </div>
+    <div
+      class="ShareBoardComponent"
+    >
+      <div
+        class="row"
+      >
+        <div>
+          Anyone with the link can view this board and all cards in it.
+        </div>
+        <div
+          class="Switch on"
+        >
+          <div
+            class="octo-switch-inner"
+          />
+        </div>
+      </div>
+      <div
+        class="row"
+      >
+        <a
+          class="shareUrl"
+          href="http://localhost/test-subpath/plugins/boards/workspace/1/shared/1/1?r=oneToken"
+          rel="noreferrer"
+          target="_blank"
+        >
+          http://localhost/test-subpath/plugins/boards/workspace/1/shared/1/1?r=oneToken
        </a>
        <button
          type="button"
@ -466,3 +537,74 @@ exports[`src/components/shareBoardComponent should match snapshot with sharing a
  </div>
 </div>
 `;
+
+exports[`src/components/shareBoardComponent should match snapshot with sharing and without workspaceId and subpath 1`] = `
+<div>
+  <div
+    class="Modal bottom"
+  >
+    <div
+      class="toolbar hideOnWidescreen"
+    >
+      <button
+        aria-label="Close"
+        class="Button IconButton"
+        title="Close"
+        type="button"
+      >
+        <i
+          class="CompassIcon icon-close CloseIcon"
+        />
+      </button>
+    </div>
+    <div
+      class="ShareBoardComponent"
+    >
+      <div
+        class="row"
+      >
+        <div>
+          Anyone with the link can view this board and all cards in it.
+        </div>
+        <div
+          class="Switch on"
+        >
+          <div
+            class="octo-switch-inner"
+          />
+        </div>
+      </div>
+      <div
+        class="row"
+      >
+        <a
+          class="shareUrl"
+          href="http://localhost/test-subpath/plugins/boards/shared/1/1?r=oneToken"
+          rel="noreferrer"
+          target="_blank"
+        >
+          http://localhost/test-subpath/plugins/boards/shared/1/1?r=oneToken
+        </a>
+        <button
+          type="button"
+        >
+          <span>
+            Copy link
+          </span>
+        </button>
+      </div>
+      <div
+        class="row"
+      >
+        <button
+          type="button"
+        >
+          <span>
+            Regenerate token
+          </span>
+        </button>
+      </div>
+    </div>
+  </div>
+</div>
+`;
--- a/webapp/src/components/shareBoardComponent.test.tsx
+++ b/webapp/src/components/shareBoardComponent.test.tsx
@ -23,8 +23,10 @@ const viewId = boardId

 jest.mock('../octoClient')
 jest.mock('../utils')
+
 const mockedOctoClient = mocked(client, true)
 const mockedUtils = mocked(Utils, true)
+
 let params = {}
 jest.mock('react-router', () => {
    const originalModule = jest.requireActual('react-router')
@ -45,14 +47,24 @@ const board = TestBlockFactory.createBoard()
 board.id = boardId

 describe('src/components/shareBoardComponent', () => {
+    const w = (window as any)
+    const oldBaseURL = w.baseURL
+
    beforeEach(() => {
        jest.clearAllMocks()
+        mockedUtils.buildURL.mockImplementation((path) => (w.baseURL || '') + path)
+
        params = {
            boardId,
            viewId,
            workspaceId,
        }
    })
+
+    afterEach(() => {
+        w.baseURL = oldBaseURL
+    })
+
    test('should match snapshot', async () => {
        mockedOctoClient.getSharing.mockResolvedValue(undefined)
        let container
@ -215,4 +227,48 @@ describe('src/components/shareBoardComponent', () => {
        expect(mockedUtils.createGuid).toBeCalledTimes(1)
        expect(container).toMatchSnapshot()
    })
+
+    test('should match snapshot with sharing and without workspaceId and subpath', async () => {
+        w.baseURL = '/test-subpath/plugins/boards'
+        const sharing:ISharing = {
+            id: boardId,
+            enabled: true,
+            token: 'oneToken',
+        }
+        params = {
+            boardId,
+            viewId,
+        }
+        mockedOctoClient.getSharing.mockResolvedValue(sharing)
+        let container
+        await act(async () => {
+            const result = render(wrapDNDIntl(
+                <ShareBoardComponent
+                    boardId={board.id}
+                    onClose={jest.fn()}
+                />), {wrapper: MemoryRouter})
+            container = result.container
+        })
+        expect(container).toMatchSnapshot()
+    })
+
+    test('should match snapshot with sharing and subpath', async () => {
+        w.baseURL = '/test-subpath/plugins/boards'
+        const sharing:ISharing = {
+            id: boardId,
+            enabled: true,
+            token: 'oneToken',
+        }
+        mockedOctoClient.getSharing.mockResolvedValue(sharing)
+        let container
+        await act(async () => {
+            const result = render(wrapDNDIntl(
+                <ShareBoardComponent
+                    boardId={board.id}
+                    onClose={jest.fn()}
+                />), {wrapper: MemoryRouter})
+            container = result.container
+        })
+        expect(container).toMatchSnapshot()
+    })
 })
--- a/webapp/src/components/shareBoardComponent.tsx
+++ b/webapp/src/components/shareBoardComponent.tsx
@ -79,18 +79,18 @@ const ShareBoardComponent = React.memo((props: Props): JSX.Element => {
    shareUrl.searchParams.set('r', readToken)

    if (match.params.workspaceId) {
-        const newPath = generatePath('/plugins/focalboard/workspace/:workspaceId/shared/:boardId/:viewId', {
+        const newPath = generatePath('/workspace/:workspaceId/shared/:boardId/:viewId', {
            boardId: match.params.boardId,
            viewId: match.params.viewId,
            workspaceId: match.params.workspaceId,
        })
-        shareUrl.pathname = newPath
+        shareUrl.pathname = Utils.buildURL(newPath)
    } else {
        const newPath = generatePath('/shared/:boardId/:viewId', {
            boardId: match.params.boardId,
            viewId: match.params.viewId,
        })
-        shareUrl.pathname = newPath
+        shareUrl.pathname = Utils.buildURL(newPath)
    }

    return (
--- a/webapp/src/pages/welcome/snapshots/welcomePage.test.tsx.snap
+++ b/webapp/src/pages/welcome/snapshots/welcomePage.test.tsx.snap
@ -19,12 +19,54 @@ exports[`pages/welcome Welcome Page shows Explore Page 1`] = `
      <img
        alt="Boards Welcome Image"
        class="WelcomePage__image WelcomePage__image--large"
-        src="test-file-stub"
+        src="http://localhost/test-file-stub"
      />
      <img
        alt="Boards Welcome Image"
        class="WelcomePage__image WelcomePage__image--small"
-        src="test-file-stub"
+        src="http://localhost/test-file-stub"
+      />
+      <button
+        class="Button filled size--large"
+        type="button"
+      >
+        <span>
+          Explore
+        </span>
+        <i
+          class="CompassIcon icon-chevron-right Icon Icon--right"
+        />
+      </button>
+    </div>
+  </div>
+</div>
+`;
+
+exports[`pages/welcome Welcome Page shows Explore Page with subpath 1`] = `
+<div>
+  <div
+    class="WelcomePage"
+  >
+    <div>
+      <h1
+        class="text-heading9"
+      >
+        Welcome To Boards
+      </h1>
+      <div
+        class="WelcomePage__subtitle"
+      >
+        Boards is a project management tool that helps define, organize, track and manage work across teams, using a familiar kanban board view
+      </div>
+      <img
+        alt="Boards Welcome Image"
+        class="WelcomePage__image WelcomePage__image--large"
+        src="http://localhost/subpath/test-file-stub"
+      />
+      <img
+        alt="Boards Welcome Image"
+        class="WelcomePage__image WelcomePage__image--small"
+        src="http://localhost/subpath/test-file-stub"
      />
      <button
        class="Button filled size--large"
--- a/webapp/src/pages/welcome/welcomePage.test.tsx
+++ b/webapp/src/pages/welcome/welcomePage.test.tsx
@ -16,10 +16,17 @@ import {wrapIntl} from '../../testUtils'

 import WelcomePage from './welcomePage'

+const w = (window as any)
+const oldBaseURL = w.baseURL
+
 beforeEach(() => {
    UserSettings.welcomePageViewed = null
 })

+afterEach(() => {
+    w.baseURL = oldBaseURL
+})
+
 describe('pages/welcome', () => {
    const history = createMemoryHistory()
    test('Welcome Page shows Explore Page', () => {
@ -32,6 +39,17 @@ describe('pages/welcome', () => {
        expect(container).toMatchSnapshot()
    })

+    test('Welcome Page shows Explore Page with subpath', () => {
+        w.baseURL = '/subpath'
+        const {container} = render(wrapIntl(
+            <Router history={history}>
+                <WelcomePage/>
+            </Router>,
+        ))
+        expect(screen.getByText('Explore')).toBeDefined()
+        expect(container).toMatchSnapshot()
+    })
+
    test('Welcome Page shows Explore Page And Then Proceeds after Clicking Explore', () => {
        history.replace = jest.fn()
        render(wrapIntl(
--- a/webapp/src/pages/welcome/welcomePage.tsx
+++ b/webapp/src/pages/welcome/welcomePage.tsx
@ -11,6 +11,7 @@ import BoardWelcomeSmallPNG from '../../../static/boards-welcome-small.png'
 import Button from '../../widgets/buttons/button'
 import CompassIcon from '../../widgets/icons/compassIcon'
 import {UserSettings} from '../../userSettings'
+import {Utils} from '../../utils'

 import './welcomePage.scss'

@ -52,14 +53,14 @@ const WelcomePage = React.memo(() => {

                {/* This image will be rendered on large screens over 2000px */}
                <img
-                    src={BoardWelcomePNG}
+                    src={Utils.buildURL(BoardWelcomePNG, true)}
                    className='WelcomePage__image WelcomePage__image--large'
                    alt='Boards Welcome Image'
                />

                {/* This image will be rendered on small screens below 2000px */}
                <img
-                    src={BoardWelcomeSmallPNG}
+                    src={Utils.buildURL(BoardWelcomeSmallPNG, true)}
                    className='WelcomePage__image WelcomePage__image--small'
                    alt='Boards Welcome Image'
                />
--- a/webapp/src/utils.ts
+++ b/webapp/src/utils.ts
@ -36,7 +36,7 @@ enum IDType {
 class Utils {
    static createGuid(idType: IDType): string {
        const data = Utils.randomArray(16)
-        return idType + this.base32encode(data, false)
+        return idType + Utils.base32encode(data, false)
    }

    static blockTypeToIDType(blockType: string | undefined): IDType {
@ -128,10 +128,10 @@ class Utils {
    static canvas : HTMLCanvasElement | undefined
    static getTextWidth(displayText: string, fontDescriptor: string): number {
        if (displayText !== '') {
-            if (!this.canvas) {
-                this.canvas = document.createElement('canvas') as HTMLCanvasElement
+            if (!Utils.canvas) {
+                Utils.canvas = document.createElement('canvas') as HTMLCanvasElement
            }
-            const context = this.canvas.getContext('2d')
+            const context = Utils.canvas.getContext('2d')
            if (context) {
                context.font = fontDescriptor
                const metrics = context.measureText(displayText)
@ -500,7 +500,7 @@ class Utils {
    }

    static getFrontendBaseURL(absolute?: boolean): string {
-        let frontendBaseURL = window.frontendBaseURL || this.getBaseURL(absolute)
+        let frontendBaseURL = window.frontendBaseURL || Utils.getBaseURL(absolute)
        frontendBaseURL = frontendBaseURL.replace(/\/+$/, '')
        if (frontendBaseURL.indexOf('/') === 0) {
            frontendBaseURL = frontendBaseURL.slice(1)
@ -512,7 +512,7 @@ class Utils {
    }

    static buildURL(path: string, absolute?: boolean): string {
-        const baseURL = this.getBaseURL()
+        const baseURL = Utils.getBaseURL()
        let finalPath = baseURL + path
        if (path.indexOf('/') !== 0) {
            finalPath = baseURL + '/' + path