From bb83ec8e9395baa0ab256d71b0adda68a330fd7b Mon Sep 17 00:00:00 2001 From: Alf Eaton Date: Tue, 14 Sep 2021 09:54:21 +0100 Subject: [PATCH] Refactor Clone Project modal (#4748) GitOrigin-RevId: 03f5164d117713efd78c9806bdda6e6d5c90f6dc --- .../app/views/project/editor/left-menu.pug | 2 - .../components/clone-project-modal-content.js | 107 +++++++++++------- .../components/clone-project-modal.js | 90 ++++----------- ...eft-menu-clone-project-modal-controller.js | 34 +++--- .../features/clone-project-modal/utils/api.js | 9 -- .../clone-project-modal-content.stories.js | 44 ------- .../stories/clone-project-modal.stories.js | 77 +++++-------- .../components/clone-project-modal.test.js | 64 ++++++++--- 8 files changed, 181 insertions(+), 246 deletions(-) delete mode 100644 services/web/frontend/js/features/clone-project-modal/utils/api.js delete mode 100644 services/web/frontend/stories/clone-project-modal-content.stories.js diff --git a/services/web/app/views/project/editor/left-menu.pug b/services/web/app/views/project/editor/left-menu.pug index 3ba85746ac..6789b05dbe 100644 --- a/services/web/app/views/project/editor/left-menu.pug +++ b/services/web/app/views/project/editor/left-menu.pug @@ -44,8 +44,6 @@ aside#left-menu.full-size( clone-project-modal( handle-hide="handleHide" - project-id="projectId" - project-name="projectName" open-project="openProject" show="show" ) diff --git a/services/web/frontend/js/features/clone-project-modal/components/clone-project-modal-content.js b/services/web/frontend/js/features/clone-project-modal/components/clone-project-modal-content.js index b1d9d38b25..585ec3f180 100644 --- a/services/web/frontend/js/features/clone-project-modal/components/clone-project-modal-content.js +++ b/services/web/frontend/js/features/clone-project-modal/components/clone-project-modal-content.js @@ -1,5 +1,6 @@ import PropTypes from 'prop-types' -import { Trans } from 'react-i18next' +import { useMemo, useState } from 'react' +import { useTranslation } from 'react-i18next' import { Modal, Alert, @@ -8,37 +9,70 @@ import { FormControl, FormGroup, } from 'react-bootstrap' -import AccessibleModal from '../../../shared/components/accessible-modal' +import { useProjectContext } from '../../../shared/context/project-context' +import { postJSON } from '../../../infrastructure/fetch-json' export default function CloneProjectModalContent({ - animation = true, - show, - cancel, - handleSubmit, - clonedProjectName, - setClonedProjectName, - error, + handleHide, inFlight, - valid, + setInFlight, + openProject, }) { + const { _id: projectId, name: projectName } = useProjectContext() + const { t } = useTranslation() + + const [error, setError] = useState() + const [clonedProjectName, setClonedProjectName] = useState( + `${projectName} (Copy)` + ) + + // valid if the cloned project has a name + const valid = useMemo(() => clonedProjectName.trim().length > 0, [ + clonedProjectName, + ]) + + // form submission: clone the project if the name is valid + const handleSubmit = event => { + event.preventDefault() + + if (!valid) { + return + } + + setError(false) + setInFlight(true) + + // clone the project + postJSON(`/project/${projectId}/clone`, { + body: { projectName: clonedProjectName }, + }) + .then(data => { + // open the cloned project + openProject(data.project_id) + }) + .catch(({ response, data }) => { + if (response?.status === 400) { + setError(data.message) + } else { + setError(true) + } + }) + .finally(() => { + setInFlight(false) + }) + } + return ( - + <> - - - + {t('copy_project')}
- + {t('new_name')} - {error.length ? ( - error - ) : ( - - )} + {error.length ? error : t('generic_something_went_wrong')} )} - - + ) } CloneProjectModalContent.propTypes = { - animation: PropTypes.bool, - show: PropTypes.bool.isRequired, - cancel: PropTypes.func.isRequired, - handleSubmit: PropTypes.func.isRequired, - clonedProjectName: PropTypes.string, - setClonedProjectName: PropTypes.func.isRequired, - error: PropTypes.oneOfType([PropTypes.bool, PropTypes.string]), - inFlight: PropTypes.bool.isRequired, - valid: PropTypes.bool.isRequired, + handleHide: PropTypes.func.isRequired, + inFlight: PropTypes.bool, + setInFlight: PropTypes.func.isRequired, + openProject: PropTypes.func.isRequired, } diff --git a/services/web/frontend/js/features/clone-project-modal/components/clone-project-modal.js b/services/web/frontend/js/features/clone-project-modal/components/clone-project-modal.js index 91158dd278..8d88604bdc 100644 --- a/services/web/frontend/js/features/clone-project-modal/components/clone-project-modal.js +++ b/services/web/frontend/js/features/clone-project-modal/components/clone-project-modal.js @@ -1,92 +1,44 @@ -import { useCallback, useEffect, useMemo, useState } from 'react' +import React, { useCallback, useState } from 'react' import PropTypes from 'prop-types' -import { cloneProject } from '../utils/api' import CloneProjectModalContent from './clone-project-modal-content' +import AccessibleModal from '../../../shared/components/accessible-modal' +import withErrorBoundary from '../../../infrastructure/error-boundary' -function CloneProjectModal({ +const CloneProjectModal = React.memo(function CloneProjectModal({ show, handleHide, - projectId, - projectName = '', openProject, }) { const [inFlight, setInFlight] = useState(false) - const [error, setError] = useState() - const [clonedProjectName, setClonedProjectName] = useState('') - // set the cloned project name when the modal opens - useEffect(() => { - if (show) { - setClonedProjectName(`${projectName} (Copy)`) - } - }, [show, projectName]) - - // reset error when the modal is opened - useEffect(() => { - if (show) { - setError(undefined) - } - }, [show]) - - // close the modal if not in flight - const cancel = useCallback(() => { + const onHide = useCallback(() => { if (!inFlight) { handleHide() } }, [handleHide, inFlight]) - // valid if the cloned project has a name - const valid = useMemo(() => !!clonedProjectName, [clonedProjectName]) - - // form submission: clone the project if the name is valid - const handleSubmit = event => { - event.preventDefault() - - if (!valid) { - return - } - - setError(false) - setInFlight(true) - - // clone the project - cloneProject(projectId, clonedProjectName) - .then(data => { - // open the cloned project - openProject(data.project_id) - }) - .catch(({ response, data }) => { - if (response?.status === 400) { - setError(data.message) - } else { - setError(true) - } - }) - .finally(() => { - setInFlight(false) - }) - } - return ( - + onHide={onHide} + id="clone-project-modal" + backdrop="static" + > + + ) -} +}) CloneProjectModal.propTypes = { handleHide: PropTypes.func.isRequired, - projectId: PropTypes.string.isRequired, - projectName: PropTypes.string, - openProject: PropTypes.func.isRequired, show: PropTypes.bool.isRequired, + openProject: PropTypes.func.isRequired, } -export default CloneProjectModal +export default withErrorBoundary(CloneProjectModal) diff --git a/services/web/frontend/js/features/clone-project-modal/controllers/left-menu-clone-project-modal-controller.js b/services/web/frontend/js/features/clone-project-modal/controllers/left-menu-clone-project-modal-controller.js index 6dac617c95..a56be78768 100644 --- a/services/web/frontend/js/features/clone-project-modal/controllers/left-menu-clone-project-modal-controller.js +++ b/services/web/frontend/js/features/clone-project-modal/controllers/left-menu-clone-project-modal-controller.js @@ -1,15 +1,12 @@ import App from '../../../base' import { react2angular } from 'react2angular' - import CloneProjectModal from '../components/clone-project-modal' - -App.component('cloneProjectModal', react2angular(CloneProjectModal)) +import { rootContext } from '../../../shared/context/root-context' export default App.controller( 'LeftMenuCloneProjectModalController', - function ($scope, ide) { + function ($scope) { $scope.show = false - $scope.projectId = ide.$scope.project_id $scope.handleHide = () => { $scope.$applyAsync(() => { @@ -17,21 +14,22 @@ export default App.controller( }) } + $scope.openCloneProjectModal = () => { + $scope.$applyAsync(() => { + $scope.show = true + }) + } + $scope.openProject = projectId => { window.location.assign(`/project/${projectId}`) } - - $scope.openCloneProjectModal = () => { - $scope.$applyAsync(() => { - const { project } = ide.$scope - - if (project) { - $scope.projectId = project._id - $scope.projectName = project.name - - $scope.show = true - } - }) - } } ) + +App.component( + 'cloneProjectModal', + react2angular( + rootContext.use(CloneProjectModal), + Object.keys(CloneProjectModal.propTypes) + ) +) diff --git a/services/web/frontend/js/features/clone-project-modal/utils/api.js b/services/web/frontend/js/features/clone-project-modal/utils/api.js deleted file mode 100644 index e3f64f0618..0000000000 --- a/services/web/frontend/js/features/clone-project-modal/utils/api.js +++ /dev/null @@ -1,9 +0,0 @@ -import { postJSON } from '../../../infrastructure/fetch-json' - -export function cloneProject(projectId, cloneName) { - return postJSON(`/project/${projectId}/clone`, { - body: { - projectName: cloneName, - }, - }) -} diff --git a/services/web/frontend/stories/clone-project-modal-content.stories.js b/services/web/frontend/stories/clone-project-modal-content.stories.js deleted file mode 100644 index 0f82f6c573..0000000000 --- a/services/web/frontend/stories/clone-project-modal-content.stories.js +++ /dev/null @@ -1,44 +0,0 @@ -import CloneProjectModalContent from '../js/features/clone-project-modal/components/clone-project-modal-content' - -export const Basic = args => { - return -} - -export const Invalid = args => { - return ( - - ) -} - -export const Inflight = args => { - return -} - -export const GenericError = args => { - return -} - -export const SpecificError = args => { - return ( - - ) -} - -export default { - title: 'Modals / Clone Project / Content', - component: CloneProjectModalContent, - args: { - animation: false, - projectId: 'original-project', - clonedProjectName: 'Project Title', - show: true, - error: false, - inFlight: false, - valid: true, - }, - argTypes: { - cancel: { action: 'cancel' }, - handleSubmit: { action: 'submit' }, - setClonedProjectName: { action: 'set project name' }, - }, -} diff --git a/services/web/frontend/stories/clone-project-modal.stories.js b/services/web/frontend/stories/clone-project-modal.stories.js index eed732a474..1f5e5bd507 100644 --- a/services/web/frontend/stories/clone-project-modal.stories.js +++ b/services/web/frontend/stories/clone-project-modal.stories.js @@ -1,68 +1,53 @@ -import PropTypes from 'prop-types' - -import CloneProjectModal from '../js/features/clone-project-modal/components/clone-project-modal' import useFetchMock from './hooks/use-fetch-mock' +import { withContextRoot } from './utils/with-context-root' +import CloneProjectModal from '../js/features/clone-project-modal/components/clone-project-modal' -export const Interactive = ({ - mockResponse = 200, - mockResponseDelay = 500, - ...args -}) => { +const project = { _id: 'original-project', name: 'Project Title' } + +export const Success = args => { useFetchMock(fetchMock => { fetchMock.post( 'express:/project/:projectId/clone', - () => { - switch (mockResponse) { - case 400: - return { status: 400, body: 'The project name is not valid' } - - default: - return mockResponse - } - }, - { delay: mockResponseDelay } + { status: 200 }, + { delay: 250 } ) }) - return + return withContextRoot(, { project }) } -Interactive.propTypes = { - mockResponse: PropTypes.number, - mockResponseDelay: PropTypes.number, + +export const GenericErrorResponse = args => { + useFetchMock(fetchMock => { + fetchMock.post( + 'express:/project/:projectId/clone', + { status: 500 }, + { delay: 250 } + ) + }) + + return withContextRoot(, { project }) +} + +export const SpecificErrorResponse = args => { + useFetchMock(fetchMock => { + fetchMock.post( + 'express:/project/:projectId/clone', + { status: 400, body: 'The project name is not valid' }, + { delay: 250 } + ) + }) + + return withContextRoot(, { project }) } export default { title: 'Modals / Clone Project', component: CloneProjectModal, args: { - projectId: 'original-project', - projectName: 'Project Title', show: true, }, argTypes: { handleHide: { action: 'close modal' }, openProject: { action: 'open project' }, - mockResponse: { - name: 'Mock Response Status', - type: { name: 'number', required: false }, - description: 'The status code that should be returned by the mock server', - defaultValue: 200, - control: { - type: 'radio', - options: [200, 500, 400], - }, - }, - mockResponseDelay: { - name: 'Mock Response Delay', - type: { name: 'number', required: false }, - description: 'The delay before returning a response from the mock server', - defaultValue: 500, - control: { - type: 'range', - min: 0, - max: 2500, - step: 250, - }, - }, }, } diff --git a/services/web/test/frontend/features/clone-project-modal/components/clone-project-modal.test.js b/services/web/test/frontend/features/clone-project-modal/components/clone-project-modal.test.js index 9fa0af35ee..df8d137448 100644 --- a/services/web/test/frontend/features/clone-project-modal/components/clone-project-modal.test.js +++ b/services/web/test/frontend/features/clone-project-modal/components/clone-project-modal.test.js @@ -1,24 +1,36 @@ -import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import { fireEvent, screen, waitFor } from '@testing-library/react' import { expect } from 'chai' -import CloneProjectModal from '../../../../../frontend/js/features/clone-project-modal/components/clone-project-modal' import sinon from 'sinon' import fetchMock from 'fetch-mock' +import CloneProjectModal from '../../../../../frontend/js/features/clone-project-modal/components/clone-project-modal' +import { renderWithEditorContext } from '../../../helpers/render-with-context' describe('', function () { - afterEach(function () { + beforeEach(function () { fetchMock.reset() }) - const modalProps = { - handleHide: sinon.stub(), - projectId: 'project-1', - projectName: 'Test Project', - openProject: sinon.stub(), - show: true, + after(function () { + fetchMock.reset() + }) + + const project = { + _id: 'project-1', + name: 'Test Project', } it('renders the translated modal title', async function () { - render() + const handleHide = sinon.stub() + const openProject = sinon.stub() + + renderWithEditorContext( + , + { scope: { project } } + ) await screen.findByText('Copy Project') }) @@ -28,14 +40,22 @@ describe('', function () { 'express:/project/:projectId/clone', { status: 200, - body: { project_id: modalProps.projectId }, + body: { project_id: 'cloned-project' }, }, { delay: 10 } ) + const handleHide = sinon.stub() const openProject = sinon.stub() - render() + renderWithEditorContext( + , + { scope: { project } } + ) const cancelButton = await screen.findByRole('button', { name: 'Cancel' }) expect(cancelButton.disabled).to.be.false @@ -88,9 +108,17 @@ describe('', function () { body: 'There was an error!', }) + const handleHide = sinon.stub() const openProject = sinon.stub() - render() + renderWithEditorContext( + , + { scope: { project } } + ) const button = await screen.findByRole('button', { name: 'Copy' }) expect(button.disabled).to.be.false @@ -117,9 +145,17 @@ describe('', function () { body: 'There was an error!', }) + const handleHide = sinon.stub() const openProject = sinon.stub() - render() + renderWithEditorContext( + , + { scope: { project } } + ) const button = await screen.findByRole('button', { name: 'Copy' }) expect(button.disabled).to.be.false