Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed checks around retirement dialog values. #768

Merged
merged 3 commits into from
Mar 23, 2017
Merged

Fixed checks around retirement dialog values. #768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b991bee
Fixed checks around retirement dialog values.
h-kataria Mar 22, 2017
3ce463c
Need to only check for existence of dialog_id key to fetch the dialog.
h-kataria Mar 22, 2017
9709fdf
Addressed code climate error
h-kataria Mar 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/assets/javascripts/controllers/catalog/catalog_item_form_controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,9 @@ ManageIQ.angular.app.controller('catalogItemFormController', ['$scope', 'catalog
if (configData.retirement_cloud_credential_id !== '')
catalog_item['config_info']['retirement']['cloud_credential_id'] = configData.retirement_cloud_credential_id;

if (configData.retirement_dialog_id !== '') {
if (angular.isDefined(vm.catalogItemModel.retirement_dialog_id) && configData.retirement_dialog_id !== '') {
catalog_item['config_info']['retirement']['dialog_id'] = configData.retirement_dialog_id;
} else if (configData.retirement_dialog_name !== '')
} else if (angular.isDefined(vm.catalogItemModel.retirement_dialog_name) && configData.retirement_dialog_name !== '')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@h-kataria - Seeing a code climate error about not having a { after this if condition.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@syncrou we usually add curly braces around if/else blocks if it has more than 1 statement in the block, so that comment can be ignored

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@h-kataria - I see now - the removed else if was that way. Looks good to me then.

👍

catalog_item['config_info']['retirement']['new_dialog_name'] = configData.retirement_dialog_name;

return catalog_item;
Expand Down
19 changes: 9 additions & 10 deletions app/controllers/catalog_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1797,11 +1797,7 @@ def fetch_playbook_details
playbook_details[:provisioning][:machine_credential] = ManageIQ::Providers::EmbeddedAnsible::AutomationManager::MachineCredential.find_by(:id => provision[:credential_id]).name
playbook_details[:provisioning][:network_credential] = ManageIQ::Providers::EmbeddedAnsible::AutomationManager::NetworkCredential.find_by(:id => provision[:network_credential_id]).name if provision[:network_credential_id]
playbook_details[:provisioning][:cloud_credential] = ManageIQ::Providers::EmbeddedAnsible::AutomationManager::CloudCredential.find_by(:id => provision[:cloud_credential_id]).name if provision[:cloud_credential_id]
dialog = provision[:dialog_id] ? Dialog.find_by(:id => provision[:dialog_id]) : Dialog.find_by(:name => provision[:dialog_name])
if dialog
playbook_details[:provisioning][:dialog] = dialog.name
playbook_details[:provisioning][:dialog_id] = dialog.id
end
fetch_dialog(playbook_details, provision[:dialog_id], :provisioning)

if @record.config_info[:retirement]
retirement = @record.config_info[:retirement]
Expand All @@ -1813,16 +1809,19 @@ def fetch_playbook_details
playbook_details[:retirement][:machine_credential] = ManageIQ::Providers::EmbeddedAnsible::AutomationManager::MachineCredential.find_by(:id => retirement[:credential_id]).name
playbook_details[:retirement][:network_credential] = ManageIQ::Providers::EmbeddedAnsible::AutomationManager::NetworkCredential.find_by(:id => retirement[:network_credential_id]).name if retirement[:network_credential_id]
playbook_details[:retirement][:cloud_credential] = ManageIQ::Providers::EmbeddedAnsible::AutomationManager::CloudCredential.find_by(:id => retirement[:cloud_credential_id]).name if retirement[:cloud_credential_id]
dialog = provision[:dialog_id] ? Dialog.find_by(:id => retirement[:dialog_id]) : Dialog.find_by(:name => retirement[:dialog_name])
if dialog
playbook_details[:retirement][:dialog] = dialog.name
playbook_details[:retirement][:dialog_id] = dialog.id
end
fetch_dialog(playbook_details, retirement[:dialog_id], :retirement)
end
end
playbook_details
end

def fetch_dialog(playbook_details, dialog_id, key)
return nil if dialog_id.nil?
dialog = Dialog.find_by(:id => dialog_id)
playbook_details[key][:dialog] = dialog.name
playbook_details[key][:dialog_id] = dialog.id
end

def open_parent_nodes(record)
existing_node = nil # Init var

Expand Down
63 changes: 63 additions & 0 deletions spec/controllers/catalog_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -739,4 +739,67 @@
expect(assigns(:edit)[:new][:available_resources].count).to eq(2)
end
end

context "#fetch_playbook_details" do
let(:auth) { FactoryGirl.create(:authentication, :name => "machine_cred", :manager_ref => 6, :type => "ManageIQ::Providers::EmbeddedAnsible::AutomationManager::MachineCredential") }
let(:repository) { FactoryGirl.create(:configuration_script_source, :manager => ems, :type => "ManageIQ::Providers::EmbeddedAnsible::AutomationManager::ConfigurationScriptSource") }
let(:inventory_root_group) { FactoryGirl.create(:inventory_root_group, :name => 'Demo Inventory') }
let(:ems) do
FactoryGirl.create(:automation_manager_ansible_tower, :inventory_root_groups => [inventory_root_group], :provider => FactoryGirl.create(:provider_embedded_ansible))
end
let(:dialog) { FactoryGirl.create(:dialog, :label => "Some Label") }
let(:playbook) do
FactoryGirl.create(:embedded_playbook,
:configuration_script_source => repository,
:manager => ems,
:inventory_root_group => inventory_root_group)
end

it "returns playbook service template details for provision & retirement tabs for summary screen" do
options = {
:name => 'test_ansible_catalog_item',
:description => 'test ansible',
:display => true,
:config_info => {
:provision => {
:new_dialog_name => 'test_dialog',
:hosts => 'many',
:credential_id => auth.id,
:repository_id => repository.id,
:playbook_id => playbook.id,
:dialog_id => dialog.id
},
:retirement => {
:new_dialog_name => 'test_dialog',
:hosts => 'many',
:credential_id => auth.id,
:repository_id => repository.id,
:playbook_id => playbook.id,
:dialog_id => dialog.id
}
}
}
service_template = double("ServiceTemplateAnsiblePlaybook", options)
controller.instance_variable_set(:@record, service_template)
playbook_details = controller.send(:fetch_playbook_details)
st_details = {
:provisioning => {
:repository => repository.name,
:playbook => playbook.name,
:machine_credential => auth.name,
:dialog => "Some Label",
:dialog_id => dialog.id
},
:retirement => {
:remove_resources => nil,
:repository => repository.name,
:playbook => playbook.name,
:machine_credential => auth.name,
:dialog => "Some Label",
:dialog_id => dialog.id
}
}
expect(playbook_details).to eq(st_details)
end
end
end
3 changes: 1 addition & 2 deletions spec/javascripts/controllers/catalog/catalog_item_form_controller_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ describe('catalogItemFormController', function() {
hosts: undefined,
extra_vars: Object({ }),
network_credential_id: undefined,
cloud_credential_id: undefined,
dialog_id: undefined
cloud_credential_id: undefined
}
}
};
Expand Down