Skip to content
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.

Sign up for GitHub

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

Show publishing api errors in publishing drawer #1367

Merged
merged 7 commits into from
Jun 9, 2022
Merged

Show publishing api errors in publishing drawer #1367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
46e6a3f
add PublishingOption component
ChristopherChudzicki May 18, 2022
c37c7e4
tweak unique url error message
ChristopherChudzicki May 18, 2022
3251185
improve IntegrationTestHelper
ChristopherChudzicki May 19, 2022
c158de6
show form errors in PublishForm
ChristopherChudzicki May 19, 2022
f212943
remove old error test
ChristopherChudzicki May 19, 2022
2bbdb7c
fix PublishForm import
ChristopherChudzicki May 19, 2022
fbf007d
format python changes
ChristopherChudzicki May 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
187 changes: 114 additions & 73 deletions static/js/components/PublishDrawer.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,36 @@
import React from "react"
import moment from "moment"
import sinon, { SinonStub } from "sinon"
import { act } from "react-dom/test-utils"
import { isEmpty } from "ramda"

import { siteApiActionUrl, siteApiDetailUrl } from "../lib/urls"
import { shouldIf } from "../test_util"
import { assertInstanceOf, shouldIf } from "../test_util"
import { makeWebsiteDetail } from "../util/factories/websites"
import IntegrationTestHelper, {
import IntegrationTestHelperOld, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there something that IntegrationTestHelperOld can test that the new one can't?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. But the old one uses Enzyme, and the new one uses @testing-library/react ("RTL") (and other packges in the @testing-library family). RTL has a somewhat different API than Enzyme, so converting tests will be a bit of effort.

I've been trying to write new tests with RTL + the new helper, and have been converting old tests when refactoring significantly. (Which is natural, since the Enzyme tests tend to be a bit brittle, so refactoring tends to break them).

TestRenderer
} from "../util/integration_test_helper_old"

import App from "../pages/App"
import { IntegrationTestHelper } from "../testing_utils"

import PublishDrawer from "./PublishDrawer"

import { Website } from "../types/websites"
import userEvent from "@testing-library/user-event"
import { waitFor, screen } from "@testing-library/react"
import * as dom from "@testing-library/dom"
import _ from "lodash"

describe("PublishDrawer", () => {
let helper: IntegrationTestHelper,
let helper: IntegrationTestHelperOld,
website: Website,
render: TestRenderer,
toggleVisibilityStub: SinonStub,
refreshWebsiteStub: SinonStub

beforeEach(() => {
helper = new IntegrationTestHelper()
helper = new IntegrationTestHelperOld()
toggleVisibilityStub = helper.sandbox.stub()
website = {
...makeWebsiteDetail(),
Expand Down Expand Up @@ -109,13 +118,13 @@ describe("PublishDrawer", () => {
it("renders the date and url", async () => {
const { wrapper } = await render()
await act(async () => {
// @ts-ignore
// @ts-expect-error
wrapper
.find("input[type='radio']")
// @ts-ignore
// @ts-expect-error
.at(idx)
// @ts-ignore
.prop("onChange")()
// @ts-expect-error
.prop("onChange")({ target: { checked: true } })
})
wrapper.update()
expect(wrapper.find(".publish-option-description").text()).toContain(
Expand All @@ -137,13 +146,13 @@ describe("PublishDrawer", () => {
it("renders the publish status", async () => {
const { wrapper } = await render()
await act(async () => {
// @ts-ignore
// @ts-expect-error
wrapper
.find("input[type='radio']")
// @ts-ignore
// @ts-expect-error
.at(idx)
// @ts-ignore
.prop("onChange")()
// @ts-expect-error
.prop("onChange")({ target: { checked: true } })
})
wrapper.update()
expect(wrapper.find("PublishStatusIndicator").prop("status")).toBe(
Expand All @@ -161,13 +170,13 @@ describe("PublishDrawer", () => {
website[publishDateField] = null
const { wrapper } = await render()
await act(async () => {
// @ts-ignore
// @ts-expect-error
wrapper
.find("input[type='radio']")
// @ts-ignore
// @ts-expect-error
.at(idx)
// @ts-ignore
.prop("onChange")()
// @ts-expect-error
.prop("onChange")({ target: { checked: true } })
})
wrapper.update()
expect(wrapper.find(".publish-option-description").text()).toContain(
Expand All @@ -178,20 +187,19 @@ describe("PublishDrawer", () => {
it("has an option with the right label", async () => {
const { wrapper } = await render()
await act(async () => {
// @ts-ignore
// @ts-expect-error
wrapper
.find("input[type='radio']")
// @ts-ignore
// @ts-expect-error
.at(idx)
// @ts-ignore
.prop("onChange")()
// @ts-expect-error
.prop("onChange")({ target: { checked: true } })
})
wrapper.update()
// @ts-ignore
expect(
wrapper
.find(".publish-option label")
// @ts-ignore
// @ts-expect-error
.at(idx)
.text()
).toBe(label)
Expand All @@ -201,8 +209,9 @@ describe("PublishDrawer", () => {
website[unpublishedField] = false
const { wrapper } = await render()
await act(async () => {
// @ts-ignore
wrapper.find(`#publish-${action}`).prop("onChange")()
const onChange = wrapper.find(`#publish-${action}`).prop("onChange")
// @ts-expect-error
onChange({ target: { checked: true } })
})
wrapper.update()
expect(wrapper.find(".btn-publish").prop("disabled")).toBe(true)
Expand All @@ -220,8 +229,9 @@ describe("PublishDrawer", () => {
website[unpublishedField] = true
const { wrapper } = await render()
await act(async () => {
// @ts-ignore
wrapper.find(`#publish-${action}`).prop("onChange")()
const onChange = wrapper.find(`#publish-${action}`).prop("onChange")
// @ts-expect-error
onChange({ target: { checked: true } })
})
wrapper.update()
expect(wrapper.find(".publish-option-description").text()).toContain(
Expand All @@ -242,50 +252,6 @@ describe("PublishDrawer", () => {
})
})

it("renders an error message if the publish didn't work", async () => {
const actionStub = helper.mockPostRequest(
siteApiActionUrl
.param({
name: website.name,
action: api
})
.toString(),
{},
500
)
const { wrapper } = await render()
await act(async () => {
// @ts-ignore
wrapper.find(`#publish-${action}`).prop("onChange")()
})
wrapper.update()
await act(async () => {
// @ts-ignore
wrapper
.find("PublishForm")
.find(".btn-publish")
.simulate("submit")
})
wrapper.update()
expect(wrapper.find(".publish-option-description").text()).toContain(
"We apologize, there was an error publishing the site. Please try again in a few minutes."
)
sinon.assert.calledOnceWithExactly(
actionStub,
`/api/websites/${website.name}/${api}/`,
"POST",
{
body: {
url_path: website.url_path
},
headers: { "X-CSRFTOKEN": "" },
credentials: undefined
}
)
sinon.assert.notCalled(refreshWebsiteStub)
sinon.assert.notCalled(toggleVisibilityStub)
})

it("publish button sends the expected request", async () => {
const actionStub = helper.mockPostRequest(
siteApiActionUrl
Expand All @@ -300,8 +266,9 @@ describe("PublishDrawer", () => {
)
const { wrapper } = await render()
await act(async () => {
// @ts-ignore
wrapper.find(`#publish-${action}`).prop("onChange")()
const onChange = wrapper.find(`#publish-${action}`).prop("onChange")
// @ts-expect-error
onChange({ target: { checked: true } })
})
wrapper.update()
expect(
Expand All @@ -311,7 +278,6 @@ describe("PublishDrawer", () => {
.prop("disabled")
).toBeFalsy()
await act(async () => {
// @ts-ignore
wrapper
.find("PublishForm")
.find(".btn-publish")
Expand Down Expand Up @@ -345,3 +311,78 @@ describe("PublishDrawer", () => {
}
)
})

describe.each([
{
publishToLabel: "Production",
api: "publish"
},
{
publishToLabel: "Staging",
api: "preview"
}
])("Publishing Drawer Errors ($publishToLabel)", ({ publishToLabel, api }) => {
const setup = (websiteDetails: Partial<Website> = {}) => {
const website = makeWebsiteDetail({ is_admin: true, ...websiteDetails })
const user = userEvent.setup()
const helper = new IntegrationTestHelper(`/sites/${website.name}?publish=`)
helper.mockGetWebsiteDetail(website)
const publishUrl = siteApiActionUrl
.param({ name: website.name, action: api })
.toString()
const setPublishResult = _.partial(helper.mockPostRequest, publishUrl)
const [result] = helper.render(<App />)
return { user, result, setPublishResult }
}

it("renders a generic error message if publihing API failed", async () => {
const { user, result, setPublishResult } = setup()
const dialog = await screen.findByRole("dialog")
const envButton = await dom.findByText(dialog, publishToLabel)
await act(() => user.click(envButton))

const publishButton = await dom.findByText(dialog, "Publish")
const form = dialog.querySelector("form")
expect(form).toContainElement(publishButton)
expect(publishButton).toHaveAttribute("type", "submit")
assertInstanceOf(form, HTMLFormElement)

setPublishResult(undefined, 500)
act(() => form.submit())
await waitFor(() => {
expect(dialog).toHaveTextContent(
"We apologize, there was a problem publishing your site."
)
})
result.unmount()
})

it("renders a specific error for url issues", async () => {
const { user, result, setPublishResult } = setup({
publish_date: null,
url_path: "some_path_in_use"
})
const dialog = await screen.findByRole("dialog")
const envButton = await dom.findByText(dialog, publishToLabel)
await act(() => user.click(envButton))

const publishButton = await dom.findByText(dialog, "Publish")
const form = dialog.querySelector("form")
expect(form).toContainElement(publishButton)
expect(publishButton).toHaveAttribute("type", "submit")
assertInstanceOf(form, HTMLFormElement)

setPublishResult({ url_path: "Some url error" }, 400)
act(() => form.submit())

await waitFor(() => {
expect(dialog).toHaveTextContent(
"We apologize, there was a problem publishing your site."
)
})
const errorMsg = await dom.findByText(dialog, "Some url error")
assertInstanceOf(errorMsg.previousSibling, HTMLInputElement)
expect(errorMsg.previousSibling.name).toBe("url_path")
result.unmount()
})
})
Loading