Skip to content

Commit

Permalink
Merge pull request #745 from Financial-Times/current-path-override
Browse files Browse the repository at this point in the history
feat: allow passing in of getCurrentPath option
  • Loading branch information
alexnaish committed Feb 5, 2020
2 parents b296b36 + 01fec6f commit 1ccb06c
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 22 deletions.
11 changes: 6 additions & 5 deletions examples/ft-ui/__test__/logged-in-user.test.js
Expand Up @@ -7,11 +7,12 @@ describe('examples/ft-ui', () => {
describe('Header link elements', () => {
it('renders the expected loggin-in user Header link elements', async () => {
await expect(page).toMatchElement('.o-header__top-column--right a[href="/myft"]', { text: 'myFT' })
await expect(
page
).toMatchElement('.o-header__nav-list--right a[href="https://myaccount.ft.com/details/core/view"]', {
text: 'Account Settings'
})
await expect(page).toMatchElement(
'.o-header__nav-list--right a[href="https://myaccount.ft.com/details/core/view"]',
{
text: 'Account Settings'
}
)
await expect(
page
).toMatchElement(
Expand Down
10 changes: 5 additions & 5 deletions package.json
Expand Up @@ -47,11 +47,11 @@
"@babel/core": "^7.8.0",
"@babel/preset-react": "^7.8.0",
"@babel/preset-typescript": "^7.8.0",
"@financial-times/athloi": "^1.0.0-beta.17",
"@storybook/addon-knobs": "^5.3.1",
"@storybook/addon-options": "^5.3.1",
"@storybook/addon-viewport": "^5.3.1",
"@storybook/react": "^5.3.1",
"@financial-times/athloi": "^1.0.0-beta.28",
"@storybook/addon-knobs": "^5.3.12",
"@storybook/addon-options": "^5.3.12",
"@storybook/addon-viewport": "^5.3.12",
"@storybook/react": "^5.3.12",
"@types/jest": "^25.0.0",
"@types/node": "^10.12.26",
"@types/react": "^16.8.20",
Expand Down
8 changes: 8 additions & 0 deletions packages/dotcom-middleware-navigation/readme.md
Expand Up @@ -45,6 +45,14 @@ The middleware accepts the following parameters. All options will be passed alon

Enables fetching hierarchical navigation data for the current path including any parent and child pages. Defaults to `false`.

### `getCurrentPath`

Enables overriding of the default current path logic. Defaults to:

```js
(request) => normalizePath(request.get('ft-vanity-url') || request.path)
```

### `interval`

See the [FT navigation documentation] for more details.
Expand Down
Expand Up @@ -136,6 +136,36 @@ describe('dotcom-middleware-navigation', () => {
})
})

describe('when handling a request with a custom getCurrentPath', () => {
it('executes the provided getCurrentPath function', async () => {
request = httpMocks.createRequest({
path: '/path/to/page/123',
headers: {
'ft-vanity-url': '/vanity-url?page=2'
}
})
const dummyPath = '/foo'
const instance = subject.init({ getCurrentPath: () => dummyPath })
await instance(request, response, next)

expect(FakeNavigation.getMenusFor).toHaveBeenCalledWith(dummyPath, 'uk')
})

it('allows overriding of how to calculate current path logic', async () => {
request = httpMocks.createRequest({
path: '/path/to/page/123',
headers: {
'ft-blocked-url': '/ig-content-test'
}
})

const instance = subject.init({ getCurrentPath: (request) => request.get('ft-blocked-url') })
await instance(request, response, next)

expect(FakeNavigation.getMenusFor).toHaveBeenCalledWith('/ig-content-test', 'uk')
})
})

describe('when something goes wrong', () => {
beforeEach(() => {
FakeNavigation.getMenusFor = jest.fn(() => {
Expand Down
6 changes: 4 additions & 2 deletions packages/dotcom-middleware-navigation/src/navigation.ts
Expand Up @@ -6,10 +6,12 @@ import normalizePath from './normalizePath'

type MiddlewareOptions = TNavOptions & {
enableSubNavigation?: boolean
getCurrentPath?: Function
}

const defaultOptions: MiddlewareOptions = {
enableSubNavigation: false
enableSubNavigation: false,
getCurrentPath: (request) => normalizePath(request.get('ft-vanity-url') || request.path)
}

export const init = (userOptions: MiddlewareOptions = {}) => {
Expand All @@ -26,7 +28,7 @@ export const init = (userOptions: MiddlewareOptions = {}) => {
// rather than the underlying path, so prefer that when available.
// <https://github.com/Financial-Times/ft.com-cdn/blob/4841fbf100e1c561a2f6729b9921ec12bb6b837c/src/vcl/next-preflight.vcl#L213-L219>
// NOTE: Next router sets the vanity header inc. any query string so it must be normalized.
const currentPath = normalizePath(request.get('ft-vanity-url') || request.path)
const currentPath = options.getCurrentPath(request)
const currentEdition = handleEdition(request, response)

const [menusData, subNavigationData] = await Promise.all([
Expand Down
4 changes: 1 addition & 3 deletions packages/dotcom-ui-header/src/index.tsx
Expand Up @@ -90,9 +90,7 @@ Header.defaultProps = defaultProps
function LogoOnly(props: Pick<THeaderProps, 'variant' | 'showLogoLink'>) {
return (
<HeaderWrapper {...props}>
<TopWrapper>
{props.showLogoLink ? <TopColumnCenter /> : <TopColumnCenterNoLink />}
</TopWrapper>
<TopWrapper>{props.showLogoLink ? <TopColumnCenter /> : <TopColumnCenterNoLink />}</TopWrapper>
</HeaderWrapper>
)
}
Expand Down
4 changes: 1 addition & 3 deletions packages/dotcom-ui-layout/src/components/Layout.tsx
Expand Up @@ -80,9 +80,7 @@ export function Layout({
}

return (
<div
className={`n-layout ${fontLoadingClassNames.join(' ')}`}
data-o-component="o-typography">
<div className={`n-layout ${fontLoadingClassNames.join(' ')}`} data-o-component="o-typography">
<EnhanceFonts />
<a
data-trackable="a11y-skip-to-help"
Expand Down
5 changes: 1 addition & 4 deletions scripts/generate-storybook-config.js
Expand Up @@ -27,8 +27,5 @@ function getPaths() {
function prepareConfigFileContents(paths) {
const storiesContents = paths.map((p) => `require('${p}')`).join('\n\t')

return fs
.readFileSync(TEMPLATE_FILE, { encoding: 'utf8' })
.replace('/* PLACEHOLDER */', storiesContents)
return fs.readFileSync(TEMPLATE_FILE, { encoding: 'utf8' }).replace('/* PLACEHOLDER */', storiesContents)
}

0 comments on commit 1ccb06c

Please sign in to comment.