すべてtrueだったらupdateするif文について

Question

前提・実現したいこと

ある条件でループを回して、全ての条件でレコードを取得することができたら
そのレコードのカラムをアップデート。できなかかったら、バリでショーンエラーを返す書き方を教えていただきたいです。

該当のソースコード・試したこと

やりたいことを伝えるための雰囲気でのコードになりますが、以下のようなイメージです。

def Workflow < ApplicationRecord

 before_save -> { update_accident_progress('payment') }

 def update_progress(status)
    return if number.blank? # numberがなければリターン

    wf = Workflow.where(number: number)
    wf.each do |w|
      as = []
      as = Status.find_by( compensation: wf.compensation, progress: status) #statusは引数で受け取っています。
    end

    if as.all?
     wf.update_all(progress: status) # 1つでもasにfalseがあれば、update_allは実行したくありません。
    else
      errors[:base] << 'ダメです。'
    end
 end
end

補足情報（FW/ツールのバージョンなど）

・Rails 5.1.7
・Ruby 2.6.3

追記

StatusはCSVファイルをnewしてモデルのように扱っています。
class Status
  attr_accessor :compensation :progress, :status

  def self.find_by(compensation:, progress:)
    STATUS_MANAGER.find_by(compensation: compensation, progress: progress)
  end

  def self.statuses
    STATUS_MANAGER.statuses
  end
end

追記2

module ProgressUpdateble
  extend ActiveSupport::Concern

  included do
    after_create -> { update_progress('payment') }
    validate :check_combination_status
  end

  def update_progress(status)
    return if number.blank?
    w = Workflow.where(approval_status: 'pending', number: number)
    w.update_all(progress: status)
  end

  def check_combination_status
    return if number.blank?
    wf = Workflow.where(number: number)
    unless wf.all? { |w| Status.find_by(compensation: w.compensation progress: progress).present? }
      errors[:base] << 'エラーメッセージ'
    end
  end
end

Answer 1

wf = Workflow.where(number: number) なもの全てが
Status.find_by( compensation: wf.compensation, progress: status)
であったら
wf.update_all(progress: status)
一つでも違うものが有ったらエラーですね？

wf = Workflow.where(number: number)
if wf.all?{|w| 
  Status.find_by( compensation: w.compensation, progress: status) 
} 
  
  wf.update_all(progress: status)
else
  ここでエラーであることを設定
end

で行けますが、問題は エラーをどのobjectにもたせるか、です。
errors.add は 配列、ないし Relationではもてず、そのインスタンスでないとだめなので。
errors という配列にもたせて返す、ということになるでしょうか

Comments

[ssk]

ご回答ありがとうございます。
追記2でvalidateとbefore_saveで役割を分けてみました。
こちらの方法で問題ないでしょうか？

[ssk]

＞一つでも違うものが有ったらエラーですね？
仰る通りです。

[winterboum]

追記２はちょっと論旨がおかしい
before_save はinstanceをsaveする前に行うことなのに、そこで多くの者のupdateが行われる。
雰囲気でなく、きちんと書いてみたほうが良かもです。
定義されていない変数を使っているので、そこだけでエラーになってしまうし。
number とか error　とか

[ssk]

ご返信ありがとうございます。
一部、追記2を修正いたしました。

更新がないので、コールバックをafter_createに変更しました。

＞雰囲気でなく、きちんと書いてみたほうが良かもです。
雰囲気ではなく、しっかりと書いたつもりでした。

＞定義されていない変数
自身がもっているものなので、numberでとることができます。

[winterboum]

混乱してきた、、、
関係するモデルは何でしょう。
moduleは何にinclude？extend?されるものでしょう。
何をトリガーに wfのチェックを行うのでしょう

[ssk]

＞moduleは何にinclude？extend?されるものでしょう。
モデルにincludeしております。

＞何をトリガーに wfのチェックを行うのでしょう
モデルの値になります。

Answer 2

module ProgressUpdateble
  extend ActiveSupport::Concern

  included do
    after_save -> { update_progress('payment') }, if: proc { self.number.present? }
    validate :check_combination_status, if: proc { self.number.present? }
  end

  def update_progress(status)
    work_flows = Workflow.where(approval_status: 'pending', number: number)
    wf.update_all progress: status
  end

  def check_combination_status
    wf = Workflow.where number: number
    return if wf.all? { |w| Status.find_by(compensation: w.compensation progress: progress).present? }
 
    errors[:base] << 'エラーメッセージ'
  end
end

before_saveよりafter_saveの方が良さそう。
うーん、なんかもっといい方法がありそうな気もする。

after_saveのif:の書き方か場所が違うかも...。
after_save -> { ... }の書き方がよくわからなかった、ごめんよ。

update_allはコールバックをスキップするのを意識しておくといいかも。
もし修正するときにsaveの無限ループに陥らないためにも。

Comments

[ssk]

ご回答いただき、ありがとうございます。情報不足で申し訳ございません。
Statusはモデルではないため、以下のエラーになってしまいました。
（StatusはCSVファイルをnewしてモデルのように扱っています）

undefined method `exists?' for AccidentStatus:Class

[rhiroe]

Status.find_by(...).present? にするか Status クラスにメソッドを追加するかで対応できそうです。

[ssk]

ありがとうございます。動作確認まですることができました。

また、ログは吐いていますが、バリデーションエラーを発生させることができませんでした。
before_saveだとバリデーションエラーを発生させることはできませんか？

ログ出力
logger.error 'Unable to update :progress to the entered status.'

[rhiroe]

before_saveではなくupdate_allがバリデーションチェックを行いません。
なので明示的にActiveRecord::RecordInvalidを発生させています。
バリデーションチェックがしたいのであればループを回して1つずつupdateすれば良いです。

[rhiroe]

あぁ、でもbefore_saveではなくbefore_validationでやるべきな気もしますね。

[rhiroe]

before_save は バリデーションが通った後の save 直前にコールバックされるものなのでそういう意味ではバリデーションエラーを発生させることができないともなるのかもしれないです。

[ssk]

ありがとうございます。
before_validationにした場合、validation前にupdate_allされる可能性があるのはまずいですね。

[ssk]

となると、before_saveとvalidateを分けた方がいいということでしょうか？

[rhiroe]

validate :validate_method
でバリデーションを用意しておくのがいいかもしれないですね。
回答編集してみます。

[ssk]

ありがとうございます。他でも使い回すので、concern化して追記2に書いてみました。
どうしょうか。。

[rhiroe]

いつの間にかafter_createになっていた。
新規作成の時しか呼ばれないけど大丈夫ですか？

[ssk]

ありがとうございます。
更新がないものなので、新規作成のみで大丈夫です。
（あとからの変更で申し訳ないです。）

[rhiroe]

うまく動くことを願ってます。

[rhiroe]

if と unless 逆に書いてました危ない危ない。修正しました。

[ssk]

修正、ありがとうございます。