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

Load custom attribute before evaluation #14785

Merged

Conversation

lpichler
Copy link
Contributor

@lpichler lpichler commented Apr 18, 2017

introduced in #11369

Loading virtual custom attributes is not needed in constructor
we need to load them when we need them and this place is in
Condition#_subst method where Tag.list is called

In addition, constructor is not called when MiqExpressin is loaded from yaml (for example as it is in used MiqReport#condition for generation reports)

This is fixing a bug when report has defined expression with virtual custom attribute and these attributes are not listed in Report#cols

@miq-bot add_label bug, core
@miq-bot assign @gtanzillo

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

Loading virtual custom attributes is not needed in constructor
we need to load them when we need them and this place is in
Condition#_subst method where Tag.list is called

In addition, constructor is not called when MiqExpressin is
loaded from yaml (for example as it is in used
MiqReport#condition for generation reports)
this feature is need to test it in RBAC - filterer
In add_custom_attribute is also checking whether attribute
has been already loaded
@lpichler lpichler force-pushed the load_custom_attribute_before_evaluation branch from 342841e to f4d298e Compare April 18, 2017 09:35
@miq-bot
Copy link
Member

miq-bot commented Apr 18, 2017

Checked commits lpichler/manageiq@549e80c~...f4d298e with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
4 files checked, 0 offenses detected
Everything looks good. 🍰

@lpichler
Copy link
Contributor Author

@miq-bot add_label fine/yes

Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@gtanzillo gtanzillo added this to the Sprint 59 Ending Apr 24, 2017 milestone Apr 20, 2017
@gtanzillo gtanzillo merged commit 291b266 into ManageIQ:master Apr 20, 2017
@lpichler lpichler deleted the load_custom_attribute_before_evaluation branch April 20, 2017 09:34
simaishi pushed a commit that referenced this pull request Apr 20, 2017
…evaluation

Load custom attribute before evaluation
(cherry picked from commit 291b266)
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit 41b005492dfd1560393d6cf8d0a06dee4f89f24e
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Thu Apr 20 11:33:31 2017 +0200

    Merge pull request #14785 from lpichler/load_custom_attribute_before_evaluation
    
    Load custom attribute before evaluation
    (cherry picked from commit 291b2666c9f3741932c14956923308c0bd2a33e9)

@simaishi
Copy link
Contributor

@lpichler Need a BZ for Euwe backport...

@simaishi
Copy link
Contributor

simaishi commented Jun 6, 2017

@lpichler ping ^

@gtanzillo
Copy link
Member

@simaishi Libor created https://bugzilla.redhat.com/show_bug.cgi?id=1459171 and I added it to the description. Thanks.

@simaishi
Copy link
Contributor

simaishi commented Jun 6, 2017

@lpichler def self.list (in app/models/tag.rb) doesn't exist in Euwe branch. Can you create an Euwe PR referencing this?

@lpichler
Copy link
Contributor Author

lpichler commented Jun 6, 2017

@simaishi EUWE PR: #15320

@simaishi
Copy link
Contributor

simaishi commented Jun 6, 2017

Backported to Euwe via #15320

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.

4 participants