From 7fcf3bde887dbf670213e2bc2f761ad6ac21df43 Mon Sep 17 00:00:00 2001 From: Alex Coles Date: Mon, 29 Jun 2015 18:27:33 +0200 Subject: [PATCH 01/11] Bump Rails dependency to latest 4.1.x Signed-off-by: Alex Coles --- openproject-costs.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openproject-costs.gemspec b/openproject-costs.gemspec index 90b90b0f9e..87125d93df 100644 --- a/openproject-costs.gemspec +++ b/openproject-costs.gemspec @@ -17,7 +17,7 @@ Gem::Specification.new do |s| s.files = Dir['{app,config,db,lib,doc}/**/*', 'README.md'] s.test_files = Dir['spec/**/*'] - s.add_dependency 'rails', '~> 4.0.13' + s.add_dependency 'rails', '~> 4.1.11' s.add_development_dependency 'factory_girl_rails', '~> 4.0' end From 8cb645f314b183a63e8e7494b4fe6ba87c4c5f95 Mon Sep 17 00:00:00 2001 From: Alex Coles Date: Sat, 29 Aug 2015 17:49:10 +0200 Subject: [PATCH 02/11] Fix obsolete calls to calculation methods w/cond. Signed-off-by: Alex Coles --- app/controllers/costlog_controller.rb | 8 ++++++-- lib/open_project/costs/attributes_helper.rb | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/controllers/costlog_controller.rb b/app/controllers/costlog_controller.rb index d6d554e354..2039f2e2d4 100644 --- a/app/controllers/costlog_controller.rb +++ b/app/controllers/costlog_controller.rb @@ -244,8 +244,12 @@ class CostlogController < ApplicationController end @from, @to = @to, @from if @from && @to && @from > @to - @from ||= (CostEntry.minimum(:spent_on, include: [:project, :user], conditions: Project.allowed_to_condition(User.current, :view_cost_entries)) || Date.today) - 1 - @to ||= (CostEntry.maximum(:spent_on, include: [:project, :user], conditions: Project.allowed_to_condition(User.current, :view_cost_entries)) || Date.today) + @from ||= (CostEntry.includes([:project, :user]) + .where(Project.allowed_to_condition(User.current, :view_cost_entries)) + .minimum(:spent_on) || Date.today) - 1 + @to ||= (CostEntry.includes([:project, :user]) + .where(Project.allowed_to_condition(User.current, :view_cost_entries)) + .maximum(:spent_on) || Date.today) end def new_default_cost_entry diff --git a/lib/open_project/costs/attributes_helper.rb b/lib/open_project/costs/attributes_helper.rb index 04b3f8bca7..c0feeba123 100644 --- a/lib/open_project/costs/attributes_helper.rb +++ b/lib/open_project/costs/attributes_helper.rb @@ -29,7 +29,7 @@ module OpenProject::Costs end def summarized_cost_entries - @summarized_cost_entries ||= cost_entries.calculate(:sum, :units, group: :cost_type) + @summarized_cost_entries ||= cost_entries.group(:cost_type).calculate(:sum, :units) end def time_entries From cf0979dec70786672909c5a4089ebe7362554046 Mon Sep 17 00:00:00 2001 From: Alex Coles Date: Sat, 29 Aug 2015 19:34:54 +0200 Subject: [PATCH 03/11] Add missing references to CostEntry scopes Signed-off-by: Alex Coles --- app/models/cost_entry.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/cost_entry.rb b/app/models/cost_entry.rb index f099793a9e..667e54cbc1 100644 --- a/app/models/cost_entry.rb +++ b/app/models/cost_entry.rb @@ -40,6 +40,7 @@ class CostEntry < ActiveRecord::Base scope :visible, lambda { |*args| where(CostEntry.visible_condition(args[0] || User.current, args[1])) .includes([:project, :user]) + .references(:project) } scope :on_work_packages, ->(work_packages) { where(work_package_id: work_packages) } From 5fff13c67e95410cf349ccfc884110ac3109aaac Mon Sep 17 00:00:00 2001 From: Alex Coles Date: Sat, 29 Aug 2015 19:57:49 +0200 Subject: [PATCH 04/11] Add missing references to TimeEntry patch scopes Signed-off-by: Alex Coles --- lib/open_project/costs/patches/time_entry_patch.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/open_project/costs/patches/time_entry_patch.rb b/lib/open_project/costs/patches/time_entry_patch.rb index 5e5742f776..2220e30604 100644 --- a/lib/open_project/costs/patches/time_entry_patch.rb +++ b/lib/open_project/costs/patches/time_entry_patch.rb @@ -32,6 +32,7 @@ module OpenProject::Costs::Patches::TimeEntryPatch scope :visible, lambda { |*args| where(TimeEntry.visible_condition(args[0] || User.current, args[1])) .includes(:project, :user) + .references(:project) } before_save :update_costs From d2822c1302838189893de0dd907dfbbc2709df69 Mon Sep 17 00:00:00 2001 From: Alex Coles Date: Sat, 29 Aug 2015 20:20:49 +0200 Subject: [PATCH 05/11] Add missing references to WorkPackage patch Signed-off-by: Alex Coles --- lib/open_project/costs/patches/work_package_patch.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/open_project/costs/patches/work_package_patch.rb b/lib/open_project/costs/patches/work_package_patch.rb index 26ff20347e..c11de5cd6a 100644 --- a/lib/open_project/costs/patches/work_package_patch.rb +++ b/lib/open_project/costs/patches/work_package_patch.rb @@ -66,8 +66,10 @@ module OpenProject::Costs::Patches::WorkPackagePatch false when 'reassign' - reassign_to = WorkPackage.includes(:project) + reassign_to = WorkPackage .where(Project.allowed_to_condition(user, :edit_cost_entries)) + .includes(:project) + .references(:projects) .find_by_id(to_do[:reassign_to_id]) if reassign_to.nil? From 770a4a609718c555fb46ff7b72cef3269298040c Mon Sep 17 00:00:00 2001 From: Alex Coles Date: Sat, 29 Aug 2015 21:07:16 +0200 Subject: [PATCH 06/11] Remove redundant .all call in AR query chains Signed-off-by: Alex Coles --- spec/models/cost_entry_spec.rb | 8 ++++---- spec/models/time_entry_spec.rb | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/models/cost_entry_spec.rb b/spec/models/cost_entry_spec.rb index 8c3728afd0..507f5f8ee5 100644 --- a/spec/models/cost_entry_spec.rb +++ b/spec/models/cost_entry_spec.rb @@ -99,7 +99,7 @@ describe CostEntry, type: :model do cost_entry.save! end - it { expect(CostEntry.visible(user2, project).all).to match_array([cost_entry]) } + it { expect(CostEntry.visible(user2, project)).to match_array([cost_entry]) } end describe "WHEN not having the view_cost_entries permission @@ -111,7 +111,7 @@ describe CostEntry, type: :model do cost_entry.save! end - it { expect(CostEntry.visible(user2, project).all).to match_array([]) } + it { expect(CostEntry.visible(user2, project)).to match_array([]) } end describe "WHEN having the view_own_cost_entries permission @@ -123,7 +123,7 @@ describe CostEntry, type: :model do cost_entry.save! end - it { expect(CostEntry.visible(user2, project).all).to match_array([]) } + it { expect(CostEntry.visible(user2, project)).to match_array([]) } end describe "WHEN having the view_own_cost_entries permission @@ -135,7 +135,7 @@ describe CostEntry, type: :model do cost_entry2.save! end - it { expect(CostEntry.visible(cost_entry2.user, project).all).to match_array([cost_entry2]) } + it { expect(CostEntry.visible(cost_entry2.user, project)).to match_array([cost_entry2]) } end end end diff --git a/spec/models/time_entry_spec.rb b/spec/models/time_entry_spec.rb index ee059b8657..2532d14323 100644 --- a/spec/models/time_entry_spec.rb +++ b/spec/models/time_entry_spec.rb @@ -306,7 +306,7 @@ describe TimeEntry, type: :model do time_entry.save! end - it { expect(TimeEntry.visible(user2, project).all).to match_array([time_entry]) } + it { expect(TimeEntry.visible(user2, project)).to match_array([time_entry]) } end describe "WHEN not having the view_time_entries permission @@ -318,7 +318,7 @@ describe TimeEntry, type: :model do time_entry.save! end - it { expect(TimeEntry.visible(user2, project).all).to match_array([]) } + it { expect(TimeEntry.visible(user2, project)).to match_array([]) } end describe "WHEN having the view_own_time_entries permission @@ -332,7 +332,7 @@ describe TimeEntry, type: :model do time_entry.save! end - it { expect(TimeEntry.visible(user2, project).all).to match_array([]) } + it { expect(TimeEntry.visible(user2, project)).to match_array([]) } end describe "WHEN having the view_own_time_entries permission @@ -346,7 +346,7 @@ describe TimeEntry, type: :model do time_entry2.save! end - it { expect(TimeEntry.visible(time_entry2.user, project).all).to match_array([time_entry2]) } + it { expect(TimeEntry.visible(time_entry2.user, project)).to match_array([time_entry2]) } end end From 27d2ceacf233e748ab026a19fd61541dc2afb580 Mon Sep 17 00:00:00 2001 From: Alex Coles Date: Sat, 29 Aug 2015 21:31:34 +0200 Subject: [PATCH 07/11] Migrate remaining scopes to use lambda/block Signed-off-by: Alex Coles --- app/models/labor_budget_item.rb | 5 ++--- lib/open_project/costs/patches/time_entry_patch.rb | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/app/models/labor_budget_item.rb b/app/models/labor_budget_item.rb index ca94a2498f..1aee14fb85 100644 --- a/app/models/labor_budget_item.rb +++ b/app/models/labor_budget_item.rb @@ -40,9 +40,8 @@ class LaborBudgetItem < ActiveRecord::Base end scope :visible_costs, lambda{|*args| - { include: [{ cost_object: :project }, :user], - conditions: LaborBudgetItem.visible_condition((args.first || User.current), args[1]) - } + includes([{ cost_object: :project }, :user]) + .where(LaborBudgetItem.visible_condition((args.first || User.current), args[1])) } def costs diff --git a/lib/open_project/costs/patches/time_entry_patch.rb b/lib/open_project/costs/patches/time_entry_patch.rb index 2220e30604..cae29dbd0f 100644 --- a/lib/open_project/costs/patches/time_entry_patch.rb +++ b/lib/open_project/costs/patches/time_entry_patch.rb @@ -50,9 +50,8 @@ module OpenProject::Costs::Patches::TimeEntryPatch (#{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) - { include: [:project, :user], - conditions: [view_time_entries, view_hourly_rates].join(' AND ') - } + includes(:project, :user) + .where([view_time_entries, view_hourly_rates].join(' AND ')) } end end From 88715c2021eb92dd401bea6dcb65e950f4411d31 Mon Sep 17 00:00:00 2001 From: Alex Coles Date: Sat, 29 Aug 2015 22:25:54 +0200 Subject: [PATCH 08/11] Fix obsolete finder w/cond. in CostTypesController Signed-off-by: Alex Coles --- app/controllers/cost_types_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/cost_types_controller.rb b/app/controllers/cost_types_controller.rb index c32ada3a27..bfb8630a06 100644 --- a/app/controllers/cost_types_controller.rb +++ b/app/controllers/cost_types_controller.rb @@ -35,7 +35,7 @@ class CostTypesController < ApplicationController 'unit_plural' => "#{CostType.table_name}.unit_plural" } sort_update sort_columns - @cost_types = CostType.find :all, order: sort_clause + @cost_types = CostType.order(sort_clause) unless params[:clear_filter] @fixed_date = Date.parse(params[:fixed_date]) rescue Date.today From 7cafbfd2a439daba0c8b13368e96fd860029790a Mon Sep 17 00:00:00 2001 From: Alex Coles Date: Sun, 30 Aug 2015 10:27:34 +0200 Subject: [PATCH 09/11] Add missing references to BudgetItem scopes Signed-off-by: Alex Coles --- app/models/labor_budget_item.rb | 1 + app/models/material_budget_item.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/app/models/labor_budget_item.rb b/app/models/labor_budget_item.rb index 1aee14fb85..dcc3921ea9 100644 --- a/app/models/labor_budget_item.rb +++ b/app/models/labor_budget_item.rb @@ -42,6 +42,7 @@ class LaborBudgetItem < ActiveRecord::Base scope :visible_costs, lambda{|*args| includes([{ cost_object: :project }, :user]) .where(LaborBudgetItem.visible_condition((args.first || User.current), args[1])) + .references(:projects) } def costs diff --git a/app/models/material_budget_item.rb b/app/models/material_budget_item.rb index b3eeaf452f..d29c4cb5ad 100644 --- a/app/models/material_budget_item.rb +++ b/app/models/material_budget_item.rb @@ -35,6 +35,7 @@ class MaterialBudgetItem < ActiveRecord::Base scope :visible_costs, lambda { |*args| where(MaterialBudgetItem.visible_condition((args.first || User.current), args[1])) .includes(cost_object: :project) + .references(:projects) } def costs From eea0bb0396db642a5350e0054d1a9bea43b73a3c Mon Sep 17 00:00:00 2001 From: Alex Coles Date: Sun, 30 Aug 2015 10:46:53 +0200 Subject: [PATCH 10/11] Fix obsolete .update_all w/cond. in cuke steps Signed-off-by: Alex Coles --- features/step_definitions/cost_steps.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/features/step_definitions/cost_steps.rb b/features/step_definitions/cost_steps.rb index 70608f3f07..2df3b30632 100755 --- a/features/step_definitions/cost_steps.rb +++ b/features/step_definitions/cost_steps.rb @@ -35,11 +35,9 @@ Given /^there (?:is|are) (\d+) (default )?hourly rate[s]? with the following:$/ end send_table_to_object(hr, table, user: Proc.new do |rate, value| unless rate.project.nil? || User.find_by_login(value).projects.include?(rate.project) - Rate.update_all({ project_id: User.find_by_login(value).projects(order: 'id ASC').last.id }, - id: rate.id) + Rate.where(id: rate.id).update_all(project_id: User.find_by_login(value).projects(order: 'id ASC').last.id) end - Rate.update_all({ user_id: User.find_by_login(value).id }, - id: rate.id) + Rate.where(id: rate.id).update_all(user_id: User.find_by_login(value).id) end, valid_from: Proc.new do |rate, value| # This works for definitions like "2 years ago" From 83926d93e309e48d22fd9e53460cabfa25581340 Mon Sep 17 00:00:00 2001 From: Alex Coles Date: Sun, 30 Aug 2015 11:52:33 +0200 Subject: [PATCH 11/11] Replace Minitest assertion in Cuke step Signed-off-by: Alex Coles --- features/step_definitions/cost_type_steps.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/features/step_definitions/cost_type_steps.rb b/features/step_definitions/cost_type_steps.rb index f57b3fc785..b19f548c5f 100644 --- a/features/step_definitions/cost_type_steps.rb +++ b/features/step_definitions/cost_type_steps.rb @@ -64,9 +64,7 @@ When /^I expect to click "([^"]*)" on a confirmation box saying "([^"]*)"$/ do | end When /^the confirmation box should have been displayed$/ do - assert page.evaluate_script('document.cookie').include?(@expected_message), - "Expected confirm box with message: '#{@expected_message}'" + - " got: '#{page.evaluate_script('document.cookie')}'" + expect(page.evaluate_script('document.cookie')).to include(@expected_message) end Then(/^the cost type "(.*?)" should not be listed on the index page$/) do |name|