Estimated hours field able to hide role based

app/controllers/projects_controller.rb

@@ -189,6 +189,8 @@ def show
 
         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
 

app/models/issue.rb

@@ -490,7 +490,6 @@ def estimated_hours=(h)
     'start_date',
     'due_date',
     'done_ratio',
-    'estimated_hours',
     'custom_field_values',
     'custom_fields',
     'lock_version',
@@ -520,6 +519,9 @@ def estimated_hours=(h)
     '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

app/models/issue_query.rb

@@ -47,19 +47,6 @@ class IssueQuery < Query
                     :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 =>
-        lambda do
-          "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)"
-        end,
-      :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),
@@ -205,10 +192,9 @@ def initialize_available_filters
     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
@@ -283,9 +269,28 @@ def available_columns
     @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}" +
@@ -307,8 +312,9 @@ def available_columns
         " 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',

app/models/journal.rb

@@ -111,6 +111,8 @@ def visible_details(user=User.current)
         detail.custom_field && detail.custom_field.visible_by?(project, user)
       elsif detail.property == 'relation'
         Issue.find_by_id(detail.value || detail.old_value).try(:visible?, user)
+      elsif detail.property == 'attr' && detail.prop_key == 'estimated_hours'
+        user.allowed_to?(:view_estimated_hours, project)
       else
         true
       end

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)

app/views/issues/show.html.erb

@@ -68,7 +68,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

app/views/projects/show.html.erb

@@ -97,24 +97,27 @@
   </div>
   <% 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 %>
   <div class="spent_time box">
     <h3 class="icon icon-time"><%= l(:label_time_tracking) %></h3>
     <ul>
-      <% if @total_estimated_hours.present? %>
+      <% if @total_estimated_hours.present? && allowed_to_view_estimated_hours %>
         <li><%= l(:field_estimated_hours) %>: <%= l_hours(@total_estimated_hours) %>
       <% end %>
-      <% if @total_hours.present? %>
-          <li><%= l(:label_spent_time) %>: <%= l_hours(@total_hours) %>
+      <% if @total_hours.present? && allowed_to_view_time_entries %>
+        <li><%= l(:label_spent_time) %>: <%= l_hours(@total_hours) %>
       <% end %>
     </ul>
-    <p>
-    <% if User.current.allowed_to?(:log_time, @project) %>
-      <%= link_to l(:button_log_time), new_project_time_entry_path(@project) %> |
+    <% if allowed_to_view_time_entries %>
+      <p>
+        <% if User.current.allowed_to?(:log_time, @project) %>
+          <%= link_to l(:button_log_time), new_project_time_entry_path(@project) %> |
+        <% end %>
+        <%= link_to(l(:label_details), project_time_entries_path(@project)) %> |
+        <%= link_to(l(:label_report), report_project_time_entries_path(@project)) %>
+      </p>
     <% end %>
-    <%= link_to(l(:label_details), project_time_entries_path(@project)) %> |
-    <%= link_to(l(:label_report), report_project_time_entries_path(@project)) %>
-    </p>
   </div>
 <% end %>
   <%= call_hook(:view_projects_show_left, :project => @project) %>

app/views/versions/show.html.erb

@@ -14,15 +14,18 @@
 <%= render(:partial => "wiki/content", :locals => {:content => @version.wiki_page.content}) if @version.wiki_page %>
 
 <div id="version-summary">
-<% 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 %>
 <fieldset class="time-tracking"><legend><%= l(:label_time_tracking) %></legend>
 <table>
-<tr>
-    <th><%= l(:field_estimated_hours) %></th>
-    <td class="total-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]) %></td>
-</tr>
-<% if User.current.allowed_to_view_all_time_entries?(@project) %>
+<% if allowed_to_view_estimated_hours %>
+  <tr>
+      <th><%= l(:field_estimated_hours) %></th>
+      <td class="total-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]) %></td>
+  </tr>
+<% end %>
+<% if allowed_to_view_all_time_entries %>
 <tr>
     <th><%= l(:label_spent_time) %></th>
     <td class="total-hours"><%= link_to html_hours(l_hours(@version.spent_hours)),

config/locales/en.yml

@@ -550,6 +550,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

db/migrate/20220714211705_add_view_estimated_hours_to_all_existing_roles.rb

@@ -0,0 +1,9 @@
+class AddViewEstimatedHoursToAllExistingRoles < ActiveRecord::Migration[6.1]
+  def up
+    Role.all.each { |role| role.add_permission! :view_estimated_hours }
+  end
+
+  def down
+    Role.all.each { |role| role.remove_permission! :view_estimated_hours }
+  end
+end

lib/redmine/default_data/loader.rb

@@ -71,6 +71,7 @@ def load(lang=nil, options={})
                   :view_calendar,
                   :log_time,
                   :view_time_entries,
+                  :view_estimated_hours,
                   :view_news,
                   :comment_news,
                   :view_documents,
@@ -102,6 +103,7 @@ def load(lang=nil, options={})
                   :view_calendar,
                   :log_time,
                   :view_time_entries,
+                  :view_estimated_hours,
                   :view_news,
                   :comment_news,
                   :view_documents,
@@ -122,6 +124,7 @@ def load(lang=nil, options={})
                                                             :view_gantt,
                                                             :view_calendar,
                                                             :view_time_entries,
+                                                            :view_estimated_hours,
                                                             :view_news,
                                                             :comment_news,
                                                             :view_documents,
@@ -137,6 +140,7 @@ def load(lang=nil, options={})
                                                            :view_gantt,
                                                            :view_calendar,
                                                            :view_time_entries,
+                                                           :view_estimated_hours,
                                                            :view_news,
                                                            :view_documents,
                                                            :view_wiki_pages,

lib/redmine/export/pdf/issues_pdf_helper.rb

@@ -57,7 +57,8 @@ def issue_to_pdf(issue, assoc={})
           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].max

lib/redmine/preparation.rb

@@ -83,6 +83,7 @@ def self.prepare
 
         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]},

test/fixtures/roles.yml

@@ -35,6 +35,7 @@ roles_001:
     - :view_calendar
     - :log_time
     - :view_time_entries
+    - :view_estimated_hours
     - :edit_time_entries
     - :delete_time_entries
     - :import_time_entries
@@ -102,6 +103,7 @@ roles_002:
     - :view_calendar
     - :log_time
     - :view_time_entries
+    - :view_estimated_hours
     - :edit_own_time_entries
     - :view_news
     - :manage_news
@@ -151,6 +153,7 @@ roles_003:
     - :view_calendar
     - :log_time
     - :view_time_entries
+    - :view_estimated_hours
     - :view_news
     - :manage_news
     - :comment_news
@@ -191,6 +194,7 @@ roles_004:
     - :view_calendar
     - :log_time
     - :view_time_entries
+    - :view_estimated_hours
     - :view_news
     - :comment_news
     - :view_documents
@@ -218,6 +222,7 @@ roles_005:
     - :view_gantt
     - :view_calendar
     - :view_time_entries
+    - :view_estimated_hours
     - :view_news
     - :view_documents
     - :view_wiki_pages

test/functional/issues_controller_test.rb

@@ -1607,6 +1607,15 @@ def test_index_with_total_estimated_hours_column
     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(

test/functional/projects_controller_test.rb

@@ -837,6 +837,29 @@ def test_show_should_spent_and_estimated_time
     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})

test/functional/versions_controller_test.rb

@@ -165,7 +165,22 @@ def test_show_issue_calculations_should_take_into_account_only_visible_issues
       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