From 8bcd508b4c7f559f27db3c05b6ae4a3a33dfff95 Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Fri, 21 Dec 2018 01:19:17 +0100
Subject: [PATCH 01/42] Add lock version to issuable helpers

---
 app/controllers/concerns/issuable_actions.rb | 3 ++-
 app/helpers/issuables_helper.rb              | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb
index 3d64ae8b775..8ef3b6502df 100644
--- a/app/controllers/concerns/issuable_actions.rb
+++ b/app/controllers/concerns/issuable_actions.rb
@@ -58,7 +58,8 @@ module IssuableActions
       title_text: issuable.title,
       description: view_context.markdown_field(issuable, :description),
       description_text: issuable.description,
-      task_status: issuable.task_status
+      task_status: issuable.task_status,
+      lock_version: issuable.lock_version
     }
 
     if issuable.edited?
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
index f8176facce9..0fee29bf7c7 100644
--- a/app/helpers/issuables_helper.rb
+++ b/app/helpers/issuables_helper.rb
@@ -269,6 +269,7 @@ module IssuablesHelper
       markdownPreviewPath: preview_markdown_path(parent),
       markdownDocsPath: help_page_path('user/markdown'),
       markdownVersion: issuable.cached_markdown_version,
+      lockVersion: issuable.lock_version,
       issuableTemplates: issuable_templates(issuable),
       initialTitleHtml: markdown_field(issuable, :title),
       initialTitleText: issuable.title,

From 45eabf921a9bb90677d1e4f59544f0a7abcbc879 Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Fri, 21 Dec 2018 01:19:41 +0100
Subject: [PATCH 02/42] Accept lockVersion as a prop and add to store

---
 .../javascripts/issue_show/components/app.vue |  6 +++++
 .../javascripts/issue_show/stores/index.js    |  2 ++
 spec/helpers/issuables_helper_spec.rb         |  1 +
 .../issue_show/components/app_spec.js         | 26 ++++++++++++++-----
 spec/javascripts/issue_show/mock_data.js      |  2 ++
 5 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue
index cd569eb3045..b6eacf839b9 100644
--- a/app/assets/javascripts/issue_show/components/app.vue
+++ b/app/assets/javascripts/issue_show/components/app.vue
@@ -130,6 +130,10 @@ export default {
       required: false,
       default: true,
     },
+    lockVersion: {
+      type: Number,
+      required: true,
+    },
   },
   data() {
     const store = new Store({
@@ -141,6 +145,7 @@ export default {
       updatedByName: this.updatedByName,
       updatedByPath: this.updatedByPath,
       taskStatus: this.initialTaskStatus,
+      lock_version: this.lockVersion,
     });
 
     return {
@@ -214,6 +219,7 @@ export default {
         this.store.setFormState({
           title: this.state.titleText,
           description: this.state.descriptionText,
+          lock_version: this.state.lock_version,
           lockedWarningVisible: false,
           updateLoading: false,
         });
diff --git a/app/assets/javascripts/issue_show/stores/index.js b/app/assets/javascripts/issue_show/stores/index.js
index 32044d6da25..2b3903def6b 100644
--- a/app/assets/javascripts/issue_show/stores/index.js
+++ b/app/assets/javascripts/issue_show/stores/index.js
@@ -6,6 +6,7 @@ export default class Store {
       description: '',
       lockedWarningVisible: false,
       updateLoading: false,
+      lock_version: 0,
     };
   }
 
@@ -22,6 +23,7 @@ export default class Store {
     this.state.updatedAt = data.updated_at;
     this.state.updatedByName = data.updated_by_name;
     this.state.updatedByPath = data.updated_by_path;
+    this.state.lock_version = data.lock_version;
   }
 
   stateShouldUpdate(data) {
diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb
index 03e3a72a82f..af319e5ebfe 100644
--- a/spec/helpers/issuables_helper_spec.rb
+++ b/spec/helpers/issuables_helper_spec.rb
@@ -190,6 +190,7 @@ describe IssuablesHelper do
         markdownDocsPath: '/help/user/markdown',
         markdownVersion: CacheMarkdownField::CACHE_COMMONMARK_VERSION,
         issuableTemplates: [],
+        lockVersion: issue.lock_version,
         projectPath: @project.path,
         projectNamespace: @project.namespace.path,
         initialTitleHtml: issue.title,
diff --git a/spec/javascripts/issue_show/components/app_spec.js b/spec/javascripts/issue_show/components/app_spec.js
index 2bd1b3996dc..9b2b6b670c3 100644
--- a/spec/javascripts/issue_show/components/app_spec.js
+++ b/spec/javascripts/issue_show/components/app_spec.js
@@ -43,6 +43,7 @@ describe('Issuable output', () => {
         initialTitleText: '',
         initialDescriptionHtml: 'test',
         initialDescriptionText: 'test',
+        lockVersion: 1,
         markdownPreviewPath: '/',
         markdownDocsPath: '/',
         projectNamespace: '/',
@@ -78,6 +79,7 @@ describe('Issuable output', () => {
         expect(formatText(editedText.innerText)).toMatch(/Edited[\s\S]+?by Some User/);
         expect(editedText.querySelector('.author-link').href).toMatch(/\/some_user$/);
         expect(editedText.querySelector('time')).toBeTruthy();
+        expect(vm.state.lock_version).toEqual(1);
       })
       .then(() => {
         vm.poll.makeRequest();
@@ -95,6 +97,7 @@ describe('Issuable output', () => {
 
         expect(editedText.querySelector('.author-link').href).toMatch(/\/other_user$/);
         expect(editedText.querySelector('time')).toBeTruthy();
+        expect(vm.state.lock_version).toEqual(2);
       })
       .then(done)
       .catch(done.fail);
@@ -255,15 +258,10 @@ describe('Issuable output', () => {
     describe('error when updating', () => {
       beforeEach(() => {
         spyOn(window, 'Flash').and.callThrough();
-        spyOn(vm.service, 'updateIssuable').and.callFake(
-          () =>
-            new Promise((resolve, reject) => {
-              reject();
-            }),
-        );
       });
 
       it('closes form on error', done => {
+        spyOn(vm.service, 'updateIssuable').and.callFake(() => Promise.resolve());
         vm.updateIssuable();
 
         setTimeout(() => {
@@ -276,6 +274,7 @@ describe('Issuable output', () => {
       });
 
       it('returns the correct error message for issuableType', done => {
+        spyOn(vm.service, 'updateIssuable').and.callFake(() => Promise.reject());
         vm.issuableType = 'merge request';
 
         Vue.nextTick(() => {
@@ -290,6 +289,20 @@ describe('Issuable output', () => {
           });
         });
       });
+
+      it('shows error mesage from backend if exists', done => {
+        const msg = 'Custom error message from backend';
+        spyOn(vm.service, 'updateIssuable').and.callFake(() =>
+          Promise.reject({ response: { data: { errors: msg } } }), // eslint-disable-line prefer-promise-reject-errors
+        );
+
+        vm.updateIssuable();
+        setTimeout(() => {
+          expect(window.Flash).toHaveBeenCalledWith(msg);
+
+          done();
+        });
+      });
     });
   });
 
@@ -420,6 +433,7 @@ describe('Issuable output', () => {
         .then(vm.$nextTick)
         .then(() => {
           expect(vm.formState.lockedWarningVisible).toEqual(true);
+          expect(vm.formState.lock_version).toEqual(1);
           expect(vm.$el.querySelector('.alert')).not.toBeNull();
         })
         .then(done)
diff --git a/spec/javascripts/issue_show/mock_data.js b/spec/javascripts/issue_show/mock_data.js
index 74b3efb014b..f4475aadb8b 100644
--- a/spec/javascripts/issue_show/mock_data.js
+++ b/spec/javascripts/issue_show/mock_data.js
@@ -8,6 +8,7 @@ export default {
     updated_at: '2015-05-15T12:31:04.428Z',
     updated_by_name: 'Some User',
     updated_by_path: '/some_user',
+    lock_version: 1,
   },
   secondRequest: {
     title: '<p>2</p>',
@@ -18,5 +19,6 @@ export default {
     updated_at: '2016-05-15T12:31:04.428Z',
     updated_by_name: 'Other User',
     updated_by_path: '/other_user',
+    lock_version: 2,
   },
 };

From f1acd5051513ee815578f7311ca88ee54e79e323 Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Mon, 7 Jan 2019 16:23:53 +0100
Subject: [PATCH 03/42] Show error message from backend

---
 app/assets/javascripts/issue_show/components/app.vue | 8 +++++++-
 spec/javascripts/issue_show/components/app_spec.js   | 4 ++--
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue
index b6eacf839b9..d65b44ca6f3 100644
--- a/app/assets/javascripts/issue_show/components/app.vue
+++ b/app/assets/javascripts/issue_show/components/app.vue
@@ -250,8 +250,14 @@ export default {
           if (error && error.name === 'SpamError') {
             this.openRecaptcha();
           } else {
+            let errMsg = `Error updating ${this.issuableType}`;
+
+            if (error && error.response && error.response.data && error.response.data.errors) {
+              errMsg = error.response.data.errors;
+            }
+
             eventHub.$emit('close.form');
-            window.Flash(`Error updating ${this.issuableType}`);
+            window.Flash(errMsg);
           }
         });
     },
diff --git a/spec/javascripts/issue_show/components/app_spec.js b/spec/javascripts/issue_show/components/app_spec.js
index 9b2b6b670c3..863d15570a0 100644
--- a/spec/javascripts/issue_show/components/app_spec.js
+++ b/spec/javascripts/issue_show/components/app_spec.js
@@ -292,8 +292,8 @@ describe('Issuable output', () => {
 
       it('shows error mesage from backend if exists', done => {
         const msg = 'Custom error message from backend';
-        spyOn(vm.service, 'updateIssuable').and.callFake(() =>
-          Promise.reject({ response: { data: { errors: msg } } }), // eslint-disable-line prefer-promise-reject-errors
+        spyOn(vm.service, 'updateIssuable').and.callFake(
+          () => Promise.reject({ response: { data: { errors: msg } } }), // eslint-disable-line prefer-promise-reject-errors
         );
 
         vm.updateIssuable();

From 83306d249e0762d21b9ca128b9ebb57a0bef6f8b Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Thu, 10 Jan 2019 15:22:28 -0600
Subject: [PATCH 04/42] Pass tasklist lock version

receive data on when there is a conflict
---
 app/assets/javascripts/issue_show/components/app.vue   |  1 +
 .../javascripts/issue_show/components/description.vue  | 10 ++++++++++
 app/assets/javascripts/task_list.js                    |  4 +++-
 app/controllers/concerns/issuable_actions.rb           |  5 ++++-
 4 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue
index d65b44ca6f3..3e71c40e896 100644
--- a/app/assets/javascripts/issue_show/components/app.vue
+++ b/app/assets/javascripts/issue_show/components/app.vue
@@ -326,6 +326,7 @@ export default {
         :task-status="state.taskStatus"
         :issuable-type="issuableType"
         :update-url="updateEndpoint"
+        :lock-version="state.lock_version"
       />
       <edited-component
         v-if="hasUpdated"
diff --git a/app/assets/javascripts/issue_show/components/description.vue b/app/assets/javascripts/issue_show/components/description.vue
index 5ca88d75063..4443d6c6760 100644
--- a/app/assets/javascripts/issue_show/components/description.vue
+++ b/app/assets/javascripts/issue_show/components/description.vue
@@ -35,6 +35,10 @@ export default {
       required: false,
       default: null,
     },
+    lockVersion: {
+      type: Number,
+      required: false,
+    },
   },
   data() {
     return {
@@ -67,8 +71,10 @@ export default {
         new TaskList({
           dataType: this.issuableType,
           fieldName: 'description',
+          lockVersion: this.lockVersion,
           selector: '.detail-page-description',
           onSuccess: this.taskListUpdateSuccess.bind(this),
+          onError: this.taskListUpdateError.bind(this),
         });
       }
     },
@@ -82,6 +88,10 @@ export default {
       }
     },
 
+    taskListUpdateError(data) {
+      // stub for error handling
+    },
+
     updateTaskStatusText() {
       const taskRegexMatches = this.taskStatus.match(/(\d+) of ((?!0)\d+)/);
       const $issuableHeader = $('.issuable-meta');
diff --git a/app/assets/javascripts/task_list.js b/app/assets/javascripts/task_list.js
index edefb3735d7..f063250c36f 100644
--- a/app/assets/javascripts/task_list.js
+++ b/app/assets/javascripts/task_list.js
@@ -8,8 +8,9 @@ export default class TaskList {
     this.selector = options.selector;
     this.dataType = options.dataType;
     this.fieldName = options.fieldName;
+    this.lockVersion = options.lockVersion;
     this.onSuccess = options.onSuccess || (() => {});
-    this.onError = function showFlash(e) {
+    this.onError = options.onError || function showFlash(e) {
       let errorMessages = '';
 
       if (e.response.data && typeof e.response.data === 'object') {
@@ -43,6 +44,7 @@ export default class TaskList {
     const patchData = {};
     patchData[this.dataType] = {
       [this.fieldName]: $target.val(),
+      ['lock_version']: this.lockVersion,
     };
 
     return axios
diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb
index 8ef3b6502df..f0b4198b2d9 100644
--- a/app/controllers/concerns/issuable_actions.rb
+++ b/app/controllers/concerns/issuable_actions.rb
@@ -162,10 +162,13 @@ module IssuableActions
       end
 
       format.json do
+        # We want to pass back the latest valid data, so reload the model
+        @issuable.reload
         render json: {
           errors: [
             "Someone edited this #{issuable.human_class_name} at the same time you did. Please refresh your browser and make sure your changes will not unintentionally remove theirs."
-          ]
+          ],
+          data: serializer.represent(@issuable)
         }, status: :conflict
       end
     end

From 1362267d45dee0a6a4311c59e72b90e3ea032da6 Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Fri, 11 Jan 2019 01:07:09 +0100
Subject: [PATCH 05/42] Fix data coming down to error handler of tasklist

---
 app/assets/javascripts/issue_show/components/description.vue | 3 ++-
 app/assets/javascripts/task_list.js                          | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/app/assets/javascripts/issue_show/components/description.vue b/app/assets/javascripts/issue_show/components/description.vue
index 4443d6c6760..d5b355a39a2 100644
--- a/app/assets/javascripts/issue_show/components/description.vue
+++ b/app/assets/javascripts/issue_show/components/description.vue
@@ -1,5 +1,6 @@
 <script>
 import $ from 'jquery';
+import createFlash from '~/flash';
 import animateMixin from '../mixins/animate';
 import TaskList from '../../task_list';
 import recaptchaModalImplementor from '../../vue_shared/mixins/recaptcha_modal_implementor';
@@ -89,7 +90,7 @@ export default {
     },
 
     taskListUpdateError(data) {
-      // stub for error handling
+      createFlash(data.errors[0]);
     },
 
     updateTaskStatusText() {
diff --git a/app/assets/javascripts/task_list.js b/app/assets/javascripts/task_list.js
index f063250c36f..fce41ef65c4 100644
--- a/app/assets/javascripts/task_list.js
+++ b/app/assets/javascripts/task_list.js
@@ -44,12 +44,12 @@ export default class TaskList {
     const patchData = {};
     patchData[this.dataType] = {
       [this.fieldName]: $target.val(),
-      ['lock_version']: this.lockVersion,
+      lock_version: this.lockVersion,
     };
 
     return axios
       .patch($target.data('updateUrl') || $('form.js-issuable-update').attr('action'), patchData)
       .then(({ data }) => this.onSuccess(data))
-      .catch(err => this.onError(err));
+      .catch(({ response }) => this.onError(response.data));
   }
 }

From 515f4b46ab06af1e7201a27f5a931c4763643c8e Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Wed, 16 Jan 2019 01:33:58 +0100
Subject: [PATCH 06/42] Update lock version after request

---
 app/assets/javascripts/task_list.js | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/app/assets/javascripts/task_list.js b/app/assets/javascripts/task_list.js
index fce41ef65c4..10368ecf248 100644
--- a/app/assets/javascripts/task_list.js
+++ b/app/assets/javascripts/task_list.js
@@ -49,7 +49,10 @@ export default class TaskList {
 
     return axios
       .patch($target.data('updateUrl') || $('form.js-issuable-update').attr('action'), patchData)
-      .then(({ data }) => this.onSuccess(data))
       .catch(({ response }) => this.onError(response.data));
+      .then(({ data }) => {
+        this.lockVersion = data.lock_version;
+        return this.onSuccess(data);
+      })
   }
 }

From b72f52f12303ae06015491844ed9dcf404cee1de Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Wed, 16 Jan 2019 01:34:30 +0100
Subject: [PATCH 07/42] Disable tasklist items while request is being done

---
 app/assets/javascripts/task_list.js | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/app/assets/javascripts/task_list.js b/app/assets/javascripts/task_list.js
index 10368ecf248..af2359d606f 100644
--- a/app/assets/javascripts/task_list.js
+++ b/app/assets/javascripts/task_list.js
@@ -34,8 +34,16 @@ export default class TaskList {
     );
   }
 
-  disable() {
+  disableTaskListItems() {
     $(`${this.selector} .js-task-list-container`).taskList('disable');
+  }
+
+  enableTaskListItems() {
+    $(`${this.selector} .js-task-list-container`).taskList('enable');
+  }
+
+  disable() {
+    this.disableTaskListItems();
     $(document).off('tasklist:changed', `${this.selector} .js-task-list-container`);
   }
 
@@ -47,12 +55,20 @@ export default class TaskList {
       lock_version: this.lockVersion,
     };
 
+    this.disableTaskListItems();
+
     return axios
       .patch($target.data('updateUrl') || $('form.js-issuable-update').attr('action'), patchData)
-      .catch(({ response }) => this.onError(response.data));
       .then(({ data }) => {
         this.lockVersion = data.lock_version;
+        this.enableTaskListItems();
+
         return this.onSuccess(data);
       })
+      .catch(({ response }) => {
+        this.enableTaskListItems();
+
+        return this.onError(response.data);
+      });
   }
 }

From 7ad76a5418a201b53963d3efd1206870fe2d9770 Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Wed, 16 Jan 2019 01:35:57 +0100
Subject: [PATCH 08/42] Handle tasklist update failed in app component

---
 app/assets/javascripts/issue_show/components/app.vue     | 9 +++++++++
 .../javascripts/issue_show/components/description.vue    | 6 ++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue
index 3e71c40e896..0428e099279 100644
--- a/app/assets/javascripts/issue_show/components/app.vue
+++ b/app/assets/javascripts/issue_show/components/app.vue
@@ -213,6 +213,14 @@ export default {
       return undefined;
     },
 
+    updateStoreState() {
+      this.service.getData()
+        .then(res => res.data)
+        .then(data => {
+          this.store.updateState(data);
+        });
+    },
+
     openForm() {
       if (!this.showForm) {
         this.showForm = true;
@@ -327,6 +335,7 @@ export default {
         :issuable-type="issuableType"
         :update-url="updateEndpoint"
         :lock-version="state.lock_version"
+        @taskListUpdateFailed="updateStoreState"
       />
       <edited-component
         v-if="hasUpdated"
diff --git a/app/assets/javascripts/issue_show/components/description.vue b/app/assets/javascripts/issue_show/components/description.vue
index d5b355a39a2..519012b7df9 100644
--- a/app/assets/javascripts/issue_show/components/description.vue
+++ b/app/assets/javascripts/issue_show/components/description.vue
@@ -89,8 +89,10 @@ export default {
       }
     },
 
-    taskListUpdateError(data) {
-      createFlash(data.errors[0]);
+    taskListUpdateError({ errors, data }) {
+      createFlash('Someone edited this issue at the same time you did and we updated the issue description.');
+
+      this.$emit('taskListUpdateFailed');
     },
 
     updateTaskStatusText() {

From 35ada101a7354fee87c4bfbcb996e99d4746bbaa Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Fri, 18 Jan 2019 01:52:09 +0100
Subject: [PATCH 09/42] =?UTF-8?q?Temporarily=20use=20tasklist=20from=20fat?=
 =?UTF-8?q?ihacet=E2=80=99s=20fork?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 Gemfile      |  2 +-
 Gemfile.lock | 11 ++++++++---
 package.json |  2 +-
 yarn.lock    |  5 ++---
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/Gemfile b/Gemfile
index 943b0260dcb..a329c9c0e28 100644
--- a/Gemfile
+++ b/Gemfile
@@ -112,7 +112,7 @@ gem 'seed-fu', '~> 2.3.7'
 
 # Markdown and HTML processing
 gem 'html-pipeline', '~> 2.8'
-gem 'deckar01-task_list', '2.0.1'
+gem 'deckar01-task_list', git: 'https://github.com/fatihacet/task_list.git'
 gem 'gitlab-markup', '~> 1.6.5'
 gem 'github-markup', '~> 1.7.0', require: 'github/markup'
 gem 'redcarpet', '~> 3.4'
diff --git a/Gemfile.lock b/Gemfile.lock
index ec6af6ffb0c..4656abcc8ea 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -1,3 +1,10 @@
+GIT
+  remote: https://github.com/fatihacet/task_list.git
+  revision: 844da3fa2830bcb159a70bf195d4693f9f22e04a
+  specs:
+    deckar01-task_list (2.0.1)
+      html-pipeline
+
 GEM
   remote: https://rubygems.org/
   specs:
@@ -143,8 +150,6 @@ GEM
     database_cleaner (1.7.0)
     debug_inspector (0.0.3)
     debugger-ruby_core_source (1.3.8)
-    deckar01-task_list (2.0.1)
-      html-pipeline
     declarative (0.0.10)
     declarative-option (0.1.0)
     descendants_tracker (0.0.4)
@@ -982,7 +987,7 @@ DEPENDENCIES
   connection_pool (~> 2.0)
   creole (~> 0.5.0)
   database_cleaner (~> 1.7.0)
-  deckar01-task_list (= 2.0.1)
+  deckar01-task_list!
   device_detector
   devise (~> 4.4)
   devise-two-factor (~> 3.0.0)
diff --git a/package.json b/package.json
index 13c0527c4a3..f7b91f43967 100644
--- a/package.json
+++ b/package.json
@@ -56,7 +56,7 @@
     "d3-time": "^1.0.8",
     "d3-time-format": "^2.1.1",
     "dateformat": "^3.0.3",
-    "deckar01-task_list": "^2.0.1",
+    "deckar01-task_list": "fatihacet/task_list#844da3f",
     "diff": "^3.4.0",
     "document-register-element": "1.3.0",
     "dropzone": "^4.2.0",
diff --git a/yarn.lock b/yarn.lock
index 5c9139fdbfa..84b291597f5 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -3032,10 +3032,9 @@ decamelize@^2.0.0:
   dependencies:
     xregexp "4.0.0"
 
-deckar01-task_list@^2.0.1:
+deckar01-task_list@fatihacet/task_list#844da3f:
   version "2.0.1"
-  resolved "https://registry.yarnpkg.com/deckar01-task_list/-/deckar01-task_list-2.0.1.tgz#fdcfb6ab5717055a82f29e863a49990a043a06a9"
-  integrity sha512-i5fT8QxJ9iV6dfgy5U0NHW91O5cKsvDc4u8JNMnZ6efQc356bA9vKuXO3732agSry+bO6TolzTmuqSRi4tkkeA==
+  resolved "https://codeload.github.com/fatihacet/task_list/tar.gz/844da3fa2830bcb159a70bf195d4693f9f22e04a"
 
 decode-uri-component@^0.2.0:
   version "0.2.0"

From 68c0276421bce7ace11128f66547cca7f61e74b0 Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Fri, 18 Jan 2019 01:52:22 +0100
Subject: [PATCH 10/42] Pass line number and source to backend

---
 app/assets/javascripts/task_list.js | 6 ++++++
 package.json                        | 2 +-
 yarn.lock                           | 2 +-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/app/assets/javascripts/task_list.js b/app/assets/javascripts/task_list.js
index af2359d606f..2c3bd61463e 100644
--- a/app/assets/javascripts/task_list.js
+++ b/app/assets/javascripts/task_list.js
@@ -49,10 +49,16 @@ export default class TaskList {
 
   update(e) {
     const $target = $(e.target);
+    const { lineNumber, lineSource } = e.detail;
     const patchData = {};
+
     patchData[this.dataType] = {
       [this.fieldName]: $target.val(),
       lock_version: this.lockVersion,
+      update_task: {
+        line_number: lineNumber,
+        line_source: lineSource,
+      },
     };
 
     this.disableTaskListItems();
diff --git a/package.json b/package.json
index f7b91f43967..863a619e63b 100644
--- a/package.json
+++ b/package.json
@@ -56,7 +56,7 @@
     "d3-time": "^1.0.8",
     "d3-time-format": "^2.1.1",
     "dateformat": "^3.0.3",
-    "deckar01-task_list": "fatihacet/task_list#844da3f",
+    "deckar01-task_list": "fatihacet/task_list",
     "diff": "^3.4.0",
     "document-register-element": "1.3.0",
     "dropzone": "^4.2.0",
diff --git a/yarn.lock b/yarn.lock
index 84b291597f5..0e2d1725c11 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -3032,7 +3032,7 @@ decamelize@^2.0.0:
   dependencies:
     xregexp "4.0.0"
 
-deckar01-task_list@fatihacet/task_list#844da3f:
+deckar01-task_list@fatihacet/task_list:
   version "2.0.1"
   resolved "https://codeload.github.com/fatihacet/task_list/tar.gz/844da3fa2830bcb159a70bf195d4693f9f22e04a"
 

From 58a005cf2c225a7d1425ccdc99377b8bf441056e Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Fri, 18 Jan 2019 14:56:10 -0600
Subject: [PATCH 11/42] Pass down the new state of the checkbox

and whitelist the `update_task` parameter
---
 app/assets/javascripts/task_list.js           | 3 ++-
 app/controllers/projects/issues_controller.rb | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/app/assets/javascripts/task_list.js b/app/assets/javascripts/task_list.js
index 2c3bd61463e..dbcac01e761 100644
--- a/app/assets/javascripts/task_list.js
+++ b/app/assets/javascripts/task_list.js
@@ -49,7 +49,7 @@ export default class TaskList {
 
   update(e) {
     const $target = $(e.target);
-    const { lineNumber, lineSource } = e.detail;
+    const { lineNumber, lineSource, checked } = e.detail;
     const patchData = {};
 
     patchData[this.dataType] = {
@@ -58,6 +58,7 @@ export default class TaskList {
       update_task: {
         line_number: lineNumber,
         line_source: lineSource,
+        checked: checked,
       },
     };
 
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index fd5f3eeaa99..aed28d49f5e 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -246,7 +246,7 @@ class Projects::IssuesController < Projects::ApplicationController
       task_num
       lock_version
       discussion_locked
-    ] + [{ label_ids: [], assignee_ids: [] }]
+    ] + [{ label_ids: [], assignee_ids: [], update_task: [:line_number, :line_source, :checked] }]
   end
 
   def authenticate_new_issue!

From e2e2f0d3788fbc1bad951eb7f2186d56f9b49c66 Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Mon, 21 Jan 2019 15:16:37 -0600
Subject: [PATCH 12/42] Pass down the index of the checkbox checked

---
 app/assets/javascripts/task_list.js | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/app/assets/javascripts/task_list.js b/app/assets/javascripts/task_list.js
index dbcac01e761..08100d1d542 100644
--- a/app/assets/javascripts/task_list.js
+++ b/app/assets/javascripts/task_list.js
@@ -49,16 +49,17 @@ export default class TaskList {
 
   update(e) {
     const $target = $(e.target);
-    const { lineNumber, lineSource, checked } = e.detail;
+    const { index, checked, lineNumber, lineSource } = e.detail;
     const patchData = {};
 
     patchData[this.dataType] = {
       [this.fieldName]: $target.val(),
       lock_version: this.lockVersion,
       update_task: {
+        index: index,
+        checked: checked,
         line_number: lineNumber,
         line_source: lineSource,
-        checked: checked,
       },
     };
 

From ff1e64902821eaa09dbdfb6acc382b33ed7f2e37 Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Mon, 21 Jan 2019 16:16:23 -0600
Subject: [PATCH 13/42] Don't consider field invalid if updating html also

---
 app/models/concerns/cache_markdown_field.rb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/models/concerns/cache_markdown_field.rb b/app/models/concerns/cache_markdown_field.rb
index 73a27326f6c..0ccc03db15a 100644
--- a/app/models/concerns/cache_markdown_field.rb
+++ b/app/models/concerns/cache_markdown_field.rb
@@ -178,7 +178,9 @@ module CacheMarkdownField
       # author and project invalidate the cache in all circumstances.
       define_method(invalidation_method) do
         changed_fields = changed_attributes.keys
-        invalidations = changed_fields & [markdown_field.to_s, *INVALIDATED_BY]
+        invalidations  = changed_fields & [markdown_field.to_s, *INVALIDATED_BY]
+        invalidations.delete(markdown_field.to_s) if changed_fields.include?("#{markdown_field}_html")
+
         !invalidations.empty? || !cached_html_up_to_date?(markdown_field)
       end
     end

From 10649c49ab371cce3119ffcccfa4c3d479f79ae4 Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Mon, 21 Jan 2019 16:17:01 -0600
Subject: [PATCH 14/42] Added 'handle_update_task' for new task handling

---
 app/controllers/concerns/issuable_actions.rb  |  3 +-
 app/controllers/projects/issues_controller.rb |  2 +-
 app/services/issues/update_service.rb         | 62 +++++++++++++++++++
 3 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb
index f0b4198b2d9..1c55cd340ec 100644
--- a/app/controllers/concerns/issuable_actions.rb
+++ b/app/controllers/concerns/issuable_actions.rb
@@ -163,12 +163,11 @@ module IssuableActions
 
       format.json do
         # We want to pass back the latest valid data, so reload the model
-        @issuable.reload
         render json: {
           errors: [
             "Someone edited this #{issuable.human_class_name} at the same time you did. Please refresh your browser and make sure your changes will not unintentionally remove theirs."
           ],
-          data: serializer.represent(@issuable)
+          data: serializer.represent(@issuable.reload) # rubocop:disable Gitlab/ModuleWithInstanceVariables
         }, status: :conflict
       end
     end
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index aed28d49f5e..df602e74cf2 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -246,7 +246,7 @@ class Projects::IssuesController < Projects::ApplicationController
       task_num
       lock_version
       discussion_locked
-    ] + [{ label_ids: [], assignee_ids: [], update_task: [:line_number, :line_source, :checked] }]
+    ] + [{ label_ids: [], assignee_ids: [], update_task: [:index, :checked, :line_number, :line_source] }]
   end
 
   def authenticate_new_issue!
diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb
index e992d682c79..c9292ed5738 100644
--- a/app/services/issues/update_service.rb
+++ b/app/services/issues/update_service.rb
@@ -8,6 +8,7 @@ module Issues
       handle_move_between_ids(issue)
       filter_spam_check_params
       change_issue_duplicate(issue)
+      handle_update_task(issue)
       move_issue_to_new_project(issue) || update(issue)
     end
 
@@ -118,6 +119,67 @@ module Issues
       end
     end
 
+    def handle_update_task(issue)
+      update_task_params = params.delete(:update_task)
+      return unless update_task_params
+
+      checkbox_index      = update_task_params[:index]
+      currently_checked   = !update_task_params[:checked]
+      changed_line_number = update_task_params[:line_number]
+      changed_line_source = update_task_params[:line_source] + "\n"
+
+      source_lines  = issue.description.lines
+      markdown_task = source_lines[changed_line_number - 1]
+
+      # binding.pry
+
+      if markdown_task == changed_line_source
+        source_checkbox = /(\[[\sxX]\])/.match(markdown_task)
+
+        if source_checkbox
+          if currently_checked
+            if source_checkbox[1] == '[x]'
+              changed_line_source.sub!(/(\[[xX]\])/, '[ ]')
+            else
+              # it's already checked by someone else, nothing to do
+              ignore = true
+            end
+          else
+            if source_checkbox[1] == '[ ]'
+              changed_line_source.sub!(/(\[[\s]\])/, '[x]')
+            else
+              # it's already unchecked by someone else, nothing to do
+              ignore = true
+            end
+          end
+        end
+
+        unless ignore
+          # replace line with proper checkbox
+          source_lines[changed_line_number - 1] = changed_line_source
+
+          params[:description] = source_lines.join
+
+          # if we update the description_html field at the same time,
+          # the cache won't be considered invalid (unless something else
+          # changed)
+          html = Nokogiri::HTML.fragment(issue.description_html)
+          html_checkbox = html.css('.task-list-item-checkbox')[checkbox_index - 1]
+          # html_checkbox = html.css(".task-list-item[data-sourcepos^='#{changed_line_number}:'] > input.task-list-item-checkbox").first
+
+          if currently_checked
+            # checkboxes[checkbox_index - 1].remove_attribute('checked')
+            html_checkbox.remove_attribute('checked')
+          else
+            # checkboxes[checkbox_index - 1][:checked] = 'checked'
+            html_checkbox[:checked] = 'checked'
+          end
+
+          params[:description_html] = html.to_html
+        end
+      end
+    end
+
     # rubocop: disable CodeReuse/ActiveRecord
     def get_issue_if_allowed(id, board_group_id = nil)
       return unless id

From 6243c04e20b480c8353ac820ac4eb7ea43b92718 Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Tue, 22 Jan 2019 21:53:08 +0100
Subject: [PATCH 15/42] Fix failing specs and lint errors.

---
 .../javascripts/issue_show/components/app.vue |  8 +++++++-
 .../issue_show/components/description.vue     |  7 +++++--
 app/assets/javascripts/task_list.js           | 20 ++++++++++---------
 .../issue_show/components/description_spec.js |  3 +++
 spec/javascripts/merge_request_spec.js        | 17 ++++++++++++++--
 spec/javascripts/notes_spec.js                | 19 ++++++++++++++++--
 6 files changed, 58 insertions(+), 16 deletions(-)

diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue
index 0428e099279..72398cb547a 100644
--- a/app/assets/javascripts/issue_show/components/app.vue
+++ b/app/assets/javascripts/issue_show/components/app.vue
@@ -214,10 +214,16 @@ export default {
     },
 
     updateStoreState() {
-      this.service.getData()
+      this.service
+        .getData()
         .then(res => res.data)
         .then(data => {
           this.store.updateState(data);
+        })
+        .catch(() => {
+          const errMsg = `Error updating ${this.issuableType}`;
+
+          window.Flash(errMsg);
         });
     },
 
diff --git a/app/assets/javascripts/issue_show/components/description.vue b/app/assets/javascripts/issue_show/components/description.vue
index 519012b7df9..56e873c6ba0 100644
--- a/app/assets/javascripts/issue_show/components/description.vue
+++ b/app/assets/javascripts/issue_show/components/description.vue
@@ -39,6 +39,7 @@ export default {
     lockVersion: {
       type: Number,
       required: false,
+      default: 0,
     },
   },
   data() {
@@ -89,8 +90,10 @@ export default {
       }
     },
 
-    taskListUpdateError({ errors, data }) {
-      createFlash('Someone edited this issue at the same time you did and we updated the issue description.');
+    taskListUpdateError() {
+      createFlash(
+        'Someone edited this issue at the same time you did and we updated the issue description.',
+      );
 
       this.$emit('taskListUpdateFailed');
     },
diff --git a/app/assets/javascripts/task_list.js b/app/assets/javascripts/task_list.js
index 08100d1d542..9ea6cba7281 100644
--- a/app/assets/javascripts/task_list.js
+++ b/app/assets/javascripts/task_list.js
@@ -10,15 +10,17 @@ export default class TaskList {
     this.fieldName = options.fieldName;
     this.lockVersion = options.lockVersion;
     this.onSuccess = options.onSuccess || (() => {});
-    this.onError = options.onError || function showFlash(e) {
-      let errorMessages = '';
+    this.onError =
+      options.onError ||
+      function showFlash(e) {
+        let errorMessages = '';
 
-      if (e.response.data && typeof e.response.data === 'object') {
-        errorMessages = e.response.data.errors.join(' ');
-      }
+        if (e.response.data && typeof e.response.data === 'object') {
+          errorMessages = e.response.data.errors.join(' ');
+        }
 
-      return new Flash(errorMessages || 'Update failed', 'alert');
-    };
+        return new Flash(errorMessages || 'Update failed', 'alert');
+      };
 
     this.init();
   }
@@ -56,8 +58,8 @@ export default class TaskList {
       [this.fieldName]: $target.val(),
       lock_version: this.lockVersion,
       update_task: {
-        index: index,
-        checked: checked,
+        index,
+        checked,
         line_number: lineNumber,
         line_source: lineSource,
       },
diff --git a/spec/javascripts/issue_show/components/description_spec.js b/spec/javascripts/issue_show/components/description_spec.js
index 463f3c89926..52148f4c66b 100644
--- a/spec/javascripts/issue_show/components/description_spec.js
+++ b/spec/javascripts/issue_show/components/description_spec.js
@@ -123,7 +123,10 @@ describe('Description component', () => {
           fieldName: 'description',
           selector: '.detail-page-description',
           onSuccess: jasmine.any(Function),
+          onError: jasmine.any(Function),
+          lockVersion: 0,
         });
+
         done();
       });
     });
diff --git a/spec/javascripts/merge_request_spec.js b/spec/javascripts/merge_request_spec.js
index 1cb49b49ca7..32623d1781a 100644
--- a/spec/javascripts/merge_request_spec.js
+++ b/spec/javascripts/merge_request_spec.js
@@ -41,15 +41,28 @@ describe('MergeRequest', function() {
     });
 
     it('submits an ajax request on tasklist:changed', done => {
-      $('.js-task-list-field').trigger('tasklist:changed');
+      const lineNumber = 8;
+      const lineSource = '- [ ] item 8';
+      const index = 3;
+      const checked = true;
+
+      $('.js-task-list-field').trigger({
+        type: 'tasklist:changed',
+        detail: { lineNumber, lineSource, index, checked },
+      });
 
       setTimeout(() => {
         expect(axios.patch).toHaveBeenCalledWith(
           `${gl.TEST_HOST}/frontend-fixtures/merge-requests-project/merge_requests/1.json`,
           {
-            merge_request: { description: '- [ ] Task List Item' },
+            merge_request: {
+              description: '- [ ] Task List Item',
+              lock_version: undefined,
+              update_task: { line_number: lineNumber, line_source: lineSource, index, checked },
+            },
           },
         );
+
         done();
       });
     });
diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js
index 694f581150f..7c869d4c326 100644
--- a/spec/javascripts/notes_spec.js
+++ b/spec/javascripts/notes_spec.js
@@ -89,10 +89,25 @@ describe('Notes', function() {
     });
 
     it('submits an ajax request on tasklist:changed', function(done) {
-      $('.js-task-list-container').trigger('tasklist:changed');
+      const lineNumber = 8;
+      const lineSource = '- [ ] item 8';
+      const index = 3;
+      const checked = true;
+
+      $('.js-task-list-container').trigger({
+        type: 'tasklist:changed',
+        detail: { lineNumber, lineSource, index, checked },
+      });
 
       setTimeout(() => {
-        expect(axios.patch).toHaveBeenCalled();
+        expect(axios.patch).toHaveBeenCalledWith(undefined, {
+          note: {
+            note: '',
+            lock_version: undefined,
+            update_task: { index, checked, line_number: lineNumber, line_source: lineSource },
+          },
+        });
+
         done();
       });
     });

From d9c5668d2915d64cbd91880e1ff1c0c743e8aa99 Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Tue, 22 Jan 2019 18:05:38 -0600
Subject: [PATCH 16/42] Refactor toggling of task list item

---
 app/models/concerns/taskable.rb               | 46 +++++++++++-
 .../issuable/common_system_notes_service.rb   |  2 +-
 app/services/issuable_base_service.rb         | 32 ++++++++
 app/services/issues/update_service.rb         | 75 +++++--------------
 4 files changed, 96 insertions(+), 59 deletions(-)

diff --git a/app/models/concerns/taskable.rb b/app/models/concerns/taskable.rb
index 603d4d62578..547afd5ee06 100644
--- a/app/models/concerns/taskable.rb
+++ b/app/models/concerns/taskable.rb
@@ -9,9 +9,11 @@ require 'task_list/filter'
 #
 # Used by MergeRequest and Issue
 module Taskable
-  COMPLETED    = 'completed'.freeze
-  INCOMPLETE   = 'incomplete'.freeze
-  ITEM_PATTERN = %r{
+  COMPLETED          = 'completed'.freeze
+  INCOMPLETE         = 'incomplete'.freeze
+  COMPLETE_PATTERN   = /(\[[xX]\])/.freeze
+  INCOMPLETE_PATTERN = /(\[[\s]\])/.freeze
+  ITEM_PATTERN       = %r{
     ^
     \s*(?:[-+*]|(?:\d+\.)) # list prefix required - task item has to be always in a list
     \s+                       # whitespace prefix has to be always presented for a list item
@@ -37,6 +39,44 @@ module Taskable
     end
   end
 
+  def self.toggle_task(content, content_html, index:, currently_checked:, line_source:, line_number:)
+    source_lines  = content.split("\n")
+    markdown_task = source_lines[line_number - 1]
+    output        = {}
+
+    if markdown_task == line_source
+      html          = Nokogiri::HTML.fragment(content_html)
+      html_checkbox = html.css('.task-list-item-checkbox')[index - 1]
+      # html_checkbox = html.css(".task-list-item[data-sourcepos^='#{changed_line_number}:'] > input.task-list-item-checkbox").first
+      updated_task  = toggle_task_source(line_source, currently_checked: currently_checked)
+
+      if html_checkbox && updated_task
+        source_lines[line_number - 1] = updated_task
+
+        if currently_checked
+          html_checkbox.remove_attribute('checked')
+        else
+          html_checkbox[:checked] = 'checked'
+        end
+
+        output[:content]      = source_lines.join("\n")
+        output[:content_html] = html.to_html
+      end
+    end
+
+    output
+  end
+
+  def self.toggle_task_source(markdown_line, currently_checked:)
+    if source_checkbox = ITEM_PATTERN.match(markdown_line)
+      if TaskList::Item.new(source_checkbox[1]).complete?
+        markdown_line.sub(COMPLETE_PATTERN, '[ ]') if currently_checked
+      else
+        markdown_line.sub(INCOMPLETE_PATTERN, '[x]') unless currently_checked
+      end
+    end
+  end
+
   # Called by `TaskList::Summary`
   def task_list_items
     return [] if description.blank?
diff --git a/app/services/issuable/common_system_notes_service.rb b/app/services/issuable/common_system_notes_service.rb
index 885e14bba8f..77f38f8882e 100644
--- a/app/services/issuable/common_system_notes_service.rb
+++ b/app/services/issuable/common_system_notes_service.rb
@@ -20,7 +20,7 @@ module Issuable
 
       create_due_date_note if issuable.previous_changes.include?('due_date')
       create_milestone_note if issuable.previous_changes.include?('milestone_id')
-      create_labels_note(old_labels) if issuable.labels != old_labels
+      create_labels_note(old_labels) if old_labels && issuable.labels != old_labels
     end
 
     private
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index 805bb5b510d..90f971faedb 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -235,6 +235,34 @@ class IssuableBaseService < BaseService
     issuable
   end
 
+  def update_task(issuable)
+    filter_params(issuable)
+    # old_associations = associations_before_update(issuable)
+    if issuable.changed? || params.present?
+      issuable.assign_attributes(params.merge(updated_by: current_user))
+      issuable.assign_attributes(last_edited_at: Time.now, last_edited_by: current_user)
+
+      before_update(issuable)
+
+      if issuable.with_transaction_returning_status { issuable.save }
+        # We do not touch as it will affect a update on updated_at field
+        ActiveRecord::Base.no_touching do
+          Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, old_labels: nil)
+        end
+
+        handle_task_changes(issuable)
+
+        invalidate_cache_counts(issuable, users: issuable.assignees.to_a)
+
+        after_update(issuable)
+
+        # execute_hooks(issuable, 'update', old_associations: old_associations)
+      end
+    end
+
+    issuable
+  end
+
   def labels_changing?(old_label_ids, new_label_ids)
     old_label_ids.sort != new_label_ids.sort
   end
@@ -317,6 +345,10 @@ class IssuableBaseService < BaseService
   def handle_changes(issuable, options)
   end
 
+  # override if needed
+  def handle_task_changes(issuable)
+  end
+
   # override if needed
   def execute_hooks(issuable, action = 'open', params = {})
   end
diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb
index c9292ed5738..8b59c501142 100644
--- a/app/services/issues/update_service.rb
+++ b/app/services/issues/update_service.rb
@@ -8,8 +8,7 @@ module Issues
       handle_move_between_ids(issue)
       filter_spam_check_params
       change_issue_duplicate(issue)
-      handle_update_task(issue)
-      move_issue_to_new_project(issue) || update(issue)
+      move_issue_to_new_project(issue) || task_change_event(issue) || update(issue)
     end
 
     def update(issue)
@@ -64,6 +63,11 @@ module Issues
       end
     end
 
+    def handle_task_changes(issuable)
+      todo_service.mark_pending_todos_as_done(issuable, current_user)
+      todo_service.update_issue(issuable, current_user)
+    end
+
     def handle_move_between_ids(issue)
       return unless params[:move_between_ids]
 
@@ -79,6 +83,8 @@ module Issues
     # rubocop: disable CodeReuse/ActiveRecord
     def change_issue_duplicate(issue)
       canonical_issue_id = params.delete(:canonical_issue_id)
+      return unless canonical_issue_id
+
       canonical_issue = IssuesFinder.new(current_user).find_by(id: canonical_issue_id)
 
       if canonical_issue
@@ -119,64 +125,23 @@ module Issues
       end
     end
 
-    def handle_update_task(issue)
+    def task_change_event(issue)
       update_task_params = params.delete(:update_task)
       return unless update_task_params
 
-      checkbox_index      = update_task_params[:index]
-      currently_checked   = !update_task_params[:checked]
-      changed_line_number = update_task_params[:line_number]
-      changed_line_source = update_task_params[:line_source] + "\n"
+      updated_content = Taskable.toggle_task(issue.description, issue.description_html,
+                                             index: update_task_params[:index],
+                                             currently_checked: !update_task_params[:checked],
+                                             line_source: update_task_params[:line_source],
+                                             line_number: update_task_params[:line_number])
 
-      source_lines  = issue.description.lines
-      markdown_task = source_lines[changed_line_number - 1]
+      unless updated_content.empty?
+        # by updating the description_html field at the same time,
+        # the markdown cache won't be considered invalid
+        params[:description]      = updated_content[:content]
+        params[:description_html] = updated_content[:content_html]
 
-      # binding.pry
-
-      if markdown_task == changed_line_source
-        source_checkbox = /(\[[\sxX]\])/.match(markdown_task)
-
-        if source_checkbox
-          if currently_checked
-            if source_checkbox[1] == '[x]'
-              changed_line_source.sub!(/(\[[xX]\])/, '[ ]')
-            else
-              # it's already checked by someone else, nothing to do
-              ignore = true
-            end
-          else
-            if source_checkbox[1] == '[ ]'
-              changed_line_source.sub!(/(\[[\s]\])/, '[x]')
-            else
-              # it's already unchecked by someone else, nothing to do
-              ignore = true
-            end
-          end
-        end
-
-        unless ignore
-          # replace line with proper checkbox
-          source_lines[changed_line_number - 1] = changed_line_source
-
-          params[:description] = source_lines.join
-
-          # if we update the description_html field at the same time,
-          # the cache won't be considered invalid (unless something else
-          # changed)
-          html = Nokogiri::HTML.fragment(issue.description_html)
-          html_checkbox = html.css('.task-list-item-checkbox')[checkbox_index - 1]
-          # html_checkbox = html.css(".task-list-item[data-sourcepos^='#{changed_line_number}:'] > input.task-list-item-checkbox").first
-
-          if currently_checked
-            # checkboxes[checkbox_index - 1].remove_attribute('checked')
-            html_checkbox.remove_attribute('checked')
-          else
-            # checkboxes[checkbox_index - 1][:checked] = 'checked'
-            html_checkbox[:checked] = 'checked'
-          end
-
-          params[:description_html] = html.to_html
-        end
+        update_task(issue)
       end
     end
 

From 4b25f1e6405fb288fd06ee610aec154efedcdb8c Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Wed, 23 Jan 2019 11:28:09 -0600
Subject: [PATCH 17/42] Ignore lock_version for task updates

---
 app/services/issues/update_service.rb | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb
index 8b59c501142..cf6173da8d4 100644
--- a/app/services/issues/update_service.rb
+++ b/app/services/issues/update_service.rb
@@ -141,6 +141,11 @@ module Issues
         params[:description]      = updated_content[:content]
         params[:description_html] = updated_content[:content_html]
 
+        # since we're updating a very specific line, we don't care whether
+        # the `lock_version` sent from the FE is the same or not.  Just
+        # make sure the data hasn't changed since we queried it
+        params[:lock_version]     = issue.lock_version
+
         update_task(issue)
       end
     end

From 1fabeecf85cc637fd68ca99a4de7bf3cb3b915bc Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Wed, 23 Jan 2019 19:12:08 +0100
Subject: [PATCH 18/42] Use original tasklist package with new version

---
 Gemfile      |  2 +-
 Gemfile.lock | 11 +++--------
 package.json |  2 +-
 yarn.lock    |  7 ++++---
 4 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/Gemfile b/Gemfile
index a329c9c0e28..5a7c309d1b8 100644
--- a/Gemfile
+++ b/Gemfile
@@ -112,7 +112,7 @@ gem 'seed-fu', '~> 2.3.7'
 
 # Markdown and HTML processing
 gem 'html-pipeline', '~> 2.8'
-gem 'deckar01-task_list', git: 'https://github.com/fatihacet/task_list.git'
+gem 'deckar01-task_list', '2.1.0'
 gem 'gitlab-markup', '~> 1.6.5'
 gem 'github-markup', '~> 1.7.0', require: 'github/markup'
 gem 'redcarpet', '~> 3.4'
diff --git a/Gemfile.lock b/Gemfile.lock
index 4656abcc8ea..812caca471a 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -1,10 +1,3 @@
-GIT
-  remote: https://github.com/fatihacet/task_list.git
-  revision: 844da3fa2830bcb159a70bf195d4693f9f22e04a
-  specs:
-    deckar01-task_list (2.0.1)
-      html-pipeline
-
 GEM
   remote: https://rubygems.org/
   specs:
@@ -150,6 +143,8 @@ GEM
     database_cleaner (1.7.0)
     debug_inspector (0.0.3)
     debugger-ruby_core_source (1.3.8)
+    deckar01-task_list (2.1.0)
+      html-pipeline
     declarative (0.0.10)
     declarative-option (0.1.0)
     descendants_tracker (0.0.4)
@@ -987,7 +982,7 @@ DEPENDENCIES
   connection_pool (~> 2.0)
   creole (~> 0.5.0)
   database_cleaner (~> 1.7.0)
-  deckar01-task_list!
+  deckar01-task_list (= 2.1.0)
   device_detector
   devise (~> 4.4)
   devise-two-factor (~> 3.0.0)
diff --git a/package.json b/package.json
index 863a619e63b..cdd779acc23 100644
--- a/package.json
+++ b/package.json
@@ -56,7 +56,7 @@
     "d3-time": "^1.0.8",
     "d3-time-format": "^2.1.1",
     "dateformat": "^3.0.3",
-    "deckar01-task_list": "fatihacet/task_list",
+    "deckar01-task_list": "^2.1.0",
     "diff": "^3.4.0",
     "document-register-element": "1.3.0",
     "dropzone": "^4.2.0",
diff --git a/yarn.lock b/yarn.lock
index 0e2d1725c11..2bd8858c77f 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -3032,9 +3032,10 @@ decamelize@^2.0.0:
   dependencies:
     xregexp "4.0.0"
 
-deckar01-task_list@fatihacet/task_list:
-  version "2.0.1"
-  resolved "https://codeload.github.com/fatihacet/task_list/tar.gz/844da3fa2830bcb159a70bf195d4693f9f22e04a"
+deckar01-task_list@^2.1.0:
+  version "2.1.0"
+  resolved "https://registry.yarnpkg.com/deckar01-task_list/-/deckar01-task_list-2.1.0.tgz#42198981614d3d2fd73e8f990b3389e00a4db1c0"
+  integrity sha512-aSwZunalq4ZnisaoW+T2CRsU9a8/UMyMLEWlmfMSK233c2HYSRc7iMF7ZgIacGw+ZbvVI38EhcAo22Fk0Tr7Pw==
 
 decode-uri-component@^0.2.0:
   version "0.2.0"

From 968e35a692b94ad0fe3bc8be03f64b5e8fec30c0 Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Wed, 23 Jan 2019 15:58:46 -0600
Subject: [PATCH 19/42] Specs for caching and issue controller changes

---
 app/controllers/concerns/issuable_actions.rb    |  4 +---
 .../projects/issues_controller_spec.rb          | 17 +++++++++++++++++
 .../concerns/cache_markdown_field_spec.rb       |  9 +++++++++
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb
index 1c55cd340ec..8ef3b6502df 100644
--- a/app/controllers/concerns/issuable_actions.rb
+++ b/app/controllers/concerns/issuable_actions.rb
@@ -162,12 +162,10 @@ module IssuableActions
       end
 
       format.json do
-        # We want to pass back the latest valid data, so reload the model
         render json: {
           errors: [
             "Someone edited this #{issuable.human_class_name} at the same time you did. Please refresh your browser and make sure your changes will not unintentionally remove theirs."
-          ],
-          data: serializer.represent(@issuable.reload) # rubocop:disable Gitlab/ModuleWithInstanceVariables
+          ]
         }, status: :conflict
       end
     end
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index c2afff6b732..26c88382acc 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -379,6 +379,23 @@ describe Projects::IssuesController do
         expect(response).to have_gitlab_http_status(200)
       end
     end
+
+    context 'when getting the changes' do
+      before do
+        project.add_developer(user)
+
+        sign_in(user)
+      end
+
+      it 'returns the necessary data' do
+        go(id: issue.iid)
+
+        data = JSON.parse(response.body)
+
+        expect(data).to include('title_text', 'description', 'description_text')
+        expect(data).to include('task_status', 'lock_version')
+      end
+    end
   end
 
   describe 'Confidential Issues' do
diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb
index ef6af232999..a8a5521a5fc 100644
--- a/spec/models/concerns/cache_markdown_field_spec.rb
+++ b/spec/models/concerns/cache_markdown_field_spec.rb
@@ -133,6 +133,15 @@ describe CacheMarkdownField do
     end
   end
 
+  context 'when a markdown field and html field are both set' do
+    it do
+      expect(thing).not_to receive(:refresh_markdown_cache)
+      thing.foo = '_look over there!_'
+      thing.foo_html = '<em>look over there!</em>'
+      thing.save
+    end
+  end
+
   context 'a non-markdown field changed' do
     shared_examples 'with cache version' do |cache_version|
       let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: cache_version) }

From edba6ee3ccdd8efcb60057d604fbf3b1a62e56ae Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Thu, 24 Jan 2019 00:44:15 +0100
Subject: [PATCH 20/42] Toggle individual task list group while requesting

---
 app/assets/javascripts/task_list.js | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/app/assets/javascripts/task_list.js b/app/assets/javascripts/task_list.js
index 9ea6cba7281..38c9e28e8b1 100644
--- a/app/assets/javascripts/task_list.js
+++ b/app/assets/javascripts/task_list.js
@@ -10,6 +10,7 @@ export default class TaskList {
     this.fieldName = options.fieldName;
     this.lockVersion = options.lockVersion;
     this.onSuccess = options.onSuccess || (() => {});
+    this.taskListContainerSelector = `${this.selector} .js-task-list-container`;
     this.onError =
       options.onError ||
       function showFlash(e) {
@@ -28,25 +29,31 @@ export default class TaskList {
   init() {
     // Prevent duplicate event bindings
     this.disable();
-    $(`${this.selector} .js-task-list-container`).taskList('enable');
+    $(this.taskListContainerSelector).taskList('enable');
     $(document).on(
       'tasklist:changed',
-      `${this.selector} .js-task-list-container`,
+      this.taskListContainerSelector,
       this.update.bind(this),
     );
   }
 
-  disableTaskListItems() {
-    $(`${this.selector} .js-task-list-container`).taskList('disable');
+  getTaskListTarget(e = {}) {
+    const $currentTarget = $(e.currentTarget);
+
+    return $currentTarget.taskList ? $currentTarget : $(this.taskListContainerSelector);
   }
 
-  enableTaskListItems() {
-    $(`${this.selector} .js-task-list-container`).taskList('enable');
+  disableTaskListItems(e) {
+    this.getTaskListTarget(e).taskList('disable');
+  }
+
+  enableTaskListItems(e) {
+    this.getTaskListTarget(e).taskList('enable');
   }
 
   disable() {
     this.disableTaskListItems();
-    $(document).off('tasklist:changed', `${this.selector} .js-task-list-container`);
+    $(document).off('tasklist:changed', this.taskListContainerSelector);
   }
 
   update(e) {
@@ -65,18 +72,18 @@ export default class TaskList {
       },
     };
 
-    this.disableTaskListItems();
+    this.disableTaskListItems(e);
 
     return axios
       .patch($target.data('updateUrl') || $('form.js-issuable-update').attr('action'), patchData)
       .then(({ data }) => {
         this.lockVersion = data.lock_version;
-        this.enableTaskListItems();
+        this.enableTaskListItems(e);
 
         return this.onSuccess(data);
       })
       .catch(({ response }) => {
-        this.enableTaskListItems();
+        this.enableTaskListItems(e);
 
         return this.onError(response.data);
       });

From e42d95fb9e412388eb3215dff674e05c73d65977 Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Thu, 24 Jan 2019 17:07:42 +0100
Subject: [PATCH 21/42] Bump up tasklist package to v2.2.0

v2.1.0 was published wrongly by the package author.
---
 Gemfile      | 2 +-
 Gemfile.lock | 4 ++--
 package.json | 2 +-
 yarn.lock    | 8 ++++----
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/Gemfile b/Gemfile
index 5a7c309d1b8..124ce7acc6d 100644
--- a/Gemfile
+++ b/Gemfile
@@ -112,7 +112,7 @@ gem 'seed-fu', '~> 2.3.7'
 
 # Markdown and HTML processing
 gem 'html-pipeline', '~> 2.8'
-gem 'deckar01-task_list', '2.1.0'
+gem 'deckar01-task_list', '2.2.0'
 gem 'gitlab-markup', '~> 1.6.5'
 gem 'github-markup', '~> 1.7.0', require: 'github/markup'
 gem 'redcarpet', '~> 3.4'
diff --git a/Gemfile.lock b/Gemfile.lock
index 812caca471a..aa21b41a74a 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -143,7 +143,7 @@ GEM
     database_cleaner (1.7.0)
     debug_inspector (0.0.3)
     debugger-ruby_core_source (1.3.8)
-    deckar01-task_list (2.1.0)
+    deckar01-task_list (2.2.0)
       html-pipeline
     declarative (0.0.10)
     declarative-option (0.1.0)
@@ -982,7 +982,7 @@ DEPENDENCIES
   connection_pool (~> 2.0)
   creole (~> 0.5.0)
   database_cleaner (~> 1.7.0)
-  deckar01-task_list (= 2.1.0)
+  deckar01-task_list (= 2.2.0)
   device_detector
   devise (~> 4.4)
   devise-two-factor (~> 3.0.0)
diff --git a/package.json b/package.json
index cdd779acc23..bd673b86fab 100644
--- a/package.json
+++ b/package.json
@@ -56,7 +56,7 @@
     "d3-time": "^1.0.8",
     "d3-time-format": "^2.1.1",
     "dateformat": "^3.0.3",
-    "deckar01-task_list": "^2.1.0",
+    "deckar01-task_list": "^2.2.0",
     "diff": "^3.4.0",
     "document-register-element": "1.3.0",
     "dropzone": "^4.2.0",
diff --git a/yarn.lock b/yarn.lock
index 2bd8858c77f..ad21820fe76 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -3032,10 +3032,10 @@ decamelize@^2.0.0:
   dependencies:
     xregexp "4.0.0"
 
-deckar01-task_list@^2.1.0:
-  version "2.1.0"
-  resolved "https://registry.yarnpkg.com/deckar01-task_list/-/deckar01-task_list-2.1.0.tgz#42198981614d3d2fd73e8f990b3389e00a4db1c0"
-  integrity sha512-aSwZunalq4ZnisaoW+T2CRsU9a8/UMyMLEWlmfMSK233c2HYSRc7iMF7ZgIacGw+ZbvVI38EhcAo22Fk0Tr7Pw==
+deckar01-task_list@^2.2.0:
+  version "2.2.0"
+  resolved "https://registry.yarnpkg.com/deckar01-task_list/-/deckar01-task_list-2.2.0.tgz#5cc3ea06f01d3d786b1a667064a462eb5d069bd3"
+  integrity sha512-NUfu5ARoD9SC2k+fBT5cBer59iKfEdawPrmfqp5+zAahTECb8z9dsuS1Xnx7jzFAmCCLnEs3z/aYucYXzNrKkQ==
 
 decode-uri-component@^0.2.0:
   version "0.2.0"

From b119fcb2e49289d26042e559d37fbfae58ac1536 Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Thu, 24 Jan 2019 16:13:35 -0600
Subject: [PATCH 22/42] Refactor and added specs

---
 app/models/concerns/taskable.rb               | 38 ---------
 app/services/issues/update_service.rb         | 16 ++--
 app/services/task_list_toggle_service.rb      | 60 ++++++++++++++
 .../services/task_list_toggle_service_spec.rb | 78 +++++++++++++++++++
 4 files changed, 146 insertions(+), 46 deletions(-)
 create mode 100644 app/services/task_list_toggle_service.rb
 create mode 100644 spec/services/task_list_toggle_service_spec.rb

diff --git a/app/models/concerns/taskable.rb b/app/models/concerns/taskable.rb
index 547afd5ee06..f147ce8ad6b 100644
--- a/app/models/concerns/taskable.rb
+++ b/app/models/concerns/taskable.rb
@@ -39,44 +39,6 @@ module Taskable
     end
   end
 
-  def self.toggle_task(content, content_html, index:, currently_checked:, line_source:, line_number:)
-    source_lines  = content.split("\n")
-    markdown_task = source_lines[line_number - 1]
-    output        = {}
-
-    if markdown_task == line_source
-      html          = Nokogiri::HTML.fragment(content_html)
-      html_checkbox = html.css('.task-list-item-checkbox')[index - 1]
-      # html_checkbox = html.css(".task-list-item[data-sourcepos^='#{changed_line_number}:'] > input.task-list-item-checkbox").first
-      updated_task  = toggle_task_source(line_source, currently_checked: currently_checked)
-
-      if html_checkbox && updated_task
-        source_lines[line_number - 1] = updated_task
-
-        if currently_checked
-          html_checkbox.remove_attribute('checked')
-        else
-          html_checkbox[:checked] = 'checked'
-        end
-
-        output[:content]      = source_lines.join("\n")
-        output[:content_html] = html.to_html
-      end
-    end
-
-    output
-  end
-
-  def self.toggle_task_source(markdown_line, currently_checked:)
-    if source_checkbox = ITEM_PATTERN.match(markdown_line)
-      if TaskList::Item.new(source_checkbox[1]).complete?
-        markdown_line.sub(COMPLETE_PATTERN, '[ ]') if currently_checked
-      else
-        markdown_line.sub(INCOMPLETE_PATTERN, '[x]') unless currently_checked
-      end
-    end
-  end
-
   # Called by `TaskList::Summary`
   def task_list_items
     return [] if description.blank?
diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb
index cf6173da8d4..9c7b8fcc6ab 100644
--- a/app/services/issues/update_service.rb
+++ b/app/services/issues/update_service.rb
@@ -129,17 +129,17 @@ module Issues
       update_task_params = params.delete(:update_task)
       return unless update_task_params
 
-      updated_content = Taskable.toggle_task(issue.description, issue.description_html,
-                                             index: update_task_params[:index],
-                                             currently_checked: !update_task_params[:checked],
-                                             line_source: update_task_params[:line_source],
-                                             line_number: update_task_params[:line_number])
+      toggler = TaskListToggleService.new(issue.description, issue.description_html,
+                                          index: update_task_params[:index],
+                                          currently_checked: !update_task_params[:checked],
+                                          line_source: update_task_params[:line_source],
+                                          line_number: update_task_params[:line_number])
 
-      unless updated_content.empty?
+      if toggler.execute
         # by updating the description_html field at the same time,
         # the markdown cache won't be considered invalid
-        params[:description]      = updated_content[:content]
-        params[:description_html] = updated_content[:content_html]
+        params[:description]      = toggler.updated_markdown
+        params[:description_html] = toggler.updated_markdown_html
 
         # since we're updating a very specific line, we don't care whether
         # the `lock_version` sent from the FE is the same or not.  Just
diff --git a/app/services/task_list_toggle_service.rb b/app/services/task_list_toggle_service.rb
new file mode 100644
index 00000000000..84a875dbaf7
--- /dev/null
+++ b/app/services/task_list_toggle_service.rb
@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+# Finds the correct checkbox in the passed in markdown/html and toggles it's state,
+# returning the updated markdown/html
+# We don't care if the text has changed above or below the specific checkbox, as long
+# the checkbox still exists at exactly the same line number and the text is equal
+# If successful, new values are available in `updated_markdown` and `updated_markdown_html`
+class TaskListToggleService
+  attr_reader :updated_markdown, :updated_markdown_html
+
+  def initialize(markdown, markdown_html, index:, currently_checked:, line_source:, line_number:)
+    @markdown, @markdown_html  = markdown, markdown_html
+    @index, @currently_checked = index, currently_checked
+    @line_source, @line_number = line_source, line_number
+
+    @updated_markdown, @updated_markdown_html = nil
+  end
+
+  def execute
+    return false unless markdown && markdown_html
+
+    !!(toggle_markdown && toggle_html)
+  end
+
+  private
+
+  attr_reader :markdown, :markdown_html, :index, :currently_checked, :line_source, :line_number
+
+  def toggle_markdown
+    source_lines  = markdown.split("\n")
+    markdown_task = source_lines[line_number - 1]
+
+    return unless markdown_task == line_source
+    return unless source_checkbox = Taskable::ITEM_PATTERN.match(markdown_task)
+
+    if TaskList::Item.new(source_checkbox[1]).complete?
+      markdown_task.sub!(Taskable::COMPLETE_PATTERN, '[ ]') if currently_checked
+    else
+      markdown_task.sub!(Taskable::INCOMPLETE_PATTERN, '[x]') unless currently_checked
+    end
+
+    source_lines[line_number - 1] = markdown_task
+    @updated_markdown = source_lines.join("\n")
+  end
+
+  def toggle_html
+    html          = Nokogiri::HTML.fragment(markdown_html)
+    html_checkbox = html.css('.task-list-item-checkbox')[index - 1]
+    # html_checkbox = html.css(".task-list-item[data-sourcepos^='#{changed_line_number}:'] > input.task-list-item-checkbox").first
+    return unless html_checkbox
+
+    if currently_checked
+      html_checkbox.remove_attribute('checked')
+    else
+      html_checkbox[:checked] = 'checked'
+    end
+
+    @updated_markdown_html = html.to_html
+  end
+end
diff --git a/spec/services/task_list_toggle_service_spec.rb b/spec/services/task_list_toggle_service_spec.rb
new file mode 100644
index 00000000000..abc27012452
--- /dev/null
+++ b/spec/services/task_list_toggle_service_spec.rb
@@ -0,0 +1,78 @@
+require 'spec_helper'
+
+describe TaskListToggleService do
+  context 'when ' do
+    let(:markdown) { <<-EOT.strip_heredoc
+        * [ ] Task 1
+        * [x] Task 2
+
+        A paragraph
+
+        1. [X] Item 1
+           - [ ] Sub-item 1
+      EOT
+    }
+
+    let(:markdown_html) { <<-EOT.strip_heredoc
+      <ul class="task-list" dir="auto">
+        <li class="task-list-item">
+          <input type="checkbox" class="task-list-item-checkbox" disabled> Task 1
+        </li>
+        <li class="task-list-item">
+          <input type="checkbox" class="task-list-item-checkbox" disabled checked> Task 2
+        </li>
+      </ul>
+      <p dir="auto">A paragraph</p>
+      <ol class="task-list" dir="auto">
+        <li class="task-list-item">
+          <input type="checkbox" class="task-list-item-checkbox" disabled checked> Item 1
+          <ul class="task-list">
+            <li class="task-list-item">
+              <input type="checkbox" class="task-list-item-checkbox" disabled> Sub-item 1
+            </li>
+          </ul>
+        </li>
+      </ol>
+      EOT
+    }
+
+    it 'checks Task 1' do
+      toggler = described_class.new(markdown, markdown_html, index: 1, currently_checked: false,
+                                    line_source: '* [ ] Task 1', line_number: 1)
+
+      expect(toggler.execute).to be_truthy
+      expect(toggler.updated_markdown.lines[0]).to eq "* [x] Task 1\n"
+      expect(toggler.updated_markdown_html).to include('disabled checked> Task 1')
+    end
+
+    it 'unchecks Item 1' do
+      toggler = described_class.new(markdown, markdown_html, index: 3, currently_checked: true,
+                                    line_source: '1. [X] Item 1', line_number: 6)
+
+      expect(toggler.execute).to be_truthy
+      expect(toggler.updated_markdown.lines[5]).to eq "1. [ ] Item 1\n"
+      expect(toggler.updated_markdown_html).to include('disabled> Item 1')
+    end
+
+    it 'returns false if line_source does not match the text' do
+      toggler = described_class.new(markdown, markdown_html, index: 2, currently_checked: true,
+                                    line_source: '* [x] Task Added', line_number: 2)
+
+      expect(toggler.execute).to be_falsey
+    end
+
+    it 'returns false if markdown is nil' do
+      toggler = described_class.new(nil, markdown_html, index: 2, currently_checked: true,
+                                    line_source: '* [x] Task Added', line_number: 2)
+
+      expect(toggler.execute).to be_falsey
+    end
+
+    it 'returns false if markdown_html is nil' do
+      toggler = described_class.new(markdown, nil, index: 2, currently_checked: true,
+                                    line_source: '* [x] Task Added', line_number: 2)
+
+      expect(toggler.execute).to be_falsey
+    end
+  end
+end

From 56506ff82f51efc6c05100ddceb5ea37b04343ad Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Fri, 25 Jan 2019 00:31:14 +0100
Subject: [PATCH 23/42] Send lockVersion from MR description task list

---
 app/assets/javascripts/merge_request.js          | 1 +
 app/assets/javascripts/task_list.js              | 6 +-----
 app/serializers/merge_request_basic_entity.rb    | 1 +
 app/views/projects/merge_requests/show.html.haml | 2 +-
 4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/app/assets/javascripts/merge_request.js b/app/assets/javascripts/merge_request.js
index 0deae478deb..ac3b47cd218 100644
--- a/app/assets/javascripts/merge_request.js
+++ b/app/assets/javascripts/merge_request.js
@@ -35,6 +35,7 @@ function MergeRequest(opts) {
       dataType: 'merge_request',
       fieldName: 'description',
       selector: '.detail-page-description',
+      lockVersion: this.$el.data('lockVersion'),
       onSuccess: result => {
         document.querySelector('#task_status').innerText = result.task_status;
         document.querySelector('#task_status_short').innerText = result.task_status_short;
diff --git a/app/assets/javascripts/task_list.js b/app/assets/javascripts/task_list.js
index 38c9e28e8b1..c3932db7be2 100644
--- a/app/assets/javascripts/task_list.js
+++ b/app/assets/javascripts/task_list.js
@@ -30,11 +30,7 @@ export default class TaskList {
     // Prevent duplicate event bindings
     this.disable();
     $(this.taskListContainerSelector).taskList('enable');
-    $(document).on(
-      'tasklist:changed',
-      this.taskListContainerSelector,
-      this.update.bind(this),
-    );
+    $(document).on('tasklist:changed', this.taskListContainerSelector, this.update.bind(this));
   }
 
   getTaskListTarget(e = {}) {
diff --git a/app/serializers/merge_request_basic_entity.rb b/app/serializers/merge_request_basic_entity.rb
index 084627f9dbe..178e72f4f0a 100644
--- a/app/serializers/merge_request_basic_entity.rb
+++ b/app/serializers/merge_request_basic_entity.rb
@@ -11,4 +11,5 @@ class MergeRequestBasicEntity < Grape::Entity
   expose :labels, using: LabelEntity
   expose :assignee, using: API::Entities::UserBasic
   expose :task_status, :task_status_short
+  expose :lock_version, :lock_version
 end
diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml
index d6f340d0ee2..0b720e5d542 100644
--- a/app/views/projects/merge_requests/show.html.haml
+++ b/app/views/projects/merge_requests/show.html.haml
@@ -7,7 +7,7 @@
 - page_card_attributes @merge_request.card_attributes
 - suggest_changes_help_path = help_page_path('user/discussions/index.md', anchor: 'suggest-changes')
 
-.merge-request{ data: { mr_action: j(params[:tab].presence || 'show'), url: merge_request_path(@merge_request, format: :json), project_path: project_path(@merge_request.project) } }
+.merge-request{ data: { mr_action: j(params[:tab].presence || 'show'), url: merge_request_path(@merge_request, format: :json), project_path: project_path(@merge_request.project), lock_version: @merge_request.lock_version } }
   = render "projects/merge_requests/mr_title"
 
   .merge-request-details.issuable-details{ data: { id: @merge_request.project.id } }

From ec66cf0a17060f8417eb261ef3770db558c35373 Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Thu, 24 Jan 2019 17:48:09 -0600
Subject: [PATCH 24/42] Raise exception if we can't match the update_task

and some additional refactoring
---
 app/services/issuable_base_service.rb         | 31 +++++++++
 app/services/issues/update_service.rb         | 27 +-------
 .../services/task_list_toggle_service_spec.rb | 63 ++++++++++---------
 3 files changed, 67 insertions(+), 54 deletions(-)

diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index 90f971faedb..dc929aa3b28 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -238,6 +238,7 @@ class IssuableBaseService < BaseService
   def update_task(issuable)
     filter_params(issuable)
     # old_associations = associations_before_update(issuable)
+
     if issuable.changed? || params.present?
       issuable.assign_attributes(params.merge(updated_by: current_user))
       issuable.assign_attributes(last_edited_at: Time.now, last_edited_by: current_user)
@@ -263,6 +264,36 @@ class IssuableBaseService < BaseService
     issuable
   end
 
+  # Handle the `update_task` event sent from UI.  Attempts to update a specific
+  # line in the markdown and cached html, bypassing any unnecessary updates or checks.
+  def update_task_event(issue)
+    update_task_params = params.delete(:update_task)
+    return unless update_task_params
+
+    toggler = TaskListToggleService.new(issue.description, issue.description_html,
+                                        index: update_task_params[:index],
+                                        currently_checked: !update_task_params[:checked],
+                                        line_source: update_task_params[:line_source],
+                                        line_number: update_task_params[:line_number])
+
+    if toggler.execute
+      # by updating the description_html field at the same time,
+      # the markdown cache won't be considered invalid
+      params[:description]      = toggler.updated_markdown
+      params[:description_html] = toggler.updated_markdown_html
+
+      # since we're updating a very specific line, we don't care whether
+      # the `lock_version` sent from the FE is the same or not.  Just
+      # make sure the data hasn't changed since we queried it
+      params[:lock_version]     = issue.lock_version
+
+      update_task(issue)
+    else
+      # if we make it here, the data is much newer than we thought it was - fail fast
+      raise ActiveRecord::StaleObjectError
+    end
+  end
+
   def labels_changing?(old_label_ids, new_label_ids)
     old_label_ids.sort != new_label_ids.sort
   end
diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb
index 9c7b8fcc6ab..cec5b5734c0 100644
--- a/app/services/issues/update_service.rb
+++ b/app/services/issues/update_service.rb
@@ -8,7 +8,7 @@ module Issues
       handle_move_between_ids(issue)
       filter_spam_check_params
       change_issue_duplicate(issue)
-      move_issue_to_new_project(issue) || task_change_event(issue) || update(issue)
+      move_issue_to_new_project(issue) || update_task_event(issue) || update(issue)
     end
 
     def update(issue)
@@ -125,31 +125,6 @@ module Issues
       end
     end
 
-    def task_change_event(issue)
-      update_task_params = params.delete(:update_task)
-      return unless update_task_params
-
-      toggler = TaskListToggleService.new(issue.description, issue.description_html,
-                                          index: update_task_params[:index],
-                                          currently_checked: !update_task_params[:checked],
-                                          line_source: update_task_params[:line_source],
-                                          line_number: update_task_params[:line_number])
-
-      if toggler.execute
-        # by updating the description_html field at the same time,
-        # the markdown cache won't be considered invalid
-        params[:description]      = toggler.updated_markdown
-        params[:description_html] = toggler.updated_markdown_html
-
-        # since we're updating a very specific line, we don't care whether
-        # the `lock_version` sent from the FE is the same or not.  Just
-        # make sure the data hasn't changed since we queried it
-        params[:lock_version]     = issue.lock_version
-
-        update_task(issue)
-      end
-    end
-
     # rubocop: disable CodeReuse/ActiveRecord
     def get_issue_if_allowed(id, board_group_id = nil)
       return unless id
diff --git a/spec/services/task_list_toggle_service_spec.rb b/spec/services/task_list_toggle_service_spec.rb
index abc27012452..facd8c193aa 100644
--- a/spec/services/task_list_toggle_service_spec.rb
+++ b/spec/services/task_list_toggle_service_spec.rb
@@ -2,7 +2,8 @@ require 'spec_helper'
 
 describe TaskListToggleService do
   context 'when ' do
-    let(:markdown) { <<-EOT.strip_heredoc
+    let(:markdown) do
+      <<-EOT.strip_heredoc
         * [ ] Task 1
         * [x] Task 2
 
@@ -11,33 +12,35 @@ describe TaskListToggleService do
         1. [X] Item 1
            - [ ] Sub-item 1
       EOT
-    }
+    end
 
-    let(:markdown_html) { <<-EOT.strip_heredoc
-      <ul class="task-list" dir="auto">
-        <li class="task-list-item">
-          <input type="checkbox" class="task-list-item-checkbox" disabled> Task 1
-        </li>
-        <li class="task-list-item">
-          <input type="checkbox" class="task-list-item-checkbox" disabled checked> Task 2
-        </li>
-      </ul>
-      <p dir="auto">A paragraph</p>
-      <ol class="task-list" dir="auto">
-        <li class="task-list-item">
-          <input type="checkbox" class="task-list-item-checkbox" disabled checked> Item 1
-          <ul class="task-list">
-            <li class="task-list-item">
-              <input type="checkbox" class="task-list-item-checkbox" disabled> Sub-item 1
-            </li>
-          </ul>
-        </li>
-      </ol>
+    let(:markdown_html) do
+      <<-EOT.strip_heredoc
+        <ul class="task-list" dir="auto">
+          <li class="task-list-item">
+            <input type="checkbox" class="task-list-item-checkbox" disabled> Task 1
+          </li>
+          <li class="task-list-item">
+            <input type="checkbox" class="task-list-item-checkbox" disabled checked> Task 2
+          </li>
+        </ul>
+        <p dir="auto">A paragraph</p>
+        <ol class="task-list" dir="auto">
+          <li class="task-list-item">
+            <input type="checkbox" class="task-list-item-checkbox" disabled checked> Item 1
+            <ul class="task-list">
+              <li class="task-list-item">
+                <input type="checkbox" class="task-list-item-checkbox" disabled> Sub-item 1
+              </li>
+            </ul>
+          </li>
+        </ol>
       EOT
-    }
+    end
 
     it 'checks Task 1' do
-      toggler = described_class.new(markdown, markdown_html, index: 1, currently_checked: false,
+      toggler = described_class.new(markdown, markdown_html,
+                                    index: 1, currently_checked: false,
                                     line_source: '* [ ] Task 1', line_number: 1)
 
       expect(toggler.execute).to be_truthy
@@ -46,7 +49,8 @@ describe TaskListToggleService do
     end
 
     it 'unchecks Item 1' do
-      toggler = described_class.new(markdown, markdown_html, index: 3, currently_checked: true,
+      toggler = described_class.new(markdown, markdown_html,
+                                    index: 3, currently_checked: true,
                                     line_source: '1. [X] Item 1', line_number: 6)
 
       expect(toggler.execute).to be_truthy
@@ -55,21 +59,24 @@ describe TaskListToggleService do
     end
 
     it 'returns false if line_source does not match the text' do
-      toggler = described_class.new(markdown, markdown_html, index: 2, currently_checked: true,
+      toggler = described_class.new(markdown, markdown_html,
+                                    index: 2, currently_checked: true,
                                     line_source: '* [x] Task Added', line_number: 2)
 
       expect(toggler.execute).to be_falsey
     end
 
     it 'returns false if markdown is nil' do
-      toggler = described_class.new(nil, markdown_html, index: 2, currently_checked: true,
+      toggler = described_class.new(nil, markdown_html,
+                                    index: 2, currently_checked: true,
                                     line_source: '* [x] Task Added', line_number: 2)
 
       expect(toggler.execute).to be_falsey
     end
 
     it 'returns false if markdown_html is nil' do
-      toggler = described_class.new(markdown, nil, index: 2, currently_checked: true,
+      toggler = described_class.new(markdown, nil,
+                                    index: 2, currently_checked: true,
                                     line_source: '* [x] Task Added', line_number: 2)
 
       expect(toggler.execute).to be_falsey

From 549821a59f06d8840a6c2d7c500e8d193f6102e1 Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Fri, 25 Jan 2019 09:41:21 -0600
Subject: [PATCH 25/42] Add back call to execute_hooks

---
 app/models/concerns/issuable.rb       | 33 +++++++++++++++------------
 app/services/issuable_base_service.rb |  6 +----
 2 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 0d363ec68b7..b1cf03551f6 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -270,26 +270,29 @@ module Issuable
 
   def to_hook_data(user, old_associations: {})
     changes = previous_changes
-    old_labels = old_associations.fetch(:labels, [])
-    old_assignees = old_associations.fetch(:assignees, [])
 
-    if old_labels != labels
-      changes[:labels] = [old_labels.map(&:hook_attrs), labels.map(&:hook_attrs)]
-    end
+    if old_associations
+      old_labels = old_associations.fetch(:labels, [])
+      old_assignees = old_associations.fetch(:assignees, [])
 
-    if old_assignees != assignees
-      if self.is_a?(Issue)
-        changes[:assignees] = [old_assignees.map(&:hook_attrs), assignees.map(&:hook_attrs)]
-      else
-        changes[:assignee] = [old_assignees&.first&.hook_attrs, assignee&.hook_attrs]
+      if old_labels != labels
+        changes[:labels] = [old_labels.map(&:hook_attrs), labels.map(&:hook_attrs)]
       end
-    end
 
-    if self.respond_to?(:total_time_spent)
-      old_total_time_spent = old_associations.fetch(:total_time_spent, nil)
+      if old_assignees != assignees
+        if self.is_a?(Issue)
+          changes[:assignees] = [old_assignees.map(&:hook_attrs), assignees.map(&:hook_attrs)]
+        else
+          changes[:assignee] = [old_assignees&.first&.hook_attrs, assignee&.hook_attrs]
+        end
+      end
 
-      if old_total_time_spent != total_time_spent
-        changes[:total_time_spent] = [old_total_time_spent, total_time_spent]
+      if self.respond_to?(:total_time_spent)
+        old_total_time_spent = old_associations.fetch(:total_time_spent, nil)
+
+        if old_total_time_spent != total_time_spent
+          changes[:total_time_spent] = [old_total_time_spent, total_time_spent]
+        end
       end
     end
 
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index dc929aa3b28..cab5dd0e28d 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -237,7 +237,6 @@ class IssuableBaseService < BaseService
 
   def update_task(issuable)
     filter_params(issuable)
-    # old_associations = associations_before_update(issuable)
 
     if issuable.changed? || params.present?
       issuable.assign_attributes(params.merge(updated_by: current_user))
@@ -252,12 +251,9 @@ class IssuableBaseService < BaseService
         end
 
         handle_task_changes(issuable)
-
         invalidate_cache_counts(issuable, users: issuable.assignees.to_a)
-
         after_update(issuable)
-
-        # execute_hooks(issuable, 'update', old_associations: old_associations)
+        execute_hooks(issuable, 'update', old_associations: nil)
       end
     end
 

From 2dc304855b7c58fda1773be9774b5d916aa0fe73 Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Fri, 25 Jan 2019 09:41:40 -0600
Subject: [PATCH 26/42] Fix for broken spec

---
 spec/fixtures/api/schemas/entities/merge_request_basic.json | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/spec/fixtures/api/schemas/entities/merge_request_basic.json b/spec/fixtures/api/schemas/entities/merge_request_basic.json
index 4c04c838cb8..3006b482d41 100644
--- a/spec/fixtures/api/schemas/entities/merge_request_basic.json
+++ b/spec/fixtures/api/schemas/entities/merge_request_basic.json
@@ -22,7 +22,8 @@
       "type": [ "array", "null" ]
     },
     "task_status": { "type": "string" },
-    "task_status_short": { "type": "string" }
+    "task_status_short": { "type": "string" },
+    "lock_version": { "type": ["string", "null"] }
   },
   "additionalProperties": false
 }

From dfeb412d9f09053a601075b04cca674787c6c275 Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Fri, 25 Jan 2019 12:50:21 -0600
Subject: [PATCH 27/42] Specs for Issue::UpdateService

---
 app/services/issuable_base_service.rb       |  7 ++-
 app/services/task_list_toggle_service.rb    | 26 +++++---
 spec/services/issues/update_service_spec.rb | 70 +++++++++++++++++++++
 3 files changed, 93 insertions(+), 10 deletions(-)

diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index cab5dd0e28d..5719a4778f0 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -267,10 +267,11 @@ class IssuableBaseService < BaseService
     return unless update_task_params
 
     toggler = TaskListToggleService.new(issue.description, issue.description_html,
-                                        index: update_task_params[:index],
-                                        currently_checked: !update_task_params[:checked],
                                         line_source: update_task_params[:line_source],
-                                        line_number: update_task_params[:line_number])
+                                        line_number: update_task_params[:line_number],
+                                        currently_checked: !update_task_params[:checked],
+                                        index: update_task_params[:index],
+                                        sourcepos: false)
 
     if toggler.execute
       # by updating the description_html field at the same time,
diff --git a/app/services/task_list_toggle_service.rb b/app/services/task_list_toggle_service.rb
index 84a875dbaf7..1b0a5965235 100644
--- a/app/services/task_list_toggle_service.rb
+++ b/app/services/task_list_toggle_service.rb
@@ -1,17 +1,21 @@
 # frozen_string_literal: true
 
 # Finds the correct checkbox in the passed in markdown/html and toggles it's state,
-# returning the updated markdown/html
+# returning the updated markdown/html.
 # We don't care if the text has changed above or below the specific checkbox, as long
-# the checkbox still exists at exactly the same line number and the text is equal
+# the checkbox still exists at exactly the same line number and the text is equal.
 # If successful, new values are available in `updated_markdown` and `updated_markdown_html`
+#
+# Note: once we've removed RedCarpet support, we can remove the `index` and `sourcepos`
+# parameters
 class TaskListToggleService
   attr_reader :updated_markdown, :updated_markdown_html
 
-  def initialize(markdown, markdown_html, index:, currently_checked:, line_source:, line_number:)
+  def initialize(markdown, markdown_html, line_source:, line_number:, currently_checked:, index:, sourcepos: false)
     @markdown, @markdown_html  = markdown, markdown_html
-    @index, @currently_checked = index, currently_checked
     @line_source, @line_number = line_source, line_number
+    @currently_checked         = currently_checked
+    @index, @use_sourcepos     = index, sourcepos
 
     @updated_markdown, @updated_markdown_html = nil
   end
@@ -24,7 +28,8 @@ class TaskListToggleService
 
   private
 
-  attr_reader :markdown, :markdown_html, :index, :currently_checked, :line_source, :line_number
+  attr_reader :markdown, :markdown_html, :index, :currently_checked
+  attr_reader :line_source, :line_number, :use_sourcepos
 
   def toggle_markdown
     source_lines  = markdown.split("\n")
@@ -45,8 +50,7 @@ class TaskListToggleService
 
   def toggle_html
     html          = Nokogiri::HTML.fragment(markdown_html)
-    html_checkbox = html.css('.task-list-item-checkbox')[index - 1]
-    # html_checkbox = html.css(".task-list-item[data-sourcepos^='#{changed_line_number}:'] > input.task-list-item-checkbox").first
+    html_checkbox = get_html_checkbox(html)
     return unless html_checkbox
 
     if currently_checked
@@ -57,4 +61,12 @@ class TaskListToggleService
 
     @updated_markdown_html = html.to_html
   end
+
+  def get_html_checkbox(html)
+    if use_sourcepos
+      html.css(".task-list-item[data-sourcepos^='#{line_number}:'] > input.task-list-item-checkbox").first
+    else
+      html.css('.task-list-item-checkbox')[index - 1]
+    end
+  end
 end
diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index ce20bf2bef6..26dc0919715 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -543,6 +543,76 @@ describe Issues::UpdateService, :mailer do
       end
     end
 
+    context 'when updating a single task' do
+      before do
+        update_issue(description: "- [ ] Task 1\n- [ ] Task 2")
+      end
+
+      it { expect(issue.tasks?).to eq(true) }
+
+      context 'when a task is marked as completed' do
+        before do
+          update_issue(update_task: { index: 1, checked: true, line_source: '- [ ] Task 1', line_number: 1})
+        end
+
+        it 'creates system note about task status change' do
+          note1 = find_note('marked the task **Task 1** as completed')
+
+          expect(note1).not_to be_nil
+
+          description_notes = find_notes('description')
+          expect(description_notes.length).to eq(1)
+        end
+      end
+
+      context 'when a task is marked as incomplete' do
+        before do
+          update_issue(description: "- [x] Task 1\n- [X] Task 2")
+          update_issue(update_task: { index: 2, checked: false, line_source: '- [X] Task 2', line_number: 2})
+        end
+
+        it 'creates system note about task status change' do
+          note1 = find_note('marked the task **Task 2** as incomplete')
+
+          expect(note1).not_to be_nil
+
+          description_notes = find_notes('description')
+          expect(description_notes.length).to eq(1)
+        end
+      end
+
+      context 'when the task position has been modified' do
+        before do
+          update_issue(description: "- [ ] Task 1\n- [ ] Task 3\n- [ ] Task 2")
+        end
+
+        it 'raises an exception' do
+          expect(Note.count).to eq(2)
+          expect do
+            update_issue(update_task: { index: 2, checked: true, line_source: '- [ ] Task 2', line_number: 2})
+          end.to raise_error(ActiveRecord::StaleObjectError)
+          expect(Note.count).to eq(2)
+        end
+      end
+
+      context 'when the content changes but not task line number' do
+        before do
+          update_issue(description: "Paragraph\n\n- [ ] Task 1\n- [x] Task 2")
+          update_issue(description: "Paragraph with more words\n\n- [ ] Task 1\n- [x] Task 2")
+          update_issue(update_task: { index: 2, checked: false, line_source: '- [x] Task 2', line_number: 4})
+        end
+
+        it 'creates system note about task status change' do
+          note1 = find_note('marked the task **Task 2** as incomplete')
+
+          expect(note1).not_to be_nil
+
+          description_notes = find_notes('description')
+          expect(description_notes.length).to eq(2)
+        end
+      end
+    end
+
     context 'updating labels' do
       let(:label3) { create(:label, project: project) }
       let(:result) { described_class.new(project, user, params).execute(issue).reload }

From 4e40d72acff087b80b8919c2d96707807a43ce4b Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Fri, 25 Jan 2019 14:16:51 -0600
Subject: [PATCH 28/42] Added changelog and danger fixes

---
 ...sts-can-be-overwritten-when-editing-simultaneously.yml | 5 +++++
 spec/services/issues/update_service_spec.rb               | 8 ++++----
 spec/services/task_list_toggle_service_spec.rb            | 2 ++
 3 files changed, 11 insertions(+), 4 deletions(-)
 create mode 100644 changelogs/unreleased/19745-forms-with-task-lists-can-be-overwritten-when-editing-simultaneously.yml

diff --git a/changelogs/unreleased/19745-forms-with-task-lists-can-be-overwritten-when-editing-simultaneously.yml b/changelogs/unreleased/19745-forms-with-task-lists-can-be-overwritten-when-editing-simultaneously.yml
new file mode 100644
index 00000000000..7b6a38c1d57
--- /dev/null
+++ b/changelogs/unreleased/19745-forms-with-task-lists-can-be-overwritten-when-editing-simultaneously.yml
@@ -0,0 +1,5 @@
+---
+title: Increase reliability and performance of clicking task items
+merge_request: 23938
+author:
+type: fixed
diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index 26dc0919715..ef76e2311b1 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -552,7 +552,7 @@ describe Issues::UpdateService, :mailer do
 
       context 'when a task is marked as completed' do
         before do
-          update_issue(update_task: { index: 1, checked: true, line_source: '- [ ] Task 1', line_number: 1})
+          update_issue(update_task: { index: 1, checked: true, line_source: '- [ ] Task 1', line_number: 1 })
         end
 
         it 'creates system note about task status change' do
@@ -568,7 +568,7 @@ describe Issues::UpdateService, :mailer do
       context 'when a task is marked as incomplete' do
         before do
           update_issue(description: "- [x] Task 1\n- [X] Task 2")
-          update_issue(update_task: { index: 2, checked: false, line_source: '- [X] Task 2', line_number: 2})
+          update_issue(update_task: { index: 2, checked: false, line_source: '- [X] Task 2', line_number: 2 })
         end
 
         it 'creates system note about task status change' do
@@ -589,7 +589,7 @@ describe Issues::UpdateService, :mailer do
         it 'raises an exception' do
           expect(Note.count).to eq(2)
           expect do
-            update_issue(update_task: { index: 2, checked: true, line_source: '- [ ] Task 2', line_number: 2})
+            update_issue(update_task: { index: 2, checked: true, line_source: '- [ ] Task 2', line_number: 2 })
           end.to raise_error(ActiveRecord::StaleObjectError)
           expect(Note.count).to eq(2)
         end
@@ -599,7 +599,7 @@ describe Issues::UpdateService, :mailer do
         before do
           update_issue(description: "Paragraph\n\n- [ ] Task 1\n- [x] Task 2")
           update_issue(description: "Paragraph with more words\n\n- [ ] Task 1\n- [x] Task 2")
-          update_issue(update_task: { index: 2, checked: false, line_source: '- [x] Task 2', line_number: 4})
+          update_issue(update_task: { index: 2, checked: false, line_source: '- [x] Task 2', line_number: 4 })
         end
 
         it 'creates system note about task status change' do
diff --git a/spec/services/task_list_toggle_service_spec.rb b/spec/services/task_list_toggle_service_spec.rb
index facd8c193aa..8ae0e209cda 100644
--- a/spec/services/task_list_toggle_service_spec.rb
+++ b/spec/services/task_list_toggle_service_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe TaskListToggleService do

From f00c6db83f1f8b0bbb35230cfede0cfddfc9f7da Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Sat, 26 Jan 2019 00:56:53 +0100
Subject: [PATCH 29/42] Add new spec for updateStoreState method

---
 .../javascripts/issue_show/components/app.vue  | 10 +++-------
 .../issue_show/components/app_spec.js          | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue
index 72398cb547a..c2c80218a87 100644
--- a/app/assets/javascripts/issue_show/components/app.vue
+++ b/app/assets/javascripts/issue_show/components/app.vue
@@ -212,9 +212,8 @@ export default {
       }
       return undefined;
     },
-
     updateStoreState() {
-      this.service
+      return this.service
         .getData()
         .then(res => res.data)
         .then(data => {
@@ -252,12 +251,9 @@ export default {
           if (window.location.pathname !== data.web_url) {
             visitUrl(data.web_url);
           }
-
-          return this.service.getData();
         })
-        .then(res => res.data)
-        .then(data => {
-          this.store.updateState(data);
+        .then(this.updateStoreState)
+        .then(() => {
           eventHub.$emit('close.form');
         })
         .catch(error => {
diff --git a/spec/javascripts/issue_show/components/app_spec.js b/spec/javascripts/issue_show/components/app_spec.js
index 863d15570a0..c3f624aebd3 100644
--- a/spec/javascripts/issue_show/components/app_spec.js
+++ b/spec/javascripts/issue_show/components/app_spec.js
@@ -140,6 +140,7 @@ describe('Issuable output', () => {
 
   describe('updateIssuable', () => {
     it('fetches new data after update', done => {
+      spyOn(vm, 'updateStoreState').and.callThrough();
       spyOn(vm.service, 'getData').and.callThrough();
       spyOn(vm.service, 'updateIssuable').and.callFake(
         () =>
@@ -155,6 +156,7 @@ describe('Issuable output', () => {
 
       vm.updateIssuable()
         .then(() => {
+          expect(vm.updateStoreState).toHaveBeenCalled();
           expect(vm.service.getData).toHaveBeenCalled();
         })
         .then(done)
@@ -452,4 +454,20 @@ describe('Issuable output', () => {
       expect(vm.$el.querySelector('.title-container .note-action-button')).toBeDefined();
     });
   });
+
+  describe('updateStoreState', () => {
+    it('should make a request and update the state of the store', done => {
+      const data = { foo: 1 };
+      spyOn(vm.store, 'updateState');
+      spyOn(vm.service, 'getData').and.returnValue(Promise.resolve({ data }));
+
+      vm.updateStoreState()
+        .then(() => {
+          expect(vm.service.getData).toHaveBeenCalled();
+          expect(vm.store.updateState).toHaveBeenCalledWith(data);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+  });
 });

From 992fcf6c87721405be59d2a22da7d21157e17f45 Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Sat, 26 Jan 2019 15:49:49 +0100
Subject: [PATCH 30/42] Add missing specs.

---
 .../issue_show/components/description.vue     |   8 +-
 app/assets/javascripts/task_list.js           |   9 +-
 locale/gitlab.pot                             |   3 +
 .../issue_show/components/app_spec.js         |  13 ++
 .../issue_show/components/description_spec.js |  14 ++
 spec/javascripts/task_list_spec.js            | 156 ++++++++++++++++++
 6 files changed, 196 insertions(+), 7 deletions(-)
 create mode 100644 spec/javascripts/task_list_spec.js

diff --git a/app/assets/javascripts/issue_show/components/description.vue b/app/assets/javascripts/issue_show/components/description.vue
index 56e873c6ba0..f42787a7147 100644
--- a/app/assets/javascripts/issue_show/components/description.vue
+++ b/app/assets/javascripts/issue_show/components/description.vue
@@ -1,6 +1,6 @@
 <script>
 import $ from 'jquery';
-import createFlash from '~/flash';
+import { __ } from '~/locale';
 import animateMixin from '../mixins/animate';
 import TaskList from '../../task_list';
 import recaptchaModalImplementor from '../../vue_shared/mixins/recaptcha_modal_implementor';
@@ -91,8 +91,10 @@ export default {
     },
 
     taskListUpdateError() {
-      createFlash(
-        'Someone edited this issue at the same time you did and we updated the issue description.',
+      window.Flash(
+        __(
+          'Someone edited this issue at the same time you did and we updated the issue description.',
+        ),
       );
 
       this.$emit('taskListUpdateFailed');
diff --git a/app/assets/javascripts/task_list.js b/app/assets/javascripts/task_list.js
index c3932db7be2..161c44fa156 100644
--- a/app/assets/javascripts/task_list.js
+++ b/app/assets/javascripts/task_list.js
@@ -9,8 +9,9 @@ export default class TaskList {
     this.dataType = options.dataType;
     this.fieldName = options.fieldName;
     this.lockVersion = options.lockVersion;
-    this.onSuccess = options.onSuccess || (() => {});
     this.taskListContainerSelector = `${this.selector} .js-task-list-container`;
+    this.updateHandler = this.update.bind(this);
+    this.onSuccess = options.onSuccess || (() => {});
     this.onError =
       options.onError ||
       function showFlash(e) {
@@ -27,10 +28,10 @@ export default class TaskList {
   }
 
   init() {
-    // Prevent duplicate event bindings
-    this.disable();
+    this.disable(); // Prevent duplicate event bindings
+
     $(this.taskListContainerSelector).taskList('enable');
-    $(document).on('tasklist:changed', this.taskListContainerSelector, this.update.bind(this));
+    $(document).on('tasklist:changed', this.taskListContainerSelector, this.updateHandler);
   }
 
   getTaskListTarget(e = {}) {
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 5b794ae587b..0f50d797c69 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -6527,6 +6527,9 @@ msgstr ""
 msgid "Snippets"
 msgstr ""
 
+msgid "Someone edited this issue at the same time you did and we updated the issue description."
+msgstr ""
+
 msgid "Something went wrong on our end"
 msgstr ""
 
diff --git a/spec/javascripts/issue_show/components/app_spec.js b/spec/javascripts/issue_show/components/app_spec.js
index c3f624aebd3..028a36d3496 100644
--- a/spec/javascripts/issue_show/components/app_spec.js
+++ b/spec/javascripts/issue_show/components/app_spec.js
@@ -469,5 +469,18 @@ describe('Issuable output', () => {
         .then(done)
         .catch(done.fail);
     });
+
+    it('should show error message if store update fails', done => {
+      spyOn(vm.service, 'getData').and.returnValue(Promise.reject());
+      spyOn(window, 'Flash');
+      vm.issuableType = 'merge request';
+
+      vm.updateStoreState()
+        .then(() => {
+          expect(window.Flash).toHaveBeenCalledWith(`Error updating ${vm.issuableType}`);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
   });
 });
diff --git a/spec/javascripts/issue_show/components/description_spec.js b/spec/javascripts/issue_show/components/description_spec.js
index 52148f4c66b..b6833079e6e 100644
--- a/spec/javascripts/issue_show/components/description_spec.js
+++ b/spec/javascripts/issue_show/components/description_spec.js
@@ -187,4 +187,18 @@ describe('Description component', () => {
   it('sets data-update-url', () => {
     expect(vm.$el.querySelector('textarea').dataset.updateUrl).toEqual(gl.TEST_HOST);
   });
+
+  describe('taskListUpdateError', () => {
+    it('should create flash notification and emit an event to parent', () => {
+      const msg =
+        'Someone edited this issue at the same time you did and we updated the issue description.';
+      spyOn(window, 'Flash');
+      spyOn(vm, '$emit');
+
+      vm.taskListUpdateError();
+
+      expect(window.Flash).toHaveBeenCalledWith(msg);
+      expect(vm.$emit).toHaveBeenCalledWith('taskListUpdateFailed');
+    });
+  });
 });
diff --git a/spec/javascripts/task_list_spec.js b/spec/javascripts/task_list_spec.js
new file mode 100644
index 00000000000..00f8b2d6d77
--- /dev/null
+++ b/spec/javascripts/task_list_spec.js
@@ -0,0 +1,156 @@
+import $ from 'jquery';
+import TaskList from '~/task_list';
+import axios from '~/lib/utils/axios_utils';
+
+describe('TaskList', () => {
+  let taskList;
+  let currentTarget;
+  const taskListOptions = {
+    selector: '.task-list',
+    dataType: 'issue',
+    fieldName: 'description',
+    lockVersion: 2,
+  };
+  const createTaskList = () => new TaskList(taskListOptions);
+
+  beforeEach(() => {
+    setFixtures(`
+      <div class="task-list">
+        <div class="js-task-list-container"></div>
+      </div>
+    `);
+
+    currentTarget = $('<div></div>');
+    taskList = createTaskList();
+  });
+
+  it('should call init when the class constructed', () => {
+    spyOn(TaskList.prototype, 'init').and.callThrough();
+    spyOn(TaskList.prototype, 'disable');
+    spyOn($.prototype, 'taskList');
+    spyOn($.prototype, 'on');
+
+    taskList = createTaskList();
+    const $taskListEl = $(taskList.taskListContainerSelector);
+
+    expect(taskList.init).toHaveBeenCalled();
+    expect(taskList.disable).toHaveBeenCalled();
+    expect($taskListEl.taskList).toHaveBeenCalledWith('enable');
+    expect($(document).on).toHaveBeenCalledWith(
+      'tasklist:changed',
+      taskList.taskListContainerSelector,
+      taskList.updateHandler,
+    );
+  });
+
+  describe('getTaskListTarget', () => {
+    it('should return currentTarget from event object if exists', () => {
+      const $target = taskList.getTaskListTarget({ currentTarget });
+
+      expect($target).toEqual(currentTarget);
+    });
+
+    // it('should return element of the taskListContainerSelector', () => {
+    //   const $target = taskList.getTaskListTarget();
+
+    //   expect($target).toEqual($(taskList.taskListContainerSelector));
+    // });
+  });
+
+  describe('disableTaskListItems', () => {
+    it('should call taskList method with disable param', () => {
+      spyOn($.prototype, 'taskList');
+
+      taskList.disableTaskListItems({ currentTarget });
+
+      expect(currentTarget.taskList).toHaveBeenCalledWith('disable');
+    });
+  });
+
+  describe('enableTaskListItems', () => {
+    it('should call taskList method with enable param', () => {
+      spyOn($.prototype, 'taskList');
+
+      taskList.enableTaskListItems({ currentTarget });
+
+      expect(currentTarget.taskList).toHaveBeenCalledWith('enable');
+    });
+  });
+
+  describe('disable', () => {
+    it('should disable task list items and off document event', () => {
+      spyOn(taskList, 'disableTaskListItems');
+      spyOn($.prototype, 'off');
+
+      taskList.disable();
+
+      expect(taskList.disableTaskListItems).toHaveBeenCalled();
+      expect($(document).off).toHaveBeenCalledWith(
+        'tasklist:changed',
+        taskList.taskListContainerSelector,
+      );
+    });
+  });
+
+  describe('update', () => {
+    it('should disable task list items and make a patch request then enable them again', done => {
+      const response = { data: { lock_version: 3 } };
+      spyOn(taskList, 'enableTaskListItems');
+      spyOn(taskList, 'disableTaskListItems');
+      spyOn(taskList, 'onSuccess');
+      spyOn(axios, 'patch').and.returnValue(Promise.resolve(response));
+
+      const value = 'hello world';
+      const endpoint = '/foo';
+      const target = $(`<input data-update-url="${endpoint}" value="${value}" />`);
+      const detail = {
+        index: 2,
+        checked: true,
+        lineNumber: 8,
+        lineSource: '- [ ] check item',
+      };
+      const event = { target, detail };
+      const patchData = {
+        [taskListOptions.dataType]: {
+          [taskListOptions.fieldName]: value,
+          lock_version: taskListOptions.lockVersion,
+          update_task: {
+            index: detail.index,
+            checked: detail.checked,
+            line_number: detail.lineNumber,
+            line_source: detail.lineSource,
+          },
+        },
+      };
+
+      taskList
+        .update(event)
+        .then(() => {
+          expect(taskList.disableTaskListItems).toHaveBeenCalledWith(event);
+          expect(axios.patch).toHaveBeenCalledWith(endpoint, patchData);
+          expect(taskList.enableTaskListItems).toHaveBeenCalledWith(event);
+          expect(taskList.onSuccess).toHaveBeenCalledWith(response.data);
+          expect(taskList.lockVersion).toEqual(response.data.lock_version);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+  });
+
+  it('should handle request error and enable task list items', done => {
+    const response = { data: { error: 1 } };
+    spyOn(taskList, 'enableTaskListItems');
+    spyOn(taskList, 'onError');
+    spyOn(axios, 'patch').and.returnValue(Promise.reject({ response })); // eslint-disable-line prefer-promise-reject-errors
+
+    const event = { detail: {} };
+    taskList
+      .update(event)
+      .then(() => {
+        expect(taskList.enableTaskListItems).toHaveBeenCalledWith(event);
+        expect(taskList.onError).toHaveBeenCalledWith(response.data);
+      })
+      .then(done)
+      .catch(done.fail);
+  });
+});

From c352e7e1621dfb3de97518eb46ca513503995ccf Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Tue, 29 Jan 2019 01:36:31 +0100
Subject: [PATCH 31/42] Some cleanup and improvements on frontent logic

---
 app/assets/javascripts/issue_show/components/app.vue | 12 +++++++-----
 .../issue_show/components/description.vue            |  2 +-
 locale/gitlab.pot                                    |  5 ++++-
 .../issue_show/components/description_spec.js        |  2 +-
 4 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue
index c2c80218a87..5dbe206c6f9 100644
--- a/app/assets/javascripts/issue_show/components/app.vue
+++ b/app/assets/javascripts/issue_show/components/app.vue
@@ -1,5 +1,6 @@
 <script>
 import Visibility from 'visibilityjs';
+import { s__, sprintf } from '~/locale';
 import { visitUrl } from '../../lib/utils/url_utility';
 import Poll from '../../lib/utils/poll';
 import eventHub from '../event_hub';
@@ -166,6 +167,9 @@ export default {
       const titleChanged = this.initialTitleText !== this.store.formState.title;
       return descriptionChanged || titleChanged;
     },
+    defaultErrorMessage() {
+      return sprintf(s__('Error updating  %{issuableType}.'), { issuableType: this.issuableType });
+    },
   },
   created() {
     this.service = new Service(this.endpoint);
@@ -220,9 +224,7 @@ export default {
           this.store.updateState(data);
         })
         .catch(() => {
-          const errMsg = `Error updating ${this.issuableType}`;
-
-          window.Flash(errMsg);
+          window.Flash(this.defaultErrorMessage);
         });
     },
 
@@ -260,10 +262,10 @@ export default {
           if (error && error.name === 'SpamError') {
             this.openRecaptcha();
           } else {
-            let errMsg = `Error updating ${this.issuableType}`;
+            let errMsg = this.defaultErrorMessage;
 
             if (error && error.response && error.response.data && error.response.data.errors) {
-              errMsg = error.response.data.errors;
+              errMsg += error.response.data.errors.join(' ');
             }
 
             eventHub.$emit('close.form');
diff --git a/app/assets/javascripts/issue_show/components/description.vue b/app/assets/javascripts/issue_show/components/description.vue
index f42787a7147..e664269b199 100644
--- a/app/assets/javascripts/issue_show/components/description.vue
+++ b/app/assets/javascripts/issue_show/components/description.vue
@@ -93,7 +93,7 @@ export default {
     taskListUpdateError() {
       window.Flash(
         __(
-          'Someone edited this issue at the same time you did and we updated the issue description.',
+          'Someone edited this issue at the same time you did. The description has been updated and you will need to make your changes again.',
         ),
       );
 
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 0f50d797c69..74846562fb5 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -3040,6 +3040,9 @@ msgstr ""
 msgid "Error saving label update."
 msgstr ""
 
+msgid "Error updating  %{issuableType}"
+msgstr ""
+
 msgid "Error updating status for all todos."
 msgstr ""
 
@@ -6527,7 +6530,7 @@ msgstr ""
 msgid "Snippets"
 msgstr ""
 
-msgid "Someone edited this issue at the same time you did and we updated the issue description."
+msgid "Someone edited this issue at the same time you did. The description has been updated and you will need to make your changes again."
 msgstr ""
 
 msgid "Something went wrong on our end"
diff --git a/spec/javascripts/issue_show/components/description_spec.js b/spec/javascripts/issue_show/components/description_spec.js
index b6833079e6e..72716b97f5f 100644
--- a/spec/javascripts/issue_show/components/description_spec.js
+++ b/spec/javascripts/issue_show/components/description_spec.js
@@ -191,7 +191,7 @@ describe('Description component', () => {
   describe('taskListUpdateError', () => {
     it('should create flash notification and emit an event to parent', () => {
       const msg =
-        'Someone edited this issue at the same time you did and we updated the issue description.';
+        'Someone edited this issue at the same time you did. The description has been updated and you will need to make your changes again.';
       spyOn(window, 'Flash');
       spyOn(vm, '$emit');
 

From a3a847f8624b5f5b10d5665725df2090a1f631ba Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Tue, 29 Jan 2019 14:50:22 +0100
Subject: [PATCH 32/42] Address review comments and fix commented spec

---
 .../javascripts/issue_show/components/app.vue | 24 +++++++-----
 .../javascripts/issue_show/stores/index.js    |  9 ++---
 app/assets/javascripts/task_list.js           |  9 ++---
 .../issue_show/components/app_spec.js         | 39 +++++++++++--------
 spec/javascripts/task_list_spec.js            |  8 ++--
 5 files changed, 47 insertions(+), 42 deletions(-)

diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue
index 5dbe206c6f9..8b3e8719247 100644
--- a/app/assets/javascripts/issue_show/components/app.vue
+++ b/app/assets/javascripts/issue_show/components/app.vue
@@ -1,6 +1,7 @@
 <script>
 import Visibility from 'visibilityjs';
-import { s__, sprintf } from '~/locale';
+import { __, s__, sprintf } from '~/locale';
+import createFlash from '~/flash';
 import { visitUrl } from '../../lib/utils/url_utility';
 import Poll from '../../lib/utils/poll';
 import eventHub from '../event_hub';
@@ -11,7 +12,6 @@ import descriptionComponent from './description.vue';
 import editedComponent from './edited.vue';
 import formComponent from './form.vue';
 import recaptchaModalImplementor from '../../vue_shared/mixins/recaptcha_modal_implementor';
-import { __ } from '~/locale';
 
 export default {
   components: {
@@ -168,7 +168,7 @@ export default {
       return descriptionChanged || titleChanged;
     },
     defaultErrorMessage() {
-      return sprintf(s__('Error updating  %{issuableType}.'), { issuableType: this.issuableType });
+      return sprintf(s__('Error updating %{issuableType}'), { issuableType: this.issuableType });
     },
   },
   created() {
@@ -224,7 +224,7 @@ export default {
           this.store.updateState(data);
         })
         .catch(() => {
-          window.Flash(this.defaultErrorMessage);
+          createFlash(this.defaultErrorMessage);
         });
     },
 
@@ -258,18 +258,20 @@ export default {
         .then(() => {
           eventHub.$emit('close.form');
         })
-        .catch(error => {
-          if (error && error.name === 'SpamError') {
+        .catch((error = {}) => {
+          const { name, response = {} } = error;
+
+          if (name === 'SpamError') {
             this.openRecaptcha();
           } else {
             let errMsg = this.defaultErrorMessage;
 
-            if (error && error.response && error.response.data && error.response.data.errors) {
-              errMsg += error.response.data.errors.join(' ');
+            if (response.data && response.data.errors) {
+              errMsg += `. ${response.data.errors.join(' ')}`;
             }
 
             eventHub.$emit('close.form');
-            window.Flash(errMsg);
+            createFlash(errMsg);
           }
         });
     },
@@ -294,7 +296,9 @@ export default {
         })
         .catch(() => {
           eventHub.$emit('close.form');
-          window.Flash(`Error deleting ${this.issuableType}`);
+          createFlash(
+            sprintf(s__('Error deleting  %{issuableType}'), { issuableType: this.issuableType }),
+          );
         });
     },
   },
diff --git a/app/assets/javascripts/issue_show/stores/index.js b/app/assets/javascripts/issue_show/stores/index.js
index 2b3903def6b..3c17e73ccec 100644
--- a/app/assets/javascripts/issue_show/stores/index.js
+++ b/app/assets/javascripts/issue_show/stores/index.js
@@ -1,3 +1,5 @@
+import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
+
 export default class Store {
   constructor(initialState) {
     this.state = initialState;
@@ -15,14 +17,9 @@ export default class Store {
       this.formState.lockedWarningVisible = true;
     }
 
+    Object.assign(this.state, convertObjectPropsToCamelCase(data));
     this.state.titleHtml = data.title;
-    this.state.titleText = data.title_text;
     this.state.descriptionHtml = data.description;
-    this.state.descriptionText = data.description_text;
-    this.state.taskStatus = data.task_status;
-    this.state.updatedAt = data.updated_at;
-    this.state.updatedByName = data.updated_by_name;
-    this.state.updatedByPath = data.updated_by_path;
     this.state.lock_version = data.lock_version;
   }
 
diff --git a/app/assets/javascripts/task_list.js b/app/assets/javascripts/task_list.js
index 161c44fa156..5172a1ef3d6 100644
--- a/app/assets/javascripts/task_list.js
+++ b/app/assets/javascripts/task_list.js
@@ -1,5 +1,6 @@
 import $ from 'jquery';
 import 'deckar01-task_list';
+import { __ } from '~/locale';
 import axios from './lib/utils/axios_utils';
 import Flash from './flash';
 
@@ -21,7 +22,7 @@ export default class TaskList {
           errorMessages = e.response.data.errors.join(' ');
         }
 
-        return new Flash(errorMessages || 'Update failed', 'alert');
+        return new Flash(errorMessages || __('Update failed'), 'alert');
       };
 
     this.init();
@@ -34,10 +35,8 @@ export default class TaskList {
     $(document).on('tasklist:changed', this.taskListContainerSelector, this.updateHandler);
   }
 
-  getTaskListTarget(e = {}) {
-    const $currentTarget = $(e.currentTarget);
-
-    return $currentTarget.taskList ? $currentTarget : $(this.taskListContainerSelector);
+  getTaskListTarget(e) {
+    return e && e.currentTarget ? $(e.currentTarget) : $(this.taskListContainerSelector);
   }
 
   disableTaskListItems(e) {
diff --git a/spec/javascripts/issue_show/components/app_spec.js b/spec/javascripts/issue_show/components/app_spec.js
index 028a36d3496..46f0d3b16b9 100644
--- a/spec/javascripts/issue_show/components/app_spec.js
+++ b/spec/javascripts/issue_show/components/app_spec.js
@@ -18,9 +18,13 @@ describe('Issuable output', () => {
   let realtimeRequestCount = 0;
   let vm;
 
-  document.body.innerHTML = '<span id="task_status"></span>';
-
   beforeEach(done => {
+    setFixtures(`
+      <div>
+        <div class="flash-container"></div>
+        <span id="task_status"></span>
+      </div>
+    `);
     spyOn(eventHub, '$emit');
 
     const IssuableDescriptionComponent = Vue.extend(issuableApp);
@@ -258,18 +262,15 @@ describe('Issuable output', () => {
     });
 
     describe('error when updating', () => {
-      beforeEach(() => {
-        spyOn(window, 'Flash').and.callThrough();
-      });
-
       it('closes form on error', done => {
         spyOn(vm.service, 'updateIssuable').and.callFake(() => Promise.resolve());
         vm.updateIssuable();
 
         setTimeout(() => {
           expect(eventHub.$emit).toHaveBeenCalledWith('close.form');
-
-          expect(window.Flash).toHaveBeenCalledWith('Error updating issue');
+          expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe(
+            `Error updating issue`,
+          );
 
           done();
         });
@@ -284,8 +285,9 @@ describe('Issuable output', () => {
 
           setTimeout(() => {
             expect(eventHub.$emit).toHaveBeenCalledWith('close.form');
-
-            expect(window.Flash).toHaveBeenCalledWith('Error updating merge request');
+            expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe(
+              `Error updating merge request`,
+            );
 
             done();
           });
@@ -295,12 +297,14 @@ describe('Issuable output', () => {
       it('shows error mesage from backend if exists', done => {
         const msg = 'Custom error message from backend';
         spyOn(vm.service, 'updateIssuable').and.callFake(
-          () => Promise.reject({ response: { data: { errors: msg } } }), // eslint-disable-line prefer-promise-reject-errors
+          () => Promise.reject({ response: { data: { errors: [msg] } } }), // eslint-disable-line prefer-promise-reject-errors
         );
 
         vm.updateIssuable();
         setTimeout(() => {
-          expect(window.Flash).toHaveBeenCalledWith(msg);
+          expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe(
+            `${vm.defaultErrorMessage}. ${msg}`,
+          );
 
           done();
         });
@@ -399,7 +403,6 @@ describe('Issuable output', () => {
     });
 
     it('closes form on error', done => {
-      spyOn(window, 'Flash').and.callThrough();
       spyOn(vm.service, 'deleteIssuable').and.callFake(
         () =>
           new Promise((resolve, reject) => {
@@ -411,8 +414,9 @@ describe('Issuable output', () => {
 
       setTimeout(() => {
         expect(eventHub.$emit).toHaveBeenCalledWith('close.form');
-
-        expect(window.Flash).toHaveBeenCalledWith('Error deleting issue');
+        expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe(
+          'Error deleting issue',
+        );
 
         done();
       });
@@ -472,12 +476,13 @@ describe('Issuable output', () => {
 
     it('should show error message if store update fails', done => {
       spyOn(vm.service, 'getData').and.returnValue(Promise.reject());
-      spyOn(window, 'Flash');
       vm.issuableType = 'merge request';
 
       vm.updateStoreState()
         .then(() => {
-          expect(window.Flash).toHaveBeenCalledWith(`Error updating ${vm.issuableType}`);
+          expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe(
+            `Error updating ${vm.issuableType}`,
+          );
         })
         .then(done)
         .catch(done.fail);
diff --git a/spec/javascripts/task_list_spec.js b/spec/javascripts/task_list_spec.js
index 00f8b2d6d77..563f402de58 100644
--- a/spec/javascripts/task_list_spec.js
+++ b/spec/javascripts/task_list_spec.js
@@ -50,11 +50,11 @@ describe('TaskList', () => {
       expect($target).toEqual(currentTarget);
     });
 
-    // it('should return element of the taskListContainerSelector', () => {
-    //   const $target = taskList.getTaskListTarget();
+    it('should return element of the taskListContainerSelector', () => {
+      const $target = taskList.getTaskListTarget();
 
-    //   expect($target).toEqual($(taskList.taskListContainerSelector));
-    // });
+      expect($target).toEqual($(taskList.taskListContainerSelector));
+    });
   });
 
   describe('disableTaskListItems', () => {

From 58cd21c2ad802c1652311f025c599e0df604669c Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Tue, 29 Jan 2019 10:46:04 -0600
Subject: [PATCH 33/42] Use the sourcepos attribute for finding tasks

---
 app/models/concerns/cache_markdown_field.rb   |  8 +++++--
 app/services/issuable_base_service.rb         | 10 ++++----
 app/services/task_list_toggle_service.rb      |  9 ++++++-
 .../concerns/cache_markdown_field_spec.rb     | 24 +++++++++++++++++++
 4 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/app/models/concerns/cache_markdown_field.rb b/app/models/concerns/cache_markdown_field.rb
index 0ccc03db15a..588204c7470 100644
--- a/app/models/concerns/cache_markdown_field.rb
+++ b/app/models/concerns/cache_markdown_field.rb
@@ -15,7 +15,7 @@ module CacheMarkdownField
   # Increment this number every time the renderer changes its output
   CACHE_REDCARPET_VERSION         = 3
   CACHE_COMMONMARK_VERSION_START  = 10
-  CACHE_COMMONMARK_VERSION        = 13
+  CACHE_COMMONMARK_VERSION        = 14
 
   # changes to these attributes cause the cache to be invalidates
   INVALIDATED_BY = %w[author project].freeze
@@ -130,13 +130,17 @@ module CacheMarkdownField
   def latest_cached_markdown_version
     return CacheMarkdownField::CACHE_COMMONMARK_VERSION unless cached_markdown_version
 
-    if cached_markdown_version < CacheMarkdownField::CACHE_COMMONMARK_VERSION_START
+    if legacy_markdown?
       CacheMarkdownField::CACHE_REDCARPET_VERSION
     else
       CacheMarkdownField::CACHE_COMMONMARK_VERSION
     end
   end
 
+  def legacy_markdown?
+    cached_markdown_version && cached_markdown_version.between?(1, CacheMarkdownField::CACHE_COMMONMARK_VERSION_START - 1)
+  end
+
   included do
     cattr_reader :cached_markdown_fields do
       FieldData.new
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index 5719a4778f0..2835d598311 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -262,16 +262,16 @@ class IssuableBaseService < BaseService
 
   # Handle the `update_task` event sent from UI.  Attempts to update a specific
   # line in the markdown and cached html, bypassing any unnecessary updates or checks.
-  def update_task_event(issue)
+  def update_task_event(issuable)
     update_task_params = params.delete(:update_task)
     return unless update_task_params
 
-    toggler = TaskListToggleService.new(issue.description, issue.description_html,
+    toggler = TaskListToggleService.new(issuable.description, issuable.description_html,
                                         line_source: update_task_params[:line_source],
                                         line_number: update_task_params[:line_number],
                                         currently_checked: !update_task_params[:checked],
                                         index: update_task_params[:index],
-                                        sourcepos: false)
+                                        sourcepos: !issuable.legacy_markdown?)
 
     if toggler.execute
       # by updating the description_html field at the same time,
@@ -282,9 +282,9 @@ class IssuableBaseService < BaseService
       # since we're updating a very specific line, we don't care whether
       # the `lock_version` sent from the FE is the same or not.  Just
       # make sure the data hasn't changed since we queried it
-      params[:lock_version]     = issue.lock_version
+      params[:lock_version]     = issuable.lock_version
 
-      update_task(issue)
+      update_task(issuable)
     else
       # if we make it here, the data is much newer than we thought it was - fail fast
       raise ActiveRecord::StaleObjectError
diff --git a/app/services/task_list_toggle_service.rb b/app/services/task_list_toggle_service.rb
index 1b0a5965235..3ba0faa5009 100644
--- a/app/services/task_list_toggle_service.rb
+++ b/app/services/task_list_toggle_service.rb
@@ -11,7 +11,7 @@
 class TaskListToggleService
   attr_reader :updated_markdown, :updated_markdown_html
 
-  def initialize(markdown, markdown_html, line_source:, line_number:, currently_checked:, index:, sourcepos: false)
+  def initialize(markdown, markdown_html, line_source:, line_number:, currently_checked:, index:, sourcepos: true)
     @markdown, @markdown_html  = markdown, markdown_html
     @line_source, @line_number = line_source, line_number
     @currently_checked         = currently_checked
@@ -62,6 +62,13 @@ class TaskListToggleService
     @updated_markdown_html = html.to_html
   end
 
+  # When using CommonMark, we should be able to use the embedded `sourcepos` attribute to
+  # target the exact line in the DOM.  For RedCarpet, we need to use the index of the checkbox
+  # that was checked and match it with what we think is the same checkbox.
+  # The reason `sourcepos` is slightly more reliable is the case where a line of text is
+  # changed from a regular line into a checkbox (or vice versa).  Then the checked index
+  # in the UI will be off from the list of checkboxes we've calculated locally.
+  # It's a rare circumstance, but since we can account for it, we do.
   def get_html_checkbox(html)
     if use_sourcepos
       html.css(".task-list-item[data-sourcepos^='#{line_number}:'] > input.task-list-item-checkbox").first
diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb
index a8a5521a5fc..1c2646c60a4 100644
--- a/spec/models/concerns/cache_markdown_field_spec.rb
+++ b/spec/models/concerns/cache_markdown_field_spec.rb
@@ -251,6 +251,30 @@ describe CacheMarkdownField do
     end
   end
 
+  describe '#legacy_markdown?' do
+    subject { thing.legacy_markdown? }
+
+    it 'returns true for redcarpet versions' do
+      thing.cached_markdown_version = CacheMarkdownField::CACHE_COMMONMARK_VERSION_START - 1
+      is_expected.to be_truthy
+    end
+
+    it 'returns false for commonmark versions' do
+      thing.cached_markdown_version = CacheMarkdownField::CACHE_COMMONMARK_VERSION_START
+      is_expected.to be_falsey
+    end
+
+    it 'returns false if nil' do
+      thing.cached_markdown_version = nil
+      is_expected.to be_falsey
+    end
+
+    it 'returns false if 0' do
+      thing.cached_markdown_version = 0
+      is_expected.to be_falsey
+    end
+  end
+
   describe '#refresh_markdown_cache' do
     before do
       thing.foo = updated_markdown

From 19dc2fb082ebeb8c1874f59282eead8a8e05fb35 Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Tue, 29 Jan 2019 13:10:36 -0600
Subject: [PATCH 34/42] Fix TaskListToggleService specs

---
 .../services/task_list_toggle_service_spec.rb | 129 ++++++++++++------
 1 file changed, 85 insertions(+), 44 deletions(-)

diff --git a/spec/services/task_list_toggle_service_spec.rb b/spec/services/task_list_toggle_service_spec.rb
index 8ae0e209cda..6b92d4e14d0 100644
--- a/spec/services/task_list_toggle_service_spec.rb
+++ b/spec/services/task_list_toggle_service_spec.rb
@@ -4,6 +4,7 @@ require 'spec_helper'
 
 describe TaskListToggleService do
   context 'when ' do
+    let(:sourcepos) { true }
     let(:markdown) do
       <<-EOT.strip_heredoc
         * [ ] Task 1
@@ -18,6 +19,87 @@ describe TaskListToggleService do
 
     let(:markdown_html) do
       <<-EOT.strip_heredoc
+        <ul data-sourcepos="1:1-3:0" class="task-list" dir="auto">
+          <li data-sourcepos="1:1-1:12" class="task-list-item">
+            <input type="checkbox" class="task-list-item-checkbox" disabled> Task 1
+          </li>
+          <li data-sourcepos="2:1-3:0" class="task-list-item">
+            <input type="checkbox" class="task-list-item-checkbox" disabled checked> Task 2
+          </li>
+        </ul>
+        <p data-sourcepos="4:1-4:11" dir="auto">A paragraph</p>
+        <ol data-sourcepos="6:1-7:19" class="task-list" dir="auto">
+          <li data-sourcepos="6:1-7:19" class="task-list-item">
+            <input type="checkbox" class="task-list-item-checkbox" disabled checked> Item 1
+            <ul data-sourcepos="7:4-7:19" class="task-list">
+              <li data-sourcepos="7:4-7:19" class="task-list-item">
+                <input type="checkbox" class="task-list-item-checkbox" disabled> Sub-item 1
+              </li>
+            </ul>
+          </li>
+        </ol>
+      EOT
+    end
+
+    shared_examples 'task lists' do
+      it 'checks Task 1' do
+        toggler = described_class.new(markdown, markdown_html,
+                                      index: 1, toggle_as_checked: true,
+                                      line_source: '* [ ] Task 1', line_number: 1,
+                                      sourcepos: sourcepos)
+
+        expect(toggler.execute).to be_truthy
+        expect(toggler.updated_markdown.lines[0]).to eq "* [x] Task 1\n"
+        expect(toggler.updated_markdown_html).to include('disabled checked> Task 1')
+      end
+
+      it 'unchecks Item 1' do
+        toggler = described_class.new(markdown, markdown_html,
+                                      index: 3, toggle_as_checked: false,
+                                      line_source: '1. [X] Item 1', line_number: 6,
+                                      sourcepos: sourcepos)
+
+        expect(toggler.execute).to be_truthy
+        expect(toggler.updated_markdown.lines[5]).to eq "1. [ ] Item 1\n"
+        expect(toggler.updated_markdown_html).to include('disabled> Item 1')
+      end
+
+      it 'returns false if line_source does not match the text' do
+        toggler = described_class.new(markdown, markdown_html,
+                                      index: 2, toggle_as_checked: false,
+                                      line_source: '* [x] Task Added', line_number: 2,
+                                      sourcepos: sourcepos)
+
+        expect(toggler.execute).to be_falsey
+      end
+
+      it 'returns false if markdown is nil' do
+        toggler = described_class.new(nil, markdown_html,
+                                      index: 2, toggle_as_checked: false,
+                                      line_source: '* [x] Task Added', line_number: 2,
+                                      sourcepos: sourcepos)
+
+        expect(toggler.execute).to be_falsey
+      end
+
+      it 'returns false if markdown_html is nil' do
+        toggler = described_class.new(markdown, nil,
+                                      index: 2, toggle_as_checked: false,
+                                      line_source: '* [x] Task Added', line_number: 2,
+                                      sourcepos: sourcepos)
+
+        expect(toggler.execute).to be_falsey
+      end
+    end
+
+    context 'when using sourcepos' do
+      it_behaves_like 'task lists'
+    end
+
+    context 'when using checkbox indexing' do
+      let(:sourcepos) { false }
+      let(:markdown_html) do
+        <<-EOT.strip_heredoc
         <ul class="task-list" dir="auto">
           <li class="task-list-item">
             <input type="checkbox" class="task-list-item-checkbox" disabled> Task 1
@@ -37,51 +119,10 @@ describe TaskListToggleService do
             </ul>
           </li>
         </ol>
-      EOT
-    end
+        EOT
+      end
 
-    it 'checks Task 1' do
-      toggler = described_class.new(markdown, markdown_html,
-                                    index: 1, currently_checked: false,
-                                    line_source: '* [ ] Task 1', line_number: 1)
-
-      expect(toggler.execute).to be_truthy
-      expect(toggler.updated_markdown.lines[0]).to eq "* [x] Task 1\n"
-      expect(toggler.updated_markdown_html).to include('disabled checked> Task 1')
-    end
-
-    it 'unchecks Item 1' do
-      toggler = described_class.new(markdown, markdown_html,
-                                    index: 3, currently_checked: true,
-                                    line_source: '1. [X] Item 1', line_number: 6)
-
-      expect(toggler.execute).to be_truthy
-      expect(toggler.updated_markdown.lines[5]).to eq "1. [ ] Item 1\n"
-      expect(toggler.updated_markdown_html).to include('disabled> Item 1')
-    end
-
-    it 'returns false if line_source does not match the text' do
-      toggler = described_class.new(markdown, markdown_html,
-                                    index: 2, currently_checked: true,
-                                    line_source: '* [x] Task Added', line_number: 2)
-
-      expect(toggler.execute).to be_falsey
-    end
-
-    it 'returns false if markdown is nil' do
-      toggler = described_class.new(nil, markdown_html,
-                                    index: 2, currently_checked: true,
-                                    line_source: '* [x] Task Added', line_number: 2)
-
-      expect(toggler.execute).to be_falsey
-    end
-
-    it 'returns false if markdown_html is nil' do
-      toggler = described_class.new(markdown, nil,
-                                    index: 2, currently_checked: true,
-                                    line_source: '* [x] Task Added', line_number: 2)
-
-      expect(toggler.execute).to be_falsey
+      it_behaves_like 'task lists'
     end
   end
 end

From e725ee0c30650ea5041afe6bc36e15a5ff2f8a07 Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Tue, 29 Jan 2019 13:11:04 -0600
Subject: [PATCH 35/42] Changes for review comments

---
 app/services/issuable_base_service.rb         | 18 +++++-----
 app/services/task_list_toggle_service.rb      | 33 +++++++++++--------
 ...verwritten-when-editing-simultaneously.yml |  2 +-
 3 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index 2835d598311..93dd6893e42 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -266,18 +266,18 @@ class IssuableBaseService < BaseService
     update_task_params = params.delete(:update_task)
     return unless update_task_params
 
-    toggler = TaskListToggleService.new(issuable.description, issuable.description_html,
-                                        line_source: update_task_params[:line_source],
-                                        line_number: update_task_params[:line_number],
-                                        currently_checked: !update_task_params[:checked],
-                                        index: update_task_params[:index],
-                                        sourcepos: !issuable.legacy_markdown?)
+    tasklist_toggler = TaskListToggleService.new(issuable.description, issuable.description_html,
+                                                 line_source: update_task_params[:line_source],
+                                                 line_number: update_task_params[:line_number],
+                                                 toggle_as_checked: update_task_params[:checked],
+                                                 index: update_task_params[:index],
+                                                 sourcepos: !issuable.legacy_markdown?)
 
-    if toggler.execute
+    if tasklist_toggler.execute
       # by updating the description_html field at the same time,
       # the markdown cache won't be considered invalid
-      params[:description]      = toggler.updated_markdown
-      params[:description_html] = toggler.updated_markdown_html
+      params[:description]      = tasklist_toggler.updated_markdown
+      params[:description_html] = tasklist_toggler.updated_markdown_html
 
       # since we're updating a very specific line, we don't care whether
       # the `lock_version` sent from the FE is the same or not.  Just
diff --git a/app/services/task_list_toggle_service.rb b/app/services/task_list_toggle_service.rb
index 3ba0faa5009..92f42a92c6b 100644
--- a/app/services/task_list_toggle_service.rb
+++ b/app/services/task_list_toggle_service.rb
@@ -11,10 +11,10 @@
 class TaskListToggleService
   attr_reader :updated_markdown, :updated_markdown_html
 
-  def initialize(markdown, markdown_html, line_source:, line_number:, currently_checked:, index:, sourcepos: true)
+  def initialize(markdown, markdown_html, line_source:, line_number:, toggle_as_checked:, index:, sourcepos: true)
     @markdown, @markdown_html  = markdown, markdown_html
     @line_source, @line_number = line_source, line_number
-    @currently_checked         = currently_checked
+    @toggle_as_checked         = toggle_as_checked
     @index, @use_sourcepos     = index, sourcepos
 
     @updated_markdown, @updated_markdown_html = nil
@@ -23,40 +23,45 @@ class TaskListToggleService
   def execute
     return false unless markdown && markdown_html
 
-    !!(toggle_markdown && toggle_html)
+    !!(toggle_markdown && toggle_markdown_html)
   end
 
   private
 
-  attr_reader :markdown, :markdown_html, :index, :currently_checked
+  attr_reader :markdown, :markdown_html, :index, :toggle_as_checked
   attr_reader :line_source, :line_number, :use_sourcepos
 
   def toggle_markdown
-    source_lines  = markdown.split("\n")
-    markdown_task = source_lines[line_number - 1]
+    source_lines      = markdown.split("\n")
+    source_line_index = line_number - 1
+    markdown_task     = source_lines[source_line_index]
 
     return unless markdown_task == line_source
     return unless source_checkbox = Taskable::ITEM_PATTERN.match(markdown_task)
 
-    if TaskList::Item.new(source_checkbox[1]).complete?
-      markdown_task.sub!(Taskable::COMPLETE_PATTERN, '[ ]') if currently_checked
+    currently_checked = TaskList::Item.new(source_checkbox[1]).complete?
+
+    # Check `toggle_as_checked` to make sure we don't accidentally replace
+    # any `[ ]` or `[x]` in the middle of the text
+    if currently_checked
+      markdown_task.sub!(Taskable::COMPLETE_PATTERN, '[ ]') unless toggle_as_checked
     else
-      markdown_task.sub!(Taskable::INCOMPLETE_PATTERN, '[x]') unless currently_checked
+      markdown_task.sub!(Taskable::INCOMPLETE_PATTERN, '[x]') if toggle_as_checked
     end
 
-    source_lines[line_number - 1] = markdown_task
+    source_lines[source_line_index] = markdown_task
     @updated_markdown = source_lines.join("\n")
   end
 
-  def toggle_html
+  def toggle_markdown_html
     html          = Nokogiri::HTML.fragment(markdown_html)
     html_checkbox = get_html_checkbox(html)
     return unless html_checkbox
 
-    if currently_checked
-      html_checkbox.remove_attribute('checked')
-    else
+    if toggle_as_checked
       html_checkbox[:checked] = 'checked'
+    else
+      html_checkbox.remove_attribute('checked')
     end
 
     @updated_markdown_html = html.to_html
diff --git a/changelogs/unreleased/19745-forms-with-task-lists-can-be-overwritten-when-editing-simultaneously.yml b/changelogs/unreleased/19745-forms-with-task-lists-can-be-overwritten-when-editing-simultaneously.yml
index 7b6a38c1d57..b1177e1717e 100644
--- a/changelogs/unreleased/19745-forms-with-task-lists-can-be-overwritten-when-editing-simultaneously.yml
+++ b/changelogs/unreleased/19745-forms-with-task-lists-can-be-overwritten-when-editing-simultaneously.yml
@@ -1,5 +1,5 @@
 ---
-title: Increase reliability and performance of clicking task items
+title: Increase reliability and performance of toggling task items
 merge_request: 23938
 author:
 type: fixed

From 26f5c7aa7cac29b087748d546c1663d6080935f2 Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Tue, 29 Jan 2019 16:58:32 -0600
Subject: [PATCH 36/42] Fix task list feature specs

---
 spec/features/task_lists_spec.rb | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/spec/features/task_lists_spec.rb b/spec/features/task_lists_spec.rb
index 9c9127980a1..e8c6c5e1832 100644
--- a/spec/features/task_lists_spec.rb
+++ b/spec/features/task_lists_spec.rb
@@ -170,8 +170,11 @@ describe 'Task Lists' do
         expect(page).to have_content("2 of 7 tasks completed")
 
         page.find('li.task-list-item', text: 'Task b').find('input').click
+        wait_for_requests
         page.find('li.task-list-item ul li.task-list-item', text: 'Task a.2').find('input').click
+        wait_for_requests
         page.find('li.task-list-item ol li.task-list-item', text: 'Task 1.1').find('input').click
+        wait_for_requests
 
         expect(page).to have_content("5 of 7 tasks completed")
 
@@ -184,25 +187,22 @@ describe 'Task Lists' do
     end
 
     describe 'nested tasks', :js do
-      context 'with Redcarpet' do
-        let(:issue) { create(:issue, description: nested_tasks_markdown_redcarpet, author: user, project: project) }
+      let(:cache_version) { CacheMarkdownField::CACHE_COMMONMARK_VERSION }
+      let!(:issue) { create(:issue, description: nested_tasks_markdown, author: user, project: project,
+                            cached_markdown_version: cache_version) }
 
-        before do
-          allow_any_instance_of(Banzai::Filter::MarkdownFilter).to receive(:engine).and_return('Redcarpet')
-          visit_issue(project, issue)
-        end
+      before do
+        visit_issue(project, issue)
+      end
+
+      context 'with Redcarpet' do
+        let(:cache_version) { CacheMarkdownField::CACHE_REDCARPET_VERSION }
+        let(:nested_tasks_markdown) { nested_tasks_markdown_redcarpet }
 
         it_behaves_like 'shared nested tasks'
       end
 
       context 'with CommonMark' do
-        let(:issue) { create(:issue, description: nested_tasks_markdown, author: user, project: project) }
-
-        before do
-          allow_any_instance_of(Banzai::Filter::MarkdownFilter).to receive(:engine).and_return('CommonMark')
-          visit_issue(project, issue)
-        end
-
         it_behaves_like 'shared nested tasks'
       end
     end

From 380db3768ff7abed63cd0cc9c7c65a4dac3fc113 Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Tue, 29 Jan 2019 17:02:25 -0600
Subject: [PATCH 37/42] Fix rubocop errors

---
 spec/features/task_lists_spec.rb | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/spec/features/task_lists_spec.rb b/spec/features/task_lists_spec.rb
index e8c6c5e1832..b549f2b5c62 100644
--- a/spec/features/task_lists_spec.rb
+++ b/spec/features/task_lists_spec.rb
@@ -188,8 +188,10 @@ describe 'Task Lists' do
 
     describe 'nested tasks', :js do
       let(:cache_version) { CacheMarkdownField::CACHE_COMMONMARK_VERSION }
-      let!(:issue) { create(:issue, description: nested_tasks_markdown, author: user, project: project,
-                            cached_markdown_version: cache_version) }
+      let!(:issue) do
+        create(:issue, description: nested_tasks_markdown, author: user, project: project,
+                       cached_markdown_version: cache_version)
+      end
 
       before do
         visit_issue(project, issue)

From 42f6be1bc16ea661f74e26949cfe74078ec0c00a Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Tue, 29 Jan 2019 20:35:04 -0600
Subject: [PATCH 38/42] Regenerate the gitlab.pot

---
 locale/gitlab.pot | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 74846562fb5..e325f027e14 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -2992,6 +2992,9 @@ msgstr ""
 msgid "Error Tracking"
 msgstr ""
 
+msgid "Error deleting  %{issuableType}"
+msgstr ""
+
 msgid "Error fetching contributors data."
 msgstr ""
 
@@ -3040,7 +3043,7 @@ msgstr ""
 msgid "Error saving label update."
 msgstr ""
 
-msgid "Error updating  %{issuableType}"
+msgid "Error updating %{issuableType}"
 msgstr ""
 
 msgid "Error updating status for all todos."
@@ -7719,6 +7722,9 @@ msgstr ""
 msgid "Update"
 msgstr ""
 
+msgid "Update failed"
+msgstr ""
+
 msgid "Update now"
 msgstr ""
 

From 69b87a979de147b0940e01224482e6b64099a563 Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Wed, 30 Jan 2019 19:22:54 +0100
Subject: [PATCH 39/42] Stop closing issue edit form in error state

---
 app/assets/javascripts/issue_show/components/app.vue | 2 --
 spec/javascripts/issue_show/components/app_spec.js   | 6 +++---
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue
index 8b3e8719247..0fba1b3bb46 100644
--- a/app/assets/javascripts/issue_show/components/app.vue
+++ b/app/assets/javascripts/issue_show/components/app.vue
@@ -270,7 +270,6 @@ export default {
               errMsg += `. ${response.data.errors.join(' ')}`;
             }
 
-            eventHub.$emit('close.form');
             createFlash(errMsg);
           }
         });
@@ -295,7 +294,6 @@ export default {
           visitUrl(data.web_url);
         })
         .catch(() => {
-          eventHub.$emit('close.form');
           createFlash(
             sprintf(s__('Error deleting  %{issuableType}'), { issuableType: this.issuableType }),
           );
diff --git a/spec/javascripts/issue_show/components/app_spec.js b/spec/javascripts/issue_show/components/app_spec.js
index 46f0d3b16b9..1a7d91179b7 100644
--- a/spec/javascripts/issue_show/components/app_spec.js
+++ b/spec/javascripts/issue_show/components/app_spec.js
@@ -263,11 +263,11 @@ describe('Issuable output', () => {
 
     describe('error when updating', () => {
       it('closes form on error', done => {
-        spyOn(vm.service, 'updateIssuable').and.callFake(() => Promise.resolve());
+        spyOn(vm.service, 'updateIssuable').and.callFake(() => Promise.reject());
         vm.updateIssuable();
 
         setTimeout(() => {
-          expect(eventHub.$emit).toHaveBeenCalledWith('close.form');
+          expect(eventHub.$emit).not.toHaveBeenCalledWith('close.form');
           expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe(
             `Error updating issue`,
           );
@@ -284,7 +284,7 @@ describe('Issuable output', () => {
           vm.updateIssuable();
 
           setTimeout(() => {
-            expect(eventHub.$emit).toHaveBeenCalledWith('close.form');
+            expect(eventHub.$emit).not.toHaveBeenCalledWith('close.form');
             expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe(
               `Error updating merge request`,
             );

From 0e6c08f58bc7faff3c29f36141872db55cad941b Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Wed, 30 Jan 2019 19:23:25 +0100
Subject: [PATCH 40/42] Simplify and unify Promise mocks in app_spec file

---
 .../issue_show/components/app_spec.js         | 104 ++++++++----------
 1 file changed, 43 insertions(+), 61 deletions(-)

diff --git a/spec/javascripts/issue_show/components/app_spec.js b/spec/javascripts/issue_show/components/app_spec.js
index 1a7d91179b7..0ccf771c7ef 100644
--- a/spec/javascripts/issue_show/components/app_spec.js
+++ b/spec/javascripts/issue_show/components/app_spec.js
@@ -146,16 +146,10 @@ describe('Issuable output', () => {
     it('fetches new data after update', done => {
       spyOn(vm, 'updateStoreState').and.callThrough();
       spyOn(vm.service, 'getData').and.callThrough();
-      spyOn(vm.service, 'updateIssuable').and.callFake(
-        () =>
-          new Promise(resolve => {
-            resolve({
-              data: {
-                confidential: false,
-                web_url: window.location.pathname,
-              },
-            });
-          }),
+      spyOn(vm.service, 'updateIssuable').and.returnValue(
+        Promise.resolve({
+          data: { web_url: window.location.pathname },
+        }),
       );
 
       vm.updateIssuable()
@@ -168,11 +162,10 @@ describe('Issuable output', () => {
     });
 
     it('correctly updates issuable data', done => {
-      spyOn(vm.service, 'updateIssuable').and.callFake(
-        () =>
-          new Promise(resolve => {
-            resolve();
-          }),
+      spyOn(vm.service, 'updateIssuable').and.returnValue(
+        Promise.resolve({
+          data: { web_url: window.location.pathname },
+        }),
       );
 
       vm.updateIssuable()
@@ -186,16 +179,13 @@ describe('Issuable output', () => {
 
     it('does not redirect if issue has not moved', done => {
       const visitUrl = spyOnDependency(issuableApp, 'visitUrl');
-      spyOn(vm.service, 'updateIssuable').and.callFake(
-        () =>
-          new Promise(resolve => {
-            resolve({
-              data: {
-                web_url: window.location.pathname,
-                confidential: vm.isConfidential,
-              },
-            });
-          }),
+      spyOn(vm.service, 'updateIssuable').and.returnValue(
+        Promise.resolve({
+          data: {
+            web_url: window.location.pathname,
+            confidential: vm.isConfidential,
+          },
+        }),
       );
 
       vm.updateIssuable();
@@ -208,16 +198,13 @@ describe('Issuable output', () => {
 
     it('redirects if returned web_url has changed', done => {
       const visitUrl = spyOnDependency(issuableApp, 'visitUrl');
-      spyOn(vm.service, 'updateIssuable').and.callFake(
-        () =>
-          new Promise(resolve => {
-            resolve({
-              data: {
-                web_url: '/testing-issue-move',
-                confidential: vm.isConfidential,
-              },
-            });
-          }),
+      spyOn(vm.service, 'updateIssuable').and.returnValue(
+        Promise.resolve({
+          data: {
+            web_url: '/testing-issue-move',
+            confidential: vm.isConfidential,
+          },
+        }),
       );
 
       vm.updateIssuable();
@@ -236,6 +223,7 @@ describe('Issuable output', () => {
         vm.handleBeforeUnloadEvent(e);
         Vue.nextTick(() => {
           expect(e.returnValue).not.toBeNull();
+
           done();
         });
       });
@@ -247,6 +235,7 @@ describe('Issuable output', () => {
         vm.handleBeforeUnloadEvent(e);
         Vue.nextTick(() => {
           expect(e.returnValue).not.toBeNull();
+
           done();
         });
       });
@@ -256,6 +245,7 @@ describe('Issuable output', () => {
         vm.handleBeforeUnloadEvent(e);
         Vue.nextTick(() => {
           expect(e.returnValue).toBeNull();
+
           done();
         });
       });
@@ -297,7 +287,8 @@ describe('Issuable output', () => {
       it('shows error mesage from backend if exists', done => {
         const msg = 'Custom error message from backend';
         spyOn(vm.service, 'updateIssuable').and.callFake(
-          () => Promise.reject({ response: { data: { errors: [msg] } } }), // eslint-disable-line prefer-promise-reject-errors
+          // eslint-disable-next-line prefer-promise-reject-errors
+          () => Promise.reject({ response: { data: { errors: [msg] } } }),
         );
 
         vm.updateIssuable();
@@ -361,21 +352,19 @@ describe('Issuable output', () => {
   describe('deleteIssuable', () => {
     it('changes URL when deleted', done => {
       const visitUrl = spyOnDependency(issuableApp, 'visitUrl');
-      spyOn(vm.service, 'deleteIssuable').and.callFake(
-        () =>
-          new Promise(resolve => {
-            resolve({
-              data: {
-                web_url: '/test',
-              },
-            });
-          }),
+      spyOn(vm.service, 'deleteIssuable').and.returnValue(
+        Promise.resolve({
+          data: {
+            web_url: '/test',
+          },
+        }),
       );
 
       vm.deleteIssuable();
 
       setTimeout(() => {
         expect(visitUrl).toHaveBeenCalledWith('/test');
+
         done();
       });
     });
@@ -383,37 +372,30 @@ describe('Issuable output', () => {
     it('stops polling when deleting', done => {
       spyOnDependency(issuableApp, 'visitUrl');
       spyOn(vm.poll, 'stop').and.callThrough();
-      spyOn(vm.service, 'deleteIssuable').and.callFake(
-        () =>
-          new Promise(resolve => {
-            resolve({
-              data: {
-                web_url: '/test',
-              },
-            });
-          }),
+      spyOn(vm.service, 'deleteIssuable').and.returnValue(
+        Promise.resolve({
+          data: {
+            web_url: '/test',
+          },
+        }),
       );
 
       vm.deleteIssuable();
 
       setTimeout(() => {
         expect(vm.poll.stop).toHaveBeenCalledWith();
+
         done();
       });
     });
 
     it('closes form on error', done => {
-      spyOn(vm.service, 'deleteIssuable').and.callFake(
-        () =>
-          new Promise((resolve, reject) => {
-            reject();
-          }),
-      );
+      spyOn(vm.service, 'deleteIssuable').and.returnValue(Promise.reject());
 
       vm.deleteIssuable();
 
       setTimeout(() => {
-        expect(eventHub.$emit).toHaveBeenCalledWith('close.form');
+        expect(eventHub.$emit).not.toHaveBeenCalledWith('close.form');
         expect(document.querySelector('.flash-container .flash-text').innerText.trim()).toBe(
           'Error deleting issue',
         );

From bb8fdcc17700e029e2a5bb7e195b07b9550b4e34 Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Thu, 31 Jan 2019 09:33:38 -0600
Subject: [PATCH 41/42] Address review comments

---
 app/services/issuable_base_service.rb         |  35 +--
 app/services/task_list_toggle_service.rb      |   2 +-
 .../concerns/cache_markdown_field_spec.rb     |   2 +-
 .../services/task_list_toggle_service_spec.rb | 214 +++++++++---------
 4 files changed, 126 insertions(+), 127 deletions(-)

diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index 93dd6893e42..842d59d26a0 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -239,8 +239,9 @@ class IssuableBaseService < BaseService
     filter_params(issuable)
 
     if issuable.changed? || params.present?
-      issuable.assign_attributes(params.merge(updated_by: current_user))
-      issuable.assign_attributes(last_edited_at: Time.now, last_edited_by: current_user)
+      issuable.assign_attributes(params.merge(updated_by: current_user,
+                                              last_edited_at: Time.now,
+                                              last_edited_by: current_user))
 
       before_update(issuable)
 
@@ -268,27 +269,27 @@ class IssuableBaseService < BaseService
 
     tasklist_toggler = TaskListToggleService.new(issuable.description, issuable.description_html,
                                                  line_source: update_task_params[:line_source],
-                                                 line_number: update_task_params[:line_number],
+                                                 line_number: update_task_params[:line_number].to_i,
                                                  toggle_as_checked: update_task_params[:checked],
-                                                 index: update_task_params[:index],
+                                                 index: update_task_params[:index].to_i,
                                                  sourcepos: !issuable.legacy_markdown?)
 
-    if tasklist_toggler.execute
-      # by updating the description_html field at the same time,
-      # the markdown cache won't be considered invalid
-      params[:description]      = tasklist_toggler.updated_markdown
-      params[:description_html] = tasklist_toggler.updated_markdown_html
-
-      # since we're updating a very specific line, we don't care whether
-      # the `lock_version` sent from the FE is the same or not.  Just
-      # make sure the data hasn't changed since we queried it
-      params[:lock_version]     = issuable.lock_version
-
-      update_task(issuable)
-    else
+    unless tasklist_toggler.execute
       # if we make it here, the data is much newer than we thought it was - fail fast
       raise ActiveRecord::StaleObjectError
     end
+
+    # by updating the description_html field at the same time,
+    # the markdown cache won't be considered invalid
+    params[:description]      = tasklist_toggler.updated_markdown
+    params[:description_html] = tasklist_toggler.updated_markdown_html
+
+    # since we're updating a very specific line, we don't care whether
+    # the `lock_version` sent from the FE is the same or not.  Just
+    # make sure the data hasn't changed since we queried it
+    params[:lock_version]     = issuable.lock_version
+
+    update_task(issuable)
   end
 
   def labels_changing?(old_label_ids, new_label_ids)
diff --git a/app/services/task_list_toggle_service.rb b/app/services/task_list_toggle_service.rb
index 92f42a92c6b..2717fc9035a 100644
--- a/app/services/task_list_toggle_service.rb
+++ b/app/services/task_list_toggle_service.rb
@@ -23,7 +23,7 @@ class TaskListToggleService
   def execute
     return false unless markdown && markdown_html
 
-    !!(toggle_markdown && toggle_markdown_html)
+    toggle_markdown && toggle_markdown_html
   end
 
   private
diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb
index 1c2646c60a4..925e2ab0955 100644
--- a/spec/models/concerns/cache_markdown_field_spec.rb
+++ b/spec/models/concerns/cache_markdown_field_spec.rb
@@ -133,7 +133,7 @@ describe CacheMarkdownField do
     end
   end
 
-  context 'when a markdown field and html field are both set' do
+  context 'when a markdown field and html field are both changed' do
     it do
       expect(thing).not_to receive(:refresh_markdown_cache)
       thing.foo = '_look over there!_'
diff --git a/spec/services/task_list_toggle_service_spec.rb b/spec/services/task_list_toggle_service_spec.rb
index 6b92d4e14d0..750ac4c40ba 100644
--- a/spec/services/task_list_toggle_service_spec.rb
+++ b/spec/services/task_list_toggle_service_spec.rb
@@ -3,126 +3,124 @@
 require 'spec_helper'
 
 describe TaskListToggleService do
-  context 'when ' do
-    let(:sourcepos) { true }
-    let(:markdown) do
-      <<-EOT.strip_heredoc
-        * [ ] Task 1
-        * [x] Task 2
+  let(:sourcepos) { true }
+  let(:markdown) do
+    <<-EOT.strip_heredoc
+      * [ ] Task 1
+      * [x] Task 2
 
-        A paragraph
+      A paragraph
 
-        1. [X] Item 1
-           - [ ] Sub-item 1
-      EOT
+      1. [X] Item 1
+         - [ ] Sub-item 1
+    EOT
+  end
+
+  let(:markdown_html) do
+    <<-EOT.strip_heredoc
+      <ul data-sourcepos="1:1-3:0" class="task-list" dir="auto">
+        <li data-sourcepos="1:1-1:12" class="task-list-item">
+          <input type="checkbox" class="task-list-item-checkbox" disabled> Task 1
+        </li>
+        <li data-sourcepos="2:1-3:0" class="task-list-item">
+          <input type="checkbox" class="task-list-item-checkbox" disabled checked> Task 2
+        </li>
+      </ul>
+      <p data-sourcepos="4:1-4:11" dir="auto">A paragraph</p>
+      <ol data-sourcepos="6:1-7:19" class="task-list" dir="auto">
+        <li data-sourcepos="6:1-7:19" class="task-list-item">
+          <input type="checkbox" class="task-list-item-checkbox" disabled checked> Item 1
+          <ul data-sourcepos="7:4-7:19" class="task-list">
+            <li data-sourcepos="7:4-7:19" class="task-list-item">
+              <input type="checkbox" class="task-list-item-checkbox" disabled> Sub-item 1
+            </li>
+          </ul>
+        </li>
+      </ol>
+    EOT
+  end
+
+  shared_examples 'task lists' do
+    it 'checks Task 1' do
+      toggler = described_class.new(markdown, markdown_html,
+                                    index: 1, toggle_as_checked: true,
+                                    line_source: '* [ ] Task 1', line_number: 1,
+                                    sourcepos: sourcepos)
+
+      expect(toggler.execute).to be_truthy
+      expect(toggler.updated_markdown.lines[0]).to eq "* [x] Task 1\n"
+      expect(toggler.updated_markdown_html).to include('disabled checked> Task 1')
     end
 
-    let(:markdown_html) do
-      <<-EOT.strip_heredoc
-        <ul data-sourcepos="1:1-3:0" class="task-list" dir="auto">
-          <li data-sourcepos="1:1-1:12" class="task-list-item">
-            <input type="checkbox" class="task-list-item-checkbox" disabled> Task 1
-          </li>
-          <li data-sourcepos="2:1-3:0" class="task-list-item">
-            <input type="checkbox" class="task-list-item-checkbox" disabled checked> Task 2
-          </li>
-        </ul>
-        <p data-sourcepos="4:1-4:11" dir="auto">A paragraph</p>
-        <ol data-sourcepos="6:1-7:19" class="task-list" dir="auto">
-          <li data-sourcepos="6:1-7:19" class="task-list-item">
-            <input type="checkbox" class="task-list-item-checkbox" disabled checked> Item 1
-            <ul data-sourcepos="7:4-7:19" class="task-list">
-              <li data-sourcepos="7:4-7:19" class="task-list-item">
-                <input type="checkbox" class="task-list-item-checkbox" disabled> Sub-item 1
-              </li>
-            </ul>
-          </li>
-        </ol>
-      EOT
+    it 'unchecks Item 1' do
+      toggler = described_class.new(markdown, markdown_html,
+                                    index: 3, toggle_as_checked: false,
+                                    line_source: '1. [X] Item 1', line_number: 6,
+                                    sourcepos: sourcepos)
+
+      expect(toggler.execute).to be_truthy
+      expect(toggler.updated_markdown.lines[5]).to eq "1. [ ] Item 1\n"
+      expect(toggler.updated_markdown_html).to include('disabled> Item 1')
     end
 
-    shared_examples 'task lists' do
-      it 'checks Task 1' do
-        toggler = described_class.new(markdown, markdown_html,
-                                      index: 1, toggle_as_checked: true,
-                                      line_source: '* [ ] Task 1', line_number: 1,
-                                      sourcepos: sourcepos)
+    it 'returns false if line_source does not match the text' do
+      toggler = described_class.new(markdown, markdown_html,
+                                    index: 2, toggle_as_checked: false,
+                                    line_source: '* [x] Task Added', line_number: 2,
+                                    sourcepos: sourcepos)
 
-        expect(toggler.execute).to be_truthy
-        expect(toggler.updated_markdown.lines[0]).to eq "* [x] Task 1\n"
-        expect(toggler.updated_markdown_html).to include('disabled checked> Task 1')
-      end
-
-      it 'unchecks Item 1' do
-        toggler = described_class.new(markdown, markdown_html,
-                                      index: 3, toggle_as_checked: false,
-                                      line_source: '1. [X] Item 1', line_number: 6,
-                                      sourcepos: sourcepos)
-
-        expect(toggler.execute).to be_truthy
-        expect(toggler.updated_markdown.lines[5]).to eq "1. [ ] Item 1\n"
-        expect(toggler.updated_markdown_html).to include('disabled> Item 1')
-      end
-
-      it 'returns false if line_source does not match the text' do
-        toggler = described_class.new(markdown, markdown_html,
-                                      index: 2, toggle_as_checked: false,
-                                      line_source: '* [x] Task Added', line_number: 2,
-                                      sourcepos: sourcepos)
-
-        expect(toggler.execute).to be_falsey
-      end
-
-      it 'returns false if markdown is nil' do
-        toggler = described_class.new(nil, markdown_html,
-                                      index: 2, toggle_as_checked: false,
-                                      line_source: '* [x] Task Added', line_number: 2,
-                                      sourcepos: sourcepos)
-
-        expect(toggler.execute).to be_falsey
-      end
-
-      it 'returns false if markdown_html is nil' do
-        toggler = described_class.new(markdown, nil,
-                                      index: 2, toggle_as_checked: false,
-                                      line_source: '* [x] Task Added', line_number: 2,
-                                      sourcepos: sourcepos)
-
-        expect(toggler.execute).to be_falsey
-      end
+      expect(toggler.execute).to be_falsey
     end
 
-    context 'when using sourcepos' do
-      it_behaves_like 'task lists'
+    it 'returns false if markdown is nil' do
+      toggler = described_class.new(nil, markdown_html,
+                                    index: 2, toggle_as_checked: false,
+                                    line_source: '* [x] Task Added', line_number: 2,
+                                    sourcepos: sourcepos)
+
+      expect(toggler.execute).to be_falsey
     end
 
-    context 'when using checkbox indexing' do
-      let(:sourcepos) { false }
-      let(:markdown_html) do
-        <<-EOT.strip_heredoc
-        <ul class="task-list" dir="auto">
-          <li class="task-list-item">
-            <input type="checkbox" class="task-list-item-checkbox" disabled> Task 1
-          </li>
-          <li class="task-list-item">
-            <input type="checkbox" class="task-list-item-checkbox" disabled checked> Task 2
-          </li>
-        </ul>
-        <p dir="auto">A paragraph</p>
-        <ol class="task-list" dir="auto">
-          <li class="task-list-item">
-            <input type="checkbox" class="task-list-item-checkbox" disabled checked> Item 1
-            <ul class="task-list">
-              <li class="task-list-item">
-                <input type="checkbox" class="task-list-item-checkbox" disabled> Sub-item 1
-              </li>
-            </ul>
-          </li>
-        </ol>
-        EOT
-      end
+    it 'returns false if markdown_html is nil' do
+      toggler = described_class.new(markdown, nil,
+                                    index: 2, toggle_as_checked: false,
+                                    line_source: '* [x] Task Added', line_number: 2,
+                                    sourcepos: sourcepos)
 
-      it_behaves_like 'task lists'
+      expect(toggler.execute).to be_falsey
     end
   end
+
+  context 'when using sourcepos' do
+    it_behaves_like 'task lists'
+  end
+
+  context 'when using checkbox indexing' do
+    let(:sourcepos) { false }
+    let(:markdown_html) do
+      <<-EOT.strip_heredoc
+      <ul class="task-list" dir="auto">
+        <li class="task-list-item">
+          <input type="checkbox" class="task-list-item-checkbox" disabled> Task 1
+        </li>
+        <li class="task-list-item">
+          <input type="checkbox" class="task-list-item-checkbox" disabled checked> Task 2
+        </li>
+      </ul>
+      <p dir="auto">A paragraph</p>
+      <ol class="task-list" dir="auto">
+        <li class="task-list-item">
+          <input type="checkbox" class="task-list-item-checkbox" disabled checked> Item 1
+          <ul class="task-list">
+            <li class="task-list-item">
+              <input type="checkbox" class="task-list-item-checkbox" disabled> Sub-item 1
+            </li>
+          </ul>
+        </li>
+      </ol>
+      EOT
+    end
+
+    it_behaves_like 'task lists'
+  end
 end

From 863ee930a6260c42d3d0252a0310ac9adff24862 Mon Sep 17 00:00:00 2001
From: Fatih Acet <acetfatih@gmail.com>
Date: Fri, 1 Feb 2019 23:41:42 +0100
Subject: [PATCH 42/42] lockVersion should be required false and default 0

---
 app/assets/javascripts/issue_show/components/app.vue | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue
index 0fba1b3bb46..fea7f0d77a5 100644
--- a/app/assets/javascripts/issue_show/components/app.vue
+++ b/app/assets/javascripts/issue_show/components/app.vue
@@ -133,7 +133,8 @@ export default {
     },
     lockVersion: {
       type: Number,
-      required: true,
+      required: false,
+      default: 0,
     },
   },
   data() {