From 32913b74b836c7b689e681a030de00da0552954a Mon Sep 17 00:00:00 2001 From: Guilherme Salazar Date: Mon, 26 Sep 2016 18:36:11 -0300 Subject: [PATCH] add "x of y tasks completed" on issuable fix issues pointed out in !6527 add task completion status feature to CHANGELOG --- CHANGELOG.md | 1 + app/assets/javascripts/issue.js | 6 +++++- app/assets/javascripts/merge_request.js | 6 +++++- app/controllers/projects/issues_controller.rb | 2 +- .../projects/merge_requests_controller.rb | 2 +- app/helpers/issuables_helper.rb | 8 ++++++++ app/models/concerns/issuable.rb | 1 + app/models/concerns/taskable.rb | 16 ++++++++++++++-- app/models/issue.rb | 1 - app/models/merge_request.rb | 1 - spec/support/taskable_shared_examples.rb | 6 ++++++ 11 files changed, 42 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99991fb805f..0120e3bb07e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ Please view this file on the master branch, on stable branches it's out of date. - Fix filtering of milestones with quotes in title (airatshigapov) - Refactor less readable existance checking code from CoffeeScript !6289 (jlogandavison) - Update mail_room and enable sentinel support to Reply By Email (!7101) + - Add task completion status in Issues and Merge Requests tabs: "X of Y tasks completed" (!6527, @gmesalazar) - Simpler arguments passed to named_route on toggle_award_url helper method - Fix typo in framework css class. !7086 (Daniel Voogsgerd) - New issue board list dropdown stays open after adding a new list diff --git a/app/assets/javascripts/issue.js b/app/assets/javascripts/issue.js index e83dae2bb3c..67ace697936 100644 --- a/app/assets/javascripts/issue.js +++ b/app/assets/javascripts/issue.js @@ -95,7 +95,11 @@ return $.ajax({ type: 'PATCH', url: $('form.js-issuable-update').attr('action'), - data: patchData + 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 diff --git a/app/assets/javascripts/merge_request.js b/app/assets/javascripts/merge_request.js index a0bce6ef381..d3bd1e846c1 100644 --- a/app/assets/javascripts/merge_request.js +++ b/app/assets/javascripts/merge_request.js @@ -97,7 +97,11 @@ return $.ajax({ type: 'PATCH', url: $('form.js-issuable-update').attr('action'), - data: patchData + 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 diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index cb649264146..3f1a1d1c511 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -112,7 +112,7 @@ class Projects::IssuesController < Projects::ApplicationController end format.json do - render json: @issue.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } }) + render json: @issue.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } }, methods: [:task_status, :task_status_short]) end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 2ee53f7ceda..30f1cf4e5be 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -278,7 +278,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @merge_request.target_project, @merge_request]) end format.json do - render json: @merge_request.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } }) + render json: @merge_request.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } }, methods: [:task_status, :task_status_short]) end end else diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 03b2db1bc91..ef6cfb235a9 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -71,6 +71,14 @@ module IssuablesHelper author_output = link_to_member(project, issuable.author, size: 24, mobile_classes: "hidden-xs", tooltip: true) author_output << link_to_member(project, issuable.author, size: 24, by_username: true, avatar: false, mobile_classes: "hidden-sm hidden-md hidden-lg") end + + if issuable.tasks? + output << " ".html_safe + output << content_tag(:span, issuable.task_status, id: "task_status", class: "hidden-xs") + output << content_tag(:span, issuable.task_status_short, id: "task_status_short", class: "hidden-sm hidden-md hidden-lg") + end + + output end def issuable_todo(issuable) diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 17c3b526c97..613444e0d70 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -12,6 +12,7 @@ module Issuable include Subscribable include StripAttribute include Awardable + include Taskable included do cache_markdown_field :title, pipeline: :single_line diff --git a/app/models/concerns/taskable.rb b/app/models/concerns/taskable.rb index a3ac577cf3e..ebc75100a54 100644 --- a/app/models/concerns/taskable.rb +++ b/app/models/concerns/taskable.rb @@ -53,10 +53,22 @@ module Taskable # Return a string that describes the current state of this Taskable's task # list items, e.g. "12 of 20 tasks completed" - def task_status + def task_status(short: false) return '' if description.blank? + prep, completed = if short + ['/', ''] + else + [' of ', ' completed'] + end + sum = tasks.summary - "#{sum.complete_count} of #{sum.item_count} #{'task'.pluralize(sum.item_count)} completed" + "#{sum.complete_count}#{prep}#{sum.item_count} #{'task'.pluralize(sum.item_count)}#{completed}" + end + + # Return a short string that describes the current state of this Taskable's + # task list items -- for small screens + def task_status_short + task_status(short: true) end end diff --git a/app/models/issue.rb b/app/models/issue.rb index e356fe06363..4f02b02c488 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -5,7 +5,6 @@ class Issue < ActiveRecord::Base include Issuable include Referable include Sortable - include Taskable include Spammable include FasterCacheKeys diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 4872f8b8649..0397c57f935 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -3,7 +3,6 @@ class MergeRequest < ActiveRecord::Base include Issuable include Referable include Sortable - include Taskable include Importable belongs_to :target_project, class_name: "Project" diff --git a/spec/support/taskable_shared_examples.rb b/spec/support/taskable_shared_examples.rb index 201614e45a4..ad1c783df4d 100644 --- a/spec/support/taskable_shared_examples.rb +++ b/spec/support/taskable_shared_examples.rb @@ -17,6 +17,8 @@ shared_examples 'a Taskable' do it 'returns the correct task status' do expect(subject.task_status).to match('2 of') expect(subject.task_status).to match('5 tasks completed') + expect(subject.task_status_short).to match('2/') + expect(subject.task_status_short).to match('5 tasks') end describe '#tasks?' do @@ -41,6 +43,8 @@ shared_examples 'a Taskable' do it 'returns the correct task status' do expect(subject.task_status).to match('0 of') expect(subject.task_status).to match('1 task completed') + expect(subject.task_status_short).to match('0/') + expect(subject.task_status_short).to match('1 task') end end @@ -54,6 +58,8 @@ shared_examples 'a Taskable' do it 'returns the correct task status' do expect(subject.task_status).to match('1 of') expect(subject.task_status).to match('1 task completed') + expect(subject.task_status_short).to match('1/') + expect(subject.task_status_short).to match('1 task') end end end