diff --git a/src/actions/summitdoc-actions.js b/src/actions/summitdoc-actions.js index 478811d18..92e10dedc 100644 --- a/src/actions/summitdoc-actions.js +++ b/src/actions/summitdoc-actions.js @@ -12,21 +12,19 @@ * */ import T from "i18n-react/dist/i18n-react"; import { - getRequest, - deleteRequest, - createAction, - stopLoading, - startLoading, - showMessage, - showSuccessMessage, authErrorHandler, + createAction, + deleteRequest, + escapeFilterValue, + getRequest, + postFile, postRequest, putRequest, - postFile, - escapeFilterValue + snackbarSuccessHandler, + startLoading, + stopLoading } from "openstack-uicore-foundation/lib/utils/actions"; import { getAccessTokenSafely, wrapFormFile } from "../utils/methods"; -import history from "../history"; import { DEFAULT_PER_PAGE } from "../utils/constants"; export const REQUEST_SUMMITDOCS = "REQUEST_SUMMITDOCS"; @@ -49,13 +47,12 @@ export const getSummitDocs = orderDir = 1 ) => async (dispatch, getState) => { + dispatch(startLoading()); const { currentSummitState } = getState(); const accessToken = await getAccessTokenSafely(); const { currentSummit } = currentSummitState; const filter = []; - dispatch(startLoading()); - if (term) { const escapedTerm = escapeFilterValue(term); filter.push(`name=@${escapedTerm}`); @@ -65,9 +62,10 @@ export const getSummitDocs = page, per_page: perPage, access_token: accessToken, - expand: "event_types", - relations: "event_types.none", - fields: "id,description,label,event_types.id,event_types.name" + expand: "event_types,selection_plan", + relations: "event_types.none,selection_plan.none", + fields: + "id,description,label,event_types.id,event_types.name,selection_plan.name,show_always" }; if (filter.length > 0) { @@ -85,19 +83,20 @@ export const getSummitDocs = createAction(RECEIVE_SUMMITDOCS), `${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/summit-documents`, authErrorHandler, - { order, orderDir, term } - )(params)(dispatch).then(() => { - dispatch(stopLoading()); - }); + { order, orderDir, term, currentPage: page, perPage } + )(params)(dispatch) + .finally(() => { + dispatch(stopLoading()); + }) + .catch(() => {}); }; export const getSummitDoc = (summitDocId) => async (dispatch, getState) => { + dispatch(startLoading()); const { currentSummitState } = getState(); const accessToken = await getAccessTokenSafely(); const { currentSummit } = currentSummitState; - dispatch(startLoading()); - const params = { access_token: accessToken }; @@ -107,9 +106,11 @@ export const getSummitDoc = (summitDocId) => async (dispatch, getState) => { createAction(RECEIVE_SUMMITDOC), `${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/summit-documents/${summitDocId}`, authErrorHandler - )(params)(dispatch).then(() => { - dispatch(stopLoading()); - }); + )(params)(dispatch) + .finally(() => { + dispatch(stopLoading()); + }) + .catch(() => {}); }; export const resetSummitDocForm = () => (dispatch) => { @@ -117,34 +118,34 @@ export const resetSummitDocForm = () => (dispatch) => { }; export const addFileToDoc = (entity, file) => async (dispatch, getState) => { + dispatch(startLoading()); const { currentSummitState } = getState(); const accessToken = await getAccessTokenSafely(); const { currentSummit } = currentSummitState; - dispatch(startLoading()); - const params = { access_token: accessToken }; - postRequest( + return postRequest( null, createAction(SUMMITDOC_FILE_ADDED), `${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/summit-documents/${entity.id}/file`, wrapFormFile(file), authErrorHandler - )(params)(dispatch).then(() => { - dispatch(stopLoading()); - }); + )(params)(dispatch) + .finally(() => { + dispatch(stopLoading()); + }) + .catch(() => {}); }; export const removeFileFromDoc = (entity) => async (dispatch, getState) => { + dispatch(startLoading()); const { currentSummitState } = getState(); const accessToken = await getAccessTokenSafely(); const { currentSummit } = currentSummitState; - dispatch(startLoading()); - const params = { access_token: accessToken }; @@ -155,60 +156,65 @@ export const removeFileFromDoc = (entity) => async (dispatch, getState) => { `${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/summit-documents/${entity.id}/file`, null, authErrorHandler - )(params)(dispatch).then(() => { - dispatch(stopLoading()); - }); + )(params)(dispatch) + .finally(() => { + dispatch(stopLoading()); + }) + .catch(() => {}); }; +// TODO: replace with snackbarErrorHandler once it handles 401s (re-login redirect) correctly. export const saveSummitDoc = (entity, file) => async (dispatch, getState) => { + dispatch(startLoading()); const { currentSummitState } = getState(); const accessToken = await getAccessTokenSafely(); const { currentSummit } = currentSummitState; - dispatch(startLoading()); - const normalizedEntity = normalizeEntity(entity); const params = { access_token: accessToken }; if (entity.id) { - putRequest( + return putRequest( createAction(UPDATE_SUMMITDOC), createAction(SUMMITDOC_UPDATED), `${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/summit-documents/${entity.id}`, normalizedEntity, authErrorHandler, entity - )(params)(dispatch).then(() => { - dispatch(showSuccessMessage(T.translate("summitdoc.saved"))); - }); - } else { - const successMessage = { - title: T.translate("general.done"), - html: T.translate("summitdoc.created"), - type: "success" - }; + )(params)(dispatch) + .then(() => { + dispatch( + snackbarSuccessHandler({ + title: T.translate("general.done"), + html: T.translate("summitdoc.saved") + }) + ); + }) + .finally(() => dispatch(stopLoading())); + } - postFile( - createAction(UPDATE_SUMMITDOC), - createAction(SUMMITDOC_ADDED), - `${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/summit-documents`, - file, - normalizedEntity, - authErrorHandler, - entity - )(params)(dispatch).then((payload) => { + return postFile( + createAction(UPDATE_SUMMITDOC), + createAction(SUMMITDOC_ADDED), + `${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/summit-documents`, + file, + normalizedEntity, + authErrorHandler, + entity + )(params)(dispatch) + .then(() => { dispatch( - showMessage(successMessage, () => { - history.push( - `/app/summits/${currentSummit.id}/summitdocs/${payload.response.id}` - ); + snackbarSuccessHandler({ + title: T.translate("general.done"), + html: T.translate("summitdoc.created") }) ); - }); - } + }) + .finally(() => dispatch(stopLoading())); }; export const deleteSummitDoc = (summitDocId) => async (dispatch, getState) => { + dispatch(startLoading()); const { currentSummitState } = getState(); const accessToken = await getAccessTokenSafely(); const { currentSummit } = currentSummitState; @@ -223,9 +229,11 @@ export const deleteSummitDoc = (summitDocId) => async (dispatch, getState) => { `${window.API_BASE_URL}/api/v1/summits/${currentSummit.id}/summit-documents/${summitDocId}`, null, authErrorHandler - )(params)(dispatch).then(() => { - dispatch(stopLoading()); - }); + )(params)(dispatch) + .finally(() => { + dispatch(stopLoading()); + }) + .catch(() => {}); }; const normalizeEntity = (entity) => { diff --git a/src/components/forms/__tests__/summitdoc-form.test.js b/src/components/forms/__tests__/summitdoc-form.test.js new file mode 100644 index 000000000..57161b5cd --- /dev/null +++ b/src/components/forms/__tests__/summitdoc-form.test.js @@ -0,0 +1,367 @@ +// ---- Mocks must come first ---- + +// jsdom does not implement scrollIntoView; polyfill so the errors effect +// (which calls scrollToError -> firstNode.scrollIntoView) does not throw. +window.HTMLElement.prototype.scrollIntoView = jest.fn(); + +jest.mock("i18n-react/dist/i18n-react", () => ({ + __esModule: true, + default: { translate: (key) => key } +})); + +jest.mock( + "openstack-uicore-foundation/lib/components/inputs/upload-input", + () => ({ + __esModule: true, + default: ({ value, handleUpload, handleRemove, disabled }) => ( +
+ + {value && ( + + )} +
+ ) + }) +); + +// The vendor Formik-input wrappers (openstack-uicore-foundation) own their own +// error/FormHelperText rendering and have their own tests; here we only need +// them to read/write real Formik state, mirroring company-form.test.js and +// payment-profile-dialog.test.js's mocking convention for these components. +jest.mock( + "openstack-uicore-foundation/lib/components/mui/formik-inputs/textfield", + () => { + const React = require("react"); + const { useField } = require("formik"); + return { + __esModule: true, + default: function MockMuiFormikTextField({ name, disabled }) { + const [field] = useField(name); + return ( + + ); + } + }; + } +); + +jest.mock( + "openstack-uicore-foundation/lib/components/mui/formik-inputs/select", + () => { + const React = require("react"); + const { useField } = require("formik"); + return { + __esModule: true, + default: function MockMuiFormikSelect({ name, disabled }) { + const [field] = useField(name); + return ( +
+ {JSON.stringify(field.value)} +
+ ); + } + }; + } +); + +jest.mock( + "openstack-uicore-foundation/lib/components/mui/formik-inputs/checkbox", + () => { + const React = require("react"); + const { useField } = require("formik"); + return { + __esModule: true, + default: function MockMuiFormikCheckbox({ name, label, ...props }) { + const [field] = useField({ name, type: "checkbox" }); + return ( + + ); + } + }; + } +); + +// ---- Now imports ---- +/* eslint-disable import/first */ +import React, { useState } from "react"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import "@testing-library/jest-dom"; +import { FormikProvider, useFormik } from "formik"; +import SummitDocForm from "../summitdoc-form"; +import { + buildValues, + validationSchema +} from "../../../pages/summitdocs/edit-summitdoc-page"; +/* eslint-enable import/first */ + +const currentSummit = { + event_types: [ + { id: 1, name: "Keynote" }, + { id: 2, name: "Panel" } + ], + selection_plans: [{ id: 10, name: "Plan A" }] +}; + +const VALID_ENTITY = { + id: 0, + name: "A doc", + label: "A label", + description: "A description", + event_types: [1], + file_preview: "", + selection_plan_id: null, + show_always: false, + web_link: "" +}; + +// Mirrors the formik wiring edit-summitdoc-page.js provides in production, so +// SummitDocForm's useFormikContext() has a real context to read from. +const Harness = ({ + entity, + onSubmit = jest.fn(), + addFileToDoc = jest.fn(), + removeFileFromDoc = jest.fn() +}) => { + const [file, setFile] = useState(null); + const formik = useFormik({ + initialValues: buildValues(entity), + validationSchema, + onSubmit: (values) => onSubmit(values, file) + }); + + return ( + + + +
{JSON.stringify(formik.values)}
+
{JSON.stringify(formik.errors)}
+
+ ); +}; + +const readFormikValues = () => + JSON.parse(screen.getByTestId("debug-values").textContent); +const readFormikErrors = () => + JSON.parse(screen.getByTestId("debug-errors").textContent); + +const clickSave = () => + userEvent.click(screen.getByRole("button", { name: "general.save" })); + +describe("SummitDocForm", () => { + it("clears and disables event types when show_always is checked", async () => { + render(); + + expect(screen.getByTestId("select-event_types")).toHaveTextContent("[1]"); + + await userEvent.click( + screen.getByRole("checkbox", { name: "summitdoc.show_always" }) + ); + + expect(readFormikValues().event_types).toEqual([]); + expect(screen.getByTestId("select-event_types")).toHaveAttribute( + "data-disabled", + "true" + ); + }); + + it("disables the file upload when a web link is entered", async () => { + render(); + + await userEvent.type( + screen.getByTestId("textfield-web_link"), + "http://example.com" + ); + + expect(screen.getByRole("button", { name: "upload-file" })).toBeDisabled(); + }); + + it("disables the web link field once a file is present", () => { + render( + + ); + + expect(screen.getByTestId("textfield-web_link")).toBeDisabled(); + }); + + it("shows an existing doc's server-side file and disables web_link for it", () => { + render( + + ); + + expect( + screen.getByRole("button", { name: "remove-file" }) + ).toBeInTheDocument(); + expect(screen.getByTestId("textfield-web_link")).toBeDisabled(); + }); + + it("holds the file in local state for a new doc and submits it together with the entity on save", async () => { + const onSubmit = jest.fn(); + const addFileToDoc = jest.fn(); + render( + + ); + + await userEvent.click(screen.getByRole("button", { name: "upload-file" })); + + expect(addFileToDoc).not.toHaveBeenCalled(); + expect( + screen.getByRole("button", { name: "remove-file" }) + ).toBeInTheDocument(); + + await clickSave(); + + expect(onSubmit).toHaveBeenCalledWith( + expect.objectContaining({ file_preview: "blob:new-file" }), + { preview: "blob:new-file" } + ); + }); + + it("uploads/removes the file directly against the API for an existing doc", async () => { + const addFileToDoc = jest.fn(); + const removeFileFromDoc = jest.fn(); + render( + + ); + + await userEvent.click(screen.getByRole("button", { name: "upload-file" })); + expect(addFileToDoc).toHaveBeenCalledWith( + expect.objectContaining({ id: 5 }), + { preview: "blob:new-file" } + ); + + await userEvent.click(screen.getByRole("button", { name: "remove-file" })); + expect(removeFileFromDoc).toHaveBeenCalledWith( + expect.objectContaining({ id: 5 }) + ); + }); + + it("blocks submit and surfaces required-field errors (including event_types) on an empty doc", async () => { + const onSubmit = jest.fn(); + render( + + ); + + await clickSave(); + + const errors = readFormikErrors(); + expect(errors.name).toBeTruthy(); + expect(errors.label).toBeTruthy(); + expect(errors.description).toBeTruthy(); + expect(errors.event_types).toBeTruthy(); + expect(errors.web_link).toBeTruthy(); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it("does not require web_link when editing a doc that already has a server-side file", async () => { + const onSubmit = jest.fn(); + render( + + ); + + // Editing an unrelated field re-runs validation against the whole + // schema - this used to flash a false "web_link required" error + // because entity.file (the existing file) never reached formik state. + await userEvent.type( + screen.getByTestId("textfield-description"), + " updated" + ); + + expect(readFormikErrors().web_link).toBeUndefined(); + }); + + it("does not require event_types when show_always is checked, and submits successfully", async () => { + const onSubmit = jest.fn(); + render( + + ); + + await userEvent.click( + screen.getByRole("checkbox", { name: "summitdoc.show_always" }) + ); + await clickSave(); + + expect(readFormikErrors().event_types).toBeUndefined(); + expect(onSubmit).toHaveBeenCalledWith( + expect.objectContaining({ show_always: true, event_types: [] }), + null + ); + }); +}); diff --git a/src/components/forms/summitdoc-form.js b/src/components/forms/summitdoc-form.js index 3af66746b..74b4fd754 100644 --- a/src/components/forms/summitdoc-form.js +++ b/src/components/forms/summitdoc-form.js @@ -9,249 +9,227 @@ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. - **/ + * */ import React from "react"; +import PropTypes from "prop-types"; import T from "i18n-react/dist/i18n-react"; -import "awesome-bootstrap-checkbox/awesome-bootstrap-checkbox.css"; -import Dropdown from "openstack-uicore-foundation/lib/components/inputs/dropdown" -import Input from "openstack-uicore-foundation/lib/components/inputs/text-input" -import TextArea from "openstack-uicore-foundation/lib/components/inputs/textarea-input" +import { useFormikContext } from "formik"; +import Box from "@mui/material/Box"; +import { Grid2 } from "@mui/material"; +import MenuItem from "@mui/material/MenuItem"; +import Tooltip from "@mui/material/Tooltip"; +import InfoOutlinedIcon from "@mui/icons-material/InfoOutlined"; +import MuiFormikTextField from "openstack-uicore-foundation/lib/components/mui/formik-inputs/textfield"; +import MuiFormikSelect from "openstack-uicore-foundation/lib/components/mui/formik-inputs/select"; +import MuiFormikCheckbox from "openstack-uicore-foundation/lib/components/mui/formik-inputs/checkbox"; import UploadInput from "openstack-uicore-foundation/lib/components/inputs/upload-input"; -import { isEmpty, scrollToError, shallowEqual } from "../../utils/methods"; - -class SummitDocForm extends React.Component { - constructor(props) { - super(props); - - this.state = { - entity: { ...props.entity }, - errors: props.errors - }; - - this.handleChange = this.handleChange.bind(this); - this.handleSubmit = this.handleSubmit.bind(this); - this.handleUploadFile = this.handleUploadFile.bind(this); - this.handleRemoveFile = this.handleRemoveFile.bind(this); - } - - componentDidUpdate(prevProps, prevState, snapshot) { - const state = {}; - scrollToError(this.props.errors); - - if (!shallowEqual(prevProps.entity, this.props.entity)) { - state.entity = { ...this.props.entity }; - state.errors = {}; - } - - if (!shallowEqual(prevProps.errors, this.props.errors)) { - state.errors = { ...this.props.errors }; - } - - if (!isEmpty(state)) { - this.setState({ ...this.state, ...state }); - } - } - - handleChange(ev) { - let entity = { ...this.state.entity }; - let errors = { ...this.state.errors }; - let { value, id } = ev.target; - - if (ev.target.type === "checkbox") { - value = ev.target.checked; - } - - if (ev.target.type === "number") { - value = parseInt(ev.target.value); - } - - errors[id] = ""; - entity[id] = value; - - if (id === "show_always" && value) { - entity.event_types = []; - } - - this.setState({ entity: entity, errors: errors }); - } - - handleSubmit(ev) { - const { entity, file } = this.state; - ev.preventDefault(); - - this.props.onSubmit(entity, file); - } - - hasErrors(field) { - let { errors } = this.state; - if (field in errors) { - return errors[field]; - } - - return ""; - } - - handleUploadFile(file) { - let entity = { ...this.state.entity }; - - if (entity.id) { - this.props.addFileToDoc(entity, file); +import useScrollToError from "../../hooks/useScrollToError"; + +const SummitDocForm = ({ + currentSummit, + addFileToDoc, + removeFileFromDoc, + setFile +}) => { + const formik = useFormikContext(); + const { values, setFieldValue, setValues } = formik; + + useScrollToError(formik, true); + + const eventTypesDDL = currentSummit.event_types.map((et) => ({ + value: et.id, + label: et.name + })); + + const selectionPlansDDL = currentSummit.selection_plans.map((sp) => ({ + value: sp.id, + label: sp.name + })); + + const handleShowAlwaysChange = (ev) => { + const { checked } = ev.target; + // Update both fields in one call - two sequential setFieldValue calls + // each trigger their own validation pass against a stale snapshot of + // the other field, flashing a spurious "required" error on event_types. + setValues({ + ...values, + show_always: checked, + event_types: checked ? [] : values.event_types + }); + }; + + const handleUploadFile = (uploadedFile) => { + if (values.id) { + addFileToDoc(values, uploadedFile); } else { - entity.file_preview = file.preview; - this.setState({ file: file, entity: entity }); + setFieldValue("file_preview", uploadedFile.preview); + setFile(uploadedFile); } - } - - handleRemoveFile(ev) { - let entity = { ...this.state.entity }; + }; - if (entity.id) { - this.props.removeFileFromDoc(entity); + const handleRemoveFile = () => { + if (values.id) { + removeFileFromDoc(values); } else { - entity.file_preview = ""; - this.setState({ file: null, entity: entity }); + setFieldValue("file_preview", ""); + setFile(null); } - } - - render() { - const { entity } = this.state; - const { currentSummit } = this.props; - - let event_types_ddl = currentSummit.event_types.map((et) => ({ - value: et.id, - label: et.name - })); - - let selection_plans_ddl = currentSummit.selection_plans.map((et) => ({ - value: et.id, - label: et.name - })); - - return ( -
- -
-
- - -
-
- - -
-
-