Load virtual custom columns from miq expression automatically #11369

lpichler · 2016-09-19T14:55:59Z

Load virtual custom columns == create dynamically method which is virtual and such methods
contain loading attributes from table CustomAttribute and we are getting ability that attributes in CustomAttribute#name have behavior as normal column.

If we have some in MiqExpression we need to load them first. This PR is moving loading when MiqExpression object is created.

this is fixing filtering advanced search by custom attributes
future clean up remove old methods for loading virtual custom columns in MiqReports #11365

@miq-bot assign @gtanzillo

can you please review?
@imtayadeway @alongoldboim

WIP: requires fix: #11380 and #11415 [unmerged]

Fryguy · 2016-09-19T15:20:53Z

lib/miq_expression.rb

@@ -1539,6 +1551,27 @@ def self._quick_search?(e)
    end
  end

+  def custom_attribute_columns_and_models(expression = nil)
+    return []


short circuit? Is this PR a WIP PR?

@Fryguy I had here some unwanted stuff

lpichler · 2016-09-19T16:40:45Z

@kbrock is it ok regard to perfomance ? methods for virtual custom attributes are created only when MiqExpression object is created and when there are already defined the creation is skipped. It is also finding recursively virtual_custom_attributes in expression for each MiqExpression object.

kbrock

Trying to wrap my mind around this.

Not sure if we need to work so hard to be lazy about these.

We are taking the hit on creation of the MiqExpression when the MiqExpression only has a custom attribute in it. So this looks good to me.

Let me know when this is no longer WIP

kbrock · 2016-09-20T22:28:04Z

app/models/mixins/custom_attribute_mixin.rb

@@ -34,6 +34,8 @@ def self.load_custom_attributes_for(cols)
    end

    def self.add_custom_attribute(custom_attribute)
+      return if respond_to?(custom_attribute)


this is great

lpichler · 2016-09-21T11:34:24Z

at this moment depends on #11415

lpichler · 2016-09-21T17:00:49Z

@miq-bot remove_label wip

lpichler · 2016-09-21T17:01:27Z

@kbrock @alongoldboim @imtayadeway - rebased and I added catching of expection.

kbrock · 2016-09-22T03:19:01Z

I'm good with this. I'll merge if @imtayadeway or @gtanzillo give a +1

imtayadeway · 2016-09-22T09:21:17Z

spec/lib/miq_expression_spec.rb

+
+    it "automatically loads virtual custom attributes from MiqExpression on class" do
+      MiqExpression.new("EQUAL" => {"field" => "#{klass}-#{virtual_custom_attribute}", "value" => "foo"}).to_sql
+      expect(vm).to respond_to(virtual_custom_attribute)


Is there a way to prove that it was the line above that made this so?

@imtayadeway I am not sure how to prove it.
maybe ?

expect(vm).not_to respond_to(virtual_custom_attribute) MiqExpression.new("EQUAL" => {"field" => "#{klass}-#{virtual_custom_attribute}", "value" => "foo"}).to_sql expect(vm).to respond_to(virtual_custom_attribute)

I like that better 👍

Could also be:

expect { MiqExpression.new(...) }.to change { vm.respond_to?(virtual_custom_attrivute) }.from(false).to(true)

But whichever you prefer :)

@imtayadeway I prefer change 👍 thanks

from MiqExpression

miq-bot · 2016-09-22T11:57:00Z

Checked commits lpichler/manageiq@062ad5e~...985ca22 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
3 files checked, 3 offenses detected

lib/miq_expression.rb

❕ - Line 1554, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for custom_attribute_columns_and_models is too high. [29.56/20]
❕ - Line 1554, Col 3 - Metrics/CyclomaticComplexity - Cyclomatic complexity for custom_attribute_columns_and_models is too high. [10/6]
❕ - Line 1554, Col 3 - Metrics/PerceivedComplexity - Perceived complexity for custom_attribute_columns_and_models is too high. [10/7]

lpichler · 2016-10-05T12:52:31Z

@gtanzillo can it be merged, if it looks good to you ? thanks!

@alongoldboim need it for #11002

alongoldboim · 2016-10-05T13:17:58Z

LGTM 👍

gtanzillo · 2016-10-05T22:17:15Z

👍 LGTM

gtanzillo · 2016-10-05T22:18:14Z

@lpichler I've tagged as euwe/no. Let me know if this is not correct.

we are loading virtual custom columns from MiqExpression when MiqExpression object is created ManageIQ#11369

lpichler · 2016-10-14T09:35:18Z

@gtanzillo this should be euwe/yes, @alongoldboim's related PR are also euwe/yes

@miq-bot add_label euwe/yes

we are loading virtual custom columns from MiqExpression when MiqExpression object is created ManageIQ#11369 (cherry picked from commit 26f94a1)

lpichler · 2016-10-14T09:49:39Z

@miq-bot remove_label euwe/no

…rom_miq_expression Load virtual custom columns from miq expression automatically (cherry picked from commit 10d93a4)

chessbyte · 2016-10-14T15:31:14Z

Euwe Backport details:

$ git log -1
commit e7832969d7eef5c2e228ddc71a05955d1fab6396
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Wed Oct 5 18:18:29 2016 -0400

    Merge pull request #11369 from lpichler/load_virtual_custom_columns_from_miq_expression

    Load virtual custom columns from miq expression automatically
    (cherry picked from commit 10d93a4a5761a3b5402fba20fccb1d807c51e174)

miq-bot assigned gtanzillo Sep 19, 2016

Fryguy reviewed Sep 19, 2016

View reviewed changes

lpichler changed the title ~~Load virtual custom columns from miq expression automatically~~ [WIP] Load virtual custom columns from miq expression automatically Sep 19, 2016

lpichler force-pushed the load_virtual_custom_columns_from_miq_expression branch from ed003ce to b17c012 Compare September 19, 2016 16:31

chessbyte added enhancement wip core labels Sep 19, 2016

lpichler force-pushed the load_virtual_custom_columns_from_miq_expression branch from b17c012 to 7e8e11c Compare September 19, 2016 16:42

lpichler mentioned this pull request Sep 20, 2016

Support container/infra/cloud provider policies in the UI #11002

Merged

gtanzillo assigned kbrock Sep 20, 2016

kbrock reviewed Sep 20, 2016

View reviewed changes

lpichler force-pushed the load_virtual_custom_columns_from_miq_expression branch 2 times, most recently from d8fa377 to 4ba36ca Compare September 21, 2016 09:15

lpichler mentioned this pull request Sep 21, 2016

Correct format of columns for MiqExpression #11415

Merged

lpichler force-pushed the load_virtual_custom_columns_from_miq_expression branch from 4ba36ca to d2e7ca5 Compare September 21, 2016 11:33

lpichler force-pushed the load_virtual_custom_columns_from_miq_expression branch from d2e7ca5 to 6880499 Compare September 21, 2016 16:59

lpichler changed the title ~~[WIP] Load virtual custom columns from miq expression automatically~~ Load virtual custom columns from miq expression automatically Sep 21, 2016

miq-bot removed the wip label Sep 21, 2016

kbrock removed their assignment Sep 22, 2016

imtayadeway reviewed Sep 22, 2016

View reviewed changes

lpichler added 3 commits September 22, 2016 11:50

Don't create same method repeately

062ad5e

Load automatically virtual custom attributes

4870779

Specs ensuring that virtual custom columns are loaded

985ca22

from MiqExpression

lpichler force-pushed the load_virtual_custom_columns_from_miq_expression branch from 6880499 to 985ca22 Compare September 22, 2016 11:55

gtanzillo added this to the Sprint 48 Ending Oct 24, 2016 milestone Oct 5, 2016

gtanzillo added the euwe/no label Oct 5, 2016

gtanzillo merged commit 10d93a4 into ManageIQ:master Oct 5, 2016

lpichler added a commit to lpichler/manageiq that referenced this pull request Oct 10, 2016

Remove loading virtual custom columns from MiqExpression for MiqReport

26f94a1

we are loading virtual custom columns from MiqExpression when MiqExpression object is created ManageIQ#11369

lpichler mentioned this pull request Oct 10, 2016

Remove old methods for loading virtual columns #11791

Merged

lpichler deleted the load_virtual_custom_columns_from_miq_expression branch October 14, 2016 09:31

miq-bot added the euwe/yes label Oct 14, 2016

miq-bot removed the euwe/no label Oct 14, 2016

lpichler mentioned this pull request Oct 14, 2016

[WIP] [DEPENDS ON 11369] [EUWE] Remove old methods for loading virtual columns #11934

Closed

chessbyte pushed a commit that referenced this pull request Oct 14, 2016

Merge pull request #11369 from lpichler/load_virtual_custom_columns_f…

e783296

…rom_miq_expression Load virtual custom columns from miq expression automatically (cherry picked from commit 10d93a4)

chessbyte added euwe/backported and removed euwe/yes labels Oct 14, 2016

lpichler mentioned this pull request Nov 3, 2016

Add specs for loading virtual custom attributes from right side in expression #12417

Merged

gtanzillo mentioned this pull request Nov 15, 2016

Enable custom attributes for chargeback reports #11794

Merged

lpichler mentioned this pull request Apr 18, 2017

Load custom attribute before evaluation #14785

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Load virtual custom columns from miq expression automatically #11369

Load virtual custom columns from miq expression automatically #11369

lpichler commented Sep 19, 2016 •

edited

Fryguy Sep 19, 2016

lpichler Sep 19, 2016

lpichler commented Sep 19, 2016 •

edited

kbrock left a comment

kbrock Sep 20, 2016

lpichler commented Sep 21, 2016

lpichler commented Sep 21, 2016

lpichler commented Sep 21, 2016

kbrock commented Sep 22, 2016

imtayadeway Sep 22, 2016

lpichler Sep 22, 2016 •

edited

imtayadeway Sep 22, 2016

lpichler Sep 22, 2016

miq-bot commented Sep 22, 2016

lpichler commented Oct 5, 2016 •

edited

alongoldboim commented Oct 5, 2016

gtanzillo commented Oct 5, 2016

gtanzillo commented Oct 5, 2016

lpichler commented Oct 14, 2016

lpichler commented Oct 14, 2016 •

edited

chessbyte commented Oct 14, 2016

Load virtual custom columns from miq expression automatically #11369

Load virtual custom columns from miq expression automatically #11369

Conversation

lpichler commented Sep 19, 2016 • edited

Fryguy Sep 19, 2016

Choose a reason for hiding this comment

lpichler Sep 19, 2016

Choose a reason for hiding this comment

lpichler commented Sep 19, 2016 • edited

kbrock left a comment

Choose a reason for hiding this comment

kbrock Sep 20, 2016

Choose a reason for hiding this comment

lpichler commented Sep 21, 2016

lpichler commented Sep 21, 2016

lpichler commented Sep 21, 2016

kbrock commented Sep 22, 2016

imtayadeway Sep 22, 2016

Choose a reason for hiding this comment

lpichler Sep 22, 2016 • edited

Choose a reason for hiding this comment

imtayadeway Sep 22, 2016

Choose a reason for hiding this comment

lpichler Sep 22, 2016

Choose a reason for hiding this comment

miq-bot commented Sep 22, 2016

lpichler commented Oct 5, 2016 • edited

alongoldboim commented Oct 5, 2016

gtanzillo commented Oct 5, 2016

gtanzillo commented Oct 5, 2016

lpichler commented Oct 14, 2016

lpichler commented Oct 14, 2016 • edited

chessbyte commented Oct 14, 2016

lpichler commented Sep 19, 2016 •

edited

lpichler commented Sep 19, 2016 •

edited

lpichler Sep 22, 2016 •

edited

lpichler commented Oct 5, 2016 •

edited

lpichler commented Oct 14, 2016 •

edited