Fixes several things about Survey List

Aligns Select All with other select buttons Add required asterisk to those items that are required Adds label for the Default and Question Type column Adds chips for multiselect items. Adds RBAC to add and edit survey. Also fixes a bug where the survey was not reloading properly after edit
2024-10-27 09:25:10 +03:00 · 2020-03-27 10:25:32 -04:00 · 2020-03-27 10:25:32 -04:00 · ed3b6385f1
commit ed3b6385f1
parent 3518fb0c17
11 changed files with 269 additions and 26 deletions
--- a/awx/ui_next/src/components/PaginatedDataList/ToolbarAddButton.jsx
+++ b/awx/ui_next/src/components/PaginatedDataList/ToolbarAddButton.jsx
@ -5,7 +5,7 @@ import { Button, Tooltip } from '@patternfly/react-core';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';

-function ToolbarAddButton({ linkTo, onClick, i18n }) {
+function ToolbarAddButton({ linkTo, onClick, i18n, isDisabled }) {
  if (!linkTo && !onClick) {
    throw new Error(
      'ToolbarAddButton requires either `linkTo` or `onClick` prop'
@ -15,6 +15,7 @@ function ToolbarAddButton({ linkTo, onClick, i18n }) {
    return (
      <Tooltip content={i18n._(t`Add`)} position="top">
        <Button
+          isDisabled={isDisabled}
          component={Link}
          to={linkTo}
          variant="primary"
--- a/awx/ui_next/src/screens/Template/Survey/SurveyList.jsx
+++ b/awx/ui_next/src/screens/Template/Survey/SurveyList.jsx
@ -22,6 +22,7 @@ function SurveyList({
  toggleSurvey,
  updateSurvey,
  deleteSurvey,
+  canAddAndEditSurvey,
  i18n,
 }) {
  const questions = survey?.spec || [];
@ -97,6 +98,7 @@ function SurveyList({
            onSelect={() => handleSelect(question)}
            onMoveUp={moveUp}
            onMoveDown={moveDown}
+            canAddAndEditSurvey={canAddAndEditSurvey}
          />
        ))}
        {isPreviewModalOpen && (
@ -169,6 +171,7 @@ function SurveyList({
        surveyEnabled={surveyEnabled}
        onToggleSurvey={toggleSurvey}
        isDeleteDisabled={selected?.length === 0}
+        canAddAndEditSurvey={canAddAndEditSurvey}
        onToggleDeleteModal={() => setIsDeleteModalOpen(true)}
      />
      {content}
--- a/awx/ui_next/src/screens/Template/Survey/SurveyList.test.jsx
+++ b/awx/ui_next/src/screens/Template/Survey/SurveyList.test.jsx
@ -43,6 +43,7 @@ describe('<SurveyList />', () => {
    await act(async () => {
      wrapper.find('Switch').invoke('onChange')(true);
    });
+
    wrapper.update();

    expect(toggleSurvey).toHaveBeenCalled();
@ -53,14 +54,18 @@ describe('<SurveyList />', () => {
    let wrapper;
    await act(async () => {
      wrapper = mountWithContexts(
-        <SurveyList survey={surveyData} deleteSurvey={deleteSurvey} />
+        <SurveyList
+          survey={surveyData}
+          deleteSurvey={deleteSurvey}
+          canAddAndEditSurvey
+        />
      );
    });
    wrapper.update();
+
    expect(wrapper.find('Button[variant="danger"]').prop('isDisabled')).toBe(
      true
    );
-
    expect(
      wrapper.find('Checkbox[aria-label="Select all"]').prop('isChecked')
    ).toBe(false);
@ -81,6 +86,7 @@ describe('<SurveyList />', () => {
    act(() => {
      wrapper.find('Button[variant="danger"]').invoke('onClick')();
    });
+
    wrapper.update();

    await act(() =>
@ -88,6 +94,7 @@ describe('<SurveyList />', () => {
    );
    expect(deleteSurvey).toHaveBeenCalled();
  });
+
  test('should render Preview button ', async () => {
    let wrapper;

@ -97,6 +104,7 @@ describe('<SurveyList />', () => {

    expect(wrapper.find('Button[aria-label="Preview"]').length).toBe(1);
  });
+
  test('Preview button should render Modal', async () => {
    let wrapper;

@ -108,6 +116,7 @@ describe('<SurveyList />', () => {

    expect(wrapper.find('SurveyPreviewModal').length).toBe(1);
  });
+
  test('Modal close button should close modal', async () => {
    let wrapper;

@ -119,11 +128,34 @@ describe('<SurveyList />', () => {

    expect(wrapper.find('SurveyPreviewModal').length).toBe(1);

-    wrapper.find('Modal').prop('onClose')();
+    act(() => wrapper.find('Modal').prop('onClose')());
+
    wrapper.update();

    expect(wrapper.find('SurveyPreviewModal').length).toBe(0);
  });
+
+  test('user without edit/delete permission cannot delete', async () => {
+    const deleteSurvey = jest.fn();
+    let wrapper;
+    await act(async () => {
+      wrapper = mountWithContexts(
+        <SurveyList survey={surveyData} deleteSurvey={deleteSurvey} />
+      );
+    });
+
+    expect(
+      wrapper
+        .find('DataToolbar')
+        .find('Checkbox')
+        .prop('isDisabled')
+    ).toBe(true);
+    expect(wrapper.find('Switch').prop('isDisabled')).toBe(true);
+    expect(wrapper.find('ToolbarAddButton').prop('isDisabled')).toBe(true);
+    expect(wrapper.find('Button[variant="danger"]').prop('isDisabled')).toBe(
+      true
+    );
+  });
 });

 describe('Survey with no questions', () => {
--- a/awx/ui_next/src/screens/Template/Survey/SurveyListItem.jsx
+++ b/awx/ui_next/src/screens/Template/Survey/SurveyListItem.jsx
@ -4,6 +4,7 @@ import { withI18n } from '@lingui/react';
 import { Link } from 'react-router-dom';
 import {
  Button as _Button,
+  Chip as _Chip,
  DataListAction as _DataListAction,
  DataListCheck,
  DataListItemCells,
@ -27,8 +28,20 @@ const Button = styled(_Button)`
  padding-bottom: 0;
  padding-left: 0;
 `;
+const Required = styled.span`
+  color: red;
+  margin-left: 5px;
+`;
+const Chip = styled(_Chip)`
+  margin-right: 5px;
+`;
+
+const Label = styled.b`
+  margin-right: 20px;
+`;

 function SurveyListItem({
+  canAddAndEditSurvey,
  question,
  i18n,
  isLast,
@ -54,7 +67,7 @@ function SurveyListItem({
              <Button
                variant="plain"
                aria-label={i18n._(t`move up`)}
-                isDisabled={isFirst}
+                isDisabled={isFirst || !canAddAndEditSurvey}
                onClick={() => onMoveUp(question)}
              >
                <CaretUpIcon />
@ -64,7 +77,7 @@ function SurveyListItem({
              <Button
                variant="plain"
                aria-label={i18n._(t`move down`)}
-                isDisabled={isLast}
+                isDisabled={isLast || !canAddAndEditSurvey}
                onClick={() => onMoveDown(question)}
              >
                <CaretDownIcon />
@ -73,6 +86,7 @@ function SurveyListItem({
          </Stack>
        </DataListAction>
        <DataListCheck
+          isDisabled={!canAddAndEditSurvey}
          checked={isChecked}
          onChange={onSelect}
          aria-labelledby="survey check"
@ -80,12 +94,43 @@ function SurveyListItem({
        <DataListItemCells
          dataListCells={[
            <DataListCell key="name">
-              <Link to={`survey/edit/${question.variable}`}>
-                {question.question_name}
-              </Link>
+              <>
+                <Link to={`survey/edit/${question.variable}`}>
+                  {question.question_name}
+                </Link>
+                {question.required && (
+                  <Required
+                    aria-label={i18n._(t`Required`)}
+                    className="pf-c-form__label-required"
+                    aria-hidden="true"
+                  >
+                    *
+                  </Required>
+                )}
+              </>
+            </DataListCell>,
+
+            <DataListCell key="type">
+              <Label>{i18n._(t`Type:`)}</Label>
+              {question.type}
+            </DataListCell>,
+            <DataListCell key="default">
+              <Label>{i18n._(t`Default:`)}</Label>
+              {[question.type].includes('password') && (
+                <span>{i18n._(t`encrypted`).toUpperCase()}</span>
+              )}
+              {[question.type].includes('multiselect') &&
+                question.default.length > 0 &&
+                question.default.split('\n').map(chip => (
+                  <Chip key={chip} isReadOnly>
+                    {chip}
+                  </Chip>
+                ))}
+              {![question.type].includes('password') &&
+                ![question.type].includes('multiselect') && (
+                  <span>{question.default}</span>
+                )}
            </DataListCell>,
-            <DataListCell key="type">{question.type}</DataListCell>,
-            <DataListCell key="default">{question.default}</DataListCell>,
          ]}
        />
      </DataListItemRow>
--- a/awx/ui_next/src/screens/Template/Survey/SurveyListItem.test.jsx
+++ b/awx/ui_next/src/screens/Template/Survey/SurveyListItem.test.jsx
@ -25,6 +25,18 @@ describe('<SurveyListItem />', () => {
    const moveDown = wrapper.find('Button[aria-label="move down"]');
    expect(moveUp.length).toBe(1);
    expect(moveDown.length).toBe(1);
+    expect(
+      wrapper
+        .find('b')
+        .at(0)
+        .text()
+    ).toBe('Type:');
+    expect(
+      wrapper
+        .find('b')
+        .at(1)
+        .text()
+    ).toBe('Default:');
    expect(wrapper.find('DataListCheck').length).toBe(1);
    expect(wrapper.find('DataListCell').length).toBe(3);
  });
@ -44,4 +56,85 @@ describe('<SurveyListItem />', () => {
    expect(moveUp).toBe(true);
    expect(moveDown).toBe(true);
  });
+  test('required item has required asterisk', () => {
+    const newItem = {
+      question_name: 'Foo',
+      default: 'Bar',
+      type: 'text',
+      id: 1,
+      required: true,
+    };
+
+    let wrapper;
+    act(() => {
+      wrapper = mountWithContexts(
+        <SurveyListItem question={newItem} isChecked={false} isFirst isLast />
+      );
+    });
+    expect(wrapper.find('span[aria-label="Required"]').length).toBe(1);
+  });
+  test('items that are not required should not have an asterisk', () => {
+    let wrapper;
+    act(() => {
+      wrapper = mountWithContexts(
+        <SurveyListItem question={item} isChecked={false} isFirst isLast />
+      );
+    });
+    expect(wrapper.find('span[aria-label="Required"]').length).toBe(0);
+  });
+  test('required item has required asterisk', () => {
+    const newItem = {
+      question_name: 'Foo',
+      default: 'a\nd\nb\ne\nf\ng\nh\ni\nk',
+      type: 'multiselect',
+      id: 1,
+    };
+
+    let wrapper;
+    act(() => {
+      wrapper = mountWithContexts(
+        <SurveyListItem question={newItem} isChecked={false} isFirst isLast />
+      );
+    });
+    expect(wrapper.find('Chip').length).toBe(9);
+    wrapper
+      .find('Chip')
+      .map(chip => expect(chip.prop('isReadOnly')).toBe(true));
+  });
+  test('items that are no required should have no an asterisk', () => {
+    const newItem = {
+      question_name: 'Foo',
+      default: '$encrypted$',
+      type: 'password',
+      id: 1,
+    };
+
+    let wrapper;
+    act(() => {
+      wrapper = mountWithContexts(
+        <SurveyListItem question={newItem} isChecked={false} isFirst isLast />
+      );
+    });
+    expect(wrapper.find('span').text()).toBe('ENCRYPTED');
+  });
+  test('users without edit/delete permissions are unable to reorder the questions', () => {
+    let wrapper;
+    act(() => {
+      wrapper = mountWithContexts(
+        <SurveyListItem
+          question={item}
+          canAddAndDelete={false}
+          isChecked={false}
+          isFirst
+          isLast
+        />
+      );
+    });
+    expect(wrapper.find('button[aria-label="move up"]').prop('disabled')).toBe(
+      true
+    );
+    expect(
+      wrapper.find('button[aria-label="move down"]').prop('disabled')
+    ).toBe(true);
+  });
 });
--- a/awx/ui_next/src/screens/Template/Survey/SurveyQuestionForm.jsx
+++ b/awx/ui_next/src/screens/Template/Survey/SurveyQuestionForm.jsx
@ -183,6 +183,9 @@ function SurveyQuestionForm({
                  type="textarea"
                  label={i18n._(t`Multiple Choice Options`)}
                  validate={required(null, i18n)}
+                  tooltip={i18n._(
+                    t`Each answer choice must be on a separate line.`
+                  )}
                  isRequired
                />
                <FormField
--- a/awx/ui_next/src/screens/Template/Survey/SurveyToolbar.jsx
+++ b/awx/ui_next/src/screens/Template/Survey/SurveyToolbar.jsx
@ -2,9 +2,10 @@ import React from 'react';
 import { useRouteMatch } from 'react-router-dom';
 import { t } from '@lingui/macro';
 import { withI18n } from '@lingui/react';
+import styled from 'styled-components';

 import {
-  DataToolbar,
+  DataToolbar as _DataToolbar,
  DataToolbarContent,
  DataToolbarGroup,
  DataToolbarItem,
@ -12,7 +13,12 @@ import {
 import { Switch, Checkbox, Button } from '@patternfly/react-core';
 import { ToolbarAddButton } from '@components/PaginatedDataList';

+const DataToolbar = styled(_DataToolbar)`
+  margin-left: 52px;
+`;
+
 function SurveyToolbar({
+  canAddAndEditSurvey,
  isAllSelected,
  onSelectAll,
  i18n,
@ -21,12 +27,14 @@ function SurveyToolbar({
  isDeleteDisabled,
  onToggleDeleteModal,
 }) {
+  isDeleteDisabled = !canAddAndEditSurvey || isDeleteDisabled;
  const match = useRouteMatch();
  return (
    <DataToolbar id="survey-toolbar">
      <DataToolbarContent>
        <DataToolbarItem>
          <Checkbox
+            isDisabled={!canAddAndEditSurvey}
            isChecked={isAllSelected}
            onChange={isChecked => {
              onSelectAll(isChecked);
@ -42,12 +50,16 @@ function SurveyToolbar({
            label={i18n._(t`On`)}
            labelOff={i18n._(t`Off`)}
            isChecked={surveyEnabled}
+            isDisabled={!canAddAndEditSurvey}
            onChange={() => onToggleSurvey(!surveyEnabled)}
          />
        </DataToolbarItem>
        <DataToolbarGroup>
          <DataToolbarItem>
-            <ToolbarAddButton linkTo={`${match.url}/add`} />
+            <ToolbarAddButton
+              isDisabled={!canAddAndEditSurvey}
+              linkTo={`${match.url}/add`}
+            />
          </DataToolbarItem>
          <DataToolbarItem>
            <Button
--- a/awx/ui_next/src/screens/Template/Survey/SurveyToolbar.test.jsx
+++ b/awx/ui_next/src/screens/Template/Survey/SurveyToolbar.test.jsx
@ -36,6 +36,7 @@ describe('<SurveyToolbar />', () => {
          isAllSelected
          onToggleDeleteModal={jest.fn()}
          onToggleSurvey={jest.fn()}
+          canAddAndEditSurvey
        />
      );
    });
@ -84,4 +85,31 @@ describe('<SurveyToolbar />', () => {
    expect(wrapper.find('Switch').length).toBe(1);
    expect(wrapper.find('Switch').prop('isChecked')).toBe(true);
  });
+  test('all action buttons in toolbar are disabled', () => {
+    let wrapper;
+    act(() => {
+      wrapper = mountWithContexts(
+        <SurveyToolbar
+          surveyEnabled
+          isDeleteDisabled={false}
+          onSelectAll={jest.fn()}
+          isAllSelected
+          onToggleDelete={jest.fn()}
+          onToggleSurvey={jest.fn()}
+          canAddAndEditSurvey={false}
+        />
+      );
+    });
+    expect(
+      wrapper
+        .find('DataToolbar')
+        .find('Checkbox')
+        .prop('isDisabled')
+    ).toBe(true);
+    expect(wrapper.find('Switch').prop('isDisabled')).toBe(true);
+    expect(wrapper.find('ToolbarAddButton').prop('isDisabled')).toBe(true);
+    expect(wrapper.find('Button[variant="danger"]').prop('isDisabled')).toBe(
+      true
+    );
+  });
 });
--- a/awx/ui_next/src/screens/Template/Template.jsx
+++ b/awx/ui_next/src/screens/Template/Template.jsx
@ -71,6 +71,9 @@ function Template({ i18n, me, setBreadcrumb }) {
  };

  const canSeeNotificationsTab = me.is_system_auditor || isNotifAdmin;
+  const canAddAndEditSurvey =
+    template?.summary_fields?.user_capabilities.edit ||
+    template?.summary_fields?.user_capabilities.delete;

  const tabsArray = [
    { name: i18n._(t`Details`), link: `${match.url}/details` },
@ -97,7 +100,7 @@ function Template({ i18n, me, setBreadcrumb }) {
      link: `${match.url}/completed_jobs`,
    },
    {
-      name: i18n._(t`Survey`),
+      name: canAddAndEditSurvey ? i18n._(t`Survey`) : i18n._(t`View Survey`),
      link: `${match.url}/survey`,
    }
  );
@ -200,7 +203,10 @@ function Template({ i18n, me, setBreadcrumb }) {
          )}
          {template && (
            <Route path="/templates/:templateType/:id/survey">
-              <TemplateSurvey template={template} />
+              <TemplateSurvey
+                template={template}
+                canAddAndEditSurvey={canAddAndEditSurvey}
+              />
            </Route>
          )}
          {!hasRolesandTemplateLoading && (
--- a/awx/ui_next/src/screens/Template/TemplateSurvey.jsx
+++ b/awx/ui_next/src/screens/Template/TemplateSurvey.jsx
@ -1,5 +1,5 @@
 import React, { useState, useEffect, useCallback } from 'react';
-import { Switch, Route, useParams } from 'react-router-dom';
+import { Switch, Route, useParams, useLocation } from 'react-router-dom';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 import { JobTemplatesAPI, WorkflowJobTemplatesAPI } from '@api';
@ -9,10 +9,12 @@ import ErrorDetail from '@components/ErrorDetail';
 import useRequest, { useDismissableError } from '@util/useRequest';
 import { SurveyList, SurveyQuestionAdd, SurveyQuestionEdit } from './Survey';

-function TemplateSurvey({ template, i18n }) {
+function TemplateSurvey({ template, canAddAndEditSurvey, i18n }) {
  const [surveyEnabled, setSurveyEnabled] = useState(template.survey_enabled);

  const { templateType } = useParams();
+  const location = useLocation();
+
  const {
    result: survey,
    request: fetchSurvey,
@ -28,9 +30,10 @@ function TemplateSurvey({ template, i18n }) {
      return data;
    }, [template.id, templateType])
  );
+
  useEffect(() => {
    fetchSurvey();
-  }, [fetchSurvey]);
+  }, [fetchSurvey, location]);

  const { request: updateSurvey, error: updateError } = useRequest(
    useCallback(
@ -82,12 +85,22 @@ function TemplateSurvey({ template, i18n }) {
  return (
    <>
      <Switch>
-        <Route path="/templates/:templateType/:id/survey/add">
-          <SurveyQuestionAdd survey={survey} updateSurvey={updateSurveySpec} />
-        </Route>
-        <Route path="/templates/:templateType/:id/survey/edit/:variable">
-          <SurveyQuestionEdit survey={survey} updateSurvey={updateSurveySpec} />
-        </Route>
+        {canAddAndEditSurvey && (
+          <Route path="/templates/:templateType/:id/survey/add">
+            <SurveyQuestionAdd
+              survey={survey}
+              updateSurvey={updateSurveySpec}
+            />
+          </Route>
+        )}
+        {canAddAndEditSurvey && (
+          <Route path="/templates/:templateType/:id/survey/edit/:variable">
+            <SurveyQuestionEdit
+              survey={survey}
+              updateSurvey={updateSurveySpec}
+            />
+          </Route>
+        )}
        <Route path="/templates/:templateType/:id/survey" exact>
          <SurveyList
            isLoading={isLoading}
@ -96,6 +109,7 @@ function TemplateSurvey({ template, i18n }) {
            toggleSurvey={toggleSurvey}
            updateSurvey={updateSurveySpec}
            deleteSurvey={deleteSurvey}
+            canAddAndEditSurvey={canAddAndEditSurvey}
          />
        </Route>
      </Switch>
--- a/awx/ui_next/src/screens/Template/WorkflowJobTemplate.jsx
+++ b/awx/ui_next/src/screens/Template/WorkflowJobTemplate.jsx
@ -116,6 +116,9 @@ class WorkflowJobTemplate extends Component {

    const canSeeNotificationsTab = me.is_system_auditor || isNotifAdmin;
    const canToggleNotifications = isNotifAdmin;
+    const canAddAndEditSurvey =
+      template?.summary_fields?.user_capabilities.edit ||
+      template?.summary_fields?.user_capabilities.delete;

    const tabsArray = [
      { name: i18n._(t`Details`), link: `${match.url}/details` },
@ -145,7 +148,7 @@ class WorkflowJobTemplate extends Component {
      link: `${match.url}/completed_jobs`,
    });
    tabsArray.push({
-      name: i18n._(t`Survey`),
+      name: canAddAndEditSurvey ? i18n._(t`Survey`) : i18n._(t`View Survey`),
      link: `${match.url}/survey`,
    });

@ -269,7 +272,10 @@ class WorkflowJobTemplate extends Component {
            )}
            {template && (
              <Route path="/templates/:templateType/:id/survey">
-                <TemplateSurvey template={template} />
+                <TemplateSurvey
+                  template={template}
+                  canAddAndEditSurvey={canAddAndEditSurvey}
+                />
              </Route>
            )}
            <Route key="not-found" path="*">