From 17406eef8a0fe067e26fd3ff8f91caf0241ae04b Mon Sep 17 00:00:00 2001 From: David Vail Date: Tue, 8 Nov 2022 11:05:48 -0500 Subject: [PATCH 1/2] Refactor collection form to allow for better composition. --- .../Containers/Collections/CollectionForm.tsx | 637 +++++++----------- .../Collections/CollectionsFormPage.tsx | 281 +++++++- .../Collections/CollectionsPage.tsx | 13 +- .../src/Containers/Collections/converter.ts | 2 +- 4 files changed, 503 insertions(+), 430 deletions(-) diff --git a/ui/apps/platform/src/Containers/Collections/CollectionForm.tsx b/ui/apps/platform/src/Containers/Collections/CollectionForm.tsx index f03140aa32f0b..dfe668994c54f 100644 --- a/ui/apps/platform/src/Containers/Collections/CollectionForm.tsx +++ b/ui/apps/platform/src/Containers/Collections/CollectionForm.tsx @@ -1,13 +1,7 @@ -import React, { useEffect, useState } from 'react'; +import React, { ReactElement, useEffect } from 'react'; import { useHistory } from 'react-router-dom'; import { - Alert, - AlertActionCloseButton, - AlertGroup, - Breadcrumb, - BreadcrumbItem, Button, - Divider, Drawer, DrawerActions, DrawerCloseButton, @@ -16,10 +10,6 @@ import { DrawerHead, DrawerPanelBody, DrawerPanelContent, - Dropdown, - DropdownItem, - DropdownSeparator, - DropdownToggle, EmptyState, EmptyStateIcon, EmptyStateVariant, @@ -33,28 +23,19 @@ import { Title, Truncate, } from '@patternfly/react-core'; -import { CaretDownIcon, CubesIcon } from '@patternfly/react-icons'; +import { CubesIcon } from '@patternfly/react-icons'; import { TableComposable, TableVariant, Tbody, Tr, Td } from '@patternfly/react-table'; -import { useFormik } from 'formik'; +import { Formik, FormikHelpers } from 'formik'; import * as yup from 'yup'; -import BreadcrumbItemLink from 'Components/BreadcrumbItemLink'; -import ConfirmationModal from 'Components/PatternFly/ConfirmationModal'; -import useSelectToggle from 'hooks/patternfly/useSelectToggle'; -import useToasts from 'hooks/patternfly/useToasts'; import { collectionsBasePath } from 'routePaths'; -import { - CollectionResponse, - createCollection, - deleteCollection, - updateCollection, -} from 'services/CollectionsService'; +import { CollectionResponse } from 'services/CollectionsService'; import { CollectionPageAction } from './collections.utils'; import RuleSelector from './RuleSelector'; import CollectionAttacher from './CollectionAttacher'; import CollectionResults from './CollectionResults'; import { Collection, ScopedResourceSelector, SelectorEntityType } from './types'; -import { generateRequest } from './converter'; +import { parseCollection } from './converter'; function AttachedCollectionTable({ collections }: { collections: CollectionResponse[] }) { return collections.length > 0 ? ( @@ -86,18 +67,22 @@ export type CollectionFormProps = { hasWriteAccessForCollections: boolean; /* The user's workflow action for this collection */ action: CollectionPageAction; - /* initial data used to populate the form */ - initialData: Collection; - /* Collection object references for the list of ids in `initialData` */ - initialEmbeddedCollections: CollectionResponse[]; + /* initial, unparsed data used to populate the form */ + collectionData: { + collection: Omit; + embeddedCollections: CollectionResponse[]; + }; /* Whether or not to display the collection results in an inline drawer. If false, will display collection results in an overlay drawer. */ useInlineDrawer: boolean; - /* Whether or not to show breadcrumb navigation at the top of the form */ - showBreadcrumbs: boolean; + isDrawerOpen: boolean; + toggleDrawer: (isOpen: boolean) => void; + onSubmit: (collection: Collection) => Promise; /* Callback used when clicking on a collection name in the CollectionAttacher section. If left undefined, collection names will not be linked. */ appendTableLinkAction?: (collectionId: string) => void; + /* content to render before the main form */ + headerContent?: ReactElement; }; function yupResourceSelectorObject() { @@ -131,140 +116,44 @@ function yupResourceSelectorObject() { function CollectionForm({ hasWriteAccessForCollections, action, - initialData, - initialEmbeddedCollections, + collectionData, useInlineDrawer, - showBreadcrumbs, + isDrawerOpen, + toggleDrawer, + onSubmit, + headerContent, }: CollectionFormProps) { const history = useHistory(); - const { - isOpen: drawerIsOpen, - toggleSelect: toggleDrawer, - closeSelect: closeDrawer, - openSelect: openDrawer, - } = useSelectToggle(useInlineDrawer); - const { - isOpen: menuIsOpen, - toggleSelect: toggleMenu, - closeSelect: closeMenu, - } = useSelectToggle(); - const [deleteId, setDeleteId] = useState(null); - const [isDeleting, setIsDeleting] = useState(false); - const { toasts, addToast, removeToast } = useToasts(); - - const { - values, - isValid, - errors, - handleChange, - handleBlur, - setFieldValue, - submitForm, - isSubmitting, - setSubmitting, - } = useFormik({ - initialValues: initialData, - onSubmit: onSaveCollection, - validationSchema: yup.object({ - name: yup.string().trim().required(), - description: yup.string(), - embeddedCollectionIds: yup.array(yup.string().trim().required()), - resourceSelector: yup.object().shape({ - Deployment: yupResourceSelectorObject(), - Namespace: yupResourceSelectorObject(), - Cluster: yupResourceSelectorObject(), - }), - }), - }); + const initialData = parseCollection(collectionData.collection); + const initialEmbeddedCollections = collectionData.embeddedCollections; useEffect(() => { toggleDrawer(useInlineDrawer); }, [toggleDrawer, useInlineDrawer]); - const pageTitle = action.type === 'create' ? 'Create collection' : values.name; const isReadOnly = action.type === 'view' || !hasWriteAccessForCollections; - function onEditCollection(id: string) { - history.push({ - pathname: `${collectionsBasePath}/${id}`, - search: 'action=edit', - }); - } - - function onCloneCollection(id: string) { - history.push({ - pathname: `${collectionsBasePath}/${id}`, - search: 'action=clone', - }); - } - - function onConfirmDeleteCollection() { - if (!deleteId) { - return; - } - setIsDeleting(true); - deleteCollection(deleteId) - .request.then(history.goBack) - .catch((err) => { - addToast(`Could not delete collection ${initialData.name}`, 'danger', err.message); - }) - .finally(() => { - setDeleteId(null); - setIsDeleting(false); - }); - } - - function onCancelDeleteCollection() { - setDeleteId(null); - } - - function onSaveCollection(collection: Collection) { - if (action.type === 'view') { - // Logically should not happen, but just in case - return; - } - - const saveServiceCall = - action.type === 'edit' - ? (payload) => updateCollection(action.collectionId, payload) - : (payload) => createCollection(payload); - - const requestPayload = generateRequest(collection); - const { request } = saveServiceCall(requestPayload); - - request - .then(() => { - history.push({ pathname: `${collectionsBasePath}` }); - }) - .catch((err) => { - addToast( - `There was an error saving collection '${values.name}'`, - 'danger', - err.message - ); - setSubmitting(false); - }); - } - function onCancelSave() { history.push({ pathname: `${collectionsBasePath}` }); } - const onResourceSelectorChange = ( - entityType: SelectorEntityType, - scopedResourceSelector: ScopedResourceSelector - ) => setFieldValue(`resourceSelector.${entityType}`, scopedResourceSelector); + const onResourceSelectorChange = + (setFieldValue: FormikHelpers['setFieldValue']) => + (entityType: SelectorEntityType, scopedResourceSelector: ScopedResourceSelector) => + setFieldValue(`resourceSelector.${entityType}`, scopedResourceSelector); - const onEmbeddedCollectionsChange = (newCollections: CollectionResponse[]) => - setFieldValue( - 'embeddedCollectionIds', - newCollections.map(({ id }) => id) - ); + const onEmbeddedCollectionsChange = + (setFieldValue: FormikHelpers['setFieldValue']) => + (newCollections: CollectionResponse[]) => + setFieldValue( + 'embeddedCollectionIds', + newCollections.map(({ id }) => id) + ); return ( <> - + Collection results See a live preview of current matches. - + toggleDrawer(false)} /> @@ -286,265 +175,231 @@ function CollectionForm({ } > - {showBreadcrumbs && ( + {headerContent} + {initialData instanceof AggregateError ? ( <> - - - Collections - - {pageTitle} - - + {initialData.errors} + {/* TODO - Handle inline UI for unsupported rule errors */} - )} - - - {pageTitle} - - - {action.type === 'view' && hasWriteAccessForCollections && ( - <> - - Actions - - } - isOpen={menuIsOpen} - dropdownItems={[ - - onEditCollection(action.collectionId) + ) : ( + { + onSubmit(collection).catch(() => { + setSubmitting(false); + }); + }} + validationSchema={yup.object({ + name: yup.string().trim().required(), + description: yup.string(), + embeddedCollectionIds: yup.array( + yup.string().trim().required() + ), + resourceSelector: yup.object().shape({ + Deployment: yupResourceSelectorObject(), + Namespace: yupResourceSelectorObject(), + Cluster: yupResourceSelectorObject(), + }), + })} + > + {({ + values, + isValid, + errors, + handleChange, + handleBlur, + setFieldValue, + submitForm, + isSubmitting, + }) => ( +
+ + + Collection details + + + + handleChange(e)} + onBlur={handleBlur} + isDisabled={isReadOnly} + /> + + + + + handleChange(e)} + onBlur={handleBlur} + isDisabled={isReadOnly} + /> + + + + + + + - Edit collection - </DropdownItem>, - <DropdownItem - key="Clone collection" - component="button" - onClick={() => - onCloneCollection(action.collectionId) + Collection rules + + {!isReadOnly && ( + <> +

+ Select deployments via rules. You can + use regular expressions (RE2 syntax). +

+ + )} + +
- - - - - Collection details - - - - handleChange(e)} - onBlur={handleBlur} + in + + - - - - - handleChange(e)} - onBlur={handleBlur} + + - - - - - - - - Collection rules - - {!isReadOnly && ( - <> -

- Select deployments via rules. You can use regular - expressions (RE2 syntax). -

- - )} - - - - - -
+
- - - Attached collections - - {isReadOnly ? ( - - ) : ( - <> -

Extend this collection by attaching other sets.

- - - )} -
-
- {action.type !== 'view' && ( -
- - -
- )} -
+ + + Attached collections + + {isReadOnly ? ( + + ) : ( + <> +

+ Extend this collection by attaching + other sets. +

+ + + )} +
+
+ {action.type !== 'view' && ( +
+ + +
+ )} + + )} + + )}
- - {toasts.map(({ key, variant, title, children }) => ( - removeToast(key)} - actionClose={ - removeToast(key)} - /> - } - > - {children} - - ))} - - - Are you sure you want to delete this collection? - ); } diff --git a/ui/apps/platform/src/Containers/Collections/CollectionsFormPage.tsx b/ui/apps/platform/src/Containers/Collections/CollectionsFormPage.tsx index e86556e9cb842..68ce0f7839b37 100644 --- a/ui/apps/platform/src/Containers/Collections/CollectionsFormPage.tsx +++ b/ui/apps/platform/src/Containers/Collections/CollectionsFormPage.tsx @@ -1,17 +1,43 @@ -import React, { ReactElement, useCallback } from 'react'; -import { PageSection } from '@patternfly/react-core'; +import React, { ReactElement, useCallback, useState } from 'react'; +import { useHistory } from 'react-router-dom'; +import { + Alert, + AlertActionCloseButton, + AlertGroup, + Breadcrumb, + BreadcrumbItem, + Button, + Divider, + Dropdown, + DropdownItem, + DropdownSeparator, + DropdownToggle, + Flex, + FlexItem, + Title, +} from '@patternfly/react-core'; import { useMediaQuery } from 'react-responsive'; import useRestQuery from 'Containers/Dashboard/hooks/useRestQuery'; import { CollectionResponse, + createCollection, + deleteCollection, getCollection, listCollections, ResolvedCollectionResponse, + updateCollection, } from 'services/CollectionsService'; +import { CaretDownIcon } from '@patternfly/react-icons'; +import BreadcrumbItemLink from 'Components/BreadcrumbItemLink'; +import { collectionsBasePath } from 'routePaths'; +import useSelectToggle from 'hooks/patternfly/useSelectToggle'; +import ConfirmationModal from 'Components/PatternFly/ConfirmationModal'; +import useToasts from 'hooks/patternfly/useToasts'; +import { values } from 'lodash'; import { CollectionPageAction } from './collections.utils'; import CollectionForm from './CollectionForm'; -import { parseCollection } from './converter'; +import { generateRequest } from './converter'; import { Collection } from './types'; export type CollectionsFormPageProps = { @@ -19,21 +45,20 @@ export type CollectionsFormPageProps = { pageAction: CollectionPageAction; }; -const noopRequest = { - request: Promise.resolve(undefined), - cancel: () => {}, -}; - -const defaultCollectionData: Collection = { +const defaultCollectionData: Omit = { name: '', description: '', inUse: false, - embeddedCollectionIds: [], - resourceSelector: { - Deployment: { type: 'All' }, - Namespace: { type: 'All' }, - Cluster: { type: 'All' }, - }, + embeddedCollections: [], + resourceSelectors: [], +}; + +const noopRequest = { + request: Promise.resolve<{ + collection: Omit; + embeddedCollections: CollectionResponse[]; + }>({ collection: defaultCollectionData, embeddedCollections: [] }), + cancel: () => {}, }; function getEmbeddedCollections({ collection }: ResolvedCollectionResponse): Promise<{ @@ -53,6 +78,7 @@ function CollectionsFormPage({ hasWriteAccessForCollections, pageAction, }: CollectionsFormPageProps) { + const history = useHistory(); const isLargeScreen = useMediaQuery({ query: '(min-width: 992px)' }); // --pf-global--breakpoint--lg const collectionId = pageAction.type !== 'create' ? pageAction.collectionId : undefined; const collectionFetcher = useCallback(() => { @@ -63,8 +89,89 @@ function CollectionsFormPage({ return { request: request.then(getEmbeddedCollections), cancel }; }, [collectionId]); const { data, loading, error } = useRestQuery(collectionFetcher); - const initialData = data ? parseCollection(data.collection) : defaultCollectionData; - const initialEmbeddedCollections = data ? data.embeddedCollections : []; + + const { toasts, addToast, removeToast } = useToasts(); + + const [isDeleting, setIsDeleting] = useState(false); + const [deleteId, setDeleteId] = useState(null); + + const { + isOpen: menuIsOpen, + toggleSelect: toggleMenu, + closeSelect: closeMenu, + } = useSelectToggle(); + + const { + isOpen: isDrawerOpen, + toggleSelect: toggleDrawer, + closeSelect: closeDrawer, + openSelect: openDrawer, + } = useSelectToggle(isLargeScreen); + + function onEditCollection(id: string) { + history.push({ + pathname: `${collectionsBasePath}/${id}`, + search: 'action=edit', + }); + } + + function onCloneCollection(id: string) { + history.push({ + pathname: `${collectionsBasePath}/${id}`, + search: 'action=clone', + }); + } + + function onConfirmDeleteCollection() { + if (!deleteId) { + return; + } + setIsDeleting(true); + deleteCollection(deleteId) + .request.then(history.goBack) + .catch((err) => { + addToast( + `Could not delete collection ${data?.collection?.name ?? ''}`, + 'danger', + err.message + ); + }) + .finally(() => { + setDeleteId(null); + setIsDeleting(false); + }); + } + + function onCancelDeleteCollection() { + setDeleteId(null); + } + + function onSubmit(collection: Collection): Promise { + if (pageAction.type === 'view') { + // Logically should not happen, but just in case + return Promise.reject(); + } + + const saveServiceCall = + pageAction.type === 'edit' + ? (payload) => updateCollection(pageAction.collectionId, payload) + : (payload) => createCollection(payload); + + const requestPayload = generateRequest(collection); + const { request } = saveServiceCall(requestPayload); + + return request + .then(() => { + history.push({ pathname: `${collectionsBasePath}` }); + }) + .catch((err) => { + addToast( + `There was an error saving collection '${values.name}'`, + 'danger', + err.message + ); + }); + } let content: ReactElement | undefined; @@ -75,39 +182,145 @@ function CollectionsFormPage({ {/* TODO - Handle UI for network errors */} ); - } else if (initialData instanceof AggregateError) { - content = ( - <> - {initialData.errors} - {/* TODO - Handle UI for parse errors */} - - ); } else if (loading) { content = <>{/* TODO - Handle UI for loading state */}; - } else if (initialData) { - if (pageAction.type === 'clone') { - initialData.name += ' (COPY)'; - } + } else if (data) { + const pageTitle = pageAction.type === 'create' ? 'Create collection' : data.collection.name; content = ( { /* TODO */ }} + headerContent={ + <> + + + Collections + + {pageTitle} + + + + + {pageTitle} + + + {pageAction.type === 'view' && hasWriteAccessForCollections && ( + <> + + Actions + + } + isOpen={menuIsOpen} + dropdownItems={[ + + onEditCollection(pageAction.collectionId) + } + > + Edit collection + , + + onCloneCollection(pageAction.collectionId) + } + > + Clone collection + , + , + + setDeleteId(pageAction.collectionId) + } + > + {data.collection.inUse + ? 'Cannot delete (in use)' + : 'Delete collection'} + , + ]} + /> + + + )} + {isDrawerOpen ? ( + + ) : ( + + )} + + + + + } /> ); } return ( <> - - {content} - + {content} + + {toasts.map(({ key, variant, title, children }) => ( + removeToast(key)} + actionClose={ + removeToast(key)} + /> + } + > + {children} + + ))} + + + Are you sure you want to delete this collection? + ); } diff --git a/ui/apps/platform/src/Containers/Collections/CollectionsPage.tsx b/ui/apps/platform/src/Containers/Collections/CollectionsPage.tsx index edb09128ccba7..dbe8d619bd1f9 100644 --- a/ui/apps/platform/src/Containers/Collections/CollectionsPage.tsx +++ b/ui/apps/platform/src/Containers/Collections/CollectionsPage.tsx @@ -1,5 +1,6 @@ import React, { useEffect } from 'react'; import { useParams } from 'react-router-dom'; +import { PageSection } from '@patternfly/react-core'; import usePermissions from 'hooks/usePermissions'; import useURLParameter from 'hooks/useURLParameter'; @@ -26,10 +27,14 @@ function CollectionsPage() { if (validPageActionProp) { return ( - + <> + + + + ); } diff --git a/ui/apps/platform/src/Containers/Collections/converter.ts b/ui/apps/platform/src/Containers/Collections/converter.ts index b4e55e88c9e16..706f9e4d64663 100644 --- a/ui/apps/platform/src/Containers/Collections/converter.ts +++ b/ui/apps/platform/src/Containers/Collections/converter.ts @@ -30,7 +30,7 @@ const LABEL_SEPARATOR = '='; * of a `Collection` that can be supported by the current UI controls. If any incompatibilities are detected * it will return a list of validation errors to the caller. */ -export function parseCollection(data: CollectionResponse): Collection | AggregateError { +export function parseCollection(data: Omit): Collection | AggregateError { const collection: Collection = { name: data.name, description: data.description, From edf079596e095dcb434bfdeb2dfb049d962ef654 Mon Sep 17 00:00:00 2001 From: David Vail Date: Thu, 10 Nov 2022 08:18:03 -0500 Subject: [PATCH 2/2] revert PageSection move --- .../Containers/Collections/CollectionsFormPage.tsx | 5 +++-- .../src/Containers/Collections/CollectionsPage.tsx | 13 ++++--------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/ui/apps/platform/src/Containers/Collections/CollectionsFormPage.tsx b/ui/apps/platform/src/Containers/Collections/CollectionsFormPage.tsx index 68ce0f7839b37..393a6d6d364cf 100644 --- a/ui/apps/platform/src/Containers/Collections/CollectionsFormPage.tsx +++ b/ui/apps/platform/src/Containers/Collections/CollectionsFormPage.tsx @@ -14,6 +14,7 @@ import { DropdownToggle, Flex, FlexItem, + PageSection, Title, } from '@patternfly/react-core'; import { useMediaQuery } from 'react-responsive'; @@ -289,7 +290,7 @@ function CollectionsFormPage({ } return ( - <> + {content} {toasts.map(({ key, variant, title, children }) => ( @@ -321,7 +322,7 @@ function CollectionsFormPage({ > Are you sure you want to delete this collection? - + ); } diff --git a/ui/apps/platform/src/Containers/Collections/CollectionsPage.tsx b/ui/apps/platform/src/Containers/Collections/CollectionsPage.tsx index dbe8d619bd1f9..edb09128ccba7 100644 --- a/ui/apps/platform/src/Containers/Collections/CollectionsPage.tsx +++ b/ui/apps/platform/src/Containers/Collections/CollectionsPage.tsx @@ -1,6 +1,5 @@ import React, { useEffect } from 'react'; import { useParams } from 'react-router-dom'; -import { PageSection } from '@patternfly/react-core'; import usePermissions from 'hooks/usePermissions'; import useURLParameter from 'hooks/useURLParameter'; @@ -27,14 +26,10 @@ function CollectionsPage() { if (validPageActionProp) { return ( - <> - - - - + ); }