Fix issue with broken survey question edit breadcrumb by altering the url scheme

awx/ui_next/src/screens/Template/Survey/SurveyListItem.jsx

@@ -94,7 +94,7 @@ function SurveyListItem({
           dataListCells={[
             <DataListCell key="name">
               <>
-                <Link to={`survey/edit/${question.variable}`}>
+                <Link to={`survey/edit?question_variable=${question.variable}`}>
                   {question.question_name}
                 </Link>
                 {question.required && (

awx/ui_next/src/screens/Template/Survey/SurveyQuestionEdit.jsx

@@ -1,5 +1,10 @@
 import React, { useState } from 'react';
-import { useHistory, useRouteMatch } from 'react-router-dom';
+import {
+  Redirect,
+  useHistory,
+  useLocation,
+  useRouteMatch,
+} from 'react-router-dom';
 import ContentLoading from '../../../components/ContentLoading';
 import { CardBody } from '../../../components/Card';
 import SurveyQuestionForm from './SurveyQuestionForm';
@@ -8,12 +13,23 @@ export default function SurveyQuestionEdit({ survey, updateSurvey }) {
   const [formError, setFormError] = useState(null);
   const history = useHistory();
   const match = useRouteMatch();
+  const { search } = useLocation();
+  const queryParams = new URLSearchParams(search);
+  const questionVariable = queryParams.get('question_variable');
 
   if (!survey) {
     return <ContentLoading />;
   }
 
-  const question = survey.spec.find(q => q.variable === match.params.variable);
+  const question = survey.spec.find(q => q.variable === questionVariable);
+
+  if (!question) {
+    return (
+      <Redirect
+        to={`/templates/${match.params.templateType}/${match.params.id}/survey`}
+      />
+    );
+  }
 
   const navigateToList = () => {
     const index = match.url.indexOf('/edit');
@@ -34,7 +50,7 @@ export default function SurveyQuestionEdit({ survey, updateSurvey }) {
         return;
       }
       const questionIndex = survey.spec.findIndex(
-        q => q.variable === match.params.variable
+        q => q.variable === questionVariable
       );
       if (questionIndex === -1) {
         throw new Error('Question not found in spec');

awx/ui_next/src/screens/Template/Survey/SurveyQuestionEdit.test.jsx

@@ -33,87 +33,124 @@ describe('<SurveyQuestionEdit />', () => {
   let history;
   let wrapper;
 
-  beforeEach(() => {
+  describe('with question_variable present', () => {
-    history = createMemoryHistory({
+    beforeEach(() => {
-      initialEntries: ['/templates/job_templates/1/survey/edit/foo'],
+      history = createMemoryHistory({
+        initialEntries: [
+          '/templates/job_templates/1/survey/edit?question_variable=foo',
+        ],
+      });
+      updateSurvey = jest.fn();
+      act(() => {
+        wrapper = mountWithContexts(
+          <Switch>
+            <Route path="/templates/:templateType/:id/survey/edit">
+              <SurveyQuestionEdit survey={survey} updateSurvey={updateSurvey} />
+            </Route>
+          </Switch>,
+          {
+            context: { router: { history } },
+          }
+        );
+      });
     });
-    updateSurvey = jest.fn();
+
-    act(() => {
+    afterEach(() => {
-      wrapper = mountWithContexts(
+      wrapper.unmount();
-        <Switch>
+    });
-          <Route path="/templates/:templateType/:id/survey/edit/:variable">
+
-            <SurveyQuestionEdit survey={survey} updateSurvey={updateSurvey} />
+    test('should render form', () => {
-          </Route>
+      expect(wrapper.find('SurveyQuestionForm')).toHaveLength(1);
-        </Switch>,
+    });
+
+    test('should call updateSurvey', () => {
+      act(() => {
+        wrapper.find('SurveyQuestionForm').invoke('handleSubmit')({
+          question_name: 'new question',
+          variable: 'question',
+          type: 'text',
+        });
+      });
+      wrapper.update();
+
+      expect(updateSurvey).toHaveBeenCalledWith([
         {
-          context: { router: { history } },
+          question_name: 'new question',
-        }
+          variable: 'question',
+          type: 'text',
+        },
+        survey.spec[1],
+      ]);
+    });
+
+    test('should set formError', async () => {
+      const realConsoleError = global.console.error;
+      global.console.error = jest.fn();
+      const err = new Error('oops');
+      updateSurvey.mockImplementation(() => {
+        throw err;
+      });
+
+      act(() => {
+        wrapper.find('SurveyQuestionForm').invoke('handleSubmit')({
+          question_name: 'new question',
+          variable: 'question',
+          type: 'text',
+        });
+      });
+      wrapper.update();
+
+      expect(wrapper.find('SurveyQuestionForm').prop('submitError')).toEqual(
+        err
       );
+      global.console.error = realConsoleError;
     });
-  });
 
-  test('should render form', () => {
+    test('should generate error for duplicate variable names', async () => {
-    expect(wrapper.find('SurveyQuestionForm')).toHaveLength(1);
+      const realConsoleError = global.console.error;
-  });
+      global.console.error = jest.fn();
 
-  test('should call updateSurvey', () => {
+      act(() => {
-    act(() => {
+        wrapper.find('SurveyQuestionForm').invoke('handleSubmit')({
-      wrapper.find('SurveyQuestionForm').invoke('handleSubmit')({
+          question_name: 'new question',
-        question_name: 'new question',
+          variable: 'bar',
-        variable: 'question',
+          type: 'text',
-        type: 'text',
+        });
       });
-    });
+      wrapper.update();
-    wrapper.update();
 
-    expect(updateSurvey).toHaveBeenCalledWith([
+      const err = wrapper.find('SurveyQuestionForm').prop('submitError');
-      {
+      expect(err.message).toEqual(
-        question_name: 'new question',
+        'Survey already contains a question with variable named “bar”'
-        variable: 'question',
+      );
-        type: 'text',
+      global.console.error = realConsoleError;
-      },
+    });
-      survey.spec[1],
-    ]);
   });
 
-  test('should set formError', async () => {
+  describe('without question_variable present', () => {
-    const realConsoleError = global.console.error;
+    test('should redirect back to the survey list', () => {
-    global.console.error = jest.fn();
+      history = createMemoryHistory({
-    const err = new Error('oops');
+        initialEntries: ['/templates/job_templates/1/survey/edit'],
-    updateSurvey.mockImplementation(() => {
-      throw err;
-    });
-
-    act(() => {
-      wrapper.find('SurveyQuestionForm').invoke('handleSubmit')({
-        question_name: 'new question',
-        variable: 'question',
-        type: 'text',
       });
-    });
+      updateSurvey = jest.fn();
-    wrapper.update();
+      act(() => {
-
+        wrapper = mountWithContexts(
-    expect(wrapper.find('SurveyQuestionForm').prop('submitError')).toEqual(err);
+          <Switch>
-    global.console.error = realConsoleError;
+            <Route path="/templates/:templateType/:id/survey/edit">
-  });
+              <SurveyQuestionEdit survey={survey} updateSurvey={updateSurvey} />
-
+            </Route>
-  test('should generate error for duplicate variable names', async () => {
+          </Switch>,
-    const realConsoleError = global.console.error;
+          {
-    global.console.error = jest.fn();
+            context: { router: { history } },
-
+          }
-    act(() => {
+        );
-      wrapper.find('SurveyQuestionForm').invoke('handleSubmit')({
-        question_name: 'new question',
-        variable: 'bar',
-        type: 'text',
       });
-    });
-    wrapper.update();
 
-    const err = wrapper.find('SurveyQuestionForm').prop('submitError');
+      expect(history.location.pathname).toEqual(
-    expect(err.message).toEqual(
+        '/templates/job_templates/1/survey'
-      'Survey already contains a question with variable named “bar”'
+      );
-    );
+
-    global.console.error = realConsoleError;
+      wrapper.unmount();
+    });
   });
 });

awx/ui_next/src/screens/Template/TemplateSurvey.jsx

@@ -100,7 +100,7 @@ function TemplateSurvey({ template, canEdit, i18n }) {
           </Route>
         )}
         {canEdit && (
-          <Route path="/templates/:templateType/:id/survey/edit/:variable">
+          <Route path="/templates/:templateType/:id/survey/edit">
             <SurveyQuestionEdit
               survey={survey}
               updateSurvey={updateSurveySpec}