From 793cb1676450a5b3e723a452893c10be1cb0f1bd Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Wed, 13 Jul 2016 19:02:48 +0000 Subject: [PATCH] Make time entries groupable (#16843). git-svn-id: http://svn.redmine.org/redmine/trunk@15649 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/timelog_controller.rb | 1 - app/helpers/issues_helper.rb | 27 +++------- app/helpers/queries_helper.rb | 31 +++++++++++ app/models/issue_query.rb | 4 ++ app/models/query.rb | 14 +++-- app/models/time_entry_query.rb | 17 +++++- app/views/issues/index.html.erb | 59 +------------------- app/views/queries/_query_form.html.erb | 62 ++++++++++++++++++++++ app/views/timelog/_date_range.html.erb | 42 +-------------- app/views/timelog/_list.html.erb | 24 ++++++++- app/views/timelog/index.html.erb | 5 +- app/views/timelog/report.html.erb | 4 -- public/stylesheets/application.css | 3 +- test/functional/timelog_controller_test.rb | 25 ++++----- 14 files changed, 167 insertions(+), 151 deletions(-) create mode 100644 app/views/queries/_query_form.html.erb diff --git a/app/controllers/timelog_controller.rb b/app/controllers/timelog_controller.rb index a63ffae82..30560dcfb 100644 --- a/app/controllers/timelog_controller.rb +++ b/app/controllers/timelog_controller.rb @@ -53,7 +53,6 @@ class TimelogController < ApplicationController @entry_count = scope.count @entry_pages = Paginator.new @entry_count, per_page_option, params['page'] @entries = scope.offset(@entry_pages.offset).limit(@entry_pages.per_page).to_a - @total_hours = scope.sum(:hours).to_f render :layout => !request.xhr? } diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 8d23d95cd..42470d14b 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -33,28 +33,13 @@ module IssuesHelper end def grouped_issue_list(issues, query, issue_count_by_group, &block) - previous_group, first = false, true - totals_by_group = query.totalable_columns.inject({}) do |h, column| - h[column] = query.total_by_group_for(column) - h - end - issue_list(issues) do |issue, level| - group_name = group_count = nil - if query.grouped? - group = query.group_by_column.value(issue) - if first || group != previous_group - if group.blank? && group != false - group_name = "(#{l(:label_blank_value)})" - else - group_name = format_object(group) - end - group_name ||= "" - group_count = issue_count_by_group[group] - group_totals = totals_by_group.map {|column, t| total_tag(column, t[group] || 0)}.join(" ").html_safe - end + ancestors = [] + grouped_query_results(issues, query, issue_count_by_group) do |issue, group_name, group_count, group_totals| + while (ancestors.any? && !issue.is_descendant_of?(ancestors.last)) + ancestors.pop end - yield issue, level, group_name, group_count, group_totals - previous_group, first = group, false + yield issue, ancestors.size, group_name, group_count, group_totals + ancestors << issue unless issue.leaf? end end diff --git a/app/helpers/queries_helper.rb b/app/helpers/queries_helper.rb index e2e09c7de..6bf8b3177 100644 --- a/app/helpers/queries_helper.rb +++ b/app/helpers/queries_helper.rb @@ -78,6 +78,11 @@ module QueriesHelper query_filters_hidden_tags(query) + query_columns_hidden_tags(query) end + def group_by_column_select_tag(query) + options = [[]] + query.groupable_columns.collect {|c| [c.caption, c.name.to_s]} + select_tag('group_by', options_for_select(options, @query.group_by)) + end + def available_block_columns_tags(query) tags = ''.html_safe query.available_block_columns.each do |column| @@ -108,6 +113,32 @@ module QueriesHelper render :partial => 'queries/columns', :locals => {:query => query, :tag_name => tag_name} end + def grouped_query_results(items, query, item_count_by_group, &block) + previous_group, first = false, true + totals_by_group = query.totalable_columns.inject({}) do |h, column| + h[column] = query.total_by_group_for(column) + h + end + items.each do |item| + group_name = group_count = nil + if query.grouped? + group = query.group_by_column.value(item) + if first || group != previous_group + if group.blank? && group != false + group_name = "(#{l(:label_blank_value)})" + else + group_name = format_object(group) + end + group_name ||= "" + group_count = item_count_by_group ? item_count_by_group[group] : nil + group_totals = totals_by_group.map {|column, t| total_tag(column, t[group] || 0)}.join(" ").html_safe + end + end + yield item, group_name, group_count, group_totals + previous_group, first = group, false + end + end + def render_query_totals(query) return unless query.totalable_columns.present? totals = query.totalable_columns.map do |column| diff --git a/app/models/issue_query.rb b/app/models/issue_query.rb index e13c46a2f..0846d7096 100644 --- a/app/models/issue_query.rb +++ b/app/models/issue_query.rb @@ -249,6 +249,10 @@ class IssueQuery < Query end end + def default_totalable_names + Setting.issue_list_default_totals.map(&:to_sym) + end + def base_scope Issue.visible.joins(:status, :project).where(statement) end diff --git a/app/models/query.rb b/app/models/query.rb index 5e8e0cd90..9a1dc0d5a 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -76,11 +76,11 @@ end class QueryCustomFieldColumn < QueryColumn - def initialize(custom_field) + def initialize(custom_field, options={}) self.name = "cf_#{custom_field.id}".to_sym self.sortable = custom_field.order_statement || false self.groupable = custom_field.group_statement || false - self.totalable = custom_field.totalable? + self.totalable = options.key?(:totalable) ? !!options[:totalable] : custom_field.totalable? @inline = true @cf = custom_field end @@ -120,8 +120,8 @@ end class QueryAssociationCustomFieldColumn < QueryCustomFieldColumn - def initialize(association, custom_field) - super(custom_field) + def initialize(association, custom_field, options={}) + super(custom_field, options) self.name = "#{association}.cf_#{custom_field.id}".to_sym # TODO: support sorting/grouping by association custom field self.sortable = false @@ -546,6 +546,10 @@ class Query < ActiveRecord::Base [] end + def default_totalable_names + [] + end + def column_names=(names) if names names = names.select {|n| n.is_a?(Symbol) || !n.blank? } @@ -583,7 +587,7 @@ class Query < ActiveRecord::Base end def totalable_names - options[:totalable_names] || Setting.issue_list_default_totals.map(&:to_sym) || [] + options[:totalable_names] || default_totalable_names || [] end def sort_criteria=(arg) diff --git a/app/models/time_entry_query.rb b/app/models/time_entry_query.rb index dde34264a..c667fb651 100644 --- a/app/models/time_entry_query.rb +++ b/app/models/time_entry_query.rb @@ -28,7 +28,7 @@ class TimeEntryQuery < Query QueryColumn.new(:activity, :sortable => "#{TimeEntryActivity.table_name}.position", :groupable => true), QueryColumn.new(:issue, :sortable => "#{Issue.table_name}.id"), QueryColumn.new(:comments), - QueryColumn.new(:hours, :sortable => "#{TimeEntry.table_name}.hours"), + QueryColumn.new(:hours, :sortable => "#{TimeEntry.table_name}.hours", :totalable => true), ] def initialize(attributes=nil, *args) @@ -105,7 +105,7 @@ class TimeEntryQuery < Query @available_columns += TimeEntryCustomField.visible. map {|cf| QueryCustomFieldColumn.new(cf) } @available_columns += IssueCustomField.visible. - map {|cf| QueryAssociationCustomFieldColumn.new(:issue, cf) } + map {|cf| QueryAssociationCustomFieldColumn.new(:issue, cf, :totalable => false) } @available_columns end @@ -113,6 +113,14 @@ class TimeEntryQuery < Query @default_columns_names ||= [:project, :spent_on, :user, :activity, :issue, :comments, :hours] end + def default_totalable_names + [:hours] + end + + def base_scope + TimeEntry.visible.where(statement) + end + def results_scope(options={}) order_option = [group_by_sort_order, options[:order]].flatten.reject(&:blank?) @@ -123,6 +131,11 @@ class TimeEntryQuery < Query includes(:activity). references(:activity) end + + # Returns sum of all the spent hours + def total_for_hours(scope) + map_total(scope.sum(:hours)) {|t| t.to_f.round(2)} + end def sql_for_issue_id_field(field, operator, value) case operator diff --git a/app/views/issues/index.html.erb b/app/views/issues/index.html.erb index d120ce71e..d0fd943f0 100644 --- a/app/views/issues/index.html.erb +++ b/app/views/issues/index.html.erb @@ -7,65 +7,10 @@

<%= @query.new_record? ? l(:label_issue_plural) : @query.name %>

<% html_title(@query.new_record? ? l(:label_issue_plural) : @query.name) %> -<%= form_tag({ :controller => 'issues', :action => 'index', :project_id => @project }, - :method => :get, :id => 'query_form') do %> -
- <%= hidden_field_tag 'set_filter', '1' %> -
-
"> - <%= l(:label_filter_plural) %> -
"> - <%= render :partial => 'queries/filters', :locals => {:query => @query} %> -
-
- -
-

- <%= link_to_function l(:button_apply), '$("#query_form").submit()', :class => 'icon icon-checked' %> - <%= link_to l(:button_clear), { :set_filter => 1, :project_id => @project }, :class => 'icon icon-reload' %> - <% if @query.new_record? %> - <% if User.current.allowed_to?(:save_queries, @project, :global => true) %> - <%= link_to_function l(:button_save), - "$('#query_form').attr('action', '#{ @project ? new_project_query_path(@project) : new_query_path }').submit()", - :class => 'icon icon-save' %> - <% end %> - <% else %> - <% if @query.editable_by?(User.current) %> - <%= link_to l(:button_edit), edit_query_path(@query), :class => 'icon icon-edit' %> - <%= delete_link query_path(@query) %> - <% end %> - <% end %> -

-
+<%= form_tag(_project_issues_path(@project), :method => :get, :id => 'query_form') do %> + <%= render :partial => 'queries/query_form' %> <% end %> -<%= error_messages_for 'query' %> <% if @query.valid? %> <% if @issues.empty? %>

<%= l(:label_no_data) %>

diff --git a/app/views/queries/_query_form.html.erb b/app/views/queries/_query_form.html.erb new file mode 100644 index 000000000..bf0b0c257 --- /dev/null +++ b/app/views/queries/_query_form.html.erb @@ -0,0 +1,62 @@ +<%= hidden_field_tag 'set_filter', '1' %> +<%= hidden_field_tag 'type', @query.type, :disabled => true, :id => 'query_type' %> + +
+
+
"> + <%= l(:label_filter_plural) %> +
"> + <%= render :partial => 'queries/filters', :locals => {:query => @query} %> +
+
+ + +
+ +

+ <%= link_to_function l(:button_apply), '$("#query_form").submit()', :class => 'icon icon-checked' %> + <%= link_to l(:button_clear), { :set_filter => 1, :project_id => @project }, :class => 'icon icon-reload' %> + <% if @query.new_record? %> + <% if User.current.allowed_to?(:save_queries, @project, :global => true) %> + <%= link_to_function l(:button_save), + "$('#query_type').prop('disabled',false);$('#query_form').attr('action', '#{ @project ? new_project_query_path(@project) : new_query_path }').submit()", + :class => 'icon icon-save' %> + <% end %> + <% else %> + <% if @query.editable_by?(User.current) %> + <%= link_to l(:button_edit), edit_query_path(@query), :class => 'icon icon-edit' %> + <%= delete_link query_path(@query) %> + <% end %> + <% end %> +

+
+ +<%= error_messages_for @query %> diff --git a/app/views/timelog/_date_range.html.erb b/app/views/timelog/_date_range.html.erb index 91b96d8b7..db07c465e 100644 --- a/app/views/timelog/_date_range.html.erb +++ b/app/views/timelog/_date_range.html.erb @@ -1,44 +1,4 @@ -
-<%= hidden_field_tag 'set_filter', '1' %> -
-
"> - <%= l(:label_filter_plural) %> -
"> - <%= render :partial => 'queries/filters', :locals => {:query => @query} %> -
-
- -
- -

- <%= link_to_function l(:button_apply), '$("#query_form").submit()', :class => 'icon icon-checked' %> - <%= link_to l(:button_clear), {:project_id => @project, :issue_id => @issue}, :class => 'icon icon-reload' %> - <% if @query.new_record? %> - <% if User.current.allowed_to?(:save_queries, @project, :global => true) %> - <%= link_to_function l(:button_save), - "$('#query_form').attr('action', '#{ @project ? new_project_query_path(@project) : new_query_path }').submit()", - :class => 'icon icon-save' %> - <% end %> - <% else %> - <% if @query.editable_by?(User.current) %> - <%= link_to l(:button_edit), edit_query_path(@query), :class => 'icon icon-edit' %> - <%= delete_link query_path(@query) %> - <% end %> - <% end %> -

- -<%= hidden_field_tag 'type', 'TimeEntryQuery' %> -
+<%= render :partial => 'queries/query_form' %>
<% query_params = request.query_parameters %> diff --git a/app/views/timelog/_list.html.erb b/app/views/timelog/_list.html.erb index 71fe41a68..3a854ccef 100644 --- a/app/views/timelog/_list.html.erb +++ b/app/views/timelog/_list.html.erb @@ -15,7 +15,22 @@ -<% entries.each do |entry| -%> +<% grouped_query_results(entries, @query, @entry_count_by_group) do |entry, group_name, group_count, group_totals| -%> + <% if group_name %> + <% reset_cycle %> + + +   + <%= group_name %> + <% if group_count %> + <%= group_count %> + <% end %> + <%= group_totals %> + <%= link_to_function("#{l(:button_collapse_all)}/#{l(:button_expand_all)}", + "toggleAllRowGroups(this)", :class => 'toggle-all') %> + + + <% end %> hascontextmenu"> <%= check_box_tag("ids[]", entry.id, false, :id => nil) %> <%= raw @query.inline_columns.map {|column| "#{column_content(column, entry)}"}.join %> @@ -32,6 +47,13 @@ <% end -%> + <% @query.block_columns.each do |column| + if (text = column_content(column, issue)) && text.present? -%> + + <%= text %> + + <% end -%> + <% end -%> <% end -%> diff --git a/app/views/timelog/index.html.erb b/app/views/timelog/index.html.erb index d6ea9690e..7af8ebd83 100644 --- a/app/views/timelog/index.html.erb +++ b/app/views/timelog/index.html.erb @@ -10,11 +10,8 @@ <%= render :partial => 'date_range' %> <% end %> -
-

<%= l(:label_total_time) %>: <%= html_hours(l_hours(@total_hours)) %>

-
- <% unless @entries.empty? %> +<%= render_query_totals(@query) %> <%= render :partial => 'list', :locals => { :entries => @entries }%> <%= pagination_links_full @entry_pages, @entry_count %> diff --git a/app/views/timelog/report.html.erb b/app/views/timelog/report.html.erb index efa04de88..c798d4873 100644 --- a/app/views/timelog/report.html.erb +++ b/app/views/timelog/report.html.erb @@ -27,10 +27,6 @@ <% end %> <% unless @report.criteria.empty? %> -
-

<%= l(:label_total_time) %>: <%= html_hours(l_hours(@report.total_hours)) %>

-
- <% unless @report.hours.empty? %>
diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css index f1914329b..9816c6508 100644 --- a/public/stylesheets/application.css +++ b/public/stylesheets/application.css @@ -314,9 +314,10 @@ table.query-columns td.buttons { text-align: center; } table.query-columns td.buttons input[type=button] {width:35px;} -.query-totals {text-align:right; margin-top:-2.3em;} +.query-totals {text-align:right;} .query-totals>span {margin-left:0.6em;} .query-totals .value {font-weight:bold;} +body.controller-issues .query-totals {margin-top:-2.3em;} td.center {text-align:center;} diff --git a/test/functional/timelog_controller_test.rb b/test/functional/timelog_controller_test.rb index b85ee67be..5e1af33f5 100644 --- a/test/functional/timelog_controller_test.rb +++ b/test/functional/timelog_controller_test.rb @@ -582,9 +582,8 @@ class TimelogControllerTest < ActionController::TestCase def test_index_all_projects get :index assert_response :success - assert_template 'index' - assert_not_nil assigns(:total_hours) - assert_equal "162.90", "%.2f" % assigns(:total_hours) + + assert_select '.total-for-hours', :text => 'Hours: 162.90' assert_select 'form#query_form[action=?]', '/time_entries' end @@ -612,8 +611,8 @@ class TimelogControllerTest < ActionController::TestCase assert_equal 4, assigns(:entries).size # project and subproject assert_equal [1, 3], assigns(:entries).collect(&:project_id).uniq.sort - assert_not_nil assigns(:total_hours) - assert_equal "162.90", "%.2f" % assigns(:total_hours) + + assert_select '.total-for-hours', :text => 'Hours: 162.90' assert_select 'form#query_form[action=?]', '/projects/ecookbook/time_entries' end @@ -646,7 +645,7 @@ class TimelogControllerTest < ActionController::TestCase @request.session[:user_id] = 2 get :index, :project_id => 'ecookbook', :issue_id => issue.id.to_s, :set_filter => 1 - assert_select '.total-hours', :text => 'Total time: 7.00 hours' + assert_select '.total-for-hours', :text => 'Hours: 7.00' end def test_index_at_project_level_with_issue_fixed_version_id_short_filter @@ -657,7 +656,7 @@ class TimelogControllerTest < ActionController::TestCase @request.session[:user_id] = 2 get :index, :project_id => 'ecookbook', :"issue.fixed_version_id" => version.id.to_s, :set_filter => 1 - assert_select '.total-hours', :text => 'Total time: 5.00 hours' + assert_select '.total-for-hours', :text => 'Hours: 5.00' end def test_index_at_project_level_with_date_range @@ -669,8 +668,8 @@ class TimelogControllerTest < ActionController::TestCase assert_template 'index' assert_not_nil assigns(:entries) assert_equal 3, assigns(:entries).size - assert_not_nil assigns(:total_hours) - assert_equal "12.90", "%.2f" % assigns(:total_hours) + + assert_select '.total-for-hours', :text => 'Hours: 12.90' assert_select 'form#query_form[action=?]', '/projects/ecookbook/time_entries' end @@ -680,8 +679,8 @@ class TimelogControllerTest < ActionController::TestCase assert_template 'index' assert_not_nil assigns(:entries) assert_equal 3, assigns(:entries).size - assert_not_nil assigns(:total_hours) - assert_equal "12.90", "%.2f" % assigns(:total_hours) + + assert_select '.total-for-hours', :text => 'Hours: 12.90' assert_select 'form#query_form[action=?]', '/projects/ecookbook/time_entries' end @@ -691,9 +690,7 @@ class TimelogControllerTest < ActionController::TestCase :op => {'spent_on' => '>t-'}, :v => {'spent_on' => ['7']} assert_response :success - assert_template 'index' - assert_not_nil assigns(:entries) - assert_not_nil assigns(:total_hours) + assert_select 'form#query_form[action=?]', '/projects/ecookbook/time_entries' end