From 9f0083a96c03ec22b1d9442a9c7530899e633301 Mon Sep 17 00:00:00 2001
From: Vinnie Okada <vokada@mrvinn.com>
Date: Sun, 5 Oct 2014 00:53:44 -0500
Subject: [PATCH] Add task lists to issues and merge requests
Make the Markdown parser recognize "[x]" or "[ ]" at the beginning of a
list item and turn it into a checkbox input. Users who can modify the
issue or MR can toggle the checkboxes directly or edit the Markdown to
manage the tasks. Task status is also displayed in the MR and issue
lists.
---
app/assets/javascripts/issue.js.coffee | 24 +++++++++
.../javascripts/merge_request.js.coffee | 23 +++++++++
app/assets/stylesheets/generic/common.scss | 3 ++
app/assets/stylesheets/generic/lists.scss | 4 ++
app/controllers/projects/issues_controller.rb | 2 +-
.../projects/merge_requests_controller.rb | 2 +-
app/models/concerns/taskable.rb | 51 +++++++++++++++++++
app/models/issue.rb | 1 +
app/models/merge_request.rb | 1 +
app/services/issues/update_service.rb | 14 ++++-
app/services/merge_requests/update_service.rb | 8 ++-
app/views/projects/issues/_issue.html.haml | 4 ++
app/views/projects/issues/show.html.haml | 2 +-
.../merge_requests/_merge_request.html.haml | 4 +-
.../merge_requests/show/_mr_box.html.haml | 2 +-
lib/gitlab/markdown.rb | 24 +++++++++
lib/redcarpet/render/gitlab_html.rb | 6 ++-
17 files changed, 167 insertions(+), 8 deletions(-)
create mode 100644 app/models/concerns/taskable.rb
diff --git a/app/assets/javascripts/issue.js.coffee b/app/assets/javascripts/issue.js.coffee
index 36935a0a159..f2b531fb2b1 100644
--- a/app/assets/javascripts/issue.js.coffee
+++ b/app/assets/javascripts/issue.js.coffee
@@ -6,4 +6,28 @@ class Issue
$(".issue-box .inline-update").on "change", "#issue_assignee_id", ->
$(this).submit()
+ if $("a.btn-close").length
+ $("li.task-list-item input:checkbox").prop("disabled", false)
+
+ $(".task-list-item input:checkbox").on "click", ->
+ is_checked = $(this).prop("checked")
+ if $(this).is(":checked")
+ state_event = "task_check"
+ else
+ state_event = "task_uncheck"
+
+ mr_url = $("form.edit-issue").first().attr("action")
+ mr_num = mr_url.match(/\d+$/)
+ task_num = 0
+ $("li.task-list-item input:checkbox").each( (index, e) =>
+ if e == this
+ task_num = index + 1
+ )
+
+ $.ajax
+ type: "PATCH"
+ url: mr_url
+ data: "issue[state_event]=" + state_event +
+ "&issue[task_num]=" + task_num
+
@Issue = Issue
diff --git a/app/assets/javascripts/merge_request.js.coffee b/app/assets/javascripts/merge_request.js.coffee
index 4c9f20ae6fa..203c721c30c 100644
--- a/app/assets/javascripts/merge_request.js.coffee
+++ b/app/assets/javascripts/merge_request.js.coffee
@@ -17,6 +17,8 @@ class MergeRequest
disableButtonIfEmptyField '#commit_message', '.accept_merge_request'
+ if $("a.close-mr-link").length
+ $("li.task-list-item input:checkbox").prop("disabled", false)
# Local jQuery finder
$: (selector) ->
@@ -72,6 +74,27 @@ class MergeRequest
this.$('.remove_source_branch_in_progress').hide()
this.$('.remove_source_branch_widget.failed').show()
+ this.$(".task-list-item input:checkbox").on "click", ->
+ is_checked = $(this).prop("checked")
+ if $(this).is(":checked")
+ state_event = "task_check"
+ else
+ state_event = "task_uncheck"
+
+ mr_url = $("form.edit-merge_request").first().attr("action")
+ mr_num = mr_url.match(/\d+$/)
+ task_num = 0
+ $("li.task-list-item input:checkbox").each( (index, e) =>
+ if e == this
+ task_num = index + 1
+ )
+
+ $.ajax
+ type: "PATCH"
+ url: mr_url
+ data: "merge_request[state_event]=" + state_event +
+ "&merge_request[task_num]=" + task_num
+
activateTab: (action) ->
this.$('.merge-request-tabs li').removeClass 'active'
this.$('.tab-content').hide()
diff --git a/app/assets/stylesheets/generic/common.scss b/app/assets/stylesheets/generic/common.scss
index 803219a2e86..cd2f4e45e3c 100644
--- a/app/assets/stylesheets/generic/common.scss
+++ b/app/assets/stylesheets/generic/common.scss
@@ -356,3 +356,6 @@ table {
font-size: 42px;
}
+.task-status {
+ margin-left: 10px;
+}
diff --git a/app/assets/stylesheets/generic/lists.scss b/app/assets/stylesheets/generic/lists.scss
index d347ab2c2e4..2653bfbf831 100644
--- a/app/assets/stylesheets/generic/lists.scss
+++ b/app/assets/stylesheets/generic/lists.scss
@@ -122,3 +122,7 @@ ul.bordered-list {
}
}
}
+
+li.task-list-item {
+ list-style-type: none;
+}
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 9e7a55b23fd..c6d526f05c5 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -152,7 +152,7 @@ class Projects::IssuesController < Projects::ApplicationController
def issue_params
params.require(:issue).permit(
:title, :assignee_id, :position, :description,
- :milestone_id, :state_event, label_ids: []
+ :milestone_id, :state_event, :task_num, label_ids: []
)
end
end
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index e13773d6465..20a733b10e1 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -250,7 +250,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
params.require(:merge_request).permit(
:title, :assignee_id, :source_project_id, :source_branch,
:target_project_id, :target_branch, :milestone_id,
- :state_event, :description, label_ids: []
+ :state_event, :description, :task_num, label_ids: []
)
end
end
diff --git a/app/models/concerns/taskable.rb b/app/models/concerns/taskable.rb
new file mode 100644
index 00000000000..410e8dc820b
--- /dev/null
+++ b/app/models/concerns/taskable.rb
@@ -0,0 +1,51 @@
+# Contains functionality for objects that can have task lists in their
+# descriptions. Task list items can be added with Markdown like "* [x] Fix
+# bugs".
+#
+# Used by MergeRequest and Issue
+module Taskable
+ TASK_PATTERN_MD = /^(?<bullet> *[*-] *)\[(?<checked>[ xX])\]/.freeze
+ TASK_PATTERN_HTML = /^<li>\[(?<checked>[ xX])\]/.freeze
+
+ # Change the state of a task list item for this Taskable. Edit the object's
+ # description by finding the nth task item and changing its checkbox
+ # placeholder to "[x]" if +checked+ is true, or "[ ]" if it's false.
+ # Note: task numbering starts with 1
+ def update_nth_task(n, checked)
+ index = 0
+ check_char = checked ? 'x' : ' '
+
+ # Do this instead of using #gsub! so that ActiveRecord detects that a field
+ # has changed.
+ self.description = self.description.gsub(TASK_PATTERN_MD) do |match|
+ index += 1
+ case index
+ when n then "#{$LAST_MATCH_INFO[:bullet]}[#{check_char}]"
+ else match
+ end
+ end
+
+ save
+ end
+
+ # Return true if this object's description has any task list items.
+ def tasks?
+ description && description.match(TASK_PATTERN_MD)
+ end
+
+ # Return a string that describes the current state of this Taskable's task
+ # list items, e.g. "20 tasks (12 done, 8 unfinished)"
+ def task_status
+ return nil unless description
+
+ num_tasks = 0
+ num_done = 0
+
+ description.scan(TASK_PATTERN_MD) do
+ num_tasks += 1
+ num_done += 1 unless $LAST_MATCH_INFO[:checked] == ' '
+ end
+
+ "#{num_tasks} tasks (#{num_done} done, #{num_tasks - num_done} unfinished)"
+ end
+end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 13152fdf94e..8a9e969248c 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -23,6 +23,7 @@ require 'file_size_validator'
class Issue < ActiveRecord::Base
include Issuable
include InternalId
+ include Taskable
ActsAsTaggableOn.strict_case_match = true
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index e0358c1889c..7c525b02f48 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -25,6 +25,7 @@ require Rails.root.join("lib/static_model")
class MergeRequest < ActiveRecord::Base
include Issuable
+ include Taskable
include InternalId
belongs_to :target_project, foreign_key: :target_project_id, class_name: "Project"
diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb
index a0e57144435..5b2746ffecf 100644
--- a/app/services/issues/update_service.rb
+++ b/app/services/issues/update_service.rb
@@ -8,9 +8,14 @@ module Issues
Issues::ReopenService.new(project, current_user, {}).execute(issue)
when 'close'
Issues::CloseService.new(project, current_user, {}).execute(issue)
+ when 'task_check'
+ issue.update_nth_task(params[:task_num].to_i, true)
+ when 'task_uncheck'
+ issue.update_nth_task(params[:task_num].to_i, false)
end
- if params.present? && issue.update_attributes(params.except(:state_event))
+ if params.present? && issue.update_attributes(params.except(:state_event,
+ :task_num))
issue.reset_events_cache
if issue.previous_changes.include?('milestone_id')
@@ -28,5 +33,12 @@ module Issues
issue
end
+
+ private
+
+ def update_task(issue, params, checked)
+ issue.update_nth_task(params[:task_num].to_i, checked)
+ params.except!(:task_num)
+ end
end
end
diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb
index 6e416a0080c..fc26619cd17 100644
--- a/app/services/merge_requests/update_service.rb
+++ b/app/services/merge_requests/update_service.rb
@@ -17,9 +17,15 @@ module MergeRequests
MergeRequests::ReopenService.new(project, current_user, {}).execute(merge_request)
when 'close'
MergeRequests::CloseService.new(project, current_user, {}).execute(merge_request)
+ when 'task_check'
+ merge_request.update_nth_task(params[:task_num].to_i, true)
+ when 'task_uncheck'
+ merge_request.update_nth_task(params[:task_num].to_i, false)
end
- if params.present? && merge_request.update_attributes(params.except(:state_event))
+ if params.present? && merge_request.update_attributes(
+ params.except(:state_event, :task_num)
+ )
merge_request.reset_events_cache
if merge_request.previous_changes.include?('milestone_id')
diff --git a/app/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml
index e089b5fa1cf..b125706781c 100644
--- a/app/views/projects/issues/_issue.html.haml
+++ b/app/views/projects/issues/_issue.html.haml
@@ -26,6 +26,10 @@
%span
%i.fa.fa-clock-o
= issue.milestone.title
+ - if issue.tasks?
+ %span.task-status
+ = issue.task_status
+
.pull-right
%small updated #{time_ago_with_tooltip(issue.updated_at, 'bottom', 'issue_update_ago')}
diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml
index 4c1ea098d98..e1849b3f8b8 100644
--- a/app/views/projects/issues/show.html.haml
+++ b/app/views/projects/issues/show.html.haml
@@ -48,7 +48,7 @@
.description
.wiki
= preserve do
- = markdown @issue.description
+ = markdown(@issue.description, parse_tasks: true)
.context
%cite.cgray
= render partial: 'issue_context', locals: { issue: @issue }
diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml
index 647e8873e9e..1ee2e1bdae8 100644
--- a/app/views/projects/merge_requests/_merge_request.html.haml
+++ b/app/views/projects/merge_requests/_merge_request.html.haml
@@ -27,7 +27,9 @@
%span
%i.fa.fa-clock-o
= merge_request.milestone.title
-
+ - if merge_request.tasks?
+ %span.task-status
+ = merge_request.task_status
.pull-right
%small updated #{time_ago_with_tooltip(merge_request.updated_at, 'bottom', 'merge_request_updated_ago')}
diff --git a/app/views/projects/merge_requests/show/_mr_box.html.haml b/app/views/projects/merge_requests/show/_mr_box.html.haml
index f1aaba2010d..7e5a4eda508 100644
--- a/app/views/projects/merge_requests/show/_mr_box.html.haml
+++ b/app/views/projects/merge_requests/show/_mr_box.html.haml
@@ -18,7 +18,7 @@
.description
.wiki
= preserve do
- = markdown @merge_request.description
+ = markdown(@merge_request.description, parse_tasks: true)
.context
%cite.cgray
diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb
index 709a74fe21e..17512a51658 100644
--- a/lib/gitlab/markdown.rb
+++ b/lib/gitlab/markdown.rb
@@ -33,6 +33,11 @@ module Gitlab
attr_reader :html_options
+ def gfm_with_tasks(text, project = @project, html_options = {})
+ text = gfm(text, project, html_options)
+ parse_tasks(text)
+ end
+
# Public: Parse the provided text with GitLab-Flavored Markdown
#
# text - the source text
@@ -265,5 +270,24 @@ module Gitlab
)
link_to("#{prefix_text}##{identifier}", url, options)
end
+
+ # Turn list items that start with "[ ]" into HTML checkbox inputs.
+ def parse_tasks(text)
+ li_tag = '<li class="task-list-item">'
+ unchecked_box = '<input type="checkbox" value="on" disabled />'
+ checked_box = unchecked_box.sub(/\/>$/, 'checked="checked" />')
+
+ # Regexp captures don't seem to work when +text+ is an
+ # ActiveSupport::SafeBuffer, hence the `String.new`
+ String.new(text).gsub(Taskable::TASK_PATTERN_HTML) do
+ checked = $LAST_MATCH_INFO[:checked].downcase == 'x'
+
+ if checked
+ "#{li_tag}#{checked_box}"
+ else
+ "#{li_tag}#{unchecked_box}"
+ end
+ end
+ end
end
end
diff --git a/lib/redcarpet/render/gitlab_html.rb b/lib/redcarpet/render/gitlab_html.rb
index bb225f1acd8..c3378d6a18f 100644
--- a/lib/redcarpet/render/gitlab_html.rb
+++ b/lib/redcarpet/render/gitlab_html.rb
@@ -47,6 +47,10 @@ class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML
unless @template.instance_variable_get("@project_wiki") || @project.nil?
full_document = h.create_relative_links(full_document)
end
- h.gfm(full_document)
+ if @options[:parse_tasks]
+ h.gfm_with_tasks(full_document)
+ else
+ h.gfm(full_document)
+ end
end
end
--
GitLab