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
influxdata · Oct 23, 2019 · f28913a · f28913a
1 parent 0bd6852
commit f28913a
Show file tree

Hide file tree

Showing 11 changed files with 136 additions and 42 deletions.
diff --git 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
@@ -19,7 +19,7 @@ describe('Tasks', () => {
     })
 
     cy.fixture('routes').then(({orgs}) => {
-      cy.get<Organization>('@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<Organization>('@org').then(({id}) => {
+  it('can delete a task', () => {
+    cy.get('@org').then(({id}: Organization) => {
       cy.get<string>('@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<Organization>('@org').then(({id}) => {
+    cy.get('@org').then(({id}: Organization) => {
       cy.get<string>('@token').then(token => {
         cy.createTask(token, id)
       })
@@ -100,7 +100,7 @@ from(bucket: "${name}")
   })
 
   it('can edit a tasks name', () => {
-    cy.get<Organization>('@org').then(({id}) => {
+    cy.get('@org').then(({id}: Organization) => {
       cy.get<string>('@token').then(token => {
         cy.createTask(token, id)
       })
@@ -119,7 +119,7 @@ from(bucket: "${name}")
     })
 
     cy.fixture('routes').then(({orgs}) => {
-      cy.get<Organization>('@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<Organization>('@org').then(({id}) => {
+      cy.get('@org').then(({id}: Organization) => {
         cy.get<string>('@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<Organization>('@org').then(({id}) => {
+        cy.get('@org').then(({id}: Organization) => {
           cy.visit(`${orgs}/${id}/tasks`)
         })
       })
@@ -160,15 +160,15 @@ from(bucket: "${name}")
   describe('searching', () => {
     it('can search by task name', () => {
       const searchName = 'beepBoop'
-      cy.get<Organization>('@org').then(({id}) => {
+      cy.get('@org').then(({id}: Organization) => {
         cy.get<string>('@token').then(token => {
           cy.createTask(token, id, searchName)
           cy.createTask(token, id)
         })
       })
 
       cy.fixture('routes').then(({orgs}) => {
-        cy.get<Organization>('@org').then(({id}) => {
+        cy.get('@org').then(({id}: Organization) => {
           cy.visit(`${orgs}/${id}/tasks`)
         })
       })
@@ -178,18 +178,102 @@ 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()
   })
 
   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>('@bucket').then(bucket => {
     cy.getByTestID('flux-editor').within(() => {

diff --git 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
@@ -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
@@ -352,6 +352,20 @@ export const selectTaskByID = (id: string) => async (
   }
 }
 
+export const setAllTaskOptionsByID = (taskID: string) => async (
+  dispatch
+): Promise<void> => {
+  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
@@ -97,6 +97,7 @@ export default class TaskForm extends PureComponent<Props, State> {
                       value={TaskSchedule.interval}
                       titleText="Run task at regular intervals"
                       onClick={this.handleChangeScheduleType}
+                      testID="task-card-every-btn"
                     >
                       Every
                     </Radio.Button>
@@ -106,6 +107,7 @@ export default class TaskForm extends PureComponent<Props, State> {
                       value={TaskSchedule.cron}
                       titleText="Use cron syntax for more control over scheduling"
                       onClick={this.handleChangeScheduleType}
+                      testID="task-card-cron-btn"
                     >
                       Cron
                     </Radio.Button>

diff --git a/ui/src/tasks/components/TaskScheduleFormField.tsx b/ui/src/tasks/components/TaskScheduleFormField.tsx
@@ -47,6 +47,7 @@ export default class TaskScheduleFormFields extends PureComponent<Props> {
               value={offset}
               placeholder="20m"
               onChange={onChangeInput}
+              testID="task-form-offset-input"
             />
           </Form.Element>
         </Grid.Column>

diff --git 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
@@ -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<Props> {
       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
@@ -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
@@ -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,
         },
       }