ruby-on-railsrubyaasm

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


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


Solution

  • 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"
        # ...
    
    1. Redundant use of self
    2. Use of = is setting the value, rather than checking for equality. (This would be done with ==.)
    3. By looping through all items like that, you're updating the order multiple times.
    4. ...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.