From 7e7679b0dff091f3a4162ae0e5ab2bce5d2b1941 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Wed, 8 May 2013 09:19:39 +0200 Subject: [PATCH 01/10] fixes cost_type creation --- app/controllers/cost_types_controller.rb | 19 +++++++++++++++++-- app/views/cost_types/_rate.html.erb | 2 +- app/views/cost_types/edit.html.erb | 4 ++-- app/views/cost_types/index.html.erb | 19 +++++++++++-------- config/routes.rb | 8 ++++---- features/step_definitions/cost_steps.rb | 6 ++---- spec/routing/cost_types_routing_spec.rb | 16 ++++++++++------ 7 files changed, 47 insertions(+), 27 deletions(-) diff --git a/app/controllers/cost_types_controller.rb b/app/controllers/cost_types_controller.rb index f95862bbdb..c173b6b1ad 100644 --- a/app/controllers/cost_types_controller.rb +++ b/app/controllers/cost_types_controller.rb @@ -6,8 +6,6 @@ class CostTypesController < ApplicationController before_filter :find_cost_type, :only => [:set_rate, :toggle_delete] before_filter :find_optional_cost_type, :only => [:edit, :update] - verify :method => :post, :only => [:set_rate, :toggle_delete], :redirect_to => { :action => :index } - helper :sort include SortHelper helper :cost_types @@ -94,6 +92,23 @@ class CostTypesController < ApplicationController flash.now[:error] = l(:notice_locking_conflict) end + def create + # TODO: method is copied over from edit + # remove code as appropriate + @cost_type = CostType.new(params[:cost_type]) + + if @cost_type.save + flash[:notice] = l(:notice_successful_update) + redirect_back_or_default(:action => 'index') + else + @cost_type.rates.build({:valid_from => Date.today}) if @cost_type.rates.empty? + render :action => "edit", :layout => !request.xhr? + end + rescue ActiveRecord::StaleObjectError + # Optimistic locking exception + flash.now[:error] = l(:notice_locking_conflict) + end + def toggle_delete @cost_type.deleted_at = @cost_type.deleted_at ? nil : DateTime.now() @cost_type.default = false diff --git a/app/views/cost_types/_rate.html.erb b/app/views/cost_types/_rate.html.erb index bbd8f33e89..f09bcd1560 100644 --- a/app/views/cost_types/_rate.html.erb +++ b/app/views/cost_types/_rate.html.erb @@ -13,7 +13,7 @@ <% unless error_messages.blank? %><%= error_messages %><% end %> -<% fields_for prefix, rate do |rate_form| %> +<%= fields_for prefix, rate do |rate_form| %> diff --git a/app/views/cost_types/edit.html.erb b/app/views/cost_types/edit.html.erb index 65aca500c9..1f4d9b5caa 100644 --- a/app/views/cost_types/edit.html.erb +++ b/app/views/cost_types/edit.html.erb @@ -1,7 +1,7 @@ <%= render :partial => 'shared/costs_header' %>

<%= l(:caption_cost_type) %>

-<%= labelled_tabular_form_for @cost_type, :url => {:action => 'update'}, :method => :put do |f| %> +<%= labelled_tabular_form_for @cost_type do |f| %> <%= error_messages_for 'cost_type' %> <%= back_url_hidden_field_tag %> @@ -13,7 +13,7 @@

<%= l :caption_rate_history %>

- <% javascript_tag do -%> + <%= javascript_tag do -%> RatesForm = new Subform('<%= escape_javascript(render(:partial => "rate", :object => CostRate.new )) %>',<%= @cost_type.rates.length %>,'rates_body'); <% end -%> diff --git a/app/views/cost_types/index.html.erb b/app/views/cost_types/index.html.erb index a1f85cb498..5e1d135b0e 100644 --- a/app/views/cost_types/index.html.erb +++ b/app/views/cost_types/index.html.erb @@ -5,28 +5,31 @@

<%= l(:caption_cost_type_plural) %>

-<%= form_tag({ :controller => 'cost_types', :action => 'index' }, :id => 'query_form') do %> -
<%= l(:label_filter_plural) %> -

- - <%= text_field_tag :fixed_date, @fixed_date, :size => 10 %><%= calendar_for('fixed_date') %> -

+<%= form_tag(cost_types_path, { :method => :get, :id => 'query_form' }) do %> +
+ <%= l(:label_filter_plural) %> +

+ + <%= text_field_tag :fixed_date, @fixed_date, :size => 10 %><%= calendar_for('fixed_date') %> +

<%= check_box_tag :include_deleted, "1", @include_deleted, :autocomplete => "off" %>

+

<%= link_to_remote l(:button_apply), { :update => "content", - :with => "Form.serialize('query_form')" + :with => "Form.serialize('query_form')", + :method => :get }, :class => 'icon icon-checked' %> <%= link_to_remote l(:button_clear), { :url => { :clear_filter => true }, + :method => :get, :update => "content", }, :class => 'icon icon-reload' %>

-
<% end %> <% end %> -<% fields_for prefix, labor_budget_item do |cost_form| %> +<%= fields_for prefix, labor_budget_item do |cost_form| %> <% end %> -<% fields_for prefix, material_budget_item do |cost_form| %> +<%= fields_for prefix, material_budget_item do |cost_form| %> <% end %> -<% fields_for prefix, rate do |rate_form| %> +<%= fields_for prefix, rate do |rate_form| %>
<%= error_messages %>
diff --git a/app/views/cost_objects/_material_budget_item.html.erb b/app/views/cost_objects/_material_budget_item.html.erb index ac56820072..1288dc7f87 100644 --- a/app/views/cost_objects/_material_budget_item.html.erb +++ b/app/views/cost_objects/_material_budget_item.html.erb @@ -12,7 +12,7 @@ -%> <% unless error_messages.blank? %>
<%= error_messages %>
diff --git a/app/views/hourly_rates/_rate.html.erb b/app/views/hourly_rates/_rate.html.erb index 204dd5c18b..cc04386f62 100644 --- a/app/views/hourly_rates/_rate.html.erb +++ b/app/views/hourly_rates/_rate.html.erb @@ -13,7 +13,7 @@ <% unless error_messages.blank? %>
<%= error_messages %>
diff --git a/app/views/hourly_rates/edit.html.erb b/app/views/hourly_rates/edit.html.erb index bea9f6d02d..beb5e178e5 100644 --- a/app/views/hourly_rates/edit.html.erb +++ b/app/views/hourly_rates/edit.html.erb @@ -6,7 +6,7 @@

<%= l(:label_current_default_rate) %>: <%= number_to_currency(default_rate.rate)%>

<% end %> -<% javascript_tag do -%> +<%= javascript_tag do -%> RatesForm = new Subform('<%= escape_javascript(render(:partial => "rate", :object => HourlyRate.new )) %>',<%= @rates.length %>,'rates_body'); <% end -%> From 43ae7c4011bee2f5c6cb86d992bc544b7a72b9c3 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Wed, 8 May 2013 09:29:06 +0200 Subject: [PATCH 03/10] loads number_helper patch --- lib/open_project/costs/engine.rb | 4 +- lib/open_project/costs/patches/i18n_patch.rb | 41 +++++++++++++------- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/lib/open_project/costs/engine.rb b/lib/open_project/costs/engine.rb index 26638f6c3c..88e234c3f1 100644 --- a/lib/open_project/costs/engine.rb +++ b/lib/open_project/costs/engine.rb @@ -1,10 +1,12 @@ +require_dependency 'open_project/costs/patches/i18n_patch' + module OpenProject::Costs class Engine < ::Rails::Engine engine_name :openproject_costs def self.settings { :default => { 'costs_currency' => 'EUR', - 'costs_currency_format' => '%n %u' }, + 'costs_currency_format' => '%n %u' }, :partial => 'settings/openproject_costs' } end diff --git a/lib/open_project/costs/patches/i18n_patch.rb b/lib/open_project/costs/patches/i18n_patch.rb index 6df5cf5268..9d34d2bed0 100644 --- a/lib/open_project/costs/patches/i18n_patch.rb +++ b/lib/open_project/costs/patches/i18n_patch.rb @@ -1,17 +1,30 @@ -module ActionView::Helpers::NumberHelper - def number_to_currency_with_l10n(number, options = {}) - options[:delimiter] = l(:currency_delimiter) unless options[:delimiter] - options[:separator] = l(:currency_separator) unless options[:separator] +module OpenProject::Costs::Patches + module NumberHelper + def self.included(base) # :nodoc: + base.class_eval do + include InstanceMethods - options[:unit] = Setting.plugin_openproject_costs['costs_currency'] unless options[:unit] - options[:format] = Setting.plugin_openproject_costs['costs_currency_format'] unless options[:format] - - # FIXME: patch ruby instead of this code - # this circumvents the broken BigDecimal#to_f on Siemens's ruby - number = number.to_s if number.is_a? BigDecimal - - number_to_currency_without_l10n(number, options) + alias_method_chain :number_to_currency, :l10n + end + end + + module InstanceMethods + def number_to_currency_with_l10n(number, options = {}) + options_with_default = { unit: Setting.plugin_openproject_costs['costs_currency'], + format: Setting.plugin_openproject_costs['costs_currency_format'], + delimiter: l(:currency_delimiter), + separator: l(:currency_separator) }.merge(options) + + # FIXME: patch ruby instead of this code + # this circumvents the broken BigDecimal#to_f on Siemens's ruby + number = number.to_s if number.is_a? BigDecimal + + number_to_currency_without_l10n(number, options_with_default) + end + end end +end - alias_method_chain :number_to_currency, :l10n -end \ No newline at end of file +unless ActionView::Helpers::NumberHelper.included_modules.include?(OpenProject::Costs::Patches::NumberHelper) + ActionView::Helpers::NumberHelper.send(:include, OpenProject::Costs::Patches::NumberHelper) +end From e00fbe66e5468ca1a4f46f4777ef307d486541d2 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Wed, 8 May 2013 09:30:01 +0200 Subject: [PATCH 04/10] removes no longer required ajax step --- features/view_own_rates.feature | 1 - 1 file changed, 1 deletion(-) diff --git a/features/view_own_rates.feature b/features/view_own_rates.feature index 41dbe8d1cd..c16bafde8c 100644 --- a/features/view_own_rates.feature +++ b/features/view_own_rates.feature @@ -53,7 +53,6 @@ Feature: Permission View Own hourly and cost rates | Labor Costs | | Unit Costs | And I follow "Apply" - And I wait for AJAX Then I should see "24.00 EUR" And I should see "10.00 EUR" And I should see "14.00 EUR" From bdac58897037be343072f4597bdd6d57b33f7d12 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Wed, 8 May 2013 09:31:24 +0200 Subject: [PATCH 05/10] fixes the standard cost project step As fixtures are no longer loaded by the core, we have to create everything ourself --- features/step_definitions/cost_steps.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/features/step_definitions/cost_steps.rb b/features/step_definitions/cost_steps.rb index ec2292e360..5e9c2f94f7 100644 --- a/features/step_definitions/cost_steps.rb +++ b/features/step_definitions/cost_steps.rb @@ -93,9 +93,13 @@ Given /^there is a standard cost control project named "([^\"]*)"$/ do |name| steps %Q{ Given there is 1 project with the following: | Name | #{name} | + And the project "#{name}" has the following trackers: + | name | + | tracker1 | And the project "#{name}" has 1 subproject And the project "#{name}" has 1 issue with: | subject | #{name}issue | + And there is a role "Manager" And the role "Manager" may have the following rights: | view_own_hourly_rate | | view_issues | @@ -105,8 +109,10 @@ Given /^there is a standard cost control project named "([^\"]*)"$/ do |name| And there is a role "Controller" And the role "Controller" may have the following rights: | View own cost entries | + And there is a role "Developer" And the role "Developer" may have the following rights: | View own cost entries | + And there is a role "Reporter" And the role "Reporter" may have the following rights: | Create issues | And there is a role "Supplier" From 756b367c807a3a70cbee38ad496eb4c688f410e6 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Wed, 8 May 2013 09:33:00 +0200 Subject: [PATCH 06/10] fixes cuke for budget creation --- features/create_budget.feature | 6 ++---- features/step_definitions/cost_object_steps.rb | 10 ++++++++++ 2 files changed, 12 insertions(+), 4 deletions(-) create mode 100644 features/step_definitions/cost_object_steps.rb diff --git a/features/create_budget.feature b/features/create_budget.feature index 9323cbff17..9281d5749a 100644 --- a/features/create_budget.feature +++ b/features/create_budget.feature @@ -13,10 +13,8 @@ Feature: Creating a Budget And I am already logged in as "testuser" When I go to the overview page of the project called "project1" - And I toggle the "Budgets" submenu - And I follow "New Budget" within "#main-menu" - And I fill in "cost_object_subject" with "budget1" - And I press "Create" + And I create a budget with the following: + | subject | budget1 | Then I should be on the show page for the budget "budget1" And I should see "Successful creation" diff --git a/features/step_definitions/cost_object_steps.rb b/features/step_definitions/cost_object_steps.rb new file mode 100644 index 0000000000..fa85df43a7 --- /dev/null +++ b/features/step_definitions/cost_object_steps.rb @@ -0,0 +1,10 @@ +When(/^I create a budget with the following:$/) do |table| + + rows = table.rows_hash + + steps %Q{And I toggle the "Budgets" submenu + And I follow "New Budget" within "#main-menu" + And I fill in "Subject" with "#{rows['subject']}"} + + click_button(I18n.t(:button_create), :exact => true) +end From 770c10aaa4d976d2aecb5aaf4236af11dcbb5a12 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Wed, 8 May 2013 09:33:28 +0200 Subject: [PATCH 07/10] renames Factory to FactoryGirl --- features/step_definitions/cost_steps.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/features/step_definitions/cost_steps.rb b/features/step_definitions/cost_steps.rb index 5e9c2f94f7..e8d8c5fb0c 100644 --- a/features/step_definitions/cost_steps.rb +++ b/features/step_definitions/cost_steps.rb @@ -76,7 +76,7 @@ end Given /^the issue "([^\"]+)" has (\d+) [Cc]ost(?: )?[Ee]ntr(?:ies|y) with the following:$/ do |issue, count, table| i = Issue.find(:last, :conditions => ["subject = '#{issue}'"]) as_admin count do - ce = Factory.build(:cost_entry, :spent_on => (table.rows_hash["date"] ? table.rows_hash["date"].to_date : Date.today), + ce = FactoryGirl.build(:cost_entry, :spent_on => (table.rows_hash["date"] ? table.rows_hash["date"].to_date : Date.today), :units => table.rows_hash["units"], :project => i.project, :issue => i, @@ -166,7 +166,7 @@ Given /^users have times and the cost type "([^\"]*)" logged on the issue "([^\" end Given /^there is a variable cost object with the following:$/ do |table| - cost_object = Factory.build(:variable_cost_object) + cost_object = FactoryGirl.build(:variable_cost_object) table_hash = table.rows_hash From e0f2b3e818ae7eace5ae2cbea7f49e0e765fe609 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Wed, 8 May 2013 09:34:22 +0200 Subject: [PATCH 08/10] fixes log unit cost steps --- features/credit_unit_costs.feature | 4 ++-- features/step_definitions/cost_steps.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) mode change 100644 => 100755 features/step_definitions/cost_steps.rb diff --git a/features/credit_unit_costs.feature b/features/credit_unit_costs.feature index 22949eae61..ab2dd92d73 100644 --- a/features/credit_unit_costs.feature +++ b/features/credit_unit_costs.feature @@ -17,8 +17,8 @@ Feature: Credit unit costs Scenario: Crediting units costs to an issue When I am already logged in as "manager" And I go to the page of the issue "issue1" - And I follow "More functions" within ".action_menu_main" - And I follow "Log unit costs" within ".action_menu_main" + And I select "Log unit costs" from the action menu And I fill in "cost_entry_units" with "100" + And I select "cost_type_1" from "Cost type" And I press "Save" Then I should be on the page of the issue "issue1" diff --git a/features/step_definitions/cost_steps.rb b/features/step_definitions/cost_steps.rb old mode 100644 new mode 100755 index e8d8c5fb0c..12ebd54faa --- a/features/step_definitions/cost_steps.rb +++ b/features/step_definitions/cost_steps.rb @@ -175,7 +175,7 @@ Given /^there is a variable cost object with the following:$/ do |table| Time.now cost_object.fixed_date = cost_object.created_on.to_date cost_object.project = (Project.find_by_identifier(table_hash["project"]) || Project.find_by_name(table_hash ["project"])) if table_hash.has_key? "project" - cost_object.author = User.current + cost_object.author = User.find_by_login(table_hash["author"]) || cost_object.project.members.first.principal cost_object.subject = table_hash["subject"] if table_hash.has_key? "subject" cost_object.save! From c30b2649a383cdd65fee950166432daecbbe100d Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Wed, 8 May 2013 10:48:34 +0200 Subject: [PATCH 09/10] fixes strong_params for cost_types nexted attributes --- lib/open_project/costs/patches/permitted_params_patch.rb | 4 ++-- spec/models/permitted_params_spec.rb | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/open_project/costs/patches/permitted_params_patch.rb b/lib/open_project/costs/patches/permitted_params_patch.rb index 0ac6b28766..e7787aa365 100644 --- a/lib/open_project/costs/patches/permitted_params_patch.rb +++ b/lib/open_project/costs/patches/permitted_params_patch.rb @@ -27,8 +27,8 @@ module OpenProject::Costs::Patches::PermittedParamsPatch :unit, :unit_plural, :default, - :new_rate_attributes, - :existing_rate_attributes) + { new_rate_attributes: [:valid_from, :rate] }, + { existing_rate_attributes: [:valid_from, :rate] }) end def labor_budget_item diff --git a/spec/models/permitted_params_spec.rb b/spec/models/permitted_params_spec.rb index 87b8780669..562d324f0b 100644 --- a/spec/models/permitted_params_spec.rb +++ b/spec/models/permitted_params_spec.rb @@ -99,15 +99,15 @@ describe PermittedParams do end it "should return new_rate_attributes" do - params = ActionController::Parameters.new(:cost_type => { "new_rate_attributes" => "new_rate_attributes_test" } ) + params = ActionController::Parameters.new(:cost_type => { "new_rate_attributes" => { "0" => { "valid_from" => "2013-05-08", "rate" => "5002" }, "1" => { "valid_from" => "2013-05-10", "rate" => "5004" } } } ) - PermittedParams.new(params, user).cost_type.should == { "new_rate_attributes" => "new_rate_attributes_test" } + PermittedParams.new(params, user).cost_type.should == { "new_rate_attributes" => { "0" => { "valid_from" => "2013-05-08", "rate" => "5002" }, "1" => { "valid_from" => "2013-05-10", "rate" => "5004" } } } end it "should return existing_rate_attributes" do - params = ActionController::Parameters.new(:cost_type => { "existing_rate_attributes" => "new_rate_attributes_test" } ) + params = ActionController::Parameters.new(:cost_type => { "existing_rate_attributes" => { "9" => { "valid_from" => "2013-05-05", "rate" => "50.0" } } } ) - PermittedParams.new(params, user).cost_type.should == { "existing_rate_attributes" => "new_rate_attributes_test" } + PermittedParams.new(params, user).cost_type.should == { "existing_rate_attributes" => { "9" => { "valid_from" => "2013-05-05", "rate" => "50.0" } } } end it "should not return project_id" do From 1b9a38b2033ed80ad9a6ee4d7d7cd5e140087753 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Wed, 8 May 2013 10:48:57 +0200 Subject: [PATCH 10/10] cleans up cost_types_controller --- app/controllers/cost_types_controller.rb | 52 ++++-------------------- 1 file changed, 7 insertions(+), 45 deletions(-) diff --git a/app/controllers/cost_types_controller.rb b/app/controllers/cost_types_controller.rb index c173b6b1ad..13156ee417 100644 --- a/app/controllers/cost_types_controller.rb +++ b/app/controllers/cost_types_controller.rb @@ -3,8 +3,7 @@ class CostTypesController < ApplicationController # Allow only admins here before_filter :require_admin - before_filter :find_cost_type, :only => [:set_rate, :toggle_delete] - before_filter :find_optional_cost_type, :only => [:edit, :update] + before_filter :find_cost_type, :only => [:edit, :update, :set_rate, :toggle_delete] helper :sort include SortHelper @@ -32,38 +31,18 @@ class CostTypesController < ApplicationController end def edit - if !@cost_type - @cost_type = CostType.new() - end - - @cost_type.attributes = permitted_params.cost_type if params[:cost_type] - - if request.post? && @cost_type.save - flash[:notice] = l(:notice_successful_update) - redirect_back_or_default(:action => 'index') - else - @cost_type.rates.build({:valid_from => Date.today}) if @cost_type.rates.empty? - render :action => "edit", :layout => !request.xhr? - end - rescue ActiveRecord::StaleObjectError - # Optimistic locking exception - flash.now[:error] = l(:notice_locking_conflict) + render :action => "edit", :layout => !request.xhr? end def update # TODO: method is copied over from edit # remove code as appropriate - if !@cost_type - @cost_type = CostType.new() - end - - @cost_type.attributes = permitted_params.cost_type if params[:cost_type] + @cost_type.attributes = permitted_params.cost_type if @cost_type.save flash[:notice] = l(:notice_successful_update) redirect_back_or_default(:action => 'index') else - @cost_type.rates.build({:valid_from => Date.today}) if @cost_type.rates.empty? render :action => "edit", :layout => !request.xhr? end rescue ActiveRecord::StaleObjectError @@ -74,28 +53,17 @@ class CostTypesController < ApplicationController def new # TODO: method is copied over from edit # remove code as appropriate - if !@cost_type - @cost_type = CostType.new() - end + @cost_type = CostType.new() - @cost_type.attributes = permitted_params.cost_type if params[:cost_type] + @cost_type.rates.build({:valid_from => Date.today}) if @cost_type.rates.empty? - if request.post? && @cost_type.save - flash[:notice] = l(:notice_successful_update) - redirect_back_or_default(:action => 'index') - else - @cost_type.rates.build({:valid_from => Date.today}) if @cost_type.rates.empty? - render :action => "edit", :layout => !request.xhr? - end - rescue ActiveRecord::StaleObjectError - # Optimistic locking exception - flash.now[:error] = l(:notice_locking_conflict) + render :action => "edit", :layout => !request.xhr? end def create # TODO: method is copied over from edit # remove code as appropriate - @cost_type = CostType.new(params[:cost_type]) + @cost_type = CostType.new(permitted_params.cost_type) if @cost_type.save flash[:notice] = l(:notice_successful_update) @@ -145,12 +113,6 @@ private render_404 end - def find_optional_cost_type - if !params[:id].blank? - @cost_type = CostType.find(params[:id]) - end - end - def default_breadcrumb l(:caption_cost_type_plural) end