Merge pull request #7964 from mr-vinn/task-lists

Add task lists to issues and merge requests
This commit is contained in:
Marin Jankovski 2014-10-06 17:29:06 +02:00
commit 0fed1b5876
30 changed files with 501 additions and 9 deletions

View File

@ -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

View File

@ -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

View File

@ -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()

View File

@ -356,3 +356,6 @@ table {
font-size: 42px;
}
.task-status {
margin-left: 10px;
}

View File

@ -122,3 +122,7 @@ ul.bordered-list {
}
}
}
li.task-list-item {
list-style-type: none;
}

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -23,6 +23,7 @@ require 'file_size_validator'
class Issue < ActiveRecord::Base
include Issuable
include InternalId
include Taskable
ActsAsTaggableOn.strict_case_match = true

View File

@ -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"

View File

@ -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

View File

@ -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')

View File

@ -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')}

View File

@ -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 }

View File

@ -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')}

View File

@ -18,7 +18,7 @@
.description
.wiki
= preserve do
= markdown @merge_request.description
= markdown(@merge_request.description, parse_tasks: true)
.context
%cite.cgray

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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 = <<EOT.gsub(/^ {6}/, '')
* [ ] Task 1
* [x] Task 2
EOT
create(:issue,
title: 'Tasks-open',
project: project,
author: project.users.first,
description: desc_text
)
end
step 'project "Shop" has "Tasks-closed" closed issue with task markdown' do
desc_text = <<EOT.gsub(/^ {6}/, '')
* [ ] Task 1
* [x] Task 2
EOT
create(:closed_issue,
title: 'Tasks-closed',
project: project,
author: project.users.first,
description: desc_text
)
end
step 'empty project "Empty Project"' do
create :empty_project, name: 'Empty Project', namespace: @user.namespace
end

View File

@ -97,6 +97,20 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
author: project.users.first)
end
step 'project "Shop" has "MR-task-open" open MR with task markdown' do
desc_text = <<EOT.gsub(/^ {6}/, '')
* [ ] Task 1
* [x] Task 2
EOT
create(:merge_request,
title: 'MR-task-open',
source_project: project,
target_project: project,
author: project.users.first,
description: desc_text
)
end
step 'I switch to the diff tab' do
visit diffs_project_merge_request_path(project, merge_request)
end

View File

@ -9,4 +9,34 @@ module SharedMarkdown
step 'Header "Description header" should have correct id and link' do
header_should_have_correct_id_and_link(1, 'Description header', 'description-header')
end
step 'I should see task checkboxes in the description' do
expect(page).to have_selector(
'div.description li.task-list-item input[type="checkbox"]'
)
end
step 'I should see the task status for issue "Tasks-open"' do
expect(find(:css, 'span.task-status').text).to eq(
'2 tasks (1 done, 1 unfinished)'
)
end
step 'I should see the task status for merge request "MR-task-open"' do
expect(find(:css, 'span.task-status').text).to eq(
'2 tasks (1 done, 1 unfinished)'
)
end
step 'Task checkboxes should be enabled' do
expect(page).to have_selector(
'div.description li.task-list-item input[type="checkbox"]:enabled'
)
end
step 'Task checkboxes should be disabled' do
expect(page).to have_selector(
'div.description li.task-list-item input[type="checkbox"]:disabled'
)
end
end

View File

@ -119,4 +119,18 @@ module SharedNote
page.should_not have_css("#comment-with-a-header")
end
end
step 'I leave a comment with task markdown' do
within('.js-main-target-form') do
fill_in 'note[note]', with: '* [x] Task item'
click_button 'Add Comment'
sleep 0.05
end
end
step 'I should not see task checkboxes in the comment' do
expect(page).not_to have_selector(
'li.note div.timeline-content input[type="checkbox"]'
)
end
end

View File

@ -292,6 +292,16 @@ module SharedPaths
visit project_issue_path(issue.project, issue)
end
step 'I visit issue page "Tasks-open"' do
issue = Issue.find_by(title: 'Tasks-open')
visit project_issue_path(issue.project, issue)
end
step 'I visit issue page "Tasks-closed"' do
issue = Issue.find_by(title: 'Tasks-closed')
visit project_issue_path(issue.project, issue)
end
step 'I visit project "Shop" labels page' do
project = Project.find_by(name: 'Shop')
visit project_labels_path(project)
@ -322,6 +332,16 @@ module SharedPaths
visit project_merge_request_path(mr.target_project, mr)
end
step 'I visit merge request page "MR-task-open"' do
mr = MergeRequest.find_by(title: 'MR-task-open')
visit project_merge_request_path(mr.target_project, mr)
end
step 'I visit merge request page "MR-task-closed"' do
mr = MergeRequest.find_by(title: 'MR-task-closed')
visit project_merge_request_path(mr.target_project, mr)
end
step 'I visit project "Shop" merge requests page' do
visit project_merge_requests_path(project)
end

View File

@ -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

View File

@ -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

View File

@ -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 = <<EOT.gsub(/^\s{8}/, '')
* [ ] valid unchecked task
* [x] valid lowercase checked task
* [X] valid uppercase checked task
* [ ] valid unchecked nested task
* [x] valid checked nested task
[ ] not an unchecked task - no list item
[x] not a checked task - no list item
* [ ] not an unchecked task - too many spaces
* [x ] not a checked task - too many spaces
* [] not an unchecked task - no spaces
* Not a task [ ] - not at beginning
EOT
@source_text_dash = <<EOT.gsub(/^\s{8}/, '')
- [ ] valid unchecked task
- [x] valid lowercase checked task
- [X] valid uppercase checked task
- [ ] valid unchecked nested task
- [x] valid checked nested task
EOT
end
it 'should render checkboxes at beginning of asterisk list items' do
rendered_text = markdown(@source_text_asterisk, parse_tasks: true)
expect(rendered_text).to match(/<input.*checkbox.*valid unchecked task/)
expect(rendered_text).to match(
/<input.*checkbox.*valid lowercase checked task/
)
expect(rendered_text).to match(
/<input.*checkbox.*valid uppercase checked task/
)
end
it 'should render checkboxes at beginning of dash list items' do
rendered_text = markdown(@source_text_dash, parse_tasks: true)
expect(rendered_text).to match(/<input.*checkbox.*valid unchecked task/)
expect(rendered_text).to match(
/<input.*checkbox.*valid lowercase checked task/
)
expect(rendered_text).to match(
/<input.*checkbox.*valid uppercase checked task/
)
end
it 'should not be confused by whitespace before bullets' do
rendered_text_asterisk = markdown(@source_text_asterisk,
parse_tasks: true)
rendered_text_dash = markdown(@source_text_dash, parse_tasks: true)
expect(rendered_text_asterisk).to match(
/<input.*checkbox.*valid unchecked nested task/
)
expect(rendered_text_asterisk).to match(
/<input.*checkbox.*valid checked nested task/
)
expect(rendered_text_dash).to match(
/<input.*checkbox.*valid unchecked nested task/
)
expect(rendered_text_dash).to match(
/<input.*checkbox.*valid checked nested task/
)
end
it 'should not render checkboxes outside of list items' do
rendered_text = markdown(@source_text_asterisk, parse_tasks: true)
expect(rendered_text).not_to match(
/<input.*checkbox.*not an unchecked task - no list item/
)
expect(rendered_text).not_to match(
/<input.*checkbox.*not a checked task - no list item/
)
end
it 'should not render checkboxes with invalid formatting' do
rendered_text = markdown(@source_text_asterisk, parse_tasks: true)
expect(rendered_text).not_to match(
/<input.*checkbox.*not an unchecked task - too many spaces/
)
expect(rendered_text).not_to match(
/<input.*checkbox.*not a checked task - too many spaces/
)
expect(rendered_text).not_to match(
/<input.*checkbox.*not an unchecked task - no spaces/
)
expect(rendered_text).not_to match(
/Not a task.*<input.*checkbox.*not at beginning/
)
end
end
end

View File

@ -60,4 +60,8 @@ describe Issue do
let(:backref_text) { "issue ##{subject.iid}" }
let(:set_mentionable_text) { ->(txt){ subject.description = txt } }
end
it_behaves_like 'a Taskable' do
let(:subject) { create :issue }
end
end

View File

@ -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

View File

@ -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 = <<EOT.gsub(/ {6}/, '')
* [ ] Task 1
* [x] Task 2
* [x] Task 3
* [ ] Task 4
* [ ] Task 5
EOT
end
it 'updates the Nth task correctly' do
subject.update_nth_task(1, true)
expect(subject.description).to match(/\[x\] Task 1/)
subject.update_nth_task(2, true)
expect(subject.description).to match('\[x\] Task 2')
subject.update_nth_task(3, false)
expect(subject.description).to match('\[ \] Task 3')
subject.update_nth_task(4, false)
expect(subject.description).to match('\[ \] Task 4')
end
it 'returns the correct task status' do
expect(subject.task_status).to match('5 tasks')
expect(subject.task_status).to match('2 done')
expect(subject.task_status).to match('3 unfinished')
end
it 'knows if it has tasks' do
expect(subject.tasks?).to be_true
subject.description = 'Now I have no tasks'
expect(subject.tasks?).to be_false
end
end