Merge branch 'task_list_refactor' into 'master'

move task_list setup and events to own class

See merge request !9016

app/assets/javascripts/issue.js

@@ -3,7 +3,7 @@
 
 require('./flash');
 require('vendor/jquery.waitforimages');
-require('vendor/task_list');
+require('./task_list');
 
 (function() {
   var bind = function(fn, me) { return function() { return fn.apply(me, arguments); }; };
@@ -11,10 +11,16 @@ require('vendor/task_list');
   this.Issue = (function() {
     function Issue() {
       this.submitNoteForm = bind(this.submitNoteForm, this);
-      // Prevent duplicate event bindings
-      this.disableTaskList();
       if ($('a.btn-close').length) {
-        this.initTaskList();
+        this.taskList = new gl.TaskList({
+          dataType: 'issue',
+          fieldName: 'description',
+          selector: '.detail-page-description',
+          onSuccess: (result) => {
+            document.querySelector('#task_status').innerText = result.task_status;
+            document.querySelector('#task_status_short').innerText = result.task_status_short;
+          }
+        });
         this.initIssueBtnEventListeners();
       }
       this.initMergeRequests();
@@ -22,11 +28,6 @@ require('vendor/task_list');
       this.initCanCreateBranch();
     }
 
-    Issue.prototype.initTaskList = function() {
-      $('.detail-page-description .js-task-list-container').taskList('enable');
-      return $(document).on('tasklist:changed', '.detail-page-description .js-task-list-container', this.updateTaskList);
-    };
-
     Issue.prototype.initIssueBtnEventListeners = function() {
       var _this, issueFailMessage;
       _this = this;
@@ -85,30 +86,6 @@ require('vendor/task_list');
       }
     };
 
-    Issue.prototype.disableTaskList = function() {
-      $('.detail-page-description .js-task-list-container').taskList('disable');
-      return $(document).off('tasklist:changed', '.detail-page-description .js-task-list-container');
-    };
-
-    Issue.prototype.updateTaskList = function() {
-      var patchData;
-      patchData = {};
-      patchData['issue'] = {
-        'description': $('.js-task-list-field', this).val()
-      };
-      return $.ajax({
-        type: 'PATCH',
-        url: $('form.js-issuable-update').attr('action'),
-        data: patchData,
-        success: function(issue) {
-          document.querySelector('#task_status').innerText = issue.task_status;
-          document.querySelector('#task_status_short').innerText = issue.task_status_short;
-        }
-      });
-    // TODO (rspeicher): Make the issue description inline-editable like a note so
-    // that we can re-use its form here
-    };
-
     Issue.prototype.initMergeRequests = function() {
       var $container;
       $container = $('#merge-requests');

app/assets/javascripts/merge_request.js

@@ -2,7 +2,7 @@
 /* global MergeRequestTabs */
 
 require('vendor/jquery.waitforimages');
-require('vendor/task_list');
+require('./task_list');
 require('./merge_request_tabs');
 
 (function() {
@@ -24,12 +24,18 @@ require('./merge_request_tabs');
         };
       })(this));
       this.initTabs();
-      // Prevent duplicate event bindings
-      this.disableTaskList();
       this.initMRBtnListeners();
       this.initCommitMessageListeners();
       if ($("a.btn-close").length) {
-        this.initTaskList();
+        this.taskList = new gl.TaskList({
+          dataType: 'merge_request',
+          fieldName: 'description',
+          selector: '.detail-page-description',
+          onSuccess: (result) => {
+            document.querySelector('#task_status').innerText = result.task_status;
+            document.querySelector('#task_status_short').innerText = result.task_status_short;
+          }
+        });
       }
     }
 
@@ -50,11 +56,6 @@ require('./merge_request_tabs');
       return this.$('.all-commits').removeClass('hide');
     };
 
-    MergeRequest.prototype.initTaskList = function() {
-      $('.detail-page-description .js-task-list-container').taskList('enable');
-      return $(document).on('tasklist:changed', '.detail-page-description .js-task-list-container', this.updateTaskList);
-    };
-
     MergeRequest.prototype.initMRBtnListeners = function() {
       var _this;
       _this = this;
@@ -85,30 +86,6 @@ require('./merge_request_tabs');
       }
     };
 
-    MergeRequest.prototype.disableTaskList = function() {
-      $('.detail-page-description .js-task-list-container').taskList('disable');
-      return $(document).off('tasklist:changed', '.detail-page-description .js-task-list-container');
-    };
-
-    MergeRequest.prototype.updateTaskList = function() {
-      var patchData;
-      patchData = {};
-      patchData['merge_request'] = {
-        'description': $('.js-task-list-field', this).val()
-      };
-      return $.ajax({
-        type: 'PATCH',
-        url: $('form.js-issuable-update').attr('action'),
-        data: patchData,
-        success: function(mergeRequest) {
-          document.querySelector('#task_status').innerText = mergeRequest.task_status;
-          document.querySelector('#task_status_short').innerText = mergeRequest.task_status_short;
-        }
-      });
-    // TODO (rspeicher): Make the merge request description inline-editable like a
-    // note so that we can re-use its form here
-    };
-
     MergeRequest.prototype.initCommitMessageListeners = function() {
       $(document).on('click', 'a.js-with-description-link', function(e) {
         var textarea = $('textarea.js-commit-message');

app/assets/javascripts/notes.js

@@ -11,7 +11,7 @@ require('./dropzone_input');
 require('./gfm_auto_complete');
 require('vendor/jquery.caret'); // required by jquery.atwho
 require('vendor/jquery.atwho');
-require('vendor/task_list');
+require('./task_list');
 
 (function() {
   var bind = function(fn, me) { return function() { return fn.apply(me, arguments); }; };
@@ -51,7 +51,11 @@ require('vendor/task_list');
       this.addBinding();
       this.setPollingInterval();
       this.setupMainTargetNoteForm();
-      this.initTaskList();
+      this.taskList = new gl.TaskList({
+        dataType: 'note',
+        fieldName: 'note',
+        selector: '.notes'
+      });
       this.collapseLongCommitList();
 
       // We are in the Merge Requests page so we need another edit form for Changes tab
@@ -125,8 +129,6 @@ require('vendor/task_list');
       $(document).off("keydown", ".js-note-text");
       $(document).off('click', '.js-comment-resolve-button');
       $(document).off("click", '.system-note-commit-list-toggler');
-      $('.note .js-task-list-container').taskList('disable');
-      return $(document).off('tasklist:changed', '.note .js-task-list-container');
     };
 
     Notes.prototype.keydownNoteText = function(e) {
@@ -286,7 +288,7 @@ require('vendor/task_list');
         // Update datetime format on the recent note
         gl.utils.localTimeAgo($notesList.find("#note_" + note.id + " .js-timeago"), false);
         this.collapseLongCommitList();
-        this.initTaskList();
+        this.taskList.init();
         this.refresh();
         return this.updateNotesCount(1);
       }
@@ -863,15 +865,6 @@ require('vendor/task_list');
       }
     };
 
-    Notes.prototype.initTaskList = function() {
-      this.enableTaskList();
-      return $(document).on('tasklist:changed', '.note .js-task-list-container', this.updateTaskList.bind(this));
-    };
-
-    Notes.prototype.enableTaskList = function() {
-      return $('.note .js-task-list-container').taskList('enable');
-    };
-
     Notes.prototype.putEditFormInPlace = function($el) {
       var $editForm = $(this.getEditFormSelector($el));
       var $note = $el.closest('.note');
@@ -896,17 +889,6 @@ require('vendor/task_list');
       $editForm.find('.referenced-users').hide();
     };
 
-    Notes.prototype.updateTaskList = function(e) {
-      var $target = $(e.target);
-      var $list = $target.closest('.js-task-list-container');
-      var $editForm = $(this.getEditFormSelector($target));
-      var $note = $list.closest('.note');
-
-      this.putEditFormInPlace($list);
-      $editForm.find('#note_note').val($note.find('.original-task-list').val());
-      $('form', $list).submit();
-    };
-
     Notes.prototype.updateNotesCount = function(updateCount) {
       return this.notesCountBadge.text(parseInt(this.notesCountBadge.text(), 10) + updateCount);
     };

app/assets/javascripts/task_list.js

+require('vendor/task_list');
+
+class TaskList {
+  constructor(options = {}) {
+    this.selector = options.selector;
+    this.dataType = options.dataType;
+    this.fieldName = options.fieldName;
+    this.onSuccess = options.onSuccess || (() => {});
+    this.init();
+  }
+
+  init() {
+    // Prevent duplicate event bindings
+    this.disable();
+    $(`${this.selector} .js-task-list-container`).taskList('enable');
+    $(document).on('tasklist:changed', `${this.selector} .js-task-list-container`, this.update.bind(this));
+  }
+
+  disable() {
+    $(`${this.selector} .js-task-list-container`).taskList('disable');
+    $(document).off('tasklist:changed', `${this.selector} .js-task-list-container`);
+  }
+
+  update(e) {
+    const $target = $(e.target);
+    const patchData = {};
+    patchData[this.dataType] = {
+      [this.fieldName]: $target.val(),
+    };
+    return $.ajax({
+      type: 'PATCH',
+      url: $target.data('update-url') || $('form.js-issuable-update').attr('action'),
+      data: patchData,
+      success: this.onSuccess,
+    });
+  }
+}
+
+window.gl = window.gl || {};
+window.gl.TaskList = TaskList;

app/views/projects/notes/_note.html.haml

@@ -71,7 +71,7 @@
         - if note_editable
           .original-note-content.hidden{ data: { post_url: namespace_project_note_path(@project.namespace, @project, note), target_id: note.noteable.id, target_type: note.noteable.class.name.underscore } }
             #{note.note}
-          %textarea.hidden.js-task-list-field.original-task-list= note.note
+          %textarea.hidden.js-task-list-field.original-task-list{ data: {update_url: namespace_project_note_path(@project.namespace, @project, note) } }= note.note
         .note-awards
           = render 'award_emoji/awards_block', awardable: note, inline: false
         - if note.system

changelogs/unreleased/task_list_refactor.yml

+---
+title: Deduplicate markdown task lists
+merge_request:
+author:

spec/javascripts/merge_request_spec.js

@@ -6,9 +6,9 @@ require('~/merge_request');
 (function() {
   describe('MergeRequest', function() {
     return describe('task lists', function() {
-      preloadFixtures('static/merge_requests_show.html.raw');
+      preloadFixtures('merge_requests/merge_request_with_task_list.html.raw');
       beforeEach(function() {
-        loadFixtures('static/merge_requests_show.html.raw');
+        loadFixtures('merge_requests/merge_request_with_task_list.html.raw');
         return this.merge = new MergeRequest();
       });
       it('modifies the Markdown field', function() {
@@ -19,7 +19,7 @@ require('~/merge_request');
       return it('submits an ajax request on tasklist:changed', function() {
         spyOn(jQuery, 'ajax').and.callFake(function(req) {
           expect(req.type).toBe('PATCH');
-          expect(req.url).toBe('/foo');
+          expect(req.url).toBe(`${gl.TEST_HOST}/frontend-fixtures/merge-requests-project/merge_requests/1.json`);
           return expect(req.data.merge_request.description).not.toBe(null);
         });
         return $('.js-task-list-field').trigger('tasklist:changed');

spec/javascripts/notes_spec.js

@@ -35,15 +35,13 @@ require('~/lib/utils/text_utility');
         expect($('.js-task-list-field').val()).toBe('- [x] Task List Item');
       });
 
-      it('submits the form on tasklist:changed', function() {
-        var submitted = false;
-        $('form').on('submit', function(e) {
-          submitted = true;
-          e.preventDefault();
+      it('submits an ajax request on tasklist:changed', function() {
+        spyOn(jQuery, 'ajax').and.callFake(function(req) {
+          expect(req.type).toBe('PATCH');
+          expect(req.url).toBe('http://test.host/frontend-fixtures/issues-project/notes/1');
+          return expect(req.data.note).not.toBe(null);
         });
-
         $('.js-task-list-field').trigger('tasklist:changed');
-        expect(submitted).toBe(true);
       });
     });