refactor(core/account): Refactor AccountSelectInput to use 'value' prop

spinnaker · Dec 7, 2018 · 307da1b · 307da1b
1 parent 6f7f543
commit 307da1b
Show file tree

Hide file tree

Showing 16 changed files with 85 additions and 91 deletions.
diff --git a/app/scripts/modules/amazon/src/loadBalancer/configure/common/LoadBalancerLocation.tsx b/app/scripts/modules/amazon/src/loadBalancer/configure/common/LoadBalancerLocation.tsx
@@ -343,11 +343,10 @@ class LoadBalancerLocationImpl extends React.Component<ILoadBalancerLocationProp
               <div className="col-md-3 sm-label-right">Account</div>
               <div className="col-md-7">
                 <AccountSelectInput
-                  component={values}
-                  field="credentials"
+                  value={values.credentials}
+                  onChange={evt => this.accountUpdated(evt.target.value)}
                   accounts={accounts}
                   provider="aws"
-                  onChange={this.accountUpdated}
                 />
               </div>
             </div>

diff --git a/...cripts/modules/amazon/src/serverGroup/configure/wizard/pages/ServerGroupBasicSettings.tsx b/...cripts/modules/amazon/src/serverGroup/configure/wizard/pages/ServerGroupBasicSettings.tsx
@@ -317,12 +317,11 @@ class ServerGroupBasicSettingsImpl extends React.Component<
           <div className="col-md-3 sm-label-right">Account</div>
           <div className="col-md-7">
             <AccountSelectInput
+              value={values.credentials}
+              onChange={evt => this.accountUpdated(evt.target.value)}
               readOnly={readOnlyFields.credentials}
-              component={values}
-              field="credentials"
               accounts={accounts}
               provider="aws"
-              onChange={this.accountUpdated}
             />
           </div>
         </div>

diff --git a/app/scripts/modules/cloudfoundry/src/loadBalancer/configure/loadBalancerDetails.tsx b/app/scripts/modules/cloudfoundry/src/loadBalancer/configure/loadBalancerDetails.tsx
@@ -133,11 +133,10 @@ class LoadBalancerDetailsImpl extends React.Component<ILoadBalancerDetailsProps,
             <div className="col-md-3 sm-label-right">Account</div>
             <div className="col-md-7">
               <AccountSelectInput
-                component={values}
-                field="credentials"
+                value={values.credentials}
+                onChange={evt => this.accountUpdated(evt.target.value)}
                 accounts={accounts}
                 provider="cloudfoundry"
-                onChange={this.accountUpdated}
               />
             </div>
           </div>

diff --git a/...s/cloudfoundry/src/pipeline/stages/deployService/CloudfoundryDeployServiceStageConfig.tsx b/...s/cloudfoundry/src/pipeline/stages/deployService/CloudfoundryDeployServiceStageConfig.tsx
@@ -424,11 +424,10 @@ export class CloudfoundryDeployServiceStageConfig extends React.Component<
       <div className="form-horizontal cloudfoundry-deploy-service-stage">
         <StageConfigField label="Account">
           <AccountSelectInput
+            value={stage.credentials}
+            onChange={evt => this.accountUpdated(evt.target.value)}
             accounts={accounts}
-            component={stage}
-            field="credentials"
             provider="cloudfoundry"
-            onChange={this.accountUpdated}
           />
         </StageConfigField>
         <RegionSelectField

diff --git a/...cloudfoundry/src/pipeline/stages/destroyService/CloudfoundryDestroyServiceStageConfig.tsx b/...cloudfoundry/src/pipeline/stages/destroyService/CloudfoundryDestroyServiceStageConfig.tsx
@@ -88,11 +88,10 @@ export class CloudfoundryDestroyServiceStageConfig extends React.Component<
       <div className="form-horizontal">
         <StageConfigField label="Account">
           <AccountSelectInput
+            value={stage.credentials}
+            onChange={evt => this.accountUpdated(evt.target.value)}
             accounts={accounts}
-            component={stage}
-            field="credentials"
             provider="cloudfoundry"
-            onChange={this.accountUpdated}
           />
         </StageConfigField>
         <RegionSelectField

diff --git a/...ts/modules/cloudfoundry/src/serverGroup/configure/wizard/sections/artifactSettings.cf.tsx b/...ts/modules/cloudfoundry/src/serverGroup/configure/wizard/sections/artifactSettings.cf.tsx
@@ -239,11 +239,10 @@ class ArtifactSettingsImpl extends React.Component<
           <div className="col-md-3 sm-label-right">Account</div>
           <div className="col-md-7">
             <AccountSelectInput
-              component={artifact}
-              field="credentials"
+              value={(artifact as any).credentials}
+              onChange={evt => this.packageAccountUpdated(evt.target.value)}
               accounts={allCloudFoundryCredentials}
               provider="cloudfoundry"
-              onChange={this.packageAccountUpdated}
             />
           </div>
         </div>

diff --git a/...ripts/modules/cloudfoundry/src/serverGroup/configure/wizard/sections/basicSettings.cf.tsx b/...ripts/modules/cloudfoundry/src/serverGroup/configure/wizard/sections/basicSettings.cf.tsx
@@ -81,11 +81,10 @@ class BasicSettingsImpl extends React.Component<
           <div className="col-md-3 sm-label-right">Account</div>
           <div className="col-md-7">
             <AccountSelectInput
-              component={values}
-              field="credentials"
+              value={values.credentials}
+              onChange={evt => this.accountUpdated(evt.target.value)}
               accounts={accounts}
               provider="cloudfoundry"
-              onChange={this.accountUpdated}
             />
           </div>
         </div>

diff --git a/app/scripts/modules/core/src/account/AccountSelectInput.tsx b/app/scripts/modules/core/src/account/AccountSelectInput.tsx
@@ -1,23 +1,20 @@
 import * as React from 'react';
 import { $q } from 'ngimport';
-import { flatten, has, isEqual, map, uniq, xor } from 'lodash';
+import { flatten, isEqual, map, uniq, xor } from 'lodash';
 
-import { IAccount } from 'core/account';
-import { AccountService } from 'core/account/AccountService';
+import { createFakeReactSyntheticEvent } from 'core/presentation/forms/inputs/utils';
+import { IFormInputProps } from 'core/presentation';
 
-export interface IAccountSelectInputProps {
+import { AccountService, IAccount } from './AccountService';
+
+export interface IAccountSelectInputProps extends IFormInputProps {
   accounts: IAccount[] | string[];
-  component: { [key: string]: any };
-  field: string;
   provider: string;
   loading?: boolean;
-  onChange?: (account: string) => void;
-  labelColumns?: number;
   readOnly?: boolean;
 }
 
 export interface IAccountSelectInputState {
-  accountContainsExpression: boolean;
   mergedAccounts: string[];
   primaryAccounts: string[];
   secondaryAccounts: string[];
@@ -27,20 +24,19 @@ const isExpression = (account: string) => !!account && account.includes('${');
 
 export class AccountSelectInput extends React.Component<IAccountSelectInputProps, IAccountSelectInputState> {
   public state: IAccountSelectInputState = {
-    accountContainsExpression: false,
     mergedAccounts: [],
     primaryAccounts: [],
     secondaryAccounts: [],
   };
 
   private groupAccounts = (accounts: IAccount[] | string[]) => {
-    const { component, field, onChange, provider } = this.props;
+    const { name, value, onChange, provider } = this.props;
 
     if (!accounts || !accounts.length) {
       return;
     }
-    if (has(component, field) && isExpression(component[field])) {
-      this.setState({ accountContainsExpression: true });
+
+    if (isExpression(value)) {
       return;
     }
 
@@ -76,10 +72,8 @@ export class AccountSelectInput extends React.Component<IAccountSelectInputProps
         mergedAccounts = flatten([primaryAccounts, secondaryAccounts]);
       }
 
-      if (component) {
-        if (!mergedAccounts.includes(component[field])) {
-          onChange('');
-        }
+      if (!mergedAccounts.includes(value)) {
+        onChange(createFakeReactSyntheticEvent({ value: '', name }));
       }
 
       this.setState({ mergedAccounts, primaryAccounts, secondaryAccounts });
@@ -97,12 +91,10 @@ export class AccountSelectInput extends React.Component<IAccountSelectInputProps
   }
 
   public render() {
-    const { component, field, onChange, readOnly } = this.props;
-    const { accountContainsExpression, primaryAccounts, secondaryAccounts } = this.state;
-
-    const value = component[field];
+    const { value, onChange, readOnly } = this.props;
+    const { primaryAccounts, secondaryAccounts } = this.state;
 
-    if (accountContainsExpression) {
+    if (isExpression(value)) {
       return (
         <div className="sm-control-field">
           <span>
@@ -112,34 +104,37 @@ export class AccountSelectInput extends React.Component<IAccountSelectInputProps
       );
     }
 
+    if (readOnly) {
+      return (
+        <div>
+          <p className="form-control-static">{value}</p>
+        </div>
+      );
+    }
+
+    const showSeparator = primaryAccounts.length > 0 && secondaryAccounts.length > 0;
+
     return (
       <div>
-        {!readOnly && (
-          <select
-            className="form-control input-sm"
-            value={value}
-            onChange={e => onChange(e.target.value)}
-            required={true}
-          >
-            <option value="" disabled={true}>
-              Select...
+        <select className="form-control input-sm" value={value} onChange={onChange} required={true}>
+          <option value="" disabled={true}>
+            Select...
+          </option>
+
+          {primaryAccounts.map(account => (
+            <option key={account} value={account}>
+              {account}
+            </option>
+          ))}
+
+          {showSeparator && <option disabled={true}>---------------</option>}
+
+          {secondaryAccounts.map(account => (
+            <option key={account} value={account}>
+              {account}
             </option>
-            {primaryAccounts.map(account => (
-              <option key={account} value={account}>
-                {account}
-              </option>
-            ))}
-            {primaryAccounts.length > 0 &&
-              secondaryAccounts.length > 0 && <option disabled={true}>---------------</option>}
-            {secondaryAccounts.map(account => (
-              <option key={account} value={account}>
-                {account}
-              </option>
-            ))}
-          </select>
-        )}
-
-        {readOnly && <p className="form-control-static">{value}</p>}
+          ))}
+        </select>
       </div>
     );
   }

diff --git a/app/scripts/modules/core/src/presentation/forms/inputs/ReactSelectInput.tsx b/app/scripts/modules/core/src/presentation/forms/inputs/ReactSelectInput.tsx
@@ -4,7 +4,7 @@ import Select, { Option, ReactSelectProps } from 'react-select';
 import { OmitControlledInputPropsFrom, StringsAsOptions, TetheredSelect } from 'core/presentation';
 import { noop } from 'core/utils';
 
-import { isStringArray, orEmptyString } from './utils';
+import { createFakeReactSyntheticEvent, isStringArray, orEmptyString } from './utils';
 import { IFormInputProps } from '../interface';
 
 interface IReactSelectInputProps extends IFormInputProps, OmitControlledInputPropsFrom<ReactSelectProps> {
@@ -20,13 +20,6 @@ export const reactSelectValidationErrorStyle = {
   boxShadow: 'inset 0 1px 1px rgba(0, 0, 0, 0.075)',
 };
 
-export const createFakeReactSyntheticEvent = (target: { name: string; value?: any }) => ({
-  persist: noop,
-  stopPropagation: noop,
-  preventDefault: noop,
-  target,
-});
-
 /**
  * Given a IControlledInputProps "field" (i.e., from Formik), returns an onChange handler
  * somewhat compatible with the controlled input pattern

diff --git a/app/scripts/modules/core/src/presentation/forms/inputs/index.ts b/app/scripts/modules/core/src/presentation/forms/inputs/index.ts
@@ -7,3 +7,4 @@ export * from './StringsAsOptions';
 export * from './TextAreaInput';
 export * from './TextInput';
 export * from './expression';
+export * from './utils';
diff --git a/app/scripts/modules/core/src/presentation/forms/inputs/utils.ts b/app/scripts/modules/core/src/presentation/forms/inputs/utils.ts
@@ -1,6 +1,8 @@
 import * as classNames from 'classnames';
 import { isUndefined, isString } from 'lodash';
 
+import { noop } from 'core/utils';
+
 import { IValidationProps } from '../interface';
 
 export const orEmptyString = (val: any) => (isUndefined(val) ? '' : val);
@@ -14,4 +16,12 @@ export const validationClassName = (validation: IValidationProps) => {
   });
 };
 
+export const createFakeReactSyntheticEvent = (target: { name?: string; value?: any }) =>
+  ({
+    persist: noop,
+    stopPropagation: noop,
+    preventDefault: noop,
+    target,
+  } as React.ChangeEvent<any>);
+
 export const isStringArray = (opts: any[]): opts is string[] => opts && opts.length && opts.every(isString);
diff --git a/app/scripts/modules/kubernetes/src/v2/manifest/selector/ManifestSelector.spec.tsx b/app/scripts/modules/kubernetes/src/v2/manifest/selector/ManifestSelector.spec.tsx
@@ -4,7 +4,13 @@ import { Creatable, Option } from 'react-select';
 import { $q } from 'ngimport';
 import Spy = jasmine.Spy;
 
-import { noop, ScopeClusterSelector, AccountService, AccountSelectInput } from '@spinnaker/core';
+import {
+  AccountSelectInput,
+  AccountService,
+  ScopeClusterSelector,
+  createFakeReactSyntheticEvent,
+  noop,
+} from '@spinnaker/core';
 
 import { ManifestKindSearchService } from 'kubernetes/v2/manifest/ManifestKindSearch';
 import { ManifestSelector } from 'kubernetes/v2/manifest/selector/ManifestSelector';
@@ -271,7 +277,7 @@ describe('<ManifestSelector />', () => {
       });
 
       const account = wrapper.find(AccountSelectInput).first();
-      account.props().onChange('my-other-account');
+      account.props().onChange(createFakeReactSyntheticEvent({ value: 'my-other-account' }));
       expect(searchService).toHaveBeenCalledWith('configMap', 'default', 'my-other-account');
     });
 
@@ -289,7 +295,7 @@ describe('<ManifestSelector />', () => {
       });
 
       const account = wrapper.find(AccountSelectInput).first();
-      account.props().onChange('my-other-account');
+      account.props().onChange(createFakeReactSyntheticEvent({ value: 'my-other-account' }));
       expect(wrapper.instance().state.selector.location).toBeFalsy();
     });
   });

diff --git a/app/scripts/modules/kubernetes/src/v2/manifest/selector/ManifestSelector.tsx b/app/scripts/modules/kubernetes/src/v2/manifest/selector/ManifestSelector.tsx
@@ -274,10 +274,9 @@ export class ManifestSelector extends React.Component<IManifestSelectorProps, IM
       <>
         <StageConfigField label="Account">
           <AccountSelectInput
-            component={selector}
-            field="account"
+            value={selector.account}
+            onChange={evt => this.handleAccountChange(evt.target.value)}
             accounts={accounts}
-            onChange={this.handleAccountChange}
             provider="'kubernetes'"
           />
         </StageConfigField>

diff --git a/app/scripts/modules/kubernetes/src/v2/manifest/wizard/BasicSettings.tsx b/app/scripts/modules/kubernetes/src/v2/manifest/wizard/BasicSettings.tsx
@@ -35,12 +35,11 @@ class ManifestBasicSettingsImpl extends React.Component<IManifestBasicSettingsPr
           </div>
           <div className="col-md-7">
             <AccountSelectInput
+              value={formik.values.command.account}
+              onChange={evt => this.accountUpdated(evt.target.value)}
               readOnly={false}
-              component={formik.values.command}
-              field="account"
               accounts={accounts}
               provider="kubernetes"
-              onChange={this.accountUpdated}
             />
           </div>
         </div>

diff --git a/app/scripts/modules/titus/src/pipeline/stages/runJob/TitusRunJobStageConfig.tsx b/app/scripts/modules/titus/src/pipeline/stages/runJob/TitusRunJobStageConfig.tsx
@@ -157,10 +157,9 @@ export class TitusRunJobStageConfig extends React.Component<IStageConfigProps, I
           </label>
           <div className="col-md-5">
             <AccountSelectInput
-              component={stage}
-              field="credentials"
+              value={stage.credentials}
+              onChange={evt => this.accountChanged(evt.target.value)}
               accounts={credentials}
-              onChange={this.accountChanged}
               provider="titus"
             />
             {stage.credentials !== undefined && (

diff --git a/...scripts/modules/titus/src/serverGroup/configure/wizard/pages/ServerGroupBasicSettings.tsx b/...scripts/modules/titus/src/serverGroup/configure/wizard/pages/ServerGroupBasicSettings.tsx
@@ -179,12 +179,11 @@ class ServerGroupBasicSettingsImpl extends React.Component<
           <div className="col-md-3 sm-label-right">Account</div>
           <div className="col-md-7">
             <AccountSelectInput
+              value={values.credentials}
+              onChange={evt => this.accountUpdated(evt.target.value)}
               readOnly={readOnlyFields.credentials}
-              component={values}
-              field="credentials"
               accounts={accounts}
               provider="titus"
-              onChange={this.accountUpdated}
             />
             {values.credentials !== undefined && (
               <div className="small">