diff --git a/.travis.yml b/.travis.yml index 91890d7123..67c75f9685 100644 --- a/.travis.yml +++ b/.travis.yml @@ -59,7 +59,7 @@ env: - COVERAGE=true matrix: - - "TEST_SUITE=plugins:spec DB=mysql GROUP_SIZE=1 GROUP=1" + - "TEST_SUITE=plugins:specs DB=mysql GROUP_SIZE=1 GROUP=1" - "TEST_SUITE=plugins:cucumber DB=mysql GROUP_SIZE=1 GROUP=1" - "TEST_SUITE=npm" diff --git a/app/models/cost_scopes.rb b/app/models/cost_scopes.rb index bea0df4637..f7e0a6704c 100644 --- a/app/models/cost_scopes.rb +++ b/app/models/cost_scopes.rb @@ -3,13 +3,13 @@ module CostScopes base_module.class_eval do def self.extended(base_class) base_class.class_eval do - scope :visible, -> (*args) { + scope :visible, ->(*args) { user = args.first || User.current with_visible_entries_on self, user: user, project: args[1] } - scope :visible_costs, -> (*args) { + scope :visible_costs, ->(*args) { user = args.first || User.current with_visible_costs_on self, user: user, project: args[1] diff --git a/app/models/labor_budget_item.rb b/app/models/labor_budget_item.rb index 83bbb3c929..8be88090c7 100644 --- a/app/models/labor_budget_item.rb +++ b/app/models/labor_budget_item.rb @@ -36,7 +36,7 @@ class LaborBudgetItem < ActiveRecord::Base table = self.arel_table view_allowed = Project.allowed_to(user, :view_hourly_rates).select(:id) - view_own_allowed = Project.allowed_to(user, :view_own_hourly_rates).select(:id) + view_own_allowed = Project.allowed_to(user, :view_own_hourly_rate).select(:id) view_or_view_own = table[:project_id] .in(view_allowed.arel) diff --git a/app/models/time_entry/time_entry_scopes.rb b/app/models/time_entry/time_entry_scopes.rb index 10f97ffbd0..e720178113 100644 --- a/app/models/time_entry/time_entry_scopes.rb +++ b/app/models/time_entry/time_entry_scopes.rb @@ -14,7 +14,7 @@ class TimeEntry table = self.arel_table view_allowed = Project.allowed_to(user, :view_hourly_rates).select(:id) - view_own_allowed = Project.allowed_to(user, :view_own_hourly_rates).select(:id) + view_own_allowed = Project.allowed_to(user, :view_own_hourly_rate).select(:id) scope.where view_or_view_own(table, view_allowed, view_own_allowed, user) end diff --git a/app/models/work_package/labor_costs.rb b/app/models/work_package/labor_costs.rb index cc0a9d7e29..0eeac91bdd 100644 --- a/app/models/work_package/labor_costs.rb +++ b/app/models/work_package/labor_costs.rb @@ -17,7 +17,7 @@ class WorkPackage end def sum_subselect(base_scope) - super.project('SUM(hours) hours') + super end end end diff --git a/lib/open_project/costs/engine.rb b/lib/open_project/costs/engine.rb index cc6c0cb173..9a88919cb0 100644 --- a/lib/open_project/costs/engine.rb +++ b/lib/open_project/costs/engine.rb @@ -356,27 +356,34 @@ module OpenProject::Costs end def self.join_costs(scope) - material = WorkPackage::MaterialCosts.new - labor = WorkPackage::LaborCosts.new - # The core adds a "LEFT OUTER JOIN time_entries" where the on clause # allows all time entries to be joined if he has the :view_time_entries. - # Because the cost scopes add another "LEFT OUTER JOIN time_entries" - # where the on clause allows all time entries to be joined if he has + # Costs will add another "LEFT OUTER JOIN time_entries". The two joins + # may or may not include each other's rows depending on the user's and the project's permissions. + # This is caused by entries being joined if he has # the :view_time_entries permission and additionally those which are # his and for which he has the :view_own_time_entries permission. - # Because costs join includes the values of the core, entries are joined twice. - # We therefore have to remove core's join. + # Because of that, entries may be joined twice. + # We therefore modify the core's join by placing it in a subquery similar to those of costs. # # This is very hacky. # # We also have to remove the sum calcualtion for time_entries.hours as # the calculation is later on performed within the subquery added by # LaborCosts. With it, we can use the value as it is calculated by the subquery. + material = WorkPackage::MaterialCosts.new + labor = WorkPackage::LaborCosts.new + time = scope.dup + + wp_table = WorkPackage.arel_table + time_join = wp_table + .outer_join(time.arel.as('spent_time_hours')) + .on(wp_table[:id].eq(time.arel_table.alias('spent_time_hours')[:id])) + scope.joins_values.reject! do |join| join.is_a?(Arel::Nodes::OuterJoin) && - join.left.is_a?(Arel::Table) && - join.left.name == 'time_entries' + join.left.is_a?(Arel::Table) && + join.left.name == 'time_entries' end scope.select_values.reject! do |select| select == "SUM(time_entries.hours) AS hours" @@ -385,16 +392,18 @@ module OpenProject::Costs material_scope = material.add_to_work_package_collection(scope.dup) labor_scope = labor.add_to_work_package_collection(scope.dup) - target_scope = scope.joins(material_scope.join_sources) + target_scope = scope + .joins(material_scope.join_sources) .joins(labor_scope.join_sources) + .joins(time_join.join_sources) .select(material_scope.select_values) .select(labor_scope.select_values) - .select('time_entries.hours') + .select('spent_time_hours.hours') target_scope.joins_values.reject! do |join| join.is_a?(Arel::Nodes::OuterJoin) && - join.left.is_a?(Arel::Nodes::TableAlias) && - join.left.right == 'descendants' + join.left.is_a?(Arel::Nodes::TableAlias) && + join.left.right == 'descendants' end target_scope.group_values.reject! do |group| diff --git a/spec/models/work_package/cost_eager_loading_spec.rb b/spec/models/work_package/cost_eager_loading_spec.rb index 8ce06b2a64..9037ab7e14 100644 --- a/spec/models/work_package/cost_eager_loading_spec.rb +++ b/spec/models/work_package/cost_eager_loading_spec.rb @@ -58,7 +58,6 @@ describe WorkPackage, 'cost eager loading', type: :model do user: user, work_package: work_package, project: project) - end let(:time_entry1) do FactoryGirl.create(:time_entry, @@ -84,9 +83,9 @@ describe WorkPackage, 'cost eager loading', type: :model do context "combining core's and cost's eager loading" do let(:scope) do - scope = WorkPackage .include_spent_hours(user) + .select('work_packages.*') .where(id: [work_package.id]) OpenProject::Costs::Engine::EagerLoadedCosts.join_costs(scope)