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

create the resource action after save for custom buttons #821

Merged
merged 1 commit into from
Apr 30, 2020

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Apr 30, 2020

otherwise this fails with Failed to create new custom button - You cannot call create unless the parent is saved on rails 5.2 🤮

Keenan caught it in https://travis-ci.com/github/ManageIQ/manageiq-cross_repo-tests/jobs/325865143#L2435

@miq-bot add_label jansa/yes, ivanchuk/yes
@miq-bot add_reviewer @jrafanie
@miq-bot add_label bug

Followup to #814
Needed for Rails 5.2: ManageIQ/manageiq#20032

    Rails 5.2 raises this error if you try to persist the child before creating the
    parent: `Failed to create new custom button - You cannot call create unless the
    parent is saved`

    Followup to ManageIQ#814
    Needed for Rails 5.2: ManageIQ/manageiq#20032
    thanks keenan and lj
@d-m-u d-m-u force-pushed the fixing_silly_create_before_parent branch from cb5a8d3 to 5c730a4 Compare April 30, 2020 17:18
@miq-bot
Copy link
Member

miq-bot commented Apr 30, 2020

Checked commit d-m-u@5c730a4 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. 👍

@abellotti abellotti self-assigned this Apr 30, 2020
custom_button.save!
custom_button.create_resource_action!(data["resource_action"].deep_symbolize_keys) if data.key?("resource_action")
Copy link
Member

@abellotti abellotti Apr 30, 2020

Choose a reason for hiding this comment

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

just curious, can the create_resource_action! method somehow fail, and we end up with a created Object, but never used/returned ? do we need a transaction for the above two so the request is atomic ? @jrafanie thoughts ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't really think so. resource action create 1) has no validations and 2) are artifacts that link dialogs, resources, and automate, there's not much of a vector for failure

Copy link
Member

Choose a reason for hiding this comment

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

yeah, probably similar to this one: ManageIQ/manageiq#20013

@d-m-u can you review if this is required and do a followup PR? This is great to fix the issue on rails 5.2

Copy link
Member

Choose a reason for hiding this comment

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

We cross our streams at the same time and didn't use a mutex

@jrafanie jrafanie merged commit da3529c into ManageIQ:master Apr 30, 2020
@d-m-u d-m-u deleted the fixing_silly_create_before_parent branch April 30, 2020 18:00
@jrafanie jrafanie self-assigned this Apr 30, 2020
@chessbyte chessbyte mentioned this pull request Apr 30, 2020
38 tasks
simaishi pushed a commit that referenced this pull request May 1, 2020
create the resource action after save for custom buttons

(cherry picked from commit da3529c)
@simaishi
Copy link
Contributor

simaishi commented May 1, 2020

Jansa backport details:

$ git log -1
commit bf293ed28e335200f98e02f4bf37678645e1f1d0
Author: Joe Rafaniello <[email protected]>
Date:   Thu Apr 30 14:00:36 2020 -0400

    Merge pull request #821 from d-m-u/fixing_silly_create_before_parent

    create the resource action after save for custom buttons

    (cherry picked from commit da3529ca5a85a3c4aa8858f8ba7d94c9a110cc9e)

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.

7 participants