Default Plan drop down selector should ask for confirmation before changing

app/controllers/api/application_plans_controller.rb

@@ -35,6 +35,8 @@ def destroy
 
   def masterize
     assign_plan!(@service, :default_application_plan)
+    flash[:notice] = 'The default plan has been changed.'
+    redirect_to admin_service_application_plans_path(@service)
   end
 
   protected

app/javascript/packs/default_plan_selector.js

@@ -1,12 +1,13 @@
 // @flow
 
-import { DefaultPlanSelectWrapper } from 'Plans'
+import { DefaultPlanSelectCardWrapper } from 'Plans'
 import { safeFromJsonString } from 'utilities'
 
 import type { Record as Plan } from 'Types'
 
 document.addEventListener('DOMContentLoaded', () => {
-  const container = document.getElementById('default_plan')
+  const containerId = 'default_plan'
+  const container = document.getElementById(containerId)
 
   if (!container) {
     return
@@ -17,9 +18,9 @@ document.addEventListener('DOMContentLoaded', () => {
   const initialDefaultPlan = safeFromJsonString<Plan>(dataset.currentPlan) || null
   const path: string = dataset.path
 
-  DefaultPlanSelectWrapper({
+  DefaultPlanSelectCardWrapper({
     initialDefaultPlan,
     plans: plans,
     path
-  }, 'default_plan')
+  }, containerId)
 })

app/javascript/src/Common/components/HelperText.jsx

@@ -0,0 +1,36 @@
+// @flow
+// TODO: Replace this component when updating patternfly-react.
+
+import * as React from 'react'
+
+import './HelperText.scss'
+
+type HelperTextProps = {
+  children: React.Node
+}
+
+type HelperTextItemProps = {
+  children: string
+}
+
+const HelperText = ({
+  children
+}: HelperTextProps): React.Node => {
+  return (
+    <div className="pf-c-helper-text">
+      {children}
+    </div>
+  )
+}
+
+const HelperTextItem = ({
+  children
+}: HelperTextItemProps): React.Node => {
+  return (
+    <div className="pf-c-helper-text__item">
+      <span className="pf-c-helper-text__item-text">{children}</span>
+    </div>
+  )
+}
+
+export { HelperText, HelperTextItem }

app/javascript/src/Common/components/HelperText.scss

@@ -0,0 +1,7 @@
+// TODO: Replace this component when updating patternfly-react.
+
+.pf-c-helper-text__item-text {
+  color: var(--pf-c-helper-text__item-text--Color);
+  font-size: .875rem;
+  padding-top: .25rem;
+}

app/javascript/src/Plans/components/DefaultPlanSelectCard.jsx

@@ -2,10 +2,17 @@
 
 import * as React from 'react'
 
-import { Form, Card, CardBody } from '@patternfly/react-core'
-import { DefaultPlanSelect } from 'Plans/components/DefaultPlanSelect'
-import { ajax, createReactWrapper } from 'utilities'
-import * as alert from 'utilities/alert'
+import {
+  ActionGroup,
+  Button,
+  Form,
+  Card,
+  CardBody
+} from '@patternfly/react-core'
+import { Select as SelectFormGroup } from 'Common/components/Select'
+import { HelperText, HelperTextItem } from 'Common/components/HelperText'
+import { createReactWrapper, CSRFToken } from 'utilities'
+
 import type { Record as Plan } from 'Types'
 
 import './DefaultPlanSelectCard.scss'
@@ -17,55 +24,60 @@ type Props = {
 }
 
 const DefaultPlanSelectCard = ({ plans, initialDefaultPlan, path: url }: Props): React.Node => {
-  const NO_DEFAULT_PLAN: Plan = { id: -1, name: '(No default plan)' }
-
-  const [defaultPlan, setDefaultPlan] = React.useState<Plan>(initialDefaultPlan ?? NO_DEFAULT_PLAN)
-
-  const [isLoading, setIsLoading] = React.useState(false)
+  // $FlowIgnore[incompatible-type] id should be a number but the controller has to recieve empty string
+  const NO_DEFAULT_PLAN: Plan = { id: '', name: '(No default plan)' }
 
-  const onSelectPlan = (plan: Plan) => {
-    const body = plan.id >= 0 ? new URLSearchParams({ id: plan.id.toString() }) : undefined
+  const [defaultPlan, setDefaultPlan] = React.useState<Plan | null>(initialDefaultPlan ?? NO_DEFAULT_PLAN)
 
-    ajax(url, { method: 'POST', body })
-      .then(data => {
-        if (data.ok) {
-          alert.notice('Default plan was updated')
-          setDefaultPlan(plan)
-        } else {
-          if (data.status === 404) {
-            alert.error("The selected plan doesn't exist.")
-          } else {
-            alert.error('Plan could not be updated')
-          }
-        }
-      })
-      .catch(err => {
-        console.error(err)
-        alert.error('An error ocurred. Please try again later.')
-      })
-      .finally(() => setIsLoading(false))
+  const availablePlans = [NO_DEFAULT_PLAN, ...plans]
 
-    setIsLoading(true)
-  }
-
-  const availablePlans = [NO_DEFAULT_PLAN, ...plans].filter(p => p.id !== defaultPlan.id)
+  // TODO: in PF4, "isDisabled" behaviour is replaced by ticking the selected item. Remove this after upgrading.
+  const mappedPlans = defaultPlan ? availablePlans.map(p => ({ ...p, disabled: p.id === defaultPlan.id })) : availablePlans
 
   return (
     <Card id="default_plan_card">
       <CardBody>
-        <Form onSubmit={e => e.preventDefault()}>
-          <DefaultPlanSelect
-            plan={defaultPlan}
-            plans={availablePlans}
-            onSelectPlan={onSelectPlan}
-            isLoading={isLoading}
+        <Form
+          acceptCharset="UTF-8"
+          method="post"
+          action={url}
+        >
+          <CSRFToken />
+          <input type="hidden" name="utf8" value="✓" />
+
+          {/* $FlowIgnore[prop-missing] description is optional */}
+          {/* $FlowIgnore[incompatible-type-arg] id can be either number or string */}
+          <SelectFormGroup
+            label="Default plan"
+            // $FlowIgnore[incompatible-type] plan is either Plan or null
+            item={defaultPlan}
+            // $FlowIgnore[incompatible-type] id can be either number or string
+            items={mappedPlans}
+            onSelect={setDefaultPlan}
+            fieldId="id"
+            name="id"
+            placeholderText={defaultPlan ? defaultPlan.name : 'Select application plan'}
           />
+          <ActionGroup>
+            <Button
+              variant="primary"
+              type="submit"
+              isDisabled={!defaultPlan || defaultPlan.id === initialDefaultPlan?.id || (defaultPlan.id === NO_DEFAULT_PLAN.id && !initialDefaultPlan)}
+            >
+              Change plan
+            </Button>
+          </ActionGroup>
         </Form>
+        <HelperText>
+          <HelperTextItem>
+            Default application plan (if any) is selected automatically upon service subscription.
+          </HelperTextItem>
+        </HelperText>
       </CardBody>
     </Card>
   )
 }
 
-const DefaultPlanSelectWrapper = (props: Props, containerId: string): void => createReactWrapper(<DefaultPlanSelectCard {...props} />, containerId)
+const DefaultPlanSelectCardWrapper = (props: Props, containerId: string): void => createReactWrapper(<DefaultPlanSelectCard {...props} />, containerId)
 
-export { DefaultPlanSelectCard, DefaultPlanSelectWrapper }
+export { DefaultPlanSelectCard, DefaultPlanSelectCardWrapper }

app/javascript/src/Plans/components/DefaultPlanSelectCard.scss

@@ -3,8 +3,19 @@
 .pf-c-card#default_plan_card {
   margin-bottom: var(--pf-c-page__main-section--PaddingBottom);
 
-  form {
+  .pf-c-form {
+    // HACK: Using flex so we can align FormGroup with ActionGroup inline
+    align-items: flex-end;
+    display: flex;
+
     // isWidthLimited isn't available in current version. See: https://github.com/patternfly/patternfly-react/blob/master/packages/react-core/src/components/Form/Form.tsx#L12
-    max-width: 500px;
+    max-width: 600px;
+
+    .pf-c-form__group {
+      flex: 2;
+      &.pf-m-action {
+        flex: 1;
+      }
+    }
   }
 }

app/javascript/src/Plans/index.js

@@ -1,5 +1,4 @@
 // @flow
 
 export * from './components/ChangePlanSelectCard'
-export * from './components/DefaultPlanSelect'
 export * from './components/DefaultPlanSelectCard'

app/javascript/src/utilities/test-utils.js

@@ -8,6 +8,17 @@
 
 import type { ReactWrapper } from 'enzyme'
 
+function selectOption <T> (wrapper: ReactWrapper<T>, name: string) {
+  openSelect(wrapper)
+  wrapper.find('.pf-c-select__menu-item')
+    .findWhere(node => node.type() === 'button' && node.text() === name)
+    .simulate('click')
+}
+
+function openSelect <T> (wrapper: ReactWrapper<T>) {
+  wrapper.find('Select button.pf-c-select__toggle-button').simulate('click')
+}
+
 function openSelectWithModal <T> (wrapper: ReactWrapper<T>) {
   // HACK: suppress error logs during this step cause wrapping it inside act() makes the test fail
   const spy = jest.spyOn(console, 'error')
@@ -66,6 +77,8 @@ function updateInput <T> (wrapper: ReactWrapper<T>, value: string, input?: strin
 }
 
 export {
+  selectOption,
+  openSelect,
   openSelectWithModal,
   closeSelectWithModal,
   mockLocation,

features/api/plans/application_plans.feature

@@ -9,6 +9,7 @@ Feature: Application plans index page
     Given a provider is logged in
     And an application plan "Basic" of provider "foo.3scale.localhost"
     And plan "Basic" has applications
+    And an application plan that is not default
     And I go to the application plans admin page
 
   Scenario: Create a simple Application plan
@@ -115,3 +116,15 @@ Feature: Application plans index page
     #   | Plan C  | 0            | published |
     #   | Plan B  | 1            | published |
     #   | Plan A  | 2            | hidden    |
+
+  Scenario: Marking a published plan as default
+    Given the application plan is published
+    Then an admin can select the application plan as default
+
+  Scenario: Marking a Hidden plan as default
+    Given the application plan is hidden
+    Then an admin can select the application plan as default
+
+  Scenario: Selected plan doesn't exist
+    Given the application plan has been deleted
+    Then an admin can't select the application plan as default