Commit ede05a1fafc0de428d532c7f5e5276200da1e709
Committed by
Joenio Costa
1 parent
14c18eaa
Exists in
staging
and in
26 other branches
Enterprises with wrong environments
* CreateEnterprise#environment uses the requestor environment as the
requestor is always required.
* Migration to fix the enterprises with wrong environments.
(ActionItem1555)
Showing
9 changed files
with
77 additions
and
13 deletions
Show diff stats
app/controllers/public/enterprise_registration_controller.rb
| @@ -16,9 +16,9 @@ class EnterpriseRegistrationController < ApplicationController | @@ -16,9 +16,9 @@ class EnterpriseRegistrationController < ApplicationController | ||
| 16 | @create_enterprise.target = Profile.find(params[:create_enterprise][:target_id]) | 16 | @create_enterprise.target = Profile.find(params[:create_enterprise][:target_id]) |
| 17 | end | 17 | end |
| 18 | elsif @validation == :admin || @validation == :none | 18 | elsif @validation == :admin || @validation == :none |
| 19 | - @create_enterprise.target = @create_enterprise.environment | 19 | + @create_enterprise.target = environment |
| 20 | end | 20 | end |
| 21 | - @create_enterprise.requestor = current_user.person | 21 | + @create_enterprise.requestor = user |
| 22 | the_action = | 22 | the_action = |
| 23 | if request.post? | 23 | if request.post? |
| 24 | if @create_enterprise.valid_before_selecting_target? | 24 | if @create_enterprise.valid_before_selecting_target? |
app/models/create_enterprise.rb
| @@ -91,7 +91,7 @@ class CreateEnterprise < Task | @@ -91,7 +91,7 @@ class CreateEnterprise < Task | ||
| 91 | end | 91 | end |
| 92 | 92 | ||
| 93 | def environment | 93 | def environment |
| 94 | - region ? region.environment : self.requestor ? self.requestor.environment : Environment.default | 94 | + requestor.environment |
| 95 | end | 95 | end |
| 96 | 96 | ||
| 97 | def available_regions | 97 | def available_regions |
db/migrate/20100730141134_set_owner_environment_to_enterprises_environment.rb
0 → 100644
| @@ -0,0 +1,14 @@ | @@ -0,0 +1,14 @@ | ||
| 1 | +class SetOwnerEnvironmentToEnterprisesEnvironment < ActiveRecord::Migration | ||
| 2 | + def self.up | ||
| 3 | + CreateEnterprise.find_all_by_status(3).each do |t| | ||
| 4 | + if(Enterprise.find_by_identifier(t.data[:identifier])) | ||
| 5 | + update("UPDATE profiles SET environment_id = '%s' WHERE identifier = '%s'" % | ||
| 6 | + [Person.find(t.requestor_id).environment.id, t.data[:identifier]]) | ||
| 7 | + end | ||
| 8 | + end | ||
| 9 | + end | ||
| 10 | + | ||
| 11 | + def self.down | ||
| 12 | + say "this migration can't be reverted" | ||
| 13 | + end | ||
| 14 | +end |
db/schema.rb
| @@ -9,7 +9,7 @@ | @@ -9,7 +9,7 @@ | ||
| 9 | # | 9 | # |
| 10 | # It's strongly recommended to check this file into your version control system. | 10 | # It's strongly recommended to check this file into your version control system. |
| 11 | 11 | ||
| 12 | -ActiveRecord::Schema.define(:version => 20100722020357) do | 12 | +ActiveRecord::Schema.define(:version => 20100730141134) do |
| 13 | 13 | ||
| 14 | create_table "article_versions", :force => true do |t| | 14 | create_table "article_versions", :force => true do |t| |
| 15 | t.integer "article_id" | 15 | t.integer "article_id" |
features/register_enterprise.feature
| @@ -5,8 +5,8 @@ Feature: register enterprise | @@ -5,8 +5,8 @@ Feature: register enterprise | ||
| 5 | 5 | ||
| 6 | Background: | 6 | Background: |
| 7 | Given the following users | 7 | Given the following users |
| 8 | - | login | name | | ||
| 9 | - | joaosilva | Joao Silva | | 8 | + | login | name | email | |
| 9 | + | joaosilva | Joao Silva | joaosilva@example.com | | ||
| 10 | 10 | ||
| 11 | And I am logged in as "joaosilva" | 11 | And I am logged in as "joaosilva" |
| 12 | And I am on Joao Silva's control panel | 12 | And I am on Joao Silva's control panel |
| @@ -80,22 +80,24 @@ Feature: register enterprise | @@ -80,22 +80,24 @@ Feature: register enterprise | ||
| 80 | Scenario: a user register an enterprise successfully through the admin | 80 | Scenario: a user register an enterprise successfully through the admin |
| 81 | validator method and the admin accepts | 81 | validator method and the admin accepts |
| 82 | Given organization_approval_method is "admin" on environment | 82 | Given organization_approval_method is "admin" on environment |
| 83 | + And the mailbox is empty | ||
| 83 | And I follow "Manage my groups" | 84 | And I follow "Manage my groups" |
| 84 | And the following states | 85 | And the following states |
| 85 | | name | | 86 | | name | |
| 86 | | Sample State | | 87 | | Sample State | |
| 87 | And I follow "Register a new enterprise" | 88 | And I follow "Register a new enterprise" |
| 88 | And I fill in the following: | 89 | And I fill in the following: |
| 89 | - | Address | my-enterprise | | ||
| 90 | - | Name | My Enterprise | | 90 | + | Address | my-enterprise | |
| 91 | + | Name | My Enterprise | | ||
| 91 | And I press "Next" | 92 | And I press "Next" |
| 92 | Then I should see "Enterprise registration completed" | 93 | Then I should see "Enterprise registration completed" |
| 93 | And I am logged in as admin | 94 | And I am logged in as admin |
| 94 | And I follow "Control panel" | 95 | And I follow "Control panel" |
| 95 | When I follow "Tasks" | 96 | When I follow "Tasks" |
| 96 | Then I should see /Processing task: Enterprise registration: "My Enterprise"/ | 97 | Then I should see /Processing task: Enterprise registration: "My Enterprise"/ |
| 97 | - And I choose "Ok" | 98 | + And the first mail is to admin_user@example.com |
| 98 | And I press "Ok" | 99 | And I press "Ok" |
| 100 | + Then the last mail is to joaosilva@example.com | ||
| 99 | And I am logged in as "joaosilva" | 101 | And I am logged in as "joaosilva" |
| 100 | And I am on Joao Silva's control panel | 102 | And I am on Joao Silva's control panel |
| 101 | When I follow "Manage my groups" | 103 | When I follow "Manage my groups" |
| @@ -104,23 +106,26 @@ Feature: register enterprise | @@ -104,23 +106,26 @@ Feature: register enterprise | ||
| 104 | Scenario: a user register an enterprise successfully through the admin | 106 | Scenario: a user register an enterprise successfully through the admin |
| 105 | validator method and the admin rejects | 107 | validator method and the admin rejects |
| 106 | Given organization_approval_method is "admin" on environment | 108 | Given organization_approval_method is "admin" on environment |
| 109 | + And the mailbox is empty | ||
| 107 | And I follow "Manage my groups" | 110 | And I follow "Manage my groups" |
| 108 | And the following states | 111 | And the following states |
| 109 | | name | | 112 | | name | |
| 110 | | Sample State | | 113 | | Sample State | |
| 111 | And I follow "Register a new enterprise" | 114 | And I follow "Register a new enterprise" |
| 112 | And I fill in the following: | 115 | And I fill in the following: |
| 113 | - | Address | my-enterprise | | ||
| 114 | - | Name | My Enterprise | | 116 | + | Address | my-enterprise | |
| 117 | + | Name | My Enterprise | | ||
| 115 | And I press "Next" | 118 | And I press "Next" |
| 116 | Then I should see "Enterprise registration completed" | 119 | Then I should see "Enterprise registration completed" |
| 117 | And I am logged in as admin | 120 | And I am logged in as admin |
| 118 | And I follow "Control panel" | 121 | And I follow "Control panel" |
| 119 | When I follow "Tasks" | 122 | When I follow "Tasks" |
| 120 | Then I should see /Processing task: Enterprise registration: "My Enterprise"/ | 123 | Then I should see /Processing task: Enterprise registration: "My Enterprise"/ |
| 124 | + And the first mail is to admin_user@example.com | ||
| 121 | And I choose "Cancel" | 125 | And I choose "Cancel" |
| 122 | And I fill in "Rejection explanation" with "This enterprise has some irregularities." | 126 | And I fill in "Rejection explanation" with "This enterprise has some irregularities." |
| 123 | - And I press "Ok" | 127 | + When I press "Ok" |
| 128 | + Then the last mail is to joaosilva@example.com | ||
| 124 | And I am logged in as "joaosilva" | 129 | And I am logged in as "joaosilva" |
| 125 | And I am on Joao Silva's control panel | 130 | And I am on Joao Silva's control panel |
| 126 | When I follow "Manage my groups" | 131 | When I follow "Manage my groups" |
features/step_definitions/noosfero_steps.rb
| @@ -205,3 +205,24 @@ end | @@ -205,3 +205,24 @@ end | ||
| 205 | Then /^The page title should contain "(.*)"$/ do |text| | 205 | Then /^The page title should contain "(.*)"$/ do |text| |
| 206 | response.should have_selector("title:contains('#{text}')") | 206 | response.should have_selector("title:contains('#{text}')") |
| 207 | end | 207 | end |
| 208 | + | ||
| 209 | +Given /^the mailbox is empty$/ do | ||
| 210 | + ActionMailer::Base.deliveries = [] | ||
| 211 | +end | ||
| 212 | + | ||
| 213 | +Given /^the (.+) mail (?:is|has) (.+) (.+)$/ do |position, field, value| | ||
| 214 | + if(/^[0-9]+$/ =~ position) | ||
| 215 | + ActionMailer::Base.deliveries[position.to_i][field] == value | ||
| 216 | + else | ||
| 217 | + ActionMailer::Base.deliveries.send(position)[field] == value | ||
| 218 | + end | ||
| 219 | +end | ||
| 220 | + | ||
| 221 | +Given /^the (.+) mail (.+) is like (.+)$/ do |position, field, regexp| | ||
| 222 | + re = Regexp.new(regexp) | ||
| 223 | + if(/^[0-9]+$/ =~ position) | ||
| 224 | + re =~ ActionMailer::Base.deliveries[position.to_i][field.to_sym] | ||
| 225 | + else | ||
| 226 | + re =~ ActionMailer::Base.deliveries.send(position)[field.to_sym] | ||
| 227 | + end | ||
| 228 | +end |
test/functional/enterprise_registration_controller_test.rb
| @@ -170,4 +170,14 @@ all_fixtures | @@ -170,4 +170,14 @@ all_fixtures | ||
| 170 | assert_no_tag :tag => 'option', :content => "Region without validator" | 170 | assert_no_tag :tag => 'option', :content => "Region without validator" |
| 171 | end | 171 | end |
| 172 | 172 | ||
| 173 | + should 'set current environment as the task target if approval method is admin' do | ||
| 174 | + environment = Environment.new(:name => "Another environment") | ||
| 175 | + environment.organization_approval_method = :admin | ||
| 176 | + environment.save | ||
| 177 | + @controller.stubs(:environment).returns(environment) | ||
| 178 | + | ||
| 179 | + get :index | ||
| 180 | + assert_equal assigns(:create_enterprise).target, environment | ||
| 181 | + end | ||
| 182 | + | ||
| 173 | end | 183 | end |
test/functional/enterprise_validation_controller_test.rb
| @@ -74,6 +74,7 @@ class EnterpriseValidationControllerTest < Test::Unit::TestCase | @@ -74,6 +74,7 @@ class EnterpriseValidationControllerTest < Test::Unit::TestCase | ||
| 74 | 74 | ||
| 75 | should 'require the user to fill in the explanation for an rejection' do | 75 | should 'require the user to fill in the explanation for an rejection' do |
| 76 | validation = CreateEnterprise.new | 76 | validation = CreateEnterprise.new |
| 77 | + validation.stubs(:environment).returns(Environment.default) | ||
| 77 | @org.expects(:find_pending_validation).with('kakakaka').returns(validation) | 78 | @org.expects(:find_pending_validation).with('kakakaka').returns(validation) |
| 78 | 79 | ||
| 79 | # FIXME: this is not working, but should. Anyway the assert_response and | 80 | # FIXME: this is not working, but should. Anyway the assert_response and |
test/unit/create_enterprise_test.rb
| @@ -13,6 +13,7 @@ class CreateEnterpriseTest < Test::Unit::TestCase | @@ -13,6 +13,7 @@ class CreateEnterpriseTest < Test::Unit::TestCase | ||
| 13 | 13 | ||
| 14 | should 'accept only numbers as foundation year' do | 14 | should 'accept only numbers as foundation year' do |
| 15 | task = CreateEnterprise.new | 15 | task = CreateEnterprise.new |
| 16 | + task.stubs(:environment).returns(Environment.default) | ||
| 16 | 17 | ||
| 17 | task.foundation_year = "test" | 18 | task.foundation_year = "test" |
| 18 | task.valid? | 19 | task.valid? |
| @@ -25,6 +26,7 @@ class CreateEnterpriseTest < Test::Unit::TestCase | @@ -25,6 +26,7 @@ class CreateEnterpriseTest < Test::Unit::TestCase | ||
| 25 | 26 | ||
| 26 | should 'require a requestor' do | 27 | should 'require a requestor' do |
| 27 | task = CreateEnterprise.new | 28 | task = CreateEnterprise.new |
| 29 | + task.stubs(:environment).returns(Environment.default) | ||
| 28 | task.valid? | 30 | task.valid? |
| 29 | 31 | ||
| 30 | assert task.errors.invalid?(:requestor_id) | 32 | assert task.errors.invalid?(:requestor_id) |
| @@ -35,6 +37,7 @@ class CreateEnterpriseTest < Test::Unit::TestCase | @@ -35,6 +37,7 @@ class CreateEnterpriseTest < Test::Unit::TestCase | ||
| 35 | 37 | ||
| 36 | should 'require a target (validator organization)' do | 38 | should 'require a target (validator organization)' do |
| 37 | task = CreateEnterprise.new | 39 | task = CreateEnterprise.new |
| 40 | + task.stubs(:environment).returns(Environment.default) | ||
| 38 | task.valid? | 41 | task.valid? |
| 39 | 42 | ||
| 40 | assert task.errors.invalid?(:target_id) | 43 | assert task.errors.invalid?(:target_id) |
| @@ -50,6 +53,7 @@ class CreateEnterpriseTest < Test::Unit::TestCase | @@ -50,6 +53,7 @@ class CreateEnterpriseTest < Test::Unit::TestCase | ||
| 50 | validator = fast_create(Organization, :name => "My organization", :identifier => 'myorg', :environment_id => environment.id) | 53 | validator = fast_create(Organization, :name => "My organization", :identifier => 'myorg', :environment_id => environment.id) |
| 51 | 54 | ||
| 52 | task = CreateEnterprise.new | 55 | task = CreateEnterprise.new |
| 56 | + task.stubs(:environment).returns(Environment.default) | ||
| 53 | 57 | ||
| 54 | task.region = region | 58 | task.region = region |
| 55 | task.target = validator | 59 | task.target = validator |
| @@ -71,6 +75,7 @@ class CreateEnterpriseTest < Test::Unit::TestCase | @@ -71,6 +75,7 @@ class CreateEnterpriseTest < Test::Unit::TestCase | ||
| 71 | 75 | ||
| 72 | should 'require an explanation for rejecting enterprise creation' do | 76 | should 'require an explanation for rejecting enterprise creation' do |
| 73 | task = CreateEnterprise.new | 77 | task = CreateEnterprise.new |
| 78 | + task.stubs(:environment).returns(Environment.default) | ||
| 74 | task.reject_explanation = nil | 79 | task.reject_explanation = nil |
| 75 | 80 | ||
| 76 | task.valid? | 81 | task.valid? |
| @@ -98,6 +103,8 @@ class CreateEnterpriseTest < Test::Unit::TestCase | @@ -98,6 +103,8 @@ class CreateEnterpriseTest < Test::Unit::TestCase | ||
| 98 | validator = fast_create(Organization, :name => "My organization", :identifier => 'myorg', :environment_id => environment.id) | 103 | validator = fast_create(Organization, :name => "My organization", :identifier => 'myorg', :environment_id => environment.id) |
| 99 | region.validators << validator | 104 | region.validators << validator |
| 100 | person = create_user('testuser').person | 105 | person = create_user('testuser').person |
| 106 | + person.environment = environment | ||
| 107 | + person.save | ||
| 101 | 108 | ||
| 102 | task = CreateEnterprise.create!({ | 109 | task = CreateEnterprise.create!({ |
| 103 | :name => 'My new enterprise', | 110 | :name => 'My new enterprise', |
| @@ -132,6 +139,8 @@ class CreateEnterpriseTest < Test::Unit::TestCase | @@ -132,6 +139,8 @@ class CreateEnterpriseTest < Test::Unit::TestCase | ||
| 132 | validator = fast_create(Organization, :name => "My organization", :identifier => 'myorg', :environment_id => environment.id) | 139 | validator = fast_create(Organization, :name => "My organization", :identifier => 'myorg', :environment_id => environment.id) |
| 133 | region.validators << validator | 140 | region.validators << validator |
| 134 | person = create_user('testuser').person | 141 | person = create_user('testuser').person |
| 142 | + person.environment = environment | ||
| 143 | + person.save | ||
| 135 | 144 | ||
| 136 | task = CreateEnterprise.create!({ | 145 | task = CreateEnterprise.create!({ |
| 137 | :name => 'My new enterprise', | 146 | :name => 'My new enterprise', |
| @@ -161,6 +170,7 @@ class CreateEnterpriseTest < Test::Unit::TestCase | @@ -161,6 +170,7 @@ class CreateEnterpriseTest < Test::Unit::TestCase | ||
| 161 | 170 | ||
| 162 | should 'override message methods from Task' do | 171 | should 'override message methods from Task' do |
| 163 | specific = CreateEnterprise.new | 172 | specific = CreateEnterprise.new |
| 173 | + specific.stubs(:environment).returns(Environment.default) | ||
| 164 | %w[ task_created_message task_finished_message task_cancelled_message ].each do |method| | 174 | %w[ task_created_message task_finished_message task_cancelled_message ].each do |method| |
| 165 | assert_nothing_raised NotImplementedError do | 175 | assert_nothing_raised NotImplementedError do |
| 166 | specific.send(method) | 176 | specific.send(method) |
| @@ -193,7 +203,9 @@ class CreateEnterpriseTest < Test::Unit::TestCase | @@ -193,7 +203,9 @@ class CreateEnterpriseTest < Test::Unit::TestCase | ||
| 193 | end | 203 | end |
| 194 | 204 | ||
| 195 | should 'provide a message to be sent to the target' do | 205 | should 'provide a message to be sent to the target' do |
| 196 | - assert_not_nil CreateEnterprise.new.target_notification_message | 206 | + task = CreateEnterprise.new |
| 207 | + task.stubs(:environment).returns(Environment.default) | ||
| 208 | + assert_not_nil task.target_notification_message | ||
| 197 | end | 209 | end |
| 198 | 210 | ||
| 199 | should 'report as approved when approved' do | 211 | should 'report as approved when approved' do |
| @@ -210,6 +222,7 @@ class CreateEnterpriseTest < Test::Unit::TestCase | @@ -210,6 +222,7 @@ class CreateEnterpriseTest < Test::Unit::TestCase | ||
| 210 | 222 | ||
| 211 | should 'refuse to create an enterprise creation request with an identifier already used by another profile' do | 223 | should 'refuse to create an enterprise creation request with an identifier already used by another profile' do |
| 212 | request = CreateEnterprise.new | 224 | request = CreateEnterprise.new |
| 225 | + request.stubs(:environment).returns(Environment.default) | ||
| 213 | request.identifier = 'testid' | 226 | request.identifier = 'testid' |
| 214 | request.valid? | 227 | request.valid? |
| 215 | assert !request.errors.invalid?(:identifier) | 228 | assert !request.errors.invalid?(:identifier) |