Support condensed virtual_attribute definitions

[kbrock]

I'm trying to guage what people think of this new syntax.

1. It has the ruby and sql definitions together rather than scattered.
2. It has 2 blocks (arel and ruby) back to back.
3. It auto generates the "use attribute from select()" line. i.e.:

   return self["archived"] if  has_attribute?("archived")
   

4. The "use attribute if from select()" is missing all over our code, so that is a win.
5. The ruby method will be defined using metadata rather than ruby. (something to be said for plain old ruby)
6. This can be added incrementally.

NOTE: If you look in the code, the t.grouping() adds parens around the SQL. This is no longer necessary and going through this will allow us to remove it.

Before

  def archived?
    return self["archived"] if  has_attribute?("archived")

    ems_id.nil? && storage_id.nil?
  end
  alias_method :archived, :archived?

  virtual_attribute :archived, :boolean, :arel => (lambda do |t|
    t.grouping(t[:ems_id].eq(nil).and(t[:storage_id].eq(nil)))
  end)

After

  virtual_attribute :archived, :boolean, :arel => -> (t) {
    t[:ems_id].eq(nil).and(t[:storage_id].eq(nil))
  } do
    ems_id.nil? && storage_id.nil?
  end
  alias_method :archived, :archived?

Before 2

class VmOrTemplate
  def normalized_state
    return self["normalized_state"] if has_attribute?("normalized_state")

    %w[archived orphaned template retired disconnected].each do |s|
      return s if send(:"#{s}?")
    end

    power_state&.downcase || "unknown"
  end

  virtual_attribute :normalized_state, :string, :arel => (lambda do |t|
    Arel::Nodes::Case.new
      .when(arel_table[:archived]).then(Arel.sql("'archived'"))
      .when(arel_table[:orphaned]).then(Arel.sql("'orphaned'"))
      .when(t[:template].eq(t.create_true)).then(Arel.sql("'template'"))
      .when(t[:retired].eq(t.create_true)).then(Arel.sql("'retired'"))
      .when(arel_table[:disconnected]).then(Arel.sql("'disconnected'"))
      .else(t.lower(t.coalesce([t[:power_state], Arel.sql("'unknown'")])))
  end)
end

After 2

class VmOrTemplate
  virtual_attribute :normalized_state, :string, :arel => -> (t) {
    Arel::Nodes::Case.new
      .when(arel_table[:archived]).then(Arel.sql("'archived'"))
      .when(arel_table[:orphaned]).then(Arel.sql("'orphaned'"))
      .when(t[:template].eq(t.create_true)).then(Arel.sql("'template'"))
      .when(t[:retired].eq(t.create_true)).then(Arel.sql("'retired'"))
      .when(arel_table[:disconnected]).then(Arel.sql("'disconnected'"))
      .else(t.lower(t.coalesce([t[:power_state], Arel.sql("'unknown'")])))
  } do
    %w[archived orphaned template retired disconnected].each do |s|
      return s if send(:"#{s}?")
    end

    power_state&.downcase || "unknown"
  end
end

[Fryguy]

The ruby method will be defined using metadata rather than ruby. (something to be said for plain old ruby)

Not sure what this means - it's still Ruby in the after as well

Unless you mean we will be building the method dynamically, and then, yeah, I agree - at least it's greppable either way.

[kbrock]

Yea. I guess it is all ruby. Even the "sql" portion is technically ruby.

But defining a method using def method ; end is not the same thing as def_method(:method) do ; end. The latter leaves a closure around and some other residue.

Also, the first is ruby debugger friendly.

[miq-bot]

This issue has been automatically marked as stale because it has not been updated for at least 3 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

[kbrock]

TODO: verify the return s in example 2 works.

Think we liked this approach and wanted to go forward with it

[miq-bot]

This issue has been automatically marked as stale because it has not been updated for at least 3 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.