-
Notifications
You must be signed in to change notification settings - Fork 274
WS-200: Migrate LiveRadio pages to NextJS - Simorgh changes #13928
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 9 commits
dca3e9e
af894ff
d0ff5ab
6cd9942
c98c92f
6c0b601
8a9bc03
92b9b1b
eb5c005
5d011f4
a5b9fda
e2225b2
5dbd991
698a103
23484b7
ca97e42
0f60440
66b9af1
a4399d4
dbd4e20
181e4c7
dd527e4
e5d073d
b290709
e7b1800
0900698
c4e38de
02488bd
b5d24d7
dc6d4c0
8ce38a9
2178537
b20b75a
95d2438
1235125
bd9a46f
52adc51
914d2e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ import { | |
| HOME_PAGE, | ||
| AUDIO_PAGE, | ||
| TV_PAGE, | ||
| LIVE_RADIO_PAGE, | ||
| } from '#app/routes/utils/pageTypes'; | ||
| import { PageTypes } from '#app/models/types/global'; | ||
| import PageDataParams from '#app/models/types/pageDataParams'; | ||
|
|
@@ -22,6 +23,7 @@ import withOptimizelyProvider from '#app/legacy/containers/PageHandlers/withOpti | |
| import { HomePageProps } from '#app/pages/HomePage/HomePage'; | ||
| import { getEnvConfig } from '#app/lib/utilities/getEnvConfig'; | ||
| import derivePageType from '#nextjs/utilities/derivePageType'; | ||
| import { LiveRadioPageProps } from '#app/pages/LiveRadioPage/types'; | ||
|
|
||
| // AV Embeds | ||
| import withMediaError from '#app/lib/utilities/episodeAvailability/withMediaError'; | ||
|
|
@@ -39,6 +41,8 @@ import handleOnDemandAudioRoute from './onDemandAudio/handleOnDemandAudioRoute'; | |
| import { OnDemandAudioProps } from './onDemandAudio/types'; | ||
| // On Demand TV | ||
| import handleOnDemandTvRoute from './onDemandTv/handleOnDemandTvRoute'; | ||
| // Live Radio | ||
| import handleLiveRadioRoute from './liveRadio/handleLiveRadioRoute'; | ||
|
|
||
| // Dynamic imports of page layouts | ||
| const AvEmbedsPageLayout = dynamic( | ||
|
|
@@ -56,6 +60,10 @@ const OnDemandTvPage = dynamic( | |
| () => import('#app/pages/OnDemandTvPage/OnDemandTvPage'), | ||
| ); | ||
|
|
||
| const LiveRadioPage = dynamic( | ||
| () => import('#app/pages/LiveRadioPage/LiveRadioPage'), | ||
| ); | ||
|
|
||
| const getPageType = ({ | ||
| resolvedUrl, | ||
| reqHeaders, | ||
|
|
@@ -87,6 +95,7 @@ const ROUTE_HANDLERS = { | |
| [HOME_PAGE]: handleHomepageRoute, | ||
| [AUDIO_PAGE]: handleOnDemandAudioRoute, | ||
| [TV_PAGE]: handleOnDemandTvRoute, | ||
| [LIVE_RADIO_PAGE]: handleLiveRadioRoute, | ||
| }; | ||
|
|
||
| export const getServerSideProps: GetServerSideProps = async context => { | ||
|
|
@@ -129,7 +138,8 @@ type PageProps = { | |
| ArticlePageProps & | ||
| HomePageProps & | ||
| OnDemandAudioProps & | ||
| OnDemandTVProps; | ||
| OnDemandTVProps & | ||
| LiveRadioPageProps; | ||
|
Comment on lines
139
to
+142
|
||
|
|
||
| export default function PageTypeToRender({ pageType, ...props }: PageProps) { | ||
| switch (pageType) { | ||
|
|
@@ -149,6 +159,8 @@ export default function PageTypeToRender({ pageType, ...props }: PageProps) { | |
| return withMediaError(OnDemandAudioPage)({ ...props }); | ||
| case TV_PAGE: | ||
| return withMediaError(OnDemandTvPage)({ ...props }); | ||
| case LIVE_RADIO_PAGE: | ||
| return withMediaError(LiveRadioPage)({ ...props }); | ||
| // Home Page | ||
| case HOME_PAGE: | ||
| return withOptimizelyProvider(HomePage)({ ...props }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,194 @@ | ||
| import { GetServerSidePropsContext } from 'next'; | ||
| import liveRadioJson from '#data/korean/bbc_korean_radio/liveradio.json'; | ||
| import { LIVE_RADIO_PAGE } from '#app/routes/utils/pageTypes'; | ||
| import { Toggles } from '#app/models/types/global'; | ||
| import * as getTogglesModule from '#app/lib/utilities/getToggles/withCache'; | ||
| import * as isTest from '#app/lib/utilities/isTest'; | ||
| import * as getPageDataModule from '../../../utilities/pageRequests/getPageData'; | ||
| import handleLiveRadioRoute from './handleLiveRadioRoute'; | ||
|
|
||
| jest.mock('../../../utilities/pageRequests/getPageData'); | ||
| jest.mock('#app/lib/utilities/getToggles/withCache'); | ||
|
|
||
| jest.mock('#app/lib/utilities/isTest', () => { | ||
| const originalModule = jest.requireActual('#app/lib/utilities/isTest'); | ||
| return { | ||
| __esModule: true, | ||
| ...originalModule, | ||
| }; | ||
| }); | ||
|
Isabella-Mitchell marked this conversation as resolved.
Outdated
|
||
|
|
||
| describe('handleLiveRadioRoute', () => { | ||
| const mockSetHeader = jest.fn(); | ||
| const mockGetServerSidePropsContext = { | ||
| req: { | ||
| headers: {}, | ||
| } as unknown as GetServerSidePropsContext['req'], | ||
| res: { | ||
| setHeader: mockSetHeader, | ||
| removeHeader: jest.fn(), | ||
| } as unknown as GetServerSidePropsContext['res'], | ||
| resolvedUrl: '/korean/bbc_korean_radio/liveradio', | ||
| query: { service: 'korean' }, | ||
| } satisfies GetServerSidePropsContext; | ||
|
|
||
| let getPageDataSpy: jest.SpyInstance; | ||
|
|
||
| beforeEach(() => { | ||
| jest.restoreAllMocks(); | ||
| jest.clearAllMocks(); | ||
| getPageDataSpy = jest | ||
| .spyOn(getPageDataModule, 'default') | ||
| .mockResolvedValue({ | ||
| data: { | ||
| pageData: liveRadioJson.data, | ||
| status: 200, | ||
| }, | ||
| }); | ||
|
|
||
| jest.spyOn(getTogglesModule, 'default').mockResolvedValue({ | ||
| liveRadioSchedule: { enabled: true }, | ||
| } as Toggles); | ||
| }); | ||
|
|
||
| it('returns expected props if data fetch succeeds', async () => { | ||
| const { props } = await handleLiveRadioRoute(mockGetServerSidePropsContext); | ||
| expect(props.status).toEqual(200); | ||
| expect(props.pageType).toEqual('liveRadio'); | ||
| expect(props.pageData).toEqual(liveRadioJson.data); | ||
| }); | ||
|
|
||
| it('should return essential data for a page to render', async () => { | ||
| const { | ||
| props: { pageData }, | ||
| } = await handleLiveRadioRoute(mockGetServerSidePropsContext); | ||
|
|
||
| expect(pageData.name).toEqual('BBC 코리아 라디오'); | ||
| expect(pageData.language).toEqual('ko'); | ||
| expect(pageData.metadata.type).toEqual('Live Radio'); | ||
| expect(pageData.summary).toEqual( | ||
| '세계와 한반도 뉴스를 공정하고 객관적으로 전달해 드립니다', | ||
| ); | ||
| expect(pageData.heading).toEqual('BBC 코리아 라디오'); | ||
| expect(pageData.bodySummary).toEqual( | ||
| '세계와 한반도 뉴스를 공정하고 객관적으로 전달해 드립니다', | ||
| ); | ||
| expect(pageData.masterBrand).toEqual('bbc_korean_radio'); | ||
| }); | ||
|
|
||
| it('should call getPageData with the correct arguments', async () => { | ||
| await handleLiveRadioRoute(mockGetServerSidePropsContext); | ||
|
|
||
| expect(getPageDataSpy).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| id: '/korean/bbc_korean_radio/liveradio', | ||
| service: 'korean', | ||
| pageType: LIVE_RADIO_PAGE, | ||
| resolvedUrl: '/korean/bbc_korean_radio/liveradio', | ||
| }), | ||
| ); | ||
| }); | ||
|
|
||
| it('should pass disableRadioSchedule as true when toggle is disabled', async () => { | ||
| jest.spyOn(getTogglesModule, 'default').mockResolvedValue({ | ||
| liveRadioSchedule: { enabled: false }, | ||
| } as Toggles); | ||
|
|
||
| await handleLiveRadioRoute(mockGetServerSidePropsContext); | ||
|
|
||
| expect(getPageDataSpy).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| disableRadioSchedule: true, | ||
| }), | ||
| ); | ||
| }); | ||
|
|
||
| it('should pass disableRadioSchedule as false when toggle is enabled', async () => { | ||
| await handleLiveRadioRoute(mockGetServerSidePropsContext); | ||
|
|
||
| expect(getPageDataSpy).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| disableRadioSchedule: false, | ||
| }), | ||
| ); | ||
| }); | ||
|
|
||
| it('returns error props if data fetch returns 500', async () => { | ||
| jest.spyOn(getPageDataModule, 'default').mockResolvedValue({ | ||
| data: { | ||
| pageData: liveRadioJson.data, | ||
| status: 500, | ||
| }, | ||
| }); | ||
|
|
||
| const result = await handleLiveRadioRoute(mockGetServerSidePropsContext); | ||
|
|
||
| expect(result).toEqual({ | ||
| props: expect.objectContaining({ | ||
| status: 500, | ||
| pageType: 'liveRadio', | ||
| pathname: '/korean/bbc_korean_radio/liveradio', | ||
| }), | ||
| }); | ||
| }); | ||
|
|
||
| it('returns error props if data fetch returns 404', async () => { | ||
| jest.spyOn(getPageDataModule, 'default').mockResolvedValue({ | ||
| data: { | ||
| pageData: liveRadioJson.data, | ||
| status: 404, | ||
| }, | ||
| }); | ||
|
|
||
| const result = await handleLiveRadioRoute(mockGetServerSidePropsContext); | ||
|
|
||
| expect(result).toEqual({ | ||
| props: expect.objectContaining({ | ||
| status: 404, | ||
| pageType: 'liveRadio', | ||
| pathname: '/korean/bbc_korean_radio/liveradio', | ||
| }), | ||
| }); | ||
| }); | ||
|
|
||
| it('throws if pageData is missing', async () => { | ||
| jest.spyOn(getPageDataModule, 'default').mockResolvedValue({ | ||
| data: { pageData: null, status: 200 }, | ||
| }); | ||
|
|
||
| await expect( | ||
| handleLiveRadioRoute(mockGetServerSidePropsContext), | ||
| ).rejects.toThrow('LiveRadioPage data is malformed'); | ||
| }); | ||
|
|
||
| it('sets correct cache-control header', async () => { | ||
| await handleLiveRadioRoute(mockGetServerSidePropsContext); | ||
|
|
||
| expect(mockSetHeader).toHaveBeenCalledWith( | ||
| 'Cache-Control', | ||
| expect.stringContaining('max-age=30'), | ||
| ); | ||
| }); | ||
|
|
||
| it('returns not found props when service is invalid', async () => { | ||
| const result = await handleLiveRadioRoute({ | ||
| ...mockGetServerSidePropsContext, | ||
| resolvedUrl: '/fakeservice', | ||
| }); | ||
|
|
||
| expect(result).toEqual({ | ||
| props: expect.objectContaining({ status: 404 }), | ||
| }); | ||
| }); | ||
|
|
||
| it('should render live assets on test environments', async () => { | ||
| jest.spyOn(isTest, 'default').mockReturnValueOnce(true); | ||
| const pageDataSpy = jest.spyOn(getPageDataModule, 'default'); | ||
|
|
||
| await handleLiveRadioRoute(mockGetServerSidePropsContext); | ||
|
|
||
| expect(pageDataSpy).toHaveBeenCalledWith( | ||
| expect.objectContaining({ rendererEnv: 'live' }), | ||
| ); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exported type name
LiveRadioPagePropsdescribes the shape ofpageData(language/name/summary/etc), not the React component props object. This is already causing confusion in the Next.js catch-all route types. Consider renaming this back to something likeLiveRadioPageData(and, if needed, introducing a separateLiveRadioPagePropstype that wraps{ pageData: LiveRadioPageData }).