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 edit credentials #20524

Merged

Conversation

NickLaMuro
Copy link
Member

@NickLaMuro NickLaMuro commented Sep 4, 2020

Fixes ManageIQ/manageiq-ui-classic#7286

There was a bug when editing credentials for EmbeddedAnsible where if you edited an attribute and there are other attributes on the particular credential class' params_to_attributes method which have to be converted, those previously would be nil'd out.

This fixes that, and modifies tests to confirm that this is now working.

Links

@Fryguy
Copy link
Member

Fryguy commented Sep 4, 2020

❗ - Line 93, Col 36 - Layout/BlockAlignment - end at 93, 35 is not aligned with previous_params_to_attrs = params_to_attrs.each_with_object({}) do |key, attrs| at 91, 8.

Wow! Pretty sure that's a rubocop bug. Its detecting the alignment to the middle of the previous_params_to_attrs because it happens to end in the same string params_to_attrs 😮

I wonder if that happens with a newer rubocop.

@Fryguy
Copy link
Member

Fryguy commented Sep 4, 2020

The rubocops in app/models/manageiq/providers/embedded_ansible/automation_manager/openstack_credential.rb should be fixed cause they are actually misaligned. Same with "Redundant curly braces around a hash parameter.". The rest I think are on unrelated lines.

@NickLaMuro NickLaMuro force-pushed the fix-embedded-ansible-credential-editing branch from 9ced20d to 39899d2 Compare September 4, 2020 20:44
There was a bug when editing credentials for EmbeddedAnsible where if
you edited an attribute and there are other attributes on the particular
credential class' params_to_attributes method which have to be
converted, those previously would be `nil`'d out.

This fixes that, and modifies tests to confirm that this is now working.
@NickLaMuro NickLaMuro force-pushed the fix-embedded-ansible-credential-editing branch from 39899d2 to 7bbdf70 Compare September 8, 2020 21:22
@miq-bot
Copy link
Member

miq-bot commented Sep 8, 2020

Checked commit NickLaMuro@7bbdf70 with ruby 2.6.3, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
10 files checked, 1 offense detected

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

  • ❗ - Line 93, Col 36 - Layout/BlockAlignment - end at 93, 35 is not aligned with previous_params_to_attrs = params_to_attrs.each_with_object({}) do |key, attrs| at 91, 8.

@NickLaMuro
Copy link
Member Author

@Fryguy I assume you are good with this now that the Rubocop errors are fixed, right?

@Fryguy Fryguy merged commit 9a23708 into ManageIQ:master Sep 10, 2020
simaishi pushed a commit that referenced this pull request Sep 18, 2020
…tial-editing

[EmbeddedAnsible] Fix edit credentials

(cherry picked from commit 9a23708)
@simaishi
Copy link
Contributor

Jansa backport details:

$ git log -1
commit 3801f07425471006e6ce1c87fa913410a6795146
Author: Jason Frey <[email protected]>
Date:   Thu Sep 10 18:17:07 2020 -0400

    Merge pull request #20524 from NickLaMuro/fix-embedded-ansible-credential-editing

    [EmbeddedAnsible] Fix edit credentials

    (cherry picked from commit 9a23708582de38c9ae0cc8332f7e46fc1a53ba08)

@tinaafitz
Copy link
Member

Hi @NickLaMuro Can this be ivanchuk/yes?

@NickLaMuro
Copy link
Member Author

@tinaafitz yeah, that would be fine. Just slipped my mind on that one.

@miq-bot add_label ivanchuk/yes

simaishi pushed a commit that referenced this pull request Sep 30, 2020
…tial-editing

[EmbeddedAnsible] Fix edit credentials

(cherry picked from commit 9a23708)

https://bugzilla.redhat.com/show_bug.cgi?id=1881513
@simaishi
Copy link
Contributor

Ivanchuk backport details:

$ git log -1
commit c0f0eeec8ee112370ea41444806d655c4d1990f1
Author: Jason Frey <[email protected]>
Date:   Thu Sep 10 18:17:07 2020 -0400

    Merge pull request #20524 from NickLaMuro/fix-embedded-ansible-credential-editing

    [EmbeddedAnsible] Fix edit credentials

    (cherry picked from commit 9a23708582de38c9ae0cc8332f7e46fc1a53ba08)

    https://bugzilla.redhat.com/show_bug.cgi?id=1881513

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.

Editing username for EmbeddedAnsible SCM credential cause key to be nil'd out
6 participants