From f44faf4e61f0bb16c299daf8eb05d501765e35e9 Mon Sep 17 00:00:00 2001 From: mabashian Date: Tue, 24 Nov 2020 12:44:09 -0500 Subject: [PATCH] Fix issue with broken survey question edit breadcrumb by altering the url scheme --- .../Template/Survey/SurveyListItem.jsx | 2 +- .../Template/Survey/SurveyQuestionEdit.jsx | 22 ++- .../Survey/SurveyQuestionEdit.test.jsx | 175 +++++++++++------- .../src/screens/Template/TemplateSurvey.jsx | 2 +- 4 files changed, 127 insertions(+), 74 deletions(-) diff --git a/awx/ui_next/src/screens/Template/Survey/SurveyListItem.jsx b/awx/ui_next/src/screens/Template/Survey/SurveyListItem.jsx index 2286510dfb..487488cb2e 100644 --- a/awx/ui_next/src/screens/Template/Survey/SurveyListItem.jsx +++ b/awx/ui_next/src/screens/Template/Survey/SurveyListItem.jsx @@ -94,7 +94,7 @@ function SurveyListItem({ dataListCells={[ <> - + {question.question_name} {question.required && ( diff --git a/awx/ui_next/src/screens/Template/Survey/SurveyQuestionEdit.jsx b/awx/ui_next/src/screens/Template/Survey/SurveyQuestionEdit.jsx index 439f445572..32e157f5f3 100644 --- a/awx/ui_next/src/screens/Template/Survey/SurveyQuestionEdit.jsx +++ b/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 ; } - const question = survey.spec.find(q => q.variable === match.params.variable); + const question = survey.spec.find(q => q.variable === questionVariable); + + if (!question) { + return ( + + ); + } 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'); diff --git a/awx/ui_next/src/screens/Template/Survey/SurveyQuestionEdit.test.jsx b/awx/ui_next/src/screens/Template/Survey/SurveyQuestionEdit.test.jsx index d1bc79e992..de51ab4e8d 100644 --- a/awx/ui_next/src/screens/Template/Survey/SurveyQuestionEdit.test.jsx +++ b/awx/ui_next/src/screens/Template/Survey/SurveyQuestionEdit.test.jsx @@ -33,87 +33,124 @@ describe('', () => { let history; let wrapper; - beforeEach(() => { - history = createMemoryHistory({ - initialEntries: ['/templates/job_templates/1/survey/edit/foo'], + describe('with question_variable present', () => { + beforeEach(() => { + history = createMemoryHistory({ + initialEntries: [ + '/templates/job_templates/1/survey/edit?question_variable=foo', + ], + }); + updateSurvey = jest.fn(); + act(() => { + wrapper = mountWithContexts( + + + + + , + { + context: { router: { history } }, + } + ); + }); }); - updateSurvey = jest.fn(); - act(() => { - wrapper = mountWithContexts( - - - - - , + + afterEach(() => { + wrapper.unmount(); + }); + + test('should render form', () => { + expect(wrapper.find('SurveyQuestionForm')).toHaveLength(1); + }); + + 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', () => { - expect(wrapper.find('SurveyQuestionForm')).toHaveLength(1); - }); + test('should generate error for duplicate variable names', async () => { + const realConsoleError = global.console.error; + global.console.error = jest.fn(); - test('should call updateSurvey', () => { - act(() => { - wrapper.find('SurveyQuestionForm').invoke('handleSubmit')({ - question_name: 'new question', - variable: 'question', - type: 'text', + act(() => { + wrapper.find('SurveyQuestionForm').invoke('handleSubmit')({ + question_name: 'new question', + variable: 'bar', + type: 'text', + }); }); - }); - wrapper.update(); + wrapper.update(); - expect(updateSurvey).toHaveBeenCalledWith([ - { - question_name: 'new question', - variable: 'question', - type: 'text', - }, - survey.spec[1], - ]); + const err = wrapper.find('SurveyQuestionForm').prop('submitError'); + expect(err.message).toEqual( + 'Survey already contains a question with variable named “bar”' + ); + global.console.error = realConsoleError; + }); }); - 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', + describe('without question_variable present', () => { + test('should redirect back to the survey list', () => { + history = createMemoryHistory({ + initialEntries: ['/templates/job_templates/1/survey/edit'], }); - }); - wrapper.update(); - - expect(wrapper.find('SurveyQuestionForm').prop('submitError')).toEqual(err); - global.console.error = realConsoleError; - }); - - test('should generate error for duplicate variable names', async () => { - const realConsoleError = global.console.error; - global.console.error = jest.fn(); - - act(() => { - wrapper.find('SurveyQuestionForm').invoke('handleSubmit')({ - question_name: 'new question', - variable: 'bar', - type: 'text', + updateSurvey = jest.fn(); + act(() => { + wrapper = mountWithContexts( + + + + + , + { + context: { router: { history } }, + } + ); }); - }); - wrapper.update(); - const err = wrapper.find('SurveyQuestionForm').prop('submitError'); - expect(err.message).toEqual( - 'Survey already contains a question with variable named “bar”' - ); - global.console.error = realConsoleError; + expect(history.location.pathname).toEqual( + '/templates/job_templates/1/survey' + ); + + wrapper.unmount(); + }); }); }); diff --git a/awx/ui_next/src/screens/Template/TemplateSurvey.jsx b/awx/ui_next/src/screens/Template/TemplateSurvey.jsx index 23b57f43ea..0badeffbf5 100644 --- a/awx/ui_next/src/screens/Template/TemplateSurvey.jsx +++ b/awx/ui_next/src/screens/Template/TemplateSurvey.jsx @@ -100,7 +100,7 @@ function TemplateSurvey({ template, canEdit, i18n }) { )} {canEdit && ( - +