From 87276b36059e7b1a4921807aa043f0994a792e94 Mon Sep 17 00:00:00 2001 From: Ondrej Ezr Date: Tue, 24 Oct 2023 15:16:43 +0200 Subject: [PATCH] feat(HMS-2255): deploy to image RG by default Use image RG by default and display clearly what the default is. This also removes the region selection entirely, as we default to the Resource Group location(region) and that's the best practice in Azure, so we don't want to promote overriding it. --- .../AzureResourceGroup.test.js | 9 +++- src/Components/AzureResourceGroup/index.js | 15 +++++-- src/Components/LaunchDescriptionList/index.js | 12 ++--- src/Components/ProvisioningWizard/helpers.js | 2 + .../steps/AccountCustomizations/azure.js | 45 ++++--------------- .../steps/ReviewDetails/index.js | 9 ++-- .../ProvisioningWizard/steps/index.js | 2 +- 7 files changed, 42 insertions(+), 52 deletions(-) diff --git a/src/Components/AzureResourceGroup/AzureResourceGroup.test.js b/src/Components/AzureResourceGroup/AzureResourceGroup.test.js index beeb84a7..c7675cb5 100644 --- a/src/Components/AzureResourceGroup/AzureResourceGroup.test.js +++ b/src/Components/AzureResourceGroup/AzureResourceGroup.test.js @@ -6,6 +6,11 @@ import { azureSourceUploadInfo } from '../../mocks/fixtures/sources.fixtures'; import AzureResourceGroup from '.'; describe('AzureResourceGroup', () => { + test('set the image resource group as placeholder to inform user', async () => { + const rgSelect = await mountSelectAndOpen('Image RG'); + expect(rgSelect).toHaveAttribute('placeholder', 'Image RG (default - image resource group)'); + }); + test('populate resource group select', async () => { await mountSelectAndOpen(); const items = await screen.findAllByLabelText(/^Resource group/); @@ -29,8 +34,8 @@ describe('AzureResourceGroup', () => { }); }); -const mountSelectAndOpen = async () => { - render(, { +const mountSelectAndOpen = async (imageResourceGroup = null) => { + render(, { contextValues: { chosenSource: '66' }, }); const selectDropdown = await screen.findByLabelText('Select resource group'); diff --git a/src/Components/AzureResourceGroup/index.js b/src/Components/AzureResourceGroup/index.js index a6d458a2..54ace67a 100644 --- a/src/Components/AzureResourceGroup/index.js +++ b/src/Components/AzureResourceGroup/index.js @@ -1,3 +1,4 @@ +import PropTypes from 'prop-types'; import React from 'react'; import { Spinner, Select, SelectOption, TextInput } from '@patternfly/react-core'; @@ -6,7 +7,7 @@ import { AZURE_RG_KEY } from '../../API/queryKeys'; import { fetchResourceGroups } from '../../API'; import { useWizardContext } from '../Common/WizardContext'; -const AzureResourceGroup = () => { +const AzureResourceGroup = ({ imageResourceGroup }) => { const [isOpen, setIsOpen] = React.useState(false); const [{ chosenSource, azureResourceGroup }, setWizardContext] = useWizardContext(); const [selection, setSelection] = React.useState(azureResourceGroup); @@ -84,15 +85,23 @@ const AzureResourceGroup = () => { isOpen={isOpen} onClear={clearSelection} selections={selection} - placeholderText="redhat-deployed (default)" + placeholderText={`${imageResourceGroup} (default - image resource group)`} typeAheadAriaLabel="Select resource group" maxHeight="220px" > - {resourceGroups.map((name, idx) => ( + {(resourceGroups || [selection]).map((name, idx) => ( ))} ); }; +AzureResourceGroup.propTypes = { + imageResourceGroup: PropTypes.string, +}; + +AzureResourceGroup.defaultProps = { + imageResourceGroup: null, +}; + export default AzureResourceGroup; diff --git a/src/Components/LaunchDescriptionList/index.js b/src/Components/LaunchDescriptionList/index.js index c623896b..bf9ba7b0 100644 --- a/src/Components/LaunchDescriptionList/index.js +++ b/src/Components/LaunchDescriptionList/index.js @@ -1,7 +1,7 @@ -import PropTypes from 'prop-types'; import React from 'react'; import { ExpandableSection, DescriptionList, DescriptionListTerm, DescriptionListGroup, DescriptionListDescription } from '@patternfly/react-core'; +import { imageProps, imageAzureResourceGroup } from '../ProvisioningWizard/helpers'; import { useSourcesData } from '../Common/Hooks/sources'; import { useWizardContext } from '../Common/WizardContext'; import { instanceType, region } from '../ProvisioningWizard/steps/ReservationProgress/helpers'; @@ -11,7 +11,7 @@ import { humanizeProvider } from '../Common/helpers'; import { TEMPLATES_KEY } from '../../API/queryKeys'; import { AZURE_PROVIDER } from '../../constants'; -const LaunchDescriptionList = ({ imageName }) => { +const LaunchDescriptionList = ({ image }) => { const [ { chosenRegion, @@ -47,16 +47,16 @@ const LaunchDescriptionList = ({ imageName }) => { Image - {imageName} + {image.name} Account {getChosenSourceName()} - {provider === AZURE_PROVIDER && azureResourceGroup && ( + {provider === AZURE_PROVIDER && ( Resource group - {azureResourceGroup} + {azureResourceGroup || {imageAzureResourceGroup(image)}} )} @@ -87,6 +87,6 @@ const LaunchDescriptionList = ({ imageName }) => { }; LaunchDescriptionList.propTypes = { - imageName: PropTypes.string.isRequired, + image: imageProps, }; export default LaunchDescriptionList; diff --git a/src/Components/ProvisioningWizard/helpers.js b/src/Components/ProvisioningWizard/helpers.js index c7092ad4..7d251315 100644 --- a/src/Components/ProvisioningWizard/helpers.js +++ b/src/Components/ProvisioningWizard/helpers.js @@ -10,3 +10,5 @@ export const imageProps = PropTypes.shape({ sourceIDs: PropTypes.arrayOf(PropTypes.string), accountIDs: PropTypes.arrayOf(PropTypes.string), }).isRequired; + +export const imageAzureResourceGroup = (image) => image.uploadOptions?.resource_group; diff --git a/src/Components/ProvisioningWizard/steps/AccountCustomizations/azure.js b/src/Components/ProvisioningWizard/steps/AccountCustomizations/azure.js index e4ff59ca..a730f3ad 100644 --- a/src/Components/ProvisioningWizard/steps/AccountCustomizations/azure.js +++ b/src/Components/ProvisioningWizard/steps/AccountCustomizations/azure.js @@ -3,17 +3,15 @@ import React from 'react'; import { Form, FormGroup, Popover, Title, Button, Text } from '@patternfly/react-core'; import { HelpIcon } from '@patternfly/react-icons'; -import { AZURE_PROVIDER } from '../../../../constants'; -import { imageProps } from '../../helpers'; +import { imageProps, imageAzureResourceGroup } from '../../helpers'; import SourcesSelect from '../../../SourcesSelect'; import InstanceCounter from '../../../InstanceCounter'; import InstanceTypesSelect from '../../../InstanceTypesSelect'; -import RegionsSelect from '../../../RegionsSelect'; import AzureResourceGroup from '../../../AzureResourceGroup'; import { useWizardContext } from '../../../Common/WizardContext'; const AccountCustomizationsAzure = ({ setStepValidated, image }) => { - const [wizardContext, setWizardContext] = useWizardContext(); + const [wizardContext] = useWizardContext(); const [validations, setValidation] = React.useState({ sources: wizardContext.chosenSource ? 'success' : 'default', types: wizardContext.chosenInstanceType ? 'success' : 'default', @@ -26,14 +24,6 @@ const AccountCustomizationsAzure = ({ setStepValidated, image }) => { setStepValidated(!errorExists); }, [validations]); - const onRegionChange = ({ region, imageID }) => { - setWizardContext((prevState) => ({ - ...prevState, - chosenRegion: region, - chosenImageID: imageID, - })); - }; - return (
@@ -56,35 +46,18 @@ const AccountCustomizationsAzure = ({ setStepValidated, image }) => { } /> </FormGroup> - <FormGroup - label="Select location" - isRequired - fieldId="azure-select-location" - labelIcon={ - <Popover headerContent={<div>Azure locations</div>}> - <Button - ouiaId="location_help" - type="button" - aria-label="More info for location field" - onClick={(e) => e.preventDefault()} - aria-describedby="azure-select-location" - className="pf-c-form__group-label-help" - variant="plain" - > - <HelpIcon noVerticalAlign /> - </Button> - </Popover> - } - > - <RegionsSelect provider={AZURE_PROVIDER} currentRegion={wizardContext.chosenRegion} onChange={onRegionChange} composeID={image.id} /> - </FormGroup> <FormGroup label="Azure resource group" fieldId="azure-resource-group-select" labelIcon={ <Popover headerContent={<div>Azure resource group</div>} - bodyContent={<div>Azure resource group to deploy the VM resources into. If left blank, defaults to ‘redhat-deployed’.</div>} + bodyContent={ + <div> + <p>Azure resource group to deploy the VM resources into. Defaults to the resource group image is located in.</p> + <p>The location (Azure region) of the resource group is used for all resources deployed.</p> + </div> + } > <Button ouiaId="resource_group_help" @@ -100,7 +73,7 @@ const AccountCustomizationsAzure = ({ setStepValidated, image }) => { </Popover> } > - <AzureResourceGroup /> + <AzureResourceGroup imageResourceGroup={imageAzureResourceGroup(image)} /> </FormGroup> <FormGroup label="Select instance size" diff --git a/src/Components/ProvisioningWizard/steps/ReviewDetails/index.js b/src/Components/ProvisioningWizard/steps/ReviewDetails/index.js index d53854a8..828eb4c5 100644 --- a/src/Components/ProvisioningWizard/steps/ReviewDetails/index.js +++ b/src/Components/ProvisioningWizard/steps/ReviewDetails/index.js @@ -1,10 +1,11 @@ -import PropTypes from 'prop-types'; import React from 'react'; + +import { imageProps } from '../../helpers'; import { Title, Text, Alert, AlertActionLink } from '@patternfly/react-core'; import LaunchDescriptionList from '../../../LaunchDescriptionList'; import useLocalStorage from '../../../Common/Hooks/useLocalStorage'; -const ReviewDetails = ({ imageName }) => { +const ReviewDetails = ({ image }) => { const [showNotificationBanner, setNotificationBanner] = useLocalStorage('rh_launch_notification_banner', true); return ( @@ -46,12 +47,12 @@ const ReviewDetails = ({ imageName }) => { </> </Alert> )} - <LaunchDescriptionList imageName={imageName} /> + <LaunchDescriptionList image={image} /> </div> ); }; ReviewDetails.propTypes = { - imageName: PropTypes.string.isRequired, + image: imageProps, }; export default ReviewDetails; diff --git a/src/Components/ProvisioningWizard/steps/index.js b/src/Components/ProvisioningWizard/steps/index.js index 96440212..2d30250c 100644 --- a/src/Components/ProvisioningWizard/steps/index.js +++ b/src/Components/ProvisioningWizard/steps/index.js @@ -78,7 +78,7 @@ const wizardSteps = ({ stepIdReached, image, stepValidation, setStepValidation, id: 5, component: ( <Loader> - <ReviewDetails imageName={image.name} /> + <ReviewDetails image={image} /> </Loader> ), canJumpTo: stepIdReached >= 5,