Manager name not updated on foreman provider edit #5
Conversation
|
@bdunne - please review |
| describe "#save" do | ||
| it "will update the name for the manager" do | ||
| provider = FactoryGirl.create(:provider_foreman, :zone => FactoryGirl.create(:zone), :name => 'Old Name') | ||
| provider.save |
There was a problem hiding this comment.
The save isn't needed since you used create above.
| it "will update the name for the manager" do | ||
| provider = FactoryGirl.create(:provider_foreman, :zone => FactoryGirl.create(:zone), :name => 'Old Name') | ||
| provider.save | ||
| expect(provider.configuration_manager.name).to eq('Old Name Configuration Manager') |
There was a problem hiding this comment.
It feels a little strange to me that we're doing this, but since we are, shouldn't we test that all managers are updated?
|
|
||
| provider.name = 'New Name' | ||
| provider.save | ||
| expect(provider.configuration_manager.name).to eq('New Name Configuration Manager') |
There was a problem hiding this comment.
@bdunne - not sure if the provisioning manager name is used in the UI - I have added a test for that as well.
lgalis
force-pushed
the
manager_name_not_updated_on_provider_edit
branch
from
d1db59b
to
fff521d
Compare
| @@ -20,7 +20,7 @@ class ManageIQ::Providers::Foreman::Provider < ::Provider | |||
|
|
|||
| delegate :api_cached?, :ensure_api_cached, :to => :connect | |||
|
|
|||
| before_create :ensure_managers | |||
| before_validation :ensure_managers | |||
There was a problem hiding this comment.
I think this should be before_save, right? Otherwise we could wind up updating the name on the managers before running the validations on the name.
There was a problem hiding this comment.
@bdunne - I think it should be before_validation, in order for the validation to run with the new names.
There was a problem hiding this comment.
I see, we're using :autosave => true on these relations.
|
@miq-bot assign @gmcculloug |
| expect(provider.configuration_manager.name).to eq('Old Name Configuration Manager') | ||
|
|
||
| provider.name = 'New Name' | ||
| provider.save |
There was a problem hiding this comment.
These two lines
provider.name = 'New Name'
provider.savecan be combined into one call:
provider.update_attributes(:name => 'New Name')There was a problem hiding this comment.
Changed to update that replaces update_attributes in newer rails.
lgalis
force-pushed
the
manager_name_not_updated_on_provider_edit
branch
from
fff521d
to
7cb3b44
Compare
lgalis
force-pushed
the
manager_name_not_updated_on_provider_edit
branch
from
7cb3b44
to
0a84fe3
Compare
|
Checked commits lgalis/manageiq-providers-foreman@13b7149~...0a84fe3 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
|
the travis errors are not related to the changes in this PR |
|
@bdunne Is this an |
|
Agreed that the failing tests are unrelated to this PR. I will merge this and open a git issue to resolve the failing tests. Thanks. |
|
Fine backport (to manageiq repo) details: |
Manager name not updated on foreman provider edit
https://bugzilla.redhat.com/show_bug.cgi?id=1450457