From 6659aad3ef651b714a66d648cef2f25e3fff8516 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sat, 30 May 2015 07:40:57 +0000 Subject: [PATCH] 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 --- app/controllers/projects_controller.rb | 2 +- app/models/role.rb | 8 +++++ app/models/time_entry.rb | 32 +++++++++++++++++-- app/models/user.rb | 6 ++++ app/views/issues/show.html.erb | 2 +- app/views/projects/_sidebar.html.erb | 6 ++-- app/views/roles/_form.html.erb | 4 +++ app/views/versions/show.html.erb | 2 +- config/locales/en.yml | 1 + config/locales/fr.yml | 3 ++ ...83158_add_roles_time_entries_visibility.rb | 9 ++++++ test/unit/time_entry_test.rb | 32 +++++++++++++++++++ 12 files changed, 100 insertions(+), 7 deletions(-) create mode 100644 db/migrate/20150526183158_add_roles_time_entries_visibility.rb 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) %>

<%= l(:label_spent_time) %>

-

<%= l_hours(@total_hours) %>

+ <% if @total_hours.present? %> +

<%= l_hours(@total_hours) %>

+ <% end %>

<% 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 @@

<%= f.select :issues_visibility, Role::ISSUES_VISIBILITY_OPTIONS.collect {|v| [l(v.last), v.first]} %>

<% end %> + <% unless @role.anonymous? %> +

<%= f.select :time_entries_visibility, Role::TIME_ENTRIES_VISIBILITY_OPTIONS.collect {|v| [l(v.last), v.first]} %>

+ <% end %> +

<%= f.select :users_visibility, Role::USERS_VISIBILITY_OPTIONS.collect {|v| [l(v.last), v.first]} %>

<% 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 @@ <%= l(:field_estimated_hours) %> <%= html_hours(l_hours(@version.estimated_hours)) %> -<% if User.current.allowed_to?(:view_time_entries, @project) %> +<% if User.current.allowed_to_view_all_time_entries?(@project) %> <%= l(:label_spent_time) %> <%= html_hours(l_hours(@version.spent_hours)) %> 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, -- 2.39.5