Stop polling when checking task lists on an issue

The polling can cause conflict errors when the poll response is stale.
This happens very often when checking multiple tasks.

Changelog: fixed

app/assets/javascripts/issue_show/components/app.vue

@@ -12,6 +12,7 @@ import {
   IssueTypePath,
   IncidentTypePath,
   IncidentType,
+  POLLING_DELAY,
 } from '../constants';
 import eventHub from '../event_hub';
 import getIssueStateQuery from '../queries/get_issue_state.query.graphql';
@@ -282,7 +283,7 @@ export default {
     });
 
     if (!Visibility.hidden()) {
-      this.poll.makeDelayedRequest(2000);
+      this.poll.makeDelayedRequest(POLLING_DELAY);
     }
 
     Visibility.change(() => {
@@ -457,6 +458,22 @@ export default {
         this.flashContainer = null;
       }
     },
+
+    taskListUpdateStarted() {
+      this.poll.stop();
+    },
+
+    taskListUpdateSucceeded() {
+      this.poll.enable();
+      this.poll.makeDelayedRequest(POLLING_DELAY);
+    },
+
+    taskListUpdateFailed() {
+      this.poll.enable();
+      this.poll.makeDelayedRequest(POLLING_DELAY);
+
+      this.updateStoreState();
+    },
   },
 };
 </script>
@@ -552,7 +569,9 @@ export default {
         :issuable-type="issuableType"
         :update-url="updateEndpoint"
         :lock-version="state.lock_version"
-        @taskListUpdateFailed="updateStoreState"
+        @taskListUpdateStarted="taskListUpdateStarted"
+        @taskListUpdateSucceeded="taskListUpdateSucceeded"
+        @taskListUpdateFailed="taskListUpdateFailed"
       />
 
       <edited-component

app/assets/javascripts/issue_show/components/description.vue

@@ -86,11 +86,21 @@ export default {
           fieldName: 'description',
           lockVersion: this.lockVersion,
           selector: '.detail-page-description',
+          onUpdate: this.taskListUpdateStarted.bind(this),
+          onSuccess: this.taskListUpdateSuccess.bind(this),
           onError: this.taskListUpdateError.bind(this),
         });
       }
     },
 
+    taskListUpdateStarted() {
+      this.$emit('taskListUpdateStarted');
+    },
+
+    taskListUpdateSuccess() {
+      this.$emit('taskListUpdateSucceeded');
+    },
+
     taskListUpdateError() {
       createFlash({
         message: sprintf(

app/assets/javascripts/issue_show/constants.js

@@ -37,3 +37,5 @@ export const IncidentTypePath = 'issues/incident';
 export const IncidentType = 'incident';
 
 export const issueState = { issueType: undefined, isDirty: false };
+
+export const POLLING_DELAY = 2000;

app/assets/javascripts/task_list.js

@@ -12,6 +12,7 @@ export default class TaskList {
     this.lockVersion = options.lockVersion;
     this.taskListContainerSelector = `${this.selector} .js-task-list-container`;
     this.updateHandler = this.update.bind(this);
+    this.onUpdate = options.onUpdate || (() => {});
     this.onSuccess = options.onSuccess || (() => {});
     this.onError =
       options.onError ||
@@ -96,6 +97,7 @@ export default class TaskList {
       },
     };
 
+    this.onUpdate();
     this.disableTaskListItems(e);
 
     return axios

spec/frontend/issue_show/components/app_spec.js

@@ -8,7 +8,7 @@ import IssuableApp from '~/issue_show/components/app.vue';
 import DescriptionComponent from '~/issue_show/components/description.vue';
 import IncidentTabs from '~/issue_show/components/incidents/incident_tabs.vue';
 import PinnedLinks from '~/issue_show/components/pinned_links.vue';
-import { IssuableStatus, IssuableStatusText } from '~/issue_show/constants';
+import { IssuableStatus, IssuableStatusText, POLLING_DELAY } from '~/issue_show/constants';
 import eventHub from '~/issue_show/event_hub';
 import axios from '~/lib/utils/axios_utils';
 import { visitUrl } from '~/lib/utils/url_utility';
@@ -643,4 +643,40 @@ describe('Issuable output', () => {
       });
     });
   });
+
+  describe('taskListUpdateStarted', () => {
+    it('stops polling', () => {
+      jest.spyOn(wrapper.vm.poll, 'stop');
+
+      wrapper.vm.taskListUpdateStarted();
+
+      expect(wrapper.vm.poll.stop).toHaveBeenCalled();
+    });
+  });
+
+  describe('taskListUpdateSucceeded', () => {
+    it('enables polling', () => {
+      jest.spyOn(wrapper.vm.poll, 'enable');
+      jest.spyOn(wrapper.vm.poll, 'makeDelayedRequest');
+
+      wrapper.vm.taskListUpdateSucceeded();
+
+      expect(wrapper.vm.poll.enable).toHaveBeenCalled();
+      expect(wrapper.vm.poll.makeDelayedRequest).toHaveBeenCalledWith(POLLING_DELAY);
+    });
+  });
+
+  describe('taskListUpdateFailed', () => {
+    it('enables polling and calls updateStoreState', () => {
+      jest.spyOn(wrapper.vm.poll, 'enable');
+      jest.spyOn(wrapper.vm.poll, 'makeDelayedRequest');
+      jest.spyOn(wrapper.vm, 'updateStoreState');
+
+      wrapper.vm.taskListUpdateFailed();
+
+      expect(wrapper.vm.poll.enable).toHaveBeenCalled();
+      expect(wrapper.vm.poll.makeDelayedRequest).toHaveBeenCalledWith(POLLING_DELAY);
+      expect(wrapper.vm.updateStoreState).toHaveBeenCalled();
+    });
+  });
 });

spec/frontend/issue_show/components/description_spec.js

@@ -114,6 +114,8 @@ describe('Description component', () => {
         dataType: 'issuableType',
         fieldName: 'description',
         selector: '.detail-page-description',
+        onUpdate: expect.any(Function),
+        onSuccess: expect.any(Function),
         onError: expect.any(Function),
         lockVersion: 0,
       });
@@ -150,6 +152,26 @@ describe('Description component', () => {
     });
   });
 
+  describe('taskListUpdateStarted', () => {
+    it('emits event to parent', () => {
+      const spy = jest.spyOn(vm, '$emit');
+
+      vm.taskListUpdateStarted();
+
+      expect(spy).toHaveBeenCalledWith('taskListUpdateStarted');
+    });
+  });
+
+  describe('taskListUpdateSuccess', () => {
+    it('emits event to parent', () => {
+      const spy = jest.spyOn(vm, '$emit');
+
+      vm.taskListUpdateSuccess();
+
+      expect(spy).toHaveBeenCalledWith('taskListUpdateSucceeded');
+    });
+  });
+
   describe('taskListUpdateError', () => {
     it('should create flash notification and emit an event to parent', () => {
       const msg =

spec/frontend/task_list_spec.js

@@ -125,6 +125,7 @@ describe('TaskList', () => {
       const response = { data: { lock_version: 3 } };
       jest.spyOn(taskList, 'enableTaskListItems').mockImplementation(() => {});
       jest.spyOn(taskList, 'disableTaskListItems').mockImplementation(() => {});
+      jest.spyOn(taskList, 'onUpdate').mockImplementation(() => {});
       jest.spyOn(taskList, 'onSuccess').mockImplementation(() => {});
       jest.spyOn(axios, 'patch').mockReturnValue(Promise.resolve(response));
 
@@ -151,8 +152,11 @@ describe('TaskList', () => {
         },
       };
 
-      taskList
-        .update(event)
+      const update = taskList.update(event);
+
+      expect(taskList.onUpdate).toHaveBeenCalled();
+
+      update
         .then(() => {
           expect(taskList.disableTaskListItems).toHaveBeenCalledWith(event);
           expect(axios.patch).toHaveBeenCalledWith(endpoint, patchData);
@@ -168,12 +172,17 @@ describe('TaskList', () => {
   it('should handle request error and enable task list items', (done) => {
     const response = { data: { error: 1 } };
     jest.spyOn(taskList, 'enableTaskListItems').mockImplementation(() => {});
+    jest.spyOn(taskList, 'onUpdate').mockImplementation(() => {});
     jest.spyOn(taskList, 'onError').mockImplementation(() => {});
     jest.spyOn(axios, 'patch').mockReturnValue(Promise.reject({ response })); // eslint-disable-line prefer-promise-reject-errors
 
     const event = { detail: {} };
-    taskList
-      .update(event)
+
+    const update = taskList.update(event);
+
+    expect(taskList.onUpdate).toHaveBeenCalled();
+
+    update
       .then(() => {
         expect(taskList.enableTaskListItems).toHaveBeenCalledWith(event);
         expect(taskList.onError).toHaveBeenCalledWith(response.data);