Skip to content

Commit

Permalink
fix: handle removed articles linked in announcements and missing arti…
Browse files Browse the repository at this point in the history 
…cles for cms users (#1057)

* fix: announcement with CTA to deleted article links to 404

* fix: return 404 if article not found and cms user
  • Loading branch information
@gidjin
gidjin authored Jun 30, 2023
1 parent eec8564 commit e9a2459
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 84 deletions.
49 changes: 49 additions & 0 deletions src/__fixtures__/data/cmsAnnouncments.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,3 +190,52 @@ export const testAnnouncementWithArticleNoSlug = {
status: 'Published',
publishedDate: '2022-05-17T13:44:39.796Z',
}

export const testAnnouncementWithDeletedArticle = {
id: 'anotherTest123',
title: 'Cool new deleted article',
body: {
document: [
{
type: 'paragraph',
children: [
{
text: 'Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor. Aenean massa. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Donec quam felis, ultricies nec, pellentesque eu, pretium quis, sem. Nulla consequat mas',
},
],
},
{
type: 'component-block',
props: {
link: {
// missing article data, like when the article is deleted but the announcement still refers to it
value: {},
discriminant: 'article',
},
ctaText: 'View deleted article',
},
children: [
{
type: 'component-inline-prop',
children: [
{
text: '',
},
],
},
],
component: 'callToAction',
},
{
type: 'paragraph',
children: [
{
text: '',
},
],
},
],
},
status: 'Published',
publishedDate: '2022-05-17T13:44:39.796Z',
}
155 changes: 81 additions & 74 deletions src/__tests__/pages/article.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,94 +72,101 @@ describe('Single article getServerSideProps', () => {
query: { article: 'test-article-slug' },
} as unknown as GetServerSidePropsContext

it('returns the article prop if the query returns a published article', async () => {
const response = await getServerSideProps(testContext)
expect(response).toEqual({
props: {
article: mockOrbitBlogArticle,
pageTitle: mockOrbitBlogArticle.title,
},
})
})

it('returns found if the query returns an unpublished article but user is a cms user', async () => {
const draftArticle = { ...mockOrbitBlogArticle, status: 'Draft' }
mockedKeystoneClient.query.mockResolvedValueOnce({
data: {
article: draftArticle,
},
loading: false,
errors: [],
networkStatus: 7,
describe('as portal user', () => {
test('returns the article prop if the query returns a published article', async () => {
const response = await getServerSideProps(testContext)
expect(response).toEqual({
props: {
article: mockOrbitBlogArticle,
pageTitle: mockOrbitBlogArticle.title,
},
})
})

mockedGetSession.mockImplementationOnce(() =>
Promise.resolve({ passport: { user: cmsUser } })
)
test('returns not found if the query returns an unpublished article', async () => {
mockedKeystoneClient.query.mockResolvedValueOnce({
data: {
article: { ...mockOrbitBlogArticle, status: 'Draft' },
},
loading: false,
errors: [],
networkStatus: 7,
})

const response = await getServerSideProps(testContext)
const response = await getServerSideProps(testContext)

expect(response).toEqual({
props: {
article: draftArticle,
pageTitle: mockOrbitBlogArticle.title,
},
expect(response).toEqual({
notFound: true,
})
})
})

it('returns not found if the query returns an unpublished article', async () => {
mockedKeystoneClient.query.mockResolvedValueOnce({
data: {
article: { ...mockOrbitBlogArticle, status: 'Draft' },
},
loading: false,
errors: [],
networkStatus: 7,
})
test('returns not found if the query returns an article published in the future', async () => {
const futureDate = DateTime.now().plus({ weeks: 2 })
mockedKeystoneClient.query.mockResolvedValueOnce({
data: {
article: {
...mockOrbitBlogArticle,
status: 'Published',
publishedDate: futureDate.toISO(),
},
},
loading: false,
errors: [],
networkStatus: 7,
})

const response = await getServerSideProps(testContext)
const response = await getServerSideProps(testContext)

expect(response).toEqual({
notFound: true,
expect(response).toEqual({
notFound: true,
})
})
})

it('returns not found if the query returns an article published in the future', async () => {
const futureDate = DateTime.now().plus({ weeks: 2 })
mockedKeystoneClient.query.mockResolvedValueOnce({
data: {
article: {
...mockOrbitBlogArticle,
status: 'Published',
publishedDate: futureDate.toISO(),
},
},
loading: false,
errors: [],
networkStatus: 7,
describe('as cms user', () => {
beforeEach(() => {
mockedGetSession.mockReset()
mockedGetSession.mockImplementationOnce(() =>
Promise.resolve({ passport: { user: cmsUser } })
)
})

const response = await getServerSideProps(testContext)
test('returns found if the query returns an unpublished article', async () => {
const draftArticle = { ...mockOrbitBlogArticle, status: 'Draft' }
mockedKeystoneClient.query.mockResolvedValueOnce({
data: {
article: draftArticle,
},
loading: false,
errors: [],
networkStatus: 7,
})

expect(response).toEqual({
notFound: true,
})
})
const response = await getServerSideProps(testContext)

it('returns not found if no article is found by the query', async () => {
mockedKeystoneClient.query.mockResolvedValueOnce({
data: {
article: null,
},
loading: false,
errors: [],
networkStatus: 7,
expect(response).toEqual({
props: {
article: draftArticle,
pageTitle: mockOrbitBlogArticle.title,
},
})
})

const response = await getServerSideProps(testContext)
test('returns not found if the query returns no article', async () => {
mockedKeystoneClient.query.mockResolvedValueOnce({
data: {
article: null,
},
loading: false,
errors: [],
networkStatus: 7,
})

expect(response).toEqual({
notFound: true,
const response = await getServerSideProps(testContext)

expect(response).toEqual({
notFound: true,
})
})
})
})
Expand All @@ -172,19 +179,19 @@ describe('Single article page', () => {
})
})

it('renders the loader while fetching the user', () => {
test('renders the loader while fetching the user', () => {
expect(screen.getByText('Content is loading...')).toBeInTheDocument()
})

it('redirects to the login page if not logged in', async () => {
test('redirects to the login page if not logged in', async () => {
await waitFor(() => {
expect(mockReplace).toHaveBeenCalledWith('/login')
})
})
})

describe('when logged in', () => {
it('renders an ORBIT Blog article', async () => {
test('renders an ORBIT Blog article', async () => {
renderWithAuth(<SingleArticlePage article={mockOrbitBlogArticle} />)

expect(
Expand All @@ -197,7 +204,7 @@ describe('Single article page', () => {
expect(await screen.findAllByRole('article')).toHaveLength(1)
})

it('renders an Internal News article', async () => {
test('renders an Internal News article', async () => {
renderWithAuth(<SingleArticlePage article={cmsInternalNewsArticle} />)

expect(
Expand Down
22 changes: 18 additions & 4 deletions src/components/AnnouncementInfo/AnnouncementInfo.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,31 +9,45 @@ import AnnouncementInfo from './AnnouncementInfo'
import {
testAnnouncementWithArticle,
testAnnouncementWithArticleNoSlug,
testAnnouncementWithDeletedArticle,
testAnnouncementWithUrl,
} from '__fixtures__/data/cmsAnnouncments'

describe('AnnouncementInfo component', () => {
it('renders an announcement with a url CTA', () => {
test('renders an announcement with a url CTA', () => {
render(<AnnouncementInfo announcement={testAnnouncementWithUrl} />)

expect(screen.getAllByText('Test Announcement')).toHaveLength(1)
expect(screen.getAllByText('View more')).toHaveLength(1)
})

it('renders an announcement with a default url if none is provided', () => {
test('renders an announcement with a default url if none is provided', () => {
render(
<AnnouncementInfo announcement={testAnnouncementWithArticleNoSlug} />
)
expect(screen.getAllByText('Cool new article')).toHaveLength(1)

const link = screen.getByRole('link', { name: 'View article' })
expect(link).toHaveAttribute('href', '/')
expect(link).toHaveAttribute('href', '/404')
})

it('renders an announcement with an article CTA', () => {
test('renders an announcement with an article CTA', () => {
render(<AnnouncementInfo announcement={testAnnouncementWithArticle} />)

expect(screen.getAllByText('Cool new article')).toHaveLength(1)
expect(screen.getAllByText('View article')).toHaveLength(1)
const link = screen.getByRole('link', { name: 'View article' })
expect(link).toHaveAttribute('href', '/articles/something')
})

test('renders an announcement with a missing article CTA', () => {
render(
<AnnouncementInfo announcement={testAnnouncementWithDeletedArticle} />
)

expect(screen.getAllByText('Cool new deleted article')).toHaveLength(1)
expect(screen.getAllByText('View deleted article')).toHaveLength(1)
const link = screen.getByRole('link', { name: 'View deleted article' })
expect(link).toHaveAttribute('href', '/404')
})
})
4 changes: 2 additions & 2 deletions src/components/AnnouncementInfo/AnnouncementInfo.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ const AnnouncementInfo = ({
{props.link.discriminant === 'article' && (
<LinkTo
href={
props.link.value.data.slug
props.link.value.data?.slug
? `/articles/${props.link.value.data.slug}`
: '/'
: '/404'
}
target="_blank"
rel="noreferrer"
Expand Down
7 changes: 3 additions & 4 deletions src/pages/articles/[article].tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,10 @@ const SingleArticlePage = ({
article,
}: InferGetServerSidePropsType<typeof getServerSideProps>) => {
const { user } = useUser()
const { category } = article

return (
<>
{category === 'ORBITBlog' ? (
{article?.category === 'ORBITBlog' ? (
<ORBITBlogArticleHeader />
) : (
<InternalNewsArticleHeader />
Expand Down Expand Up @@ -85,10 +84,10 @@ export const getServerSideProps: GetServerSideProps = async (context) => {
variables: { slug },
})

// if article is not published return 404
// if article is not published or not found return 404
// unless the current user is a CMS user or admin
// then allow them to see any article
if (!isPublished(article) && !isCmsUser(user)) {
if (!article || (!isPublished(article) && !isCmsUser(user))) {
return {
notFound: true,
}
Expand Down

0 comments on commit e9a2459

Please sign in to comment.