From 2ecaa0e9aa02bfca0515b19834cce20fede45757 Mon Sep 17 00:00:00 2001 From: Marius BALTEANU Date: Sat, 5 Dec 2020 20:23:55 +0200 Subject: [PATCH] * Include only visible issues in subtasks stats * Get subtasks using only one query * Show all subtasks count as badge * Add tests --- app/helpers/issues_helper.rb | 46 +++++++++++++++++++++++ app/views/issues/show.html.erb | 14 +------ public/stylesheets/application.css | 6 +++ test/functional/issues_controller_test.rb | 41 ++++++++++++++++++++ 4 files changed, 95 insertions(+), 12 deletions(-) diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 0bcb90ad0..4926b6cc5 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -143,6 +143,52 @@ module IssuesHelper s.html_safe end + # Renders descendants stats (total descendants (open - closed)) with query links + def render_descendants_stats(issue) + # Get issue descendants grouped by status type (open/closed) using a single query + subtasks_grouped = issue.descendants.visible.joins(:status).select(:is_closed, :id).group(:is_closed).reorder(:is_closed).count(:id) + # Cast keys to boolean in order to have consistent results between database types + subtasks_grouped.transform_keys! {|k| ActiveModel::Type::Boolean.new.cast(k)} + + open_subtasks = subtasks_grouped[false].to_i + closed_subtasks = subtasks_grouped[true].to_i + all_subtasks = open_subtasks + closed_subtasks + + return if all_subtasks == 0 + + all_block = content_tag( + 'span', + link_to(all_subtasks, issues_path(parent_id: "~#{issue.id}", set_filter: true, status_id: '*')), + class: 'badge badge-issues-count' + ) + + closed_block = content_tag( + 'span', + link_to_if( + closed_subtasks > 0, + l(:label_x_closed_issues_abbr, count: closed_subtasks), + issues_path(parent_id: "~#{issue.id}", set_filter: true, status_id: 'c') + ), + class: 'closed' + ) + + open_block = content_tag( + 'span', + link_to_if( + open_subtasks > 0, + l(:label_x_open_issues_abbr, :count => open_subtasks), + issues_path(:parent_id => "~#{issue.id}", :set_filter => true, :status_id => 'o') + ), + class: 'open' + ) + + content_tag( + "span", + "#{all_block} (#{open_block} — #{closed_block})".html_safe, + :class => 'issues-stat' + ) + end + # Renders the list of related issues on the issue details view def render_issue_relations(issue, relations) manage_relations = User.current.allowed_to?(:manage_issue_relations, issue.project) diff --git a/app/views/issues/show.html.erb b/app/views/issues/show.html.erb index f40f206d7..3a4e1b40f 100644 --- a/app/views/issues/show.html.erb +++ b/app/views/issues/show.html.erb @@ -109,18 +109,8 @@ end %> <%= link_to_new_subtask(@issue) if User.current.allowed_to?(:manage_subtasks, @project) %>

-<%=l(:label_subtask_plural)%> -<% if !@issue.leaf? %> - (<%= link_to(l(:label_x_issues, :count => @issue.descendants.count), - issues_path(:parent_id => "~#{@issue.id}", :set_filter => true, :status_id => '*')) %> - : <%= link_to_if( @issue.descendants.select(&:closed?).count > 0, - l(:label_x_closed_issues_abbr, :count => @issue.descendants.select(&:closed?).count ), - issues_path(:parent_id => "~#{@issue.id}", :set_filter => true, :status_id => 'c')) %> - — - <%= link_to_if( @issue.descendants.open.count > 0, - l(:label_x_open_issues_abbr, :count => @issue.descendants.open.count ), - issues_path(:parent_id => "~#{@issue.id}", :set_filter => true, :status_id => 'o')) %>) -<% end %> + <%=l(:label_subtask_plural)%> + <%= render_descendants_stats(@issue) unless @issue.leaf? %>

<%= form_tag({}, :data => {:cm_url => issues_context_menu_path}) do %> <%= render_descendants_tree(@issue) unless @issue.leaf? %> diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css index 70911e4f6..02c7e81e9 100644 --- a/public/stylesheets/application.css +++ b/public/stylesheets/application.css @@ -540,6 +540,8 @@ body.controller-issues h2.inline-flex {padding-right: 0} #issue_tree td.checkbox, #relations td.checkbox {display:none;} #issue_tree td.subject, #relations td.subject {width: 50%;} #issue_tree td.buttons, #relations td.buttons {padding:0;} +#issue_tree .issues-stat {font-size: 80%} +#issue_tree .issues-stat .badge {bottom: initial;} #trackers_description {display:none;} #trackers_description dt {font-weight: bold; text-decoration: underline;} @@ -1469,6 +1471,10 @@ td.gantt_selected_column .gantt_hdr,.gantt_selected_column_container { color: #1D781D; border: 1px solid #1D781D; } +.badge-issues-count { + background: #EEEEEE; +} + /***** Tooltips *****/ .ui-tooltip { background: #000; diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 07533a854..fa4cecbaf 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -2299,6 +2299,47 @@ class IssuesControllerTest < Redmine::ControllerTest end end + def test_show_should_show_subtasks_stats + @request.session[:user_id] = 1 + child1 = Issue.generate!(parent_issue_id: 1, subject: 'Open child issue') + Issue.generate!(parent_issue_id: 1, subject: 'Closed child issue', status_id: 5) + Issue.generate!(parent_issue_id: child1.id, subject: 'Open child of child') + # Issue not visible for anonymous + Issue.generate!(parent_issue_id: 1, subject: 'Private child', project_id: 5) + + get(:show, params: {:id => 1}) + assert_response :success + + assert_select 'div#issue_tree span.issues-stat' do + assert_select 'span.badge', text: '4' + assert_select 'span.open a[href=?]', "/issues?parent_id=~1&set_filter=true&status_id=o", text: '3 open' + assert_select 'span.closed a[href=?]', "/issues?parent_id=~1&set_filter=true&status_id=c", text: '1 closed' + end + end + + def test_show_subtasks_stats_should_not_link_if_issue_has_zero_open_or_closed_subtasks + child1 = Issue.generate!(parent_issue_id: 1, subject: 'Open child issue') + + get(:show, params: {:id => 1}) + assert_response :success + + assert_select 'div#issue_tree span.issues-stat' do + assert_select 'span.open a[href=?]', "/issues?parent_id=~1&set_filter=true&status_id=o", text: '1 open' + assert_select 'span.closed', text: '0 closed' + assert_select 'span.closed a', 0 + end + end + + def test_show_should_not_show_subtasks_stats_if_subtasks_are_not_visible + # Issue not visible for anonymous + Issue.generate!(parent_issue_id: 1, subject: 'Private child', project_id: 5) + + get(:show, params: {:id => 1}) + assert_response :success + + assert_select 'div#issue_tree span.issues-stat', 0 + end + def test_show_should_list_parents issue = Issue. create!( -- 2.22.0