diff --git a/CHANGELOG b/CHANGELOG index 857a5bc9234..14bb88f22a5 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -18,6 +18,7 @@ v 7.4.0 - Add Pushover service integration (Sullivan Senechal) - Add select field type for services options (Sullivan Senechal) - Add cross-project references to the Markdown parser (Vinnie Okada) + - Add task lists to issue and merge request descriptions (Vinnie Okada) v 7.3.2 - Fix creating new file via web editor 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 = /^(? *[*-] *)\[(?[ xX])\]/.freeze + TASK_PATTERN_HTML = /^
  • \[(?[ 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/doc/markdown/markdown.md b/doc/markdown/markdown.md index 5c095ed1487..6d96da76ad7 100644 --- a/doc/markdown/markdown.md +++ b/doc/markdown/markdown.md @@ -10,6 +10,7 @@ * [Code and Syntax Highlighting](#code-and-syntax-highlighting) * [Emoji](#emoji) * [Special GitLab references](#special-gitlab-references) +* [Task lists](#task-lists) **[Standard Markdown](#standard-markdown)** @@ -183,6 +184,18 @@ GFM also recognizes references to commits, issues, and merge requests in other p - namespace/project!123 : for merge requests - namespace/project@1234567 : for commits +## Task Lists + +You can add task lists to merge request and issue descriptions to keep track of to-do items. To create a task, add an unordered list to the description in an issue or merge request, formatted like so: + +```no-highlight +* [x] Completed task +* [ ] Unfinished task + * [x] Nested task +``` + +Task lists can only be created in descriptions, not in titles or comments. Task item state can be managed by editing the description's Markdown or by clicking the rendered checkboxes. + # Standard Markdown ## Headers diff --git a/features/project/issues/issues.feature b/features/project/issues/issues.feature index ae6a03ce865..e989569ccd4 100644 --- a/features/project/issues/issues.feature +++ b/features/project/issues/issues.feature @@ -126,3 +126,36 @@ Feature: Project Issues When I click label 'bug' And I should see "Release 0.4" in issues And I should not see "Tweet control" in issues + + Scenario: Issue description should render task checkboxes + Given project "Shop" has "Tasks-open" open issue with task markdown + When I visit issue page "Tasks-open" + Then I should see task checkboxes in the description + + @javascript + Scenario: Issue notes should not render task checkboxes + Given project "Shop" has "Tasks-open" open issue with task markdown + When I visit issue page "Tasks-open" + And I leave a comment with task markdown + Then I should not see task checkboxes in the comment + + # Task status in issues list + + Scenario: Issues list should display task status + Given project "Shop" has "Tasks-open" open issue with task markdown + When I visit project "Shop" issues page + Then I should see the task status for issue "Tasks-open" + + # Toggling task items + + @javascript + Scenario: Task checkboxes should be enabled for an open issue + Given project "Shop" has "Tasks-open" open issue with task markdown + When I visit issue page "Tasks-open" + Then Task checkboxes should be enabled + + @javascript + Scenario: Task checkboxes should be disabled for a closed issue + Given project "Shop" has "Tasks-closed" closed issue with task markdown + When I visit issue page "Tasks-closed" + Then Task checkboxes should be disabled diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature index 60302c44ddb..e1e0edd0545 100644 --- a/features/project/merge_requests.feature +++ b/features/project/merge_requests.feature @@ -96,6 +96,16 @@ Feature: Project Merge Requests And I leave a comment with a header containing "Comment with a header" Then The comment with the header should not have an ID + Scenario: Merge request description should render task checkboxes + Given project "Shop" has "MR-task-open" open MR with task markdown + When I visit merge request page "MR-task-open" + Then I should see task checkboxes in the description + + Scenario: Merge request notes should not render task checkboxes + Given project "Shop" has "MR-task-open" open MR with task markdown + When I visit merge request page "MR-task-open" + Then I should not see task checkboxes in the comment + # Toggling inline comments @javascript @@ -155,3 +165,26 @@ Feature: Project Merge Requests And I leave a comment like "Line is wrong" on line 39 of the second file And I click Side-by-side Diff tab Then I should see comments on the side-by-side diff page + + # Task status in issues list + + @now + Scenario: Merge requests list should display task status + Given project "Shop" has "MR-task-open" open MR with task markdown + When I visit project "Shop" merge requests page + Then I should see the task status for merge request "MR-task-open" + + # Toggling task items + + @javascript + Scenario: Task checkboxes should be enabled for an open merge request + Given project "Shop" has "MR-task-open" open MR with task markdown + When I visit merge request page "MR-task-open" + Then Task checkboxes should be enabled + + @javascript + Scenario: Task checkboxes should be disabled for a closed merge request + Given project "Shop" has "MR-task-open" open MR with task markdown + And I visit merge request page "MR-task-open" + And I click link "Close" + Then Task checkboxes should be disabled diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb index b55b3c6c8a2..26c3c7c14bc 100644 --- a/features/steps/project/issues/issues.rb +++ b/features/steps/project/issues/issues.rb @@ -153,6 +153,32 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps author: project.users.first) end + step 'project "Shop" has "Tasks-open" open issue with task markdown' do + desc_text = <$/, '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 diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index 73b3d91e96e..15033f07432 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -616,7 +616,7 @@ describe GitlabMarkdownHelper do end end - describe "markdwon for empty repository" do + describe 'markdown for empty repository' do before do @project = empty_project @repository = empty_project.repository @@ -652,4 +652,103 @@ describe GitlabMarkdownHelper do helper.render_wiki_content(@wiki) end end + + describe '#gfm_with_tasks' do + before(:all) do + @source_text_asterisk = <(txt){ subject.description = txt } } end + + it_behaves_like 'a Taskable' do + let(:subject) { create :issue } + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index c40f75290ed..7b0d261d72f 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -119,4 +119,8 @@ describe MergeRequest do let(:backref_text) { "merge request !#{subject.iid}" } let(:set_mentionable_text) { ->(txt){ subject.title = txt } } end + + it_behaves_like 'a Taskable' do + let(:subject) { create :merge_request, :simple } + end end diff --git a/spec/support/taskable_shared_examples.rb b/spec/support/taskable_shared_examples.rb new file mode 100644 index 00000000000..42252675683 --- /dev/null +++ b/spec/support/taskable_shared_examples.rb @@ -0,0 +1,42 @@ +# Specs for task state functionality for issues and merge requests. +# +# Requires a context containing: +# let(:subject) { Issue or MergeRequest } +shared_examples 'a Taskable' do + before do + subject.description = <