diff --git a/lib/open_project/costs/attributes_helper.rb b/lib/open_project/costs/attributes_helper.rb index c0feeba123..5d516e7139 100644 --- a/lib/open_project/costs/attributes_helper.rb +++ b/lib/open_project/costs/attributes_helper.rb @@ -33,11 +33,23 @@ module OpenProject::Costs end def time_entries - @work_package.time_entries.visible(@user, @work_package.project) + if @work_package.time_entries.loaded? + @work_package.time_entries.select do |time_entry| + time_entry.visible_by?(@user) + end + else + @work_package.time_entries.visible(@user, @work_package.project) + end end def cost_entries - @cost_entries ||= @work_package.cost_entries.visible(@user, @work_package.project) + @cost_entries ||= if @work_package.cost_entries.loaded? + @work_package.cost_entries.select do |cost_entry| + cost_entry.visible_by?(@user) + end + else + @work_package.cost_entries.visible(@user, @work_package.project) + end end private diff --git a/lib/open_project/costs/engine.rb b/lib/open_project/costs/engine.rb index cdc6e0b4d8..c7bfd69365 100644 --- a/lib/open_project/costs/engine.rb +++ b/lib/open_project/costs/engine.rb @@ -286,6 +286,14 @@ module OpenProject::Costs # TODO: this recreates the original behaviour # however, it might not be desirable to allow assigning of cost_object regardless of the permissions PermittedParams.permit(:new_work_package, :cost_object_id) + + require 'api/v3/work_packages/work_package_representer' + + API::V3::WorkPackages::WorkPackageRepresenter.to_eager_load += [{ time_entries: [:project, + :user] }, + { cost_entries: [:project, + :user] }, + :cost_object] end config.to_prepare do |_app| diff --git a/lib/open_project/costs/patches/time_entry_patch.rb b/lib/open_project/costs/patches/time_entry_patch.rb index e0dfc1971d..03e5d4acdb 100644 --- a/lib/open_project/costs/patches/time_entry_patch.rb +++ b/lib/open_project/costs/patches/time_entry_patch.rb @@ -28,17 +28,20 @@ module OpenProject::Costs::Patches::TimeEntryPatch base.class_eval do belongs_to :rate, -> { where(type: ['HourlyRate', 'DefaultHourlyRate']) }, class_name: 'Rate' - scope :visible, lambda { |*args| - where(TimeEntry.visible_condition(args[0] || User.current, args[1])) - .includes(:project, :user) - .references(:project) - } - before_save :update_costs - def self.visible_condition(user, project) - %{ (#{Project.allowed_to_condition(user, :view_time_entries, project: project)} OR - (#{Project.allowed_to_condition(user, :view_own_time_entries, project: project)} AND #{TimeEntry.table_name}.user_id = #{user.id})) } + def self.visible_condition(user, table_alias: nil, project: nil) + options = {} + options[:project_alias] = table_alias if table_alias + options[:project] = project if project + + %{ (#{Project.allowed_to_condition(user, + :view_time_entries, + options)} OR + (#{Project.allowed_to_condition(user, + :view_own_time_entries, + options)} AND + #{TimeEntry.table_name}.user_id = #{user.id})) } end scope :visible_costs, lambda{|*args| @@ -47,7 +50,7 @@ module OpenProject::Costs::Patches::TimeEntryPatch view_hourly_rates = %{ (#{Project.allowed_to_condition(user, :view_hourly_rates, project: project)} OR (#{Project.allowed_to_condition(user, :view_own_hourly_rate, project: project)} AND #{TimeEntry.table_name}.user_id = #{user.id})) } - view_time_entries = TimeEntry.visible_condition(user, project) + view_time_entries = TimeEntry.visible_condition(user, project: project) includes(:project, :user) .where([view_time_entries, view_hourly_rates].join(' AND ')) @@ -126,7 +129,8 @@ module OpenProject::Costs::Patches::TimeEntryPatch end def visible_by?(usr) - usr.allowed_to?(:view_time_entries, project) + usr.allowed_to?(:view_time_entries, project) || + (user_id == usr.id && usr.allowed_to?(:view_own_time_entries, project)) end def costs_visible_by?(usr) diff --git a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb index 0f7df0cebf..daee36e5e5 100644 --- a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb @@ -243,4 +243,24 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do it { is_expected.not_to have_json_path('_links/log_costs') } end end + + describe '.to_eager_load' do + it 'includes cost entries with dependencies' do + expect(described_class.to_eager_load.any? { |el| + el.is_a?(Hash) && el[:cost_entries] == [:project, :user] + }).to be_truthy + end + + it 'includes time entries with dependencies' do + expect(described_class.to_eager_load.any? { |el| + el.is_a?(Hash) && el[:time_entries] == [:project, :user] + }).to be_truthy + end + + it 'includes the cost objects' do + expect(described_class.to_eager_load.any? { |el| + el == :cost_object + }).to be_truthy + end + end end diff --git a/spec/models/time_entry_spec.rb b/spec/models/time_entry_spec.rb index 2532d14323..ea786b39e2 100644 --- a/spec/models/time_entry_spec.rb +++ b/spec/models/time_entry_spec.rb @@ -295,6 +295,54 @@ describe TimeEntry, type: :model do end end + describe 'visible_by?' do + context 'when not having the necessary permissions' do + before do + is_member(project, user, []) + end + + it 'is visible' do + expect(time_entry.visible_by?(user)).to be_falsey + end + end + + context 'when having the view_time_entries permission' do + before do + is_member(project, user, [:view_time_entries]) + end + + it 'is visible' do + expect(time_entry.visible_by?(user)).to be_truthy + end + end + + context 'when having the view_own_time_entries permission ' + + 'and being the owner of the time entry' do + before do + is_member(project, user, [:view_own_time_entries]) + + time_entry.user = user + end + + it 'is visible' do + expect(time_entry.visible_by?(user)).to be_truthy + end + end + + context 'when having the view_own_time_entries permission ' + + 'and not being the owner of the time entry' do + before do + is_member(project, user, [:view_own_time_entries]) + + time_entry.user = FactoryGirl.build :user + end + + it 'is visible' do + expect(time_entry.visible_by?(user)).to be_falsey + end + end + end + describe 'class' do describe '#visible' do describe "WHEN having the view_time_entries permission