How to check if all records of an instance have one of many values from a list of values?

Question

I'm looking for a solution for a complex query.

Goal : I want to know if all record of an instance have a value, and do some action when all records of the instance have the same value.

Order Model :

has_many :items
Columns : treated

Item Model :

belongs_to :order
Columns : order_id / state

Seed exemple :

Order.create
@item1 = Item.create(order_id: 1, state: pending)
@item2 = Item.create(order_id: 1, state: pending)

In a callback action, on the Item Model, I've done :

def treatment
    self.order.items.each do |item|
      if item.state = "order_accepted" or item.state = "order_refused"
        item.order.update_attributes(treated: "true")
      end
    end
  end

So i'm trying to have a result like :

"If all my Items have the states "order_accepted" or "order_refused", then update the Order to 'treated'."

Please help! Many thanks

Tom Lord · Accepted Answer · 2018-07-17T09:02:19.427

Your question title does not match the description. You're not checking whether "all records have the same value", you're checking that all records have one of many values (from a list).

There are a few issues with your attempt:

self.order.items.each do |item|
  if item.state = "order_accepted" or item.state = "order_refused"
    # ...

Redundant use of self
Use of = is setting the value, rather than checking for equality. (This would be done with ==.)
By looping through all items like that, you're updating the order multiple times.
...And since you're performing the check for each item, rather than all items, you're not checking that they all have the desired value.

A minimal change to your attempt that would work is:

if order.items.all? { |item| item.state == "order_accepted" || item.state == "order_refused" }
  order.update_attributes(treated: "true")
end

However, that's not a great solution since you're loading all objects into memory and looping through them. This is inefficient, especially if there are lots of items. A better approach is to perform the check directly in SQL rather than in ruby:

if order.items.where.not(state: ["order_accepted", "order_refused"]).empty?
  order.update_attribute(:treated, "true")
end

To tidy this code up a little, I would define that query as a scope on the Item model, such as:

class Item < ApplicationRecord
  scope :not_completed, -> { where.not(state: ["order_accepted", "order_refused"]) }
end

And then perhaps define a method on the Order model too:

class Order < ApplicationRecord
  def complete?
    items.not_completed.empty?
  end
end

Then in the code above, you can just write:

def treatment
  # ...
  order.update_attribute(:treated, "true") if order.complete?
  # ...
end

Also, I suspect that should really be true rather than "true". true is a boolean; "true" is a string. Use the correct data type for the job.

Rails may be automatically casting the value to a boolean on your behalf (?), but I would consider it bad practice to use the wrong data type, regardless.

the `order` appears to be what is being updated so might make more sense for `Order` to have a `complete?` method such that `items.not_completed.empty?` then the call back would just be `order.update_attribute(:treated, true) if order.complete?`. Also I believe rails handles the type casting for `true` and `"true"` without issue. — engineersmnky, Jul 16 '18 at 16:52
Yeah, true. I'll add that suggestion as an extension, thanks. — Tom Lord, Jul 16 '18 at 16:54
Hi @TomLord & engineersmnky, thanks a lot(!) for the solution. It's pretty elegant. The redondant use of Self was the most tricky problem form me (with the simple "="). Thanks again guys! — , Jul 17 '18 at 07:33

How to check if all records of an instance have one of many values from a list of values?

1 Answers1