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

Delegate custom attributes to images in ChargebackContainerImage #14395

Merged
merged 1 commit into from
Mar 20, 2017

Conversation

zeari
Copy link

@zeari zeari commented Mar 19, 2017

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1434159
Custom attributes in this report would turn out nil crash the report since they actually belong to the container image.

@lpichler Please review
cc @simon3z

@miq-bot add_label bug, chargeback, providers/containers

@zeari zeari force-pushed the delegate_custom_attributes_to_images branch 2 times, most recently from e0ed308 to 2663497 Compare March 19, 2017 09:53
@zeari zeari force-pushed the delegate_custom_attributes_to_images branch from 2663497 to b580851 Compare March 20, 2017 16:23
@zeari
Copy link
Author

zeari commented Mar 20, 2017

it turns out the entity was set correctly by ChargebackContainerImage but was overridden by the abstract Chargeback class

cc @isimluk @lpichler

@miq-bot
Copy link
Member

miq-bot commented Mar 20, 2017

Checked commit zeari@b580851 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks good. 🍪

@isimluk
Copy link
Member

isimluk commented Mar 20, 2017

@zeari This is good find.

Copy link
Contributor

@lpichler lpichler left a comment

Choose a reason for hiding this comment

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

@isimluk @options[:groupby_tag].present? is always false for container images (there is no such option in UI) and then init_extra_fields(consumption) will init the entity as container image object - so we are using origin code for loading custom attributes

Copy link
Member

@isimluk isimluk left a comment

Choose a reason for hiding this comment

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

👍

@isimluk isimluk added this to the Sprint 57 Ending Mar 27, 2017 milestone Mar 20, 2017
@isimluk isimluk self-assigned this Mar 20, 2017
@isimluk isimluk merged commit 6d5d478 into ManageIQ:master Mar 20, 2017
@zeari
Copy link
Author

zeari commented Mar 21, 2017

@miq-bot remove_label euwe/no
@miq-bot add_label euwe/yes

@miq-bot miq-bot added euwe/yes and removed euwe/no labels Mar 21, 2017
simaishi pushed a commit that referenced this pull request Mar 21, 2017
…ages

Delegate custom attributes to images in ChargebackContainerImage
(cherry picked from commit 6d5d478)

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

Euwe backport details:

 $ git log -1
commit 40cfb807ee4311dcc85672f23ee61bd0ed4c1ca9
Author: Šimon Lukašík <[email protected]>
Date:   Mon Mar 20 17:52:37 2017 +0100

    Merge pull request #14395 from zeari/delegate_custom_attributes_to_images
    
    Delegate custom attributes to images in ChargebackContainerImage
    (cherry picked from commit 6d5d478e0bc6f2726bee568d49da070d6e94794a)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1434411

agrare pushed a commit to agrare/manageiq that referenced this pull request Apr 19, 2017
…es_to_images

Delegate custom attributes to images in ChargebackContainerImage
(cherry picked from commit 6d5d478)

https://bugzilla.redhat.com/show_bug.cgi?id=1434411
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