diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index c5c98d6fd..89de825d2 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -174,6 +174,8 @@ class ProjectsController < ApplicationController if User.current.allowed_to_view_all_time_entries?(@project) @total_hours = TimeEntry.visible.where(cond).sum(:hours).to_f + end + if User.current.allowed_to?(:view_estimated_hours, @project) @total_estimated_hours = Issue.visible.where(cond).sum(:estimated_hours).to_f end diff --git a/app/models/issue.rb b/app/models/issue.rb index 63bc8cabe..0510828bc 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -467,7 +467,6 @@ class Issue < ActiveRecord::Base 'start_date', 'due_date', 'done_ratio', - 'estimated_hours', 'custom_field_values', 'custom_fields', 'lock_version', @@ -498,6 +497,9 @@ class Issue < ActiveRecord::Base 'deleted_attachment_ids', :if => lambda {|issue, user| issue.attachments_deletable?(user)}) + safe_attributes 'estimated_hours', + :if => lambda {|issue, user| user.allowed_to?(:view_estimated_hours, issue.project)} + def safe_attribute_names(user=nil) names = super names -= disabled_core_fields diff --git a/app/models/issue_query.rb b/app/models/issue_query.rb index bfb02144b..740c68185 100644 --- a/app/models/issue_query.rb +++ b/app/models/issue_query.rb @@ -37,14 +37,6 @@ class IssueQuery < Query QueryColumn.new(:fixed_version, :sortable => lambda {Version.fields_for_order_statement}, :groupable => true), QueryColumn.new(:start_date, :sortable => "#{Issue.table_name}.start_date", :groupable => true), QueryColumn.new(:due_date, :sortable => "#{Issue.table_name}.due_date", :groupable => true), - QueryColumn.new(:estimated_hours, :sortable => "#{Issue.table_name}.estimated_hours", :totalable => true), - QueryColumn.new( - :total_estimated_hours, - :sortable => -> { - "COALESCE((SELECT SUM(estimated_hours) FROM #{Issue.table_name} subtasks" + - " WHERE #{Issue.visible_condition(User.current).gsub(/\bissues\b/, 'subtasks')} AND subtasks.root_id = #{Issue.table_name}.root_id AND subtasks.lft >= #{Issue.table_name}.lft AND subtasks.rgt <= #{Issue.table_name}.rgt), 0)" - }, - :default_order => 'desc'), QueryColumn.new(:done_ratio, :sortable => "#{Issue.table_name}.done_ratio", :groupable => true), TimestampQueryColumn.new(:created_on, :sortable => "#{Issue.table_name}.created_on", :default_order => 'desc', :groupable => true), TimestampQueryColumn.new(:closed_on, :sortable => "#{Issue.table_name}.closed_on", :default_order => 'desc', :groupable => true), @@ -155,12 +147,9 @@ class IssueQuery < Query add_available_filter "closed_on", :type => :date_past add_available_filter "start_date", :type => :date add_available_filter "due_date", :type => :date - add_available_filter "estimated_hours", :type => :float - - if User.current.allowed_to?(:view_time_entries, project, :global => true) - add_available_filter "spent_time", :type => :float, :label => :label_spent_time + if User.current.allowed_to?(:view_estimated_hours, nil, :global => true) + add_available_filter "estimated_hours", :type => :float end - add_available_filter "done_ratio", :type => :integer if User.current.allowed_to?(:set_issues_private, nil, :global => true) || @@ -225,9 +214,28 @@ class IssueQuery < Query @available_columns = self.class.available_columns.dup @available_columns += issue_custom_fields.visible.collect {|cf| QueryCustomFieldColumn.new(cf) } + if User.current.allowed_to?(:view_estimated_hours, project, :global => true) + # insert the columns after due_date or at the end + index = @available_columns.rindex {|column| column.name == :due_date} + index = (index ? index + 1 : -1) + + @available_columns.insert index, QueryColumn.new(:estimated_hours, + :sortable => "#{Issue.table_name}.estimated_hours", + :totalable => true + ) + index = (index.negative? ? index : index + 1) + @available_columns.insert index, QueryColumn.new(:total_estimated_hours, + :sortable => -> { + "COALESCE((SELECT SUM(estimated_hours) FROM #{Issue.table_name} subtasks" + + " WHERE #{Issue.visible_condition(User.current).gsub(/\bissues\b/, 'subtasks')} AND subtasks.root_id = #{Issue.table_name}.root_id AND subtasks.lft >= #{Issue.table_name}.lft AND subtasks.rgt <= #{Issue.table_name}.rgt), 0)" + }, + :default_order => 'desc' + ) + end + if User.current.allowed_to?(:view_time_entries, project, :global => true) - # insert the columns after total_estimated_hours or at the end - index = @available_columns.find_index {|column| column.name == :total_estimated_hours} + # insert the columns after total_estimated_hours or the columns after due_date or at the end + index = @available_columns.rindex {|column| column.name == :total_estimated_hours || column.name == :due_date } index = (index ? index + 1 : -1) subselect = "SELECT SUM(hours) FROM #{TimeEntry.table_name}" + @@ -249,8 +257,9 @@ class IssueQuery < Query " WHERE (#{TimeEntry.visible_condition(User.current)})" + " AND subtasks.root_id = #{Issue.table_name}.root_id AND subtasks.lft >= #{Issue.table_name}.lft AND subtasks.rgt <= #{Issue.table_name}.rgt" + index = (index.negative? ? index : index + 1) @available_columns.insert( - index + 1, + index, QueryColumn.new(:total_spent_hours, :sortable => "COALESCE((#{subselect}), 0)", :default_order => 'desc', diff --git a/app/views/issues/show.api.rsb b/app/views/issues/show.api.rsb index f474ed9c6..02c23c37f 100644 --- a/app/views/issues/show.api.rsb +++ b/app/views/issues/show.api.rsb @@ -16,8 +16,10 @@ api.issue do api.due_date @issue.due_date api.done_ratio @issue.done_ratio api.is_private @issue.is_private - api.estimated_hours @issue.estimated_hours - api.total_estimated_hours @issue.total_estimated_hours + if User.current.allowed_to?(:view_estimated_hours, @project) + api.estimated_hours @issue.estimated_hours + api.total_estimated_hours @issue.total_estimated_hours + end if User.current.allowed_to?(:view_time_entries, @project) api.spent_hours(@issue.spent_hours) api.total_spent_hours(@issue.total_spent_hours) diff --git a/app/views/issues/show.html.erb b/app/views/issues/show.html.erb index 7c180a4d3..7df68d842 100644 --- a/app/views/issues/show.html.erb +++ b/app/views/issues/show.html.erb @@ -64,7 +64,7 @@ unless @issue.disabled_core_fields.include?('done_ratio') rows.right l(:field_done_ratio), progress_bar(@issue.done_ratio, :legend => "#{@issue.done_ratio}%"), :class => 'progress' end - unless @issue.disabled_core_fields.include?('estimated_hours') + if User.current.allowed_to?(:view_estimated_hours, @project) && !@issue.disabled_core_fields.include?('estimated_hours') rows.right l(:field_estimated_hours), issue_estimated_hours_details(@issue), :class => 'estimated-hours' end if User.current.allowed_to?(:view_time_entries, @project) && @issue.total_spent_hours > 0 diff --git a/app/views/projects/show.html.erb b/app/views/projects/show.html.erb index de99eeabe..5b0109445 100644 --- a/app/views/projects/show.html.erb +++ b/app/views/projects/show.html.erb @@ -94,26 +94,29 @@ <% end %> - <% if User.current.allowed_to?(:view_time_entries, @project) %> + <% allowed_to_view_time_entries, allowed_to_view_estimated_hours = User.current.allowed_to?(:view_time_entries, @project), User.current.allowed_to?(:view_estimated_hours, @project) %> + <% if allowed_to_view_time_entries || allowed_to_view_estimated_hours %>

<%= l(:label_time_tracking) %>

+ <% if allowed_to_view_time_entries %>

- <% if User.current.allowed_to?(:log_time, @project) %> + <% if User.current.allowed_to?(:log_time, @project) %> <%= link_to l(:button_log_time), new_project_time_entry_path(@project) %> | - <% end %> + <% end %> <%= link_to(l(:label_details), project_time_entries_path(@project)) %> | <%= link_to(l(:label_report), report_project_time_entries_path(@project)) %>

+ <% end %>
-<% end %> + <% end %> <%= call_hook(:view_projects_show_left, :project => @project) %> @@ -134,7 +137,7 @@ <% end %> diff --git a/app/views/versions/show.html.erb b/app/views/versions/show.html.erb index d73485a8c..e984dd12e 100644 --- a/app/views/versions/show.html.erb +++ b/app/views/versions/show.html.erb @@ -14,15 +14,18 @@ <%= render(:partial => "wiki/content", :locals => {:content => @version.wiki_page.content}) if @version.wiki_page %>
-<% if @version.visible_fixed_issues.estimated_hours > 0 || User.current.allowed_to?(:view_time_entries, @project) %> +<% allowed_to_view_all_time_entries, allowed_to_view_estimated_hours = User.current.allowed_to_view_all_time_entries?(@project), User.current.allowed_to?(:view_estimated_hours, @project) %> +<% if (@version.visible_fixed_issues.estimated_hours > 0 && allowed_to_view_estimated_hours) || allowed_to_view_all_time_entries %>
<%= l(:label_time_tracking) %> +<% if allowed_to_view_estimated_hours %> -<% if User.current.allowed_to_view_all_time_entries?(@project) %> +<% end %> +<% if allowed_to_view_all_time_entries %>
<%= l(:field_estimated_hours) %> <%= link_to html_hours(l_hours(@version.visible_fixed_issues.estimated_hours)), project_issues_path(@version.project, :set_filter => 1, :status_id => '*', :fixed_version_id => @version.id, :c => [:tracker, :status, :subject, :estimated_hours], :t => [:estimated_hours]) %>
<%= l(:label_spent_time) %> <%= link_to html_hours(l_hours(@version.spent_hours)), diff --git a/config/locales/en.yml b/config/locales/en.yml index 3f0fd2d77..df501b356 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -519,6 +519,7 @@ en: permission_delete_issue_watchers: Delete watchers permission_log_time: Log spent time permission_view_time_entries: View spent time + permission_view_estimated_hours: View estimated time permission_edit_time_entries: Edit time logs permission_edit_own_time_entries: Edit own time logs permission_view_news: View news diff --git a/lib/redmine.rb b/lib/redmine.rb index 9ecca63c3..d41b09439 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -127,6 +127,7 @@ Redmine::AccessControl.map do |map| map.project_module :time_tracking do |map| map.permission :view_time_entries, {:timelog => [:index, :report, :show]}, :read => true + map.permission :view_estimated_hours, {}, :read => true map.permission :log_time, {:timelog => [:new, :create]}, :require => :loggedin map.permission :edit_time_entries, {:timelog => [:edit, :update, :destroy, :bulk_edit, :bulk_update]}, :require => :member map.permission :edit_own_time_entries, {:timelog => [:edit, :update, :destroy,:bulk_edit, :bulk_update]}, :require => :loggedin diff --git a/lib/redmine/default_data/loader.rb b/lib/redmine/default_data/loader.rb index cccc807fe..d75f1c6fb 100644 --- a/lib/redmine/default_data/loader.rb +++ b/lib/redmine/default_data/loader.rb @@ -68,6 +68,7 @@ module Redmine :view_calendar, :log_time, :view_time_entries, + :view_estimated_hours, :view_news, :comment_news, :view_documents, @@ -95,6 +96,7 @@ module Redmine :view_calendar, :log_time, :view_time_entries, + :view_estimated_hours, :view_news, :comment_news, :view_documents, @@ -114,6 +116,7 @@ module Redmine :view_gantt, :view_calendar, :view_time_entries, + :view_estimated_hours, :view_news, :comment_news, :view_documents, @@ -129,6 +132,7 @@ module Redmine :view_gantt, :view_calendar, :view_time_entries, + :view_estimated_hours, :view_news, :view_documents, :view_wiki_pages, diff --git a/lib/redmine/export/pdf/issues_pdf_helper.rb b/lib/redmine/export/pdf/issues_pdf_helper.rb index 652622755..37dd64a26 100644 --- a/lib/redmine/export/pdf/issues_pdf_helper.rb +++ b/lib/redmine/export/pdf/issues_pdf_helper.rb @@ -57,7 +57,8 @@ module Redmine right << [l(:field_start_date), format_date(issue.start_date)] unless issue.disabled_core_fields.include?('start_date') right << [l(:field_due_date), format_date(issue.due_date)] unless issue.disabled_core_fields.include?('due_date') right << [l(:field_done_ratio), "#{issue.done_ratio}%"] unless issue.disabled_core_fields.include?('done_ratio') - right << [l(:field_estimated_hours), l_hours(issue.estimated_hours)] unless issue.disabled_core_fields.include?('estimated_hours') + right << [l(:field_estimated_hours), l_hours(issue.estimated_hours)] if \ + User.current.allowed_to?(:view_estimated_hours, issue.project) && !issue.disabled_core_fields.include?('estimated_hours') right << [l(:label_spent_time), l_hours(issue.total_spent_hours)] if User.current.allowed_to?(:view_time_entries, issue.project) rows = left.size > right.size ? left.size : right.size diff --git a/test/fixtures/roles.yml b/test/fixtures/roles.yml index 13433874e..deb5d7753 100644 --- a/test/fixtures/roles.yml +++ b/test/fixtures/roles.yml @@ -34,6 +34,7 @@ roles_001: - :view_calendar - :log_time - :view_time_entries + - :view_estimated_hours - :edit_time_entries - :delete_time_entries - :import_time_entries @@ -94,6 +95,7 @@ roles_002: - :view_calendar - :log_time - :view_time_entries + - :view_estimated_hours - :edit_own_time_entries - :view_news - :manage_news @@ -141,6 +143,7 @@ roles_003: - :view_calendar - :log_time - :view_time_entries + - :view_estimated_hours - :view_news - :manage_news - :comment_news @@ -179,6 +182,7 @@ roles_004: - :view_calendar - :log_time - :view_time_entries + - :view_estimated_hours - :view_news - :comment_news - :view_documents @@ -206,6 +210,7 @@ roles_005: - :view_gantt - :view_calendar - :view_time_entries + - :view_estimated_hours - :view_news - :view_documents - :view_wiki_pages diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 11d93cf21..f75a4a62d 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -1519,6 +1519,15 @@ class IssuesControllerTest < Redmine::ControllerTest assert_select 'table.issues td.total_estimated_hours' end + def test_index_should_not_show_estimated_hours_column_without_permission + Role.anonymous.remove_permission! :view_estimated_hours + get :index, :params => { + :set_filter => 1, + :c => %w(subject estimated_hours) + } + assert_select 'td.estimated_hours', 0 + end + def test_index_should_not_show_spent_hours_column_without_permission Role.anonymous.remove_permission! :view_time_entries get( diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb index 6be670566..54431180c 100644 --- a/test/functional/projects_controller_test.rb +++ b/test/functional/projects_controller_test.rb @@ -759,6 +759,29 @@ class ProjectsControllerTest < Redmine::ControllerTest end end + def test_show_by_non_admin_user_with_view_estimated_hours_permission_should_show_estimated_time + @request.session[:user_id] = 2 # manager + get :show, :params => { + :id => 'ecookbook' + } + + assert_select 'div.spent_time.box>ul' do + assert_select '>li', :text => 'Estimated time: 203.50 hours' + end + end + + def test_show_by_non_admin_user_without_view_estimated_hours_permission_should_not_show_estimated_time + Role.find_by_name('Manager').remove_permission! :view_estimated_hours + @request.session[:user_id] = 2 # manager + get :show, :params => { + :id => 'ecookbook' + } + + assert_select 'div.spent_time.box>ul' do + assert_select '>li', :text => 'Estimated time: 203.50 hours', :count => 0 + end + end + def test_settings @request.session[:user_id] = 2 # manager get(:settings, :params => {:id => 1}) diff --git a/test/functional/versions_controller_test.rb b/test/functional/versions_controller_test.rb index b298153bc..3957db43f 100644 --- a/test/functional/versions_controller_test.rb +++ b/test/functional/versions_controller_test.rb @@ -161,7 +161,22 @@ class VersionsControllerTest < Redmine::ControllerTest assert_select 'a', :text => '1 open' end - assert_select '.time-tracking td.total-hours a:first-child', :text => '2.00 hours' + assert_select '.time-tracking tr:first-child' do + assert_select 'th', :text => 'Estimated time' + assert_select 'td.total-hours a', :text => '2.00 hours' + end + + Role.non_member.remove_permission! :view_estimated_hours + + get :show, :params => {:id => 4} + assert_response :success + + assert_select 'p.progress-info' do + assert_select 'a', :text => '1 issue' + assert_select 'a', :text => '1 open' + end + + assert_select '.time-tracking th', :text => 'Estimated time', :count => 0 end def test_show_should_link_to_spent_time_on_version diff --git a/test/integration/api_test/issues_test.rb b/test/integration/api_test/issues_test.rb index f524d8052..9a2aa5f67 100644 --- a/test/integration/api_test/issues_test.rb +++ b/test/integration/api_test/issues_test.rb @@ -427,6 +427,26 @@ class Redmine::ApiTest::IssuesTest < Redmine::ApiTest::Base end end + test "GET /issues/:id.xml should contains total_spent_hours, and should not contains estimated_hours and total_estimated_hours when permission does not exists" do + parent = Issue.find(3) + parent.update_columns :estimated_hours => 2.0 + child = Issue.generate!(:parent_issue_id => parent.id, :estimated_hours => 3.0) + TimeEntry.create!(:project => child.project, :issue => child, :user => child.author, :spent_on => child.author.today, + :hours => '2.5', :comments => '', :activity_id => TimeEntryActivity.first.id) + # remove permission! + Role.anonymous.remove_permission! :view_estimated_hours + + get '/issues/3.xml' + + assert_equal 'application/xml', response.content_type + assert_select 'issue' do + assert_select 'estimated_hours', false + assert_select 'total_estimated_hours', false + assert_select 'spent_hours', parent.spent_hours.to_s + assert_select 'total_spent_hours', (parent.spent_hours.to_f + 2.5).to_s + end + end + test "GET /issues/:id.xml should contains visible spent_hours only" do user = User.find_by_login('jsmith') Role.find(1).update(:time_entries_visibility => 'own') @@ -475,6 +495,25 @@ class Redmine::ApiTest::IssuesTest < Redmine::ApiTest::Base assert_nil json['issue']['total_spent_hours'] end + test "GET /issues/:id.json should contains total_spent_hours, and should not contains estimated_hours and total_estimated_hours when permission does not exists" do + parent = Issue.find(3) + parent.update_columns :estimated_hours => 2.0 + child = Issue.generate!(:parent_issue_id => parent.id, :estimated_hours => 3.0) + TimeEntry.create!(:project => child.project, :issue => child, :user => child.author, :spent_on => child.author.today, + :hours => '2.5', :comments => '', :activity_id => TimeEntryActivity.first.id) + # remove permission! + Role.anonymous.remove_permission! :view_estimated_hours + + get '/issues/3.json' + + assert_equal 'application/json', response.content_type + json = ActiveSupport::JSON.decode(response.body) + assert_nil json['issue']['estimated_hours'] + assert_nil json['issue']['total_estimated_hours'] + assert_equal parent.spent_hours, json['issue']['spent_hours'] + assert_equal (parent.spent_hours.to_f + 2.5), json['issue']['total_spent_hours'] + end + test "GET /issues/:id.json should contains visible spent_hours only" do user = User.find_by_login('jsmith') Role.find(1).update(:time_entries_visibility => 'own') diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb index 524b37072..3a3ae019a 100644 --- a/test/unit/mail_handler_test.rb +++ b/test/unit/mail_handler_test.rb @@ -43,6 +43,10 @@ class MailHandlerTest < ActiveSupport::TestCase end def test_add_issue_with_specific_overrides + project = Project.find_by_name('OnlineStore') + project.enabled_module_names += [:time_tracking] + project.save! + issue = submit_email('ticket_on_given_project.eml', :allow_override => ['status', 'start_date', 'due_date', 'assigned_to', @@ -72,6 +76,10 @@ class MailHandlerTest < ActiveSupport::TestCase end def test_add_issue_with_all_overrides + project = Project.find_by_name('OnlineStore') + project.enabled_module_names += [:time_tracking] + project.save! + issue = submit_email('ticket_on_given_project.eml', :allow_override => 'all') assert issue.is_a?(Issue) assert !issue.new_record?