From f28913a1939f9d4a170bcffc67428cbde269dd44 Mon Sep 17 00:00:00 2001 From: Ariel Salem Date: Wed, 23 Oct 2019 13:45:27 -0700 Subject: [PATCH] Fix/tasks edit (#15549) * fix(ui/tasks): edit & update functionality has been fixed. Also fixed the issue where task form data didn't persist when toggling between schedule task options * chore(CHANGELOG): updated changelog with current PR * fix(ui/tasks): updated reducer test to more accurately reflect the current reducer functionality and updated the parameter name for consistency * fix(linter): removed extra whitespace * chore(comment): removed old comment from PR since it's no longer necessary --- CHANGELOG.md | 1 + ui/cypress/e2e/tasks.test.ts | 110 +++++++++++++++--- ui/cypress/index.d.ts | 2 + ui/cypress/support/commands.ts | 5 + ui/src/tasks/actions/index.ts | 14 +++ ui/src/tasks/components/TaskForm.tsx | 2 + .../components/TaskScheduleFormField.tsx | 1 + .../__snapshots__/TaskForm.test.tsx.snap | 2 + ui/src/tasks/containers/TaskEditPage.tsx | 11 +- ui/src/tasks/reducers/index.ts | 16 --- ui/src/tasks/reducers/tasks.test.ts | 14 ++- 11 files changed, 136 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8dd256a8e78..b58e7ff327c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ 1. [15452](https://github.com/influxdata/influxdb/pull/15452): Log error as info message on unauthorized API call attempts 1. [15504](https://github.com/influxdata/influxdb/pull/15504): Ensure members&owners eps 404 when /org resource does not exist 1. [15510](https://github.com/influxdata/influxdb/pull/15510): UI/Telegraf sort functionality fixed +1. [15549](https://github.com/influxdata/influxdb/pull/15549): UI/Task edit functionality fixed ## v2.0.0-alpha.18 [2019-09-26] diff --git a/ui/cypress/e2e/tasks.test.ts b/ui/cypress/e2e/tasks.test.ts index 6bb2950ba05..4c627cc0f9e 100644 --- a/ui/cypress/e2e/tasks.test.ts +++ b/ui/cypress/e2e/tasks.test.ts @@ -19,7 +19,7 @@ describe('Tasks', () => { }) cy.fixture('routes').then(({orgs}) => { - cy.get('@org').then(({id}) => { + cy.get('@org').then(({id}: Organization) => { cy.visit(`${orgs}/${id}/tasks`) }) }) @@ -60,8 +60,8 @@ from(bucket: "${name}") .and('contain', taskName) }) - it.only('can delete a task', () => { - cy.get('@org').then(({id}) => { + it('can delete a task', () => { + cy.get('@org').then(({id}: Organization) => { cy.get('@token').then(token => { cy.createTask(token, id) cy.createTask(token, id) @@ -86,7 +86,7 @@ from(bucket: "${name}") }) it('can disable a task', () => { - cy.get('@org').then(({id}) => { + cy.get('@org').then(({id}: Organization) => { cy.get('@token').then(token => { cy.createTask(token, id) }) @@ -100,7 +100,7 @@ from(bucket: "${name}") }) it('can edit a tasks name', () => { - cy.get('@org').then(({id}) => { + cy.get('@org').then(({id}: Organization) => { cy.get('@token').then(token => { cy.createTask(token, id) }) @@ -119,7 +119,7 @@ from(bucket: "${name}") }) cy.fixture('routes').then(({orgs}) => { - cy.get('@org').then(({id}) => { + cy.get('@org').then(({id}: Organization) => { cy.visit(`${orgs}/${id}/tasks`) }) }) @@ -131,7 +131,7 @@ from(bucket: "${name}") it('can click to filter tasks by labels', () => { const newLabelName = 'click-me' - cy.get('@org').then(({id}) => { + cy.get('@org').then(({id}: Organization) => { cy.get('@token').then(token => { cy.createTask(token, id).then(({body}) => { cy.createAndAddLabel('tasks', id, body.id, newLabelName) @@ -144,7 +144,7 @@ from(bucket: "${name}") }) cy.fixture('routes').then(({orgs}) => { - cy.get('@org').then(({id}) => { + cy.get('@org').then(({id}: Organization) => { cy.visit(`${orgs}/${id}/tasks`) }) }) @@ -160,7 +160,7 @@ from(bucket: "${name}") describe('searching', () => { it('can search by task name', () => { const searchName = 'beepBoop' - cy.get('@org').then(({id}) => { + cy.get('@org').then(({id}: Organization) => { cy.get('@token').then(token => { cy.createTask(token, id, searchName) cy.createTask(token, id) @@ -168,7 +168,7 @@ from(bucket: "${name}") }) cy.fixture('routes').then(({orgs}) => { - cy.get('@org').then(({id}) => { + cy.get('@org').then(({id}: Organization) => { cy.visit(`${orgs}/${id}/tasks`) }) }) @@ -178,9 +178,93 @@ from(bucket: "${name}") cy.getByTestID('task-card').should('have.length', 1) }) }) + + describe('update & persist data', () => { + // address a bug that was reported when editing tasks: + // https://github.com/influxdata/influxdb/issues/15534 + const taskName = 'Task' + const interval = '12h' + const offset = '30m' + beforeEach(() => { + createFirstTask( + taskName, + ({name}) => { + return `import "influxdata/influxdb/v1" + v1.tagValues(bucket: "${name}", tag: "_field") + from(bucket: "${name}") + |> range(start: -2m)` + }, + interval, + offset + ) + cy.contains('Save').click() + cy.getByTestID('task-card') + .should('have.length', 1) + .and('contain', taskName) + + cy.getByTestID('task-card--name') + .contains(taskName) + .click() + // verify that the previously input data exists + cy.getByInputValue(taskName) + cy.getByInputValue(interval) + cy.getByInputValue(offset) + }) + + it('can update a task', () => { + const newTask = 'Taskr[sic]' + const newInterval = '24h' + const newOffset = '7h' + // updates the data + cy.getByTestID('task-form-name') + .clear() + .type(newTask) + cy.getByTestID('task-form-schedule-input') + .clear() + .type(newInterval) + cy.getByTestID('task-form-offset-input') + .clear() + .type(newOffset) + + cy.contains('Save').click() + // checks to see if the data has been updated once saved + cy.getByTestID('task-card--name').contains(newTask) + }) + + it('persists data when toggling between scheduling tasks', () => { + // toggles schedule task from every to cron + cy.getByTestID('task-card-cron-btn').click() + + // checks to see if the cron helper text exists + cy.getByTestID('form--box').should('have.length', 1) + + const cronInput = '0 2 * * *' + // checks to see if the cron data is set to the initial value + cy.getByInputValue('') + cy.getByInputValue(offset) + + cy.getByTestID('task-form-schedule-input').type(cronInput) + // toggles schedule task back to every from cron + cy.getByTestID('task-card-every-btn').click() + // checks to see if the initial interval data for every persists + cy.getByInputValue(interval) + cy.getByInputValue(offset) + // toggles back to cron from every + cy.getByTestID('task-card-cron-btn').click() + // checks to see if the cron data persists + cy.getByInputValue(cronInput) + cy.getByInputValue(offset) + cy.contains('Save').click() + }) + }) }) -function createFirstTask(name: string, flux: (bucket: Bucket) => string) { +function createFirstTask( + name: string, + flux: (bucket: Bucket) => string, + interval: string = '24h', + offset: string = '20m' +) { cy.getByTestID('empty-tasks-list').within(() => { cy.getByTestID('add-resource-dropdown--button').click() }) @@ -188,8 +272,8 @@ function createFirstTask(name: string, flux: (bucket: Bucket) => string) { cy.getByTestID('add-resource-dropdown--new').click() cy.getByInputName('name').type(name) - cy.getByInputName('interval').type('24h') - cy.getByInputName('offset').type('20m') + cy.getByTestID('task-form-schedule-input').type(interval) + cy.getByTestID('task-form-offset-input').type(offset) cy.get('@bucket').then(bucket => { cy.getByTestID('flux-editor').within(() => { diff --git a/ui/cypress/index.d.ts b/ui/cypress/index.d.ts index e8c5b3c86a6..c9f9e47ba38 100644 --- a/ui/cypress/index.d.ts +++ b/ui/cypress/index.d.ts @@ -9,6 +9,7 @@ import { flush, getByTestID, getByInputName, + getByInputValue, getByTitle, createTask, createVariable, @@ -42,6 +43,7 @@ declare global { flush: typeof flush getByTestID: typeof getByTestID getByInputName: typeof getByInputName + getByInputValue: typeof getByInputValue getByTitle: typeof getByTitle getByTestIDSubStr: typeof getByTestIDSubStr createAndAddLabel: typeof createAndAddLabel diff --git a/ui/cypress/support/commands.ts b/ui/cypress/support/commands.ts index 85c2b1d36ca..6e7cc23d4c5 100644 --- a/ui/cypress/support/commands.ts +++ b/ui/cypress/support/commands.ts @@ -361,6 +361,10 @@ export const getByInputName = (name: string): Cypress.Chainable => { return cy.get(`input[name=${name}]`) } +export const getByInputValue = (value: string): Cypress.Chainable => { + return cy.get(`input[value='${value}']`) +} + export const getByTitle = (name: string): Cypress.Chainable => { return cy.get(`[title="${name}"]`) } @@ -398,6 +402,7 @@ Cypress.Commands.add('fluxEqual', fluxEqual) // getters Cypress.Commands.add('getByTestID', getByTestID) Cypress.Commands.add('getByInputName', getByInputName) +Cypress.Commands.add('getByInputValue', getByInputValue) Cypress.Commands.add('getByTitle', getByTitle) Cypress.Commands.add('getByTestIDSubStr', getByTestIDSubStr) diff --git a/ui/src/tasks/actions/index.ts b/ui/src/tasks/actions/index.ts index 61198960861..a0c0b85fae7 100644 --- a/ui/src/tasks/actions/index.ts +++ b/ui/src/tasks/actions/index.ts @@ -352,6 +352,20 @@ export const selectTaskByID = (id: string) => async ( } } +export const setAllTaskOptionsByID = (taskID: string) => async ( + dispatch +): Promise => { + try { + const task = await client.tasks.get(taskID) + dispatch(setAllTaskOptions(task)) + } catch (e) { + console.error(e) + dispatch(goToTasks()) + const message = getErrorMessage(e) + dispatch(notify(taskNotFound(message))) + } +} + export const selectTask = (task: Task) => ( dispatch, getState: GetStateFunc diff --git a/ui/src/tasks/components/TaskForm.tsx b/ui/src/tasks/components/TaskForm.tsx index 2c127c18bb2..07f764d0773 100644 --- a/ui/src/tasks/components/TaskForm.tsx +++ b/ui/src/tasks/components/TaskForm.tsx @@ -97,6 +97,7 @@ export default class TaskForm extends PureComponent { value={TaskSchedule.interval} titleText="Run task at regular intervals" onClick={this.handleChangeScheduleType} + testID="task-card-every-btn" > Every @@ -106,6 +107,7 @@ export default class TaskForm extends PureComponent { value={TaskSchedule.cron} titleText="Use cron syntax for more control over scheduling" onClick={this.handleChangeScheduleType} + testID="task-card-cron-btn" > Cron diff --git a/ui/src/tasks/components/TaskScheduleFormField.tsx b/ui/src/tasks/components/TaskScheduleFormField.tsx index 965732740eb..029bc0c1ef5 100644 --- a/ui/src/tasks/components/TaskScheduleFormField.tsx +++ b/ui/src/tasks/components/TaskScheduleFormField.tsx @@ -47,6 +47,7 @@ export default class TaskScheduleFormFields extends PureComponent { value={offset} placeholder="20m" onChange={onChangeInput} + testID="task-form-offset-input" /> diff --git a/ui/src/tasks/components/__snapshots__/TaskForm.test.tsx.snap b/ui/src/tasks/components/__snapshots__/TaskForm.test.tsx.snap index fa48904e6fc..27a466682f0 100644 --- a/ui/src/tasks/components/__snapshots__/TaskForm.test.tsx.snap +++ b/ui/src/tasks/components/__snapshots__/TaskForm.test.tsx.snap @@ -35,6 +35,7 @@ exports[`TaskForm rendering renders 1`] = ` active={false} id="every" onClick={[Function]} + testID="task-card-every-btn" titleText="Run task at regular intervals" value="interval" > @@ -44,6 +45,7 @@ exports[`TaskForm rendering renders 1`] = ` active={false} id="cron" onClick={[Function]} + testID="task-card-cron-btn" titleText="Use cron syntax for more control over scheduling" value="cron" > diff --git a/ui/src/tasks/containers/TaskEditPage.tsx b/ui/src/tasks/containers/TaskEditPage.tsx index 0acbe19f413..f70bf491ed4 100644 --- a/ui/src/tasks/containers/TaskEditPage.tsx +++ b/ui/src/tasks/containers/TaskEditPage.tsx @@ -18,7 +18,7 @@ import { cancel, setTaskOption, clearTask, - setAllTaskOptions, + setAllTaskOptionsByID, } from 'src/tasks/actions' // Utils @@ -50,7 +50,7 @@ interface DispatchProps { cancel: typeof cancel selectTaskByID: typeof selectTaskByID clearTask: typeof clearTask - setAllTaskOptions: typeof setAllTaskOptions + setAllTaskOptionsByID: typeof setAllTaskOptionsByID } type Props = OwnProps & StateProps & DispatchProps @@ -65,10 +65,7 @@ class TaskEditPage extends PureComponent { params: {id}, } = this.props this.props.selectTaskByID(id) - - const {currentTask} = this.props - - this.props.setAllTaskOptions(currentTask) + this.props.setAllTaskOptionsByID(id) } public componentWillUnmount() { @@ -158,7 +155,7 @@ const mdtp: DispatchProps = { updateScript, cancel, selectTaskByID, - setAllTaskOptions, + setAllTaskOptionsByID, clearTask, } diff --git a/ui/src/tasks/reducers/index.ts b/ui/src/tasks/reducers/index.ts index e3a8ed9618d..613e6c915c3 100644 --- a/ui/src/tasks/reducers/index.ts +++ b/ui/src/tasks/reducers/index.ts @@ -90,22 +90,6 @@ export default ( } case 'SET_TASK_OPTION': const {key, value} = action.payload - - if (key === 'taskScheduleType') { - if (value === TaskSchedule.cron) { - return { - ...state, - taskOptions: {...state.taskOptions, interval: '', [key]: value}, - } - } - if (value === TaskSchedule.interval) { - return { - ...state, - taskOptions: {...state.taskOptions, cron: '', [key]: value}, - } - } - } - return { ...state, taskOptions: {...state.taskOptions, [key]: value}, diff --git a/ui/src/tasks/reducers/tasks.test.ts b/ui/src/tasks/reducers/tasks.test.ts index 2057b85649b..623148628df 100644 --- a/ui/src/tasks/reducers/tasks.test.ts +++ b/ui/src/tasks/reducers/tasks.test.ts @@ -7,9 +7,10 @@ import {TaskSchedule} from 'src/utils/taskOptionsToFluxScript' describe('tasksReducer', () => { describe('setTaskOption', () => { - it('clears the cron property from the task options when interval is selected', () => { + it('should not clear the cron property from the task options when interval is selected', () => { const initialState = defaultState - initialState.taskOptions = {...defaultTaskOptions, cron: '0 2 * * *'} + const cron = '0 2 * * *' + initialState.taskOptions = {...defaultTaskOptions, cron} const actual = tasksReducer( initialState, @@ -21,16 +22,17 @@ describe('tasksReducer', () => { taskOptions: { ...defaultTaskOptions, taskScheduleType: TaskSchedule.interval, - cron: '', + cron, }, } expect(actual).toEqual(expected) }) - it('clears the interval property from the task options when cron is selected', () => { + it('should not clear the interval property from the task options when cron is selected', () => { const initialState = defaultState - initialState.taskOptions = {...defaultTaskOptions, interval: '24h'} // todo(docmerlin): allow for time units larger than 1d, right now h is the longest unit our s + const interval = '24h' + initialState.taskOptions = {...defaultTaskOptions, interval} // todo(docmerlin): allow for time units larger than 1d, right now h is the longest unit our s const actual = tasksReducer( initialState, @@ -42,7 +44,7 @@ describe('tasksReducer', () => { taskOptions: { ...defaultTaskOptions, taskScheduleType: TaskSchedule.cron, - interval: '', + interval, }, }