Adds a role setting that viewing all or own time entries (#8929).

git-svn-id: http://svn.redmine.org/redmine/trunk@14275 e93f8b46-1217-0410-a6f0-8f06a7374b81

12 files changed, 100 insertions, 7 deletions

diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 38636a054..71007f383 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -143,7 +143,7 @@ class ProjectsController < ApplicationController
     @open_issues_by_tracker = Issue.visible.open.where(cond).group(:tracker).count
     @total_issues_by_tracker = Issue.visible.where(cond).group(:tracker).count
 
-    if User.current.allowed_to?(:view_time_entries, @project)
+    if User.current.allowed_to_view_all_time_entries?(@project)
       @total_hours = TimeEntry.visible.where(cond).sum(:hours).to_f
     end
 
diff --git a/app/models/role.rb b/app/models/role.rb
index f1adc01b2..43f32b726 100644
--- a/app/models/role.rb
+++ b/app/models/role.rb
@@ -39,6 +39,11 @@ class Role < ActiveRecord::Base
     ['own', :label_issues_visibility_own]
   ]
 
+  TIME_ENTRIES_VISIBILITY_OPTIONS = [
+    ['all', :label_time_entries_visibility_all],
+    ['own', :label_time_entries_visibility_own]
+  ]
+
   USERS_VISIBILITY_OPTIONS = [
     ['all', :label_users_visibility_all],
     ['members_of_visible_projects', :label_users_visibility_members_of_visible_projects]
@@ -75,6 +80,9 @@ class Role < ActiveRecord::Base
   validates_inclusion_of :users_visibility,
     :in => USERS_VISIBILITY_OPTIONS.collect(&:first),
     :if => lambda {|role| role.respond_to?(:users_visibility) && role.users_visibility_changed?}
+  validates_inclusion_of :time_entries_visibility,
+    :in => TIME_ENTRIES_VISIBILITY_OPTIONS.collect(&:first),
+    :if => lambda {|role| role.respond_to?(:time_entries_visibility) && role.time_entries_visibility_changed?}
 
   # Copies attributes from another role, arg can be an id or a Role
   def copy_from(arg, options={})
diff --git a/app/models/time_entry.rb b/app/models/time_entry.rb
index 87d27c1de..c5a917d96 100644
--- a/app/models/time_entry.rb
+++ b/app/models/time_entry.rb
@@ -46,7 +46,7 @@ class TimeEntry < ActiveRecord::Base
 
   scope :visible, lambda {|*args|
     joins(:project).
-    where(Project.allowed_to_condition(args.shift || User.current, :view_time_entries, *args))
+    where(TimeEntry.visible_condition(args.shift || User.current, *args))
   }
   scope :on_issue, lambda {|issue|
     joins(:issue).
@@ -55,6 +55,32 @@ class TimeEntry < ActiveRecord::Base
 
   safe_attributes 'hours', 'comments', 'project_id', 'issue_id', 'activity_id', 'spent_on', 'custom_field_values', 'custom_fields'
 
+  # Returns a SQL conditions string used to find all time entries visible by the specified user
+  def self.visible_condition(user, options={})
+    Project.allowed_to_condition(user, :view_time_entries, options) do |role, user|
+      if role.time_entries_visibility == 'all'
+        nil
+      elsif role.time_entries_visibility == 'own' && user.id && user.logged?
+        "#{table_name}.user_id = #{user.id}"
+      else
+        '1=0'
+      end
+    end
+  end
+
+  # Returns true if user or current user is allowed to view the time entry
+  def visible?(user=nil)
+    (user || User.current).allowed_to?(:view_time_entries, self.project) do |role, user|
+      if role.time_entries_visibility == 'all'
+        true
+      elsif role.time_entries_visibility == 'own'
+        self.user == user
+      else
+        false
+      end
+    end
+  end
+
   def initialize(attributes=nil, *args)
     super
     if new_record? && self.activity.nil?
@@ -116,7 +142,9 @@ class TimeEntry < ActiveRecord::Base
 
   # Returns true if the time entry can be edited by usr, otherwise false
   def editable_by?(usr)
-    (usr == user && usr.allowed_to?(:edit_own_time_entries, project)) || usr.allowed_to?(:edit_time_entries, project)
+    visible?(usr) && (
+      (usr == user && usr.allowed_to?(:edit_own_time_entries, project)) || usr.allowed_to?(:edit_time_entries, project)
+    )
   end
 
   # Returns the custom_field_values that can be edited by the given user
diff --git a/app/models/user.rb b/app/models/user.rb
index ebfd02ea0..cdaebcf9e 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -634,6 +634,12 @@ class User < Principal
     allowed_to?(action, nil, options.reverse_merge(:global => true), &block)
   end
 
+  def allowed_to_view_all_time_entries?(context)
+    allowed_to?(:view_time_entries, context) do |role, user|
+      role.time_entries_visibility == 'all'
+    end
+  end
+
   # Returns true if the user is allowed to delete the user's own account
   def own_account_deletable?
     Setting.unsubscribe? &&
diff --git a/app/views/issues/show.html.erb b/app/views/issues/show.html.erb
index 2065ce3b3..b91b512a3 100644
--- a/app/views/issues/show.html.erb
+++ b/app/views/issues/show.html.erb
@@ -62,7 +62,7 @@
       rows.right l(:field_estimated_hours), issue_estimated_hours_details(@issue), :class => 'estimated-hours'
     end
   end
-  if User.current.allowed_to?(:view_time_entries, @project)
+  if User.current.allowed_to_view_all_time_entries?(@project)
     if @issue.total_spent_hours > 0
       rows.right l(:label_spent_time), issue_spent_hours_details(@issue), :class => 'spent-time'
     end
diff --git a/app/views/projects/_sidebar.html.erb b/app/views/projects/_sidebar.html.erb
index 0f5c24186..803686ae1 100644
--- a/app/views/projects/_sidebar.html.erb
+++ b/app/views/projects/_sidebar.html.erb
@@ -1,6 +1,8 @@
-<% if @total_hours.present? %>
+<% if User.current.allowed_to?(:view_time_entries, @project) %>
   <h3><%= l(:label_spent_time) %></h3>
-  <p><span class="icon icon-time"><%= l_hours(@total_hours) %></span></p>
+  <% if @total_hours.present? %>
+    <p><span class="icon icon-time"><%= l_hours(@total_hours) %></span></p>
+  <% end %>
   <p>
   <% if User.current.allowed_to?(:log_time, @project) %>
     <%= link_to l(:button_log_time), new_project_time_entry_path(@project) %> |
diff --git a/app/views/roles/_form.html.erb b/app/views/roles/_form.html.erb
index 1d4ce3a2a..03cbe58d2 100644
--- a/app/views/roles/_form.html.erb
+++ b/app/views/roles/_form.html.erb
@@ -10,6 +10,10 @@
     <p><%= f.select :issues_visibility, Role::ISSUES_VISIBILITY_OPTIONS.collect {|v| [l(v.last), v.first]} %></p>
   <% end %>
 
+  <% unless @role.anonymous? %>
+    <p><%= f.select :time_entries_visibility, Role::TIME_ENTRIES_VISIBILITY_OPTIONS.collect {|v| [l(v.last), v.first]} %></p>
+  <% end %>
+
   <p><%= f.select :users_visibility, Role::USERS_VISIBILITY_OPTIONS.collect {|v| [l(v.last), v.first]} %></p>
 
   <% if @role.new_record? && @roles.any? %>
diff --git a/app/views/versions/show.html.erb b/app/views/versions/show.html.erb
index d05729e51..0a45b9c84 100644
--- a/app/views/versions/show.html.erb
+++ b/app/views/versions/show.html.erb
@@ -19,7 +19,7 @@
     <th><%= l(:field_estimated_hours) %></th>
     <td class="total-hours"><%= html_hours(l_hours(@version.estimated_hours)) %></td>
 </tr>
-<% if User.current.allowed_to?(:view_time_entries, @project) %>
+<% if User.current.allowed_to_view_all_time_entries?(@project) %>
 <tr>
     <th><%= l(:label_spent_time) %></th>
     <td class="total-hours"><%= html_hours(l_hours(@version.spent_hours)) %></td>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 92b2ffdeb..02bc82022 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -341,6 +341,7 @@ en:
   field_must_change_passwd: Must change password at next logon
   field_default_status: Default status
   field_users_visibility: Users visibility
+  field_time_entries_visibility: Time logs visibility
 
   setting_app_title: Application title
   setting_app_subtitle: Application subtitle
diff --git a/config/locales/fr.yml b/config/locales/fr.yml
index 539375277..fbaaf7e61 100644
--- a/config/locales/fr.yml
+++ b/config/locales/fr.yml
@@ -361,6 +361,7 @@ fr:
   field_must_change_passwd: Doit changer de mot de passe à la prochaine connexion
   field_default_status: Statut par défaut
   field_users_visibility: Visibilité des utilisateurs
+  field_time_entries_visibility: Visibilité du temps passé
 
   setting_app_title: Titre de l'application
   setting_app_subtitle: Sous-titre de l'application
@@ -962,6 +963,8 @@ fr:
   label_disable_notifications: Désactiver les notifications
   label_blank_value: non renseigné
   label_parent_task_attributes: Attributs des tâches parentes
+  label_time_entries_visibility_all: Tous les temps passés
+  label_time_entries_visibility_own: Ses propres temps passés
 
   button_login: Connexion
   button_submit: Soumettre
diff --git a/db/migrate/20150526183158_add_roles_time_entries_visibility.rb b/db/migrate/20150526183158_add_roles_time_entries_visibility.rb
new file mode 100644
index 000000000..991a14f02
--- /dev/null
+++ b/db/migrate/20150526183158_add_roles_time_entries_visibility.rb
@@ -0,0 +1,9 @@
+class AddRolesTimeEntriesVisibility < ActiveRecord::Migration
+  def self.up
+    add_column :roles, :time_entries_visibility, :string, :limit => 30, :default => 'all', :null => false
+  end
+
+  def self.down
+    remove_column :roles, :time_entries_visibility
+  end
+end
diff --git a/test/unit/time_entry_test.rb b/test/unit/time_entry_test.rb
index b3731a516..9154c6008 100644
--- a/test/unit/time_entry_test.rb
+++ b/test/unit/time_entry_test.rb
@@ -27,6 +27,38 @@ class TimeEntryTest < ActiveSupport::TestCase
            :groups_users,
            :enabled_modules
 
+  def test_visibility_with_permission_to_view_all_time_entries
+    user = User.generate!
+    role = Role.generate!(:permissions => [:view_time_entries], :time_entries_visibility => 'all')
+    Role.non_member.remove_permission! :view_time_entries
+    project = Project.find(1)
+    User.add_to_project user, project, role
+    own = TimeEntry.generate! :user => user, :project => project
+    other = TimeEntry.generate! :user => User.find(2), :project => project
+
+    assert TimeEntry.visible(user).find_by_id(own.id)
+    assert TimeEntry.visible(user).find_by_id(other.id)
+
+    assert own.visible?(user)
+    assert other.visible?(user)
+  end
+
+  def test_visibility_with_permission_to_view_own_time_entries
+    user = User.generate!
+    role = Role.generate!(:permissions => [:view_time_entries], :time_entries_visibility => 'own')
+    Role.non_member.remove_permission! :view_time_entries
+    project = Project.find(1)
+    User.add_to_project user, project, role
+    own = TimeEntry.generate! :user => user, :project => project
+    other = TimeEntry.generate! :user => User.find(2), :project => project
+
+    assert TimeEntry.visible(user).find_by_id(own.id)
+    assert_nil TimeEntry.visible(user).find_by_id(other.id)
+
+    assert own.visible?(user)
+    assert_equal false, other.visible?(user)
+  end
+
   def test_hours_format
     assertions = { "2"      => 2.0,
                    "21.1"   => 21.1,