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.

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

[EmbeddedAnsible] Fix manager ref for credentials #18972

Conversation

NickLaMuro
Copy link
Member

@NickLaMuro NickLaMuro commented Jul 13, 2019

Makes sure we are properly assigning the manager (resource) when creating AutomationManager::Credential records.

Also updates the specs, not only to test this on create, but to also properly initialize the records for the UPDATE and DELETE specs (how I instantiated things were should have been a bit of a tip off that this was coded wrong... oh well...)

Steps for Testing/QA

TODO: Want to confirm this fixes a few things on a VM, but need to make a separate branch to integrate this commit with the work in #18969 to confirm this is doing what I want it to.

  • Enable EmbeddedAnsible
  • Add a credential
  • Create a catalog and catalog item
  • When ordering the catalog item, you should see the credential in the dropdown:

Screen Shot 2019-07-12 at 9 44 30 PM

Makes sure we are properly assigning the manager (resource) when
creating AutomationManager::Credential records.

Also updates the specs, not only to test this on create, but to also
properly initialize the records for the UPDATE and DELETE specs (how I
instantiated things were should have been a bit of a tip off that this
was coded wrong... oh well...)
@NickLaMuro
Copy link
Member Author

cc @carbonin

This should fix the issue we were discussing earlier today.

@miq-bot add_labels embedded ansible, bug

@miq-bot
Copy link
Member

miq-bot commented Jul 13, 2019

Checked commit NickLaMuro@5d7ebae with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 1 offense detected

spec/models/manageiq/providers/embedded_ansible/automation_manager/credential_spec.rb

  • ❗ - Line 49, Col 9 - Performance/RedundantMerge - Use params_to_attributes[:resource] = manager instead of params_to_attributes.merge!(:resource => manager).

@carbonin carbonin self-assigned this Jul 15, 2019
@carbonin carbonin merged commit 8bebef6 into ManageIQ:master Jul 15, 2019
@carbonin carbonin added this to the Sprint 116 Ending Jul 22, 2019 milestone Jul 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants