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

Add nil check to serializer for invalid categories #16951

Merged
merged 1 commit into from Feb 5, 2018

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Feb 5, 2018

Fixes issue with dialog editor not loading on import of dialogs with invalid tag controls.

Fixes 1/2 of https://bugzilla.redhat.com/show_bug.cgi?id=1540713

This bz is two issues: the editor won't load because https://github.com/ManageIQ/manageiq/blob/master/app/models/dialog_field_serializer.rb#L26 returns nil for non-valid categories which this PR fixes, and the editor doesn't correctly save changed, valid categories which is an editor problem (because I can replicate it with the import piece taken out, that is, creating a new dialog with a tag control that has a category, saving it, then editing that dialog and changing the category results in the new category not saving as it should.)

@d-m-u
Copy link
Contributor Author

d-m-u commented Feb 5, 2018

@miq-bot add_label bug
@miq_bot assign @gmcculloug
@eclarizio @syncrou

@miq-bot miq-bot added the bug label Feb 5, 2018
@@ -23,7 +23,7 @@ def serialize(dialog_field, all_attributes = false)

if dialog_field.type == "DialogFieldTagControl"
category = Category.find_by(:id => dialog_field.category)
dialog_field.options.merge!(:category_name => category.name, :category_description => category.description)
dialog_field.options.merge!(:category_name => category.name, :category_description => category.description) if category.present?
Copy link
Member

Choose a reason for hiding this comment

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

The present? shouldn't be necessary since find_by will return either a record or nil

@miq-bot
Copy link
Member

miq-bot commented Feb 5, 2018

Checked commit d-m-u@60fde31 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

@gmcculloug gmcculloug self-assigned this Feb 5, 2018
@gmcculloug gmcculloug merged commit 0e35ec3 into ManageIQ:master Feb 5, 2018
@d-m-u d-m-u deleted the bz_1540713 branch February 5, 2018 18:04
simaishi pushed a commit that referenced this pull request Feb 6, 2018
Add nil check to serializer for invalid categories
(cherry picked from commit 0e35ec3)

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

simaishi commented Feb 6, 2018

Gaprindashvili backport details:

$ git log -1
commit d8bd20b4d234ffecbc4a216e24b6f6fe24db92b4
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Mon Feb 5 13:01:56 2018 -0500

    Merge pull request #16951 from d-m-u/bz_1540713
    
    Add nil check to serializer for invalid categories
    (cherry picked from commit 0e35ec3d10d8431e2838087f85c740dc053c2f24)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1542606

@agrare agrare added this to the Sprint 79 Ending Feb 12, 2018 milestone Jun 4, 2018
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.

None yet

7 participants