Merge pull request #8869 from mabashian/6510-schedule-detail-prompts

Fix prompted values section of schedule details

Reviewed-by: https://github.com/apps/softwarefactory-project-zuul

awx/ui_next/src/components/PromptDetail/PromptDetail.jsx

@@ -5,7 +5,7 @@ import { withI18n } from '@lingui/react';
 import { t, Trans } from '@lingui/macro';
 import { Link } from 'react-router-dom';
 import styled from 'styled-components';
-import { Chip, Divider } from '@patternfly/react-core';
+import { Chip, Divider, Title } from '@patternfly/react-core';
 import { toTitleCase } from '../../util/strings';
 
 import CredentialChip from '../CredentialChip';
@@ -18,9 +18,19 @@ import PromptInventorySourceDetail from './PromptInventorySourceDetail';
 import PromptJobTemplateDetail from './PromptJobTemplateDetail';
 import PromptWFJobTemplateDetail from './PromptWFJobTemplateDetail';
 
-const PromptHeader = styled.h2`
+const PromptTitle = styled(Title)`
-  font-weight: bold;
+  margin-top: var(--pf-global--spacer--xl);
-  margin: var(--pf-global--spacer--lg) 0;
+  --pf-c-title--m-md--FontWeight: 700;
+  grid-column: 1 / -1;
+`;
+
+const PromptDivider = styled(Divider)`
+  margin-top: var(--pf-global--spacer--lg);
+  margin-bottom: var(--pf-global--spacer--lg);
+`;
+
+const PromptDetailList = styled(DetailList)`
+  padding: 0px var(--pf-global--spacer--lg);
 `;
 
 function formatTimeout(timeout) {
@@ -136,9 +146,11 @@ function PromptDetail({ i18n, resource, launchConfig = {}, overrides = {} }) {
 
       {hasPromptData(launchConfig) && hasOverrides && (
         <>
-          <Divider css="margin-top: var(--pf-global--spacer--lg)" />
+          <PromptTitle headingLevel="h2">
-          <PromptHeader>{i18n._(t`Prompted Values`)}</PromptHeader>
+            {i18n._(t`Prompted Values`)}
-          <DetailList aria-label={i18n._(t`Prompt Overrides`)}>
+          </PromptTitle>
+          <PromptDivider />
+          <PromptDetailList aria-label={i18n._(t`Prompt Overrides`)}>
             {launchConfig.ask_job_type_on_launch && (
               <Detail
                 label={i18n._(t`Job Type`)}
@@ -250,7 +262,7 @@ function PromptDetail({ i18n, resource, launchConfig = {}, overrides = {} }) {
                 value={overrides.extra_vars}
               />
             )}
-          </DetailList>
+          </PromptDetailList>
         </>
       )}
     </>

awx/ui_next/src/components/Schedule/ScheduleDetail/ScheduleDetail.jsx

@@ -5,7 +5,7 @@ import { RRule, rrulestr } from 'rrule';
 import styled from 'styled-components';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
-import { Chip, Title, Button } from '@patternfly/react-core';
+import { Chip, Divider, Title, Button } from '@patternfly/react-core';
 import { Schedule } from '../../../types';
 import AlertModal from '../../AlertModal';
 import { CardBody, CardActionsRow } from '../../Card';
@@ -27,11 +27,21 @@ import ErrorDetail from '../../ErrorDetail';
 import ChipGroup from '../../ChipGroup';
 import { VariablesDetail } from '../../CodeMirrorInput';
 
+const PromptDivider = styled(Divider)`
+  margin-top: var(--pf-global--spacer--lg);
+  margin-bottom: var(--pf-global--spacer--lg);
+`;
+
 const PromptTitle = styled(Title)`
+  margin-top: 40px;
   --pf-c-title--m-md--FontWeight: 700;
   grid-column: 1 / -1;
 `;
 
+const PromptDetailList = styled(DetailList)`
+  padding: 0px 20px;
+`;
+
 function ScheduleDetail({ schedule, i18n }) {
   const {
     id,
@@ -41,6 +51,7 @@ function ScheduleDetail({ schedule, i18n }) {
     dtend,
     dtstart,
     extra_data,
+    inventory,
     job_tags,
     job_type,
     limit,
@@ -52,12 +63,21 @@ function ScheduleDetail({ schedule, i18n }) {
     skip_tags,
     summary_fields,
     timezone,
+    verbosity,
   } = schedule;
 
   const history = useHistory();
   const { pathname } = useLocation();
   const pathRoot = pathname.substr(0, pathname.indexOf('schedules'));
 
+  const VERBOSITY = {
+    0: i18n._(t`0 (Normal)`),
+    1: i18n._(t`1 (Verbose)`),
+    2: i18n._(t`2 (More Verbose)`),
+    3: i18n._(t`3 (Debug)`),
+    4: i18n._(t`4 (Connection Debug)`),
+  };
+
   const {
     request: deleteSchedule,
     isLoading: isDeleteLoading,
@@ -140,18 +160,34 @@ function ScheduleDetail({ schedule, i18n }) {
     survey_enabled,
   } = launchData || {};
 
+  const showCredentialsDetail =
+    ask_credential_on_launch && credentials.length > 0;
+  const showInventoryDetail = ask_inventory_on_launch && inventory;
+  const showVariablesDetail =
+    (ask_variables_on_launch || survey_enabled) &&
+    ((typeof extra_data === 'string' && extra_data !== '') ||
+      (typeof extra_data === 'object' && Object.keys(extra_data).length > 0));
+  const showTagsDetail = ask_tags_on_launch && job_tags && job_tags.length > 0;
+  const showSkipTagsDetail =
+    ask_skip_tags_on_launch && skip_tags && skip_tags.length > 0;
+  const showDiffModeDetail =
+    ask_diff_mode_on_launch && typeof diff_mode === 'boolean';
+  const showLimitDetail = ask_limit_on_launch && limit;
+  const showJobTypeDetail = ask_job_type_on_launch && job_type;
+  const showSCMBranchDetail = ask_scm_branch_on_launch && scm_branch;
+  const showVerbosityDetail = ask_verbosity_on_launch && VERBOSITY[verbosity];
+
   const showPromptedFields =
-    ask_credential_on_launch ||
+    showCredentialsDetail ||
-    ask_diff_mode_on_launch ||
+    showDiffModeDetail ||
-    ask_inventory_on_launch ||
+    showInventoryDetail ||
-    ask_job_type_on_launch ||
+    showJobTypeDetail ||
-    ask_limit_on_launch ||
+    showLimitDetail ||
-    ask_scm_branch_on_launch ||
+    showSCMBranchDetail ||
-    ask_skip_tags_on_launch ||
+    showSkipTagsDetail ||
-    ask_tags_on_launch ||
+    showTagsDetail ||
-    ask_variables_on_launch ||
+    showVerbosityDetail ||
-    ask_verbosity_on_launch ||
+    showVariablesDetail;
-    survey_enabled;
 
   if (isLoading) {
     return <ContentLoading />;
@@ -189,15 +225,18 @@ function ScheduleDetail({ schedule, i18n }) {
           date={modified}
           user={summary_fields.modified_by}
         />
-        {showPromptedFields && (
+      </DetailList>
-          <>
+      {showPromptedFields && (
-            <PromptTitle headingLevel="h2">
+        <>
-              {i18n._(t`Prompted Fields`)}
+          <PromptTitle headingLevel="h2">
-            </PromptTitle>
+            {i18n._(t`Prompted Values`)}
+          </PromptTitle>
+          <PromptDivider />
+          <PromptDetailList>
             {ask_job_type_on_launch && (
               <Detail label={i18n._(t`Job Type`)} value={job_type} />
             )}
-            {ask_inventory_on_launch && (
+            {showInventoryDetail && (
               <Detail
                 label={i18n._(t`Inventory`)}
                 value={
@@ -226,13 +265,19 @@ function ScheduleDetail({ schedule, i18n }) {
             {ask_limit_on_launch && (
               <Detail label={i18n._(t`Limit`)} value={limit} />
             )}
-            {ask_diff_mode_on_launch && typeof diff_mode === 'boolean' && (
+            {ask_verbosity_on_launch && (
+              <Detail
+                label={i18n._(t`Verbosity`)}
+                value={VERBOSITY[verbosity]}
+              />
+            )}
+            {showDiffModeDetail && (
               <Detail
                 label={i18n._(t`Show Changes`)}
                 value={diff_mode ? i18n._(t`On`) : i18n._(t`Off`)}
               />
             )}
-            {ask_credential_on_launch && (
+            {showCredentialsDetail && (
               <Detail
                 fullWidth
                 label={i18n._(t`Credentials`)}
@@ -245,7 +290,7 @@ function ScheduleDetail({ schedule, i18n }) {
                 }
               />
             )}
-            {ask_tags_on_launch && job_tags && job_tags.length > 0 && (
+            {showTagsDetail && (
               <Detail
                 fullWidth
                 label={i18n._(t`Job Tags`)}
@@ -263,7 +308,7 @@ function ScheduleDetail({ schedule, i18n }) {
                 }
               />
             )}
-            {ask_skip_tags_on_launch && skip_tags && skip_tags.length > 0 && (
+            {showSkipTagsDetail && (
               <Detail
                 fullWidth
                 label={i18n._(t`Skip Tags`)}
@@ -281,16 +326,16 @@ function ScheduleDetail({ schedule, i18n }) {
                 }
               />
             )}
-            {(ask_variables_on_launch || survey_enabled) && (
+            {showVariablesDetail && (
               <VariablesDetail
                 value={extra_data}
                 rows={4}
                 label={i18n._(t`Variables`)}
               />
             )}
-          </>
+          </PromptDetailList>
-        )}
+        </>
-      </DetailList>
+      )}
       <CardActionsRow>
         {summary_fields?.user_capabilities?.edit && (
           <Button

awx/ui_next/src/components/Schedule/ScheduleDetail/ScheduleDetail.test.jsx

@@ -73,10 +73,6 @@ const schedule = {
       first_name: '',
       last_name: '',
     },
-    inventory: {
-      id: 1,
-      name: 'Test Inventory',
-    },
   },
   created: '2020-03-03T20:38:54.210306Z',
   modified: '2020-03-03T20:38:54.210336Z',
@@ -88,6 +84,27 @@ const schedule = {
   dtend: '2020-07-06T04:00:00Z',
   next_run: '2020-03-16T04:00:00Z',
   extra_data: {},
+  inventory: null,
+  scm_branch: null,
+  job_type: null,
+  job_tags: null,
+  skip_tags: null,
+  limit: null,
+  diff_mode: null,
+  verbosity: null,
+};
+
+const scheduleWithPrompts = {
+  ...schedule,
+  job_type: 'run',
+  inventory: 1,
+  job_tags: 'tag1',
+  skip_tags: 'tag2',
+  scm_branch: 'foo/branch',
+  limit: 'localhost',
+  diff_mode: true,
+  verbosity: 1,
+  extra_data: { foo: 'fii' },
 };
 
 SchedulesAPI.createPreview.mockResolvedValue({
@@ -159,13 +176,14 @@ describe('<ScheduleDetail />', () => {
     expect(wrapper.find('Detail[label="Repeat Frequency"]').length).toBe(1);
     expect(wrapper.find('Detail[label="Created"]').length).toBe(1);
     expect(wrapper.find('Detail[label="Last Modified"]').length).toBe(1);
-    expect(wrapper.find('Title[children="Prompted Fields"]').length).toBe(0);
+    expect(wrapper.find('Title[children="Prompted Values"]').length).toBe(0);
     expect(wrapper.find('Detail[label="Job Type"]').length).toBe(0);
     expect(wrapper.find('Detail[label="Inventory"]').length).toBe(0);
     expect(wrapper.find('Detail[label="Source Control Branch"]').length).toBe(
       0
     );
     expect(wrapper.find('Detail[label="Limit"]').length).toBe(0);
+    expect(wrapper.find('Detail[label="Verbosity"]').length).toBe(0);
     expect(wrapper.find('Detail[label="Show Changes"]').length).toBe(0);
     expect(wrapper.find('Detail[label="Credentials"]').length).toBe(0);
     expect(wrapper.find('Detail[label="Job Tags"]').length).toBe(0);
@@ -189,18 +207,6 @@ describe('<ScheduleDetail />', () => {
       },
     });
     JobTemplatesAPI.readLaunch.mockResolvedValueOnce(allPrompts);
-    const scheduleWithPrompts = {
-      ...schedule,
-      job_type: 'run',
-      inventory: 1,
-      job_tags: 'tag1',
-      skip_tags: 'tag2',
-      scm_branch: 'foo/branch',
-      limit: 'localhost',
-      diff_mode: true,
-      verbosity: 1,
-      extra_data: { foo: 'fii' },
-    };
     await act(async () => {
       wrapper = mountWithContexts(
         <Route
@@ -245,7 +251,7 @@ describe('<ScheduleDetail />', () => {
     expect(wrapper.find('Detail[label="Repeat Frequency"]').length).toBe(1);
     expect(wrapper.find('Detail[label="Created"]').length).toBe(1);
     expect(wrapper.find('Detail[label="Last Modified"]').length).toBe(1);
-    expect(wrapper.find('Title[children="Prompted Fields"]').length).toBe(1);
+    expect(wrapper.find('Title[children="Prompted Values"]').length).toBe(1);
     expect(
       wrapper
         .find('Detail[label="Job Type"]')
@@ -265,12 +271,102 @@ describe('<ScheduleDetail />', () => {
         .find('dd')
         .text()
     ).toBe('localhost');
+    expect(
+      wrapper
+        .find('Detail[label="Verbosity"]')
+        .find('dd')
+        .text()
+    ).toBe('1 (Verbose)');
     expect(wrapper.find('Detail[label="Show Changes"]').length).toBe(1);
     expect(wrapper.find('Detail[label="Credentials"]').length).toBe(1);
     expect(wrapper.find('Detail[label="Job Tags"]').length).toBe(1);
     expect(wrapper.find('Detail[label="Skip Tags"]').length).toBe(1);
     expect(wrapper.find('VariablesDetail').length).toBe(1);
   });
+  test('prompt values section should be hidden if no overrides are present on the schedule but ask_ options are all true', async () => {
+    SchedulesAPI.readCredentials.mockResolvedValueOnce({
+      data: {
+        count: 0,
+        results: [],
+      },
+    });
+    JobTemplatesAPI.readLaunch.mockResolvedValueOnce(allPrompts);
+    await act(async () => {
+      wrapper = mountWithContexts(
+        <Route
+          path="/templates/job_template/:id/schedules/:scheduleId"
+          component={() => <ScheduleDetail schedule={schedule} />}
+        />,
+        {
+          context: {
+            router: {
+              history,
+              route: {
+                location: history.location,
+                match: { params: { id: 1 } },
+              },
+            },
+          },
+        }
+      );
+    });
+    await waitForElement(wrapper, 'ContentLoading', el => el.length === 0);
+    expect(wrapper.find('Title[children="Prompted Values"]').length).toBe(0);
+    expect(wrapper.find('Detail[label="Job Type"]').length).toBe(0);
+    expect(wrapper.find('Detail[label="Inventory"]').length).toBe(0);
+    expect(wrapper.find('Detail[label="Source Control Branch"]').length).toBe(
+      0
+    );
+    expect(wrapper.find('Detail[label="Limit"]').length).toBe(0);
+    expect(wrapper.find('Detail[label="Verbosity"]').length).toBe(0);
+    expect(wrapper.find('Detail[label="Show Changes"]').length).toBe(0);
+    expect(wrapper.find('Detail[label="Credentials"]').length).toBe(0);
+    expect(wrapper.find('Detail[label="Job Tags"]').length).toBe(0);
+    expect(wrapper.find('Detail[label="Skip Tags"]').length).toBe(0);
+    expect(wrapper.find('VariablesDetail').length).toBe(0);
+  });
+  test('prompt values section should be hidden if overrides are present on the schedule but ask_ options are all false', async () => {
+    SchedulesAPI.readCredentials.mockResolvedValueOnce({
+      data: {
+        count: 0,
+        results: [],
+      },
+    });
+    JobTemplatesAPI.readLaunch.mockResolvedValueOnce(noPrompts);
+    await act(async () => {
+      wrapper = mountWithContexts(
+        <Route
+          path="/templates/job_template/:id/schedules/:scheduleId"
+          component={() => <ScheduleDetail schedule={scheduleWithPrompts} />}
+        />,
+        {
+          context: {
+            router: {
+              history,
+              route: {
+                location: history.location,
+                match: { params: { id: 1 } },
+              },
+            },
+          },
+        }
+      );
+    });
+    await waitForElement(wrapper, 'ContentLoading', el => el.length === 0);
+    expect(wrapper.find('Title[children="Prompted Values"]').length).toBe(0);
+    expect(wrapper.find('Detail[label="Job Type"]').length).toBe(0);
+    expect(wrapper.find('Detail[label="Inventory"]').length).toBe(0);
+    expect(wrapper.find('Detail[label="Source Control Branch"]').length).toBe(
+      0
+    );
+    expect(wrapper.find('Detail[label="Limit"]').length).toBe(0);
+    expect(wrapper.find('Detail[label="Verbosity"]').length).toBe(0);
+    expect(wrapper.find('Detail[label="Show Changes"]').length).toBe(0);
+    expect(wrapper.find('Detail[label="Credentials"]').length).toBe(0);
+    expect(wrapper.find('Detail[label="Job Tags"]').length).toBe(0);
+    expect(wrapper.find('Detail[label="Skip Tags"]').length).toBe(0);
+    expect(wrapper.find('VariablesDetail').length).toBe(0);
+  });
   test('error shown when error encountered fetching credentials', async () => {
     SchedulesAPI.readCredentials.mockRejectedValueOnce(
       new Error({