【Rails API】パラメーターから受け取った値とDBの値を足して更新をしたい。update, params

Question

class UserController < ApplicationController

  def update
    user = User.find(params[:id])
    if user.present?
      user.update(update_params)
      render status: 200
    else
      render status: 400
    end
  end

  private
  
  def update_params
    params.permit(:money)
  end

end

例えば上記の簡単なコードがあるとします (認証,認可,トランザクション等は省いています。)
パラメーターはidとmoneyがokuraremasu.またuserテーブルにはmoneyカラムがあるとします。

やりたいこと

結果的に user.money + params[:money] のようにDBの値を更新したいです。
単純にストロングパラメーターを使わなければupdateの引数で user.money + params[:money] とすればいけそうですが、

この場合のベストプラクティスとしてどうするべきか時間が結構経ってしまったので質問させていただきます。

宜しくお願い致します。

Answer 1

結果的に user.money + params[:money] のようにDBの値を更新したいです。

という事であれば、単純にincrementすればいいのではないでしょうか。

# そもそもfindはレコードがないと例外が発生するので、
# user.present?は実行不可能。
# やるなら、user = User.find_by(id: params[:id])とするべき
user = User.find(params[:id])

# 行ロックをかけないと同時アクセス時に値が保証できないので
# 行ロックしたほうが良い
user.with_lock do
  user.increment!(:money, params[:money])
end

Comments

[rzz]

mingosさん ご回答ありがとうございました。
find_byはpresent?が出来ていたのですが、findでは該当しないidを送ってみても、例外が発生していたので何でだろうって思ってました。
また行ロック、incrementメソッドも知りませんでした。
本当に勉強になります。

1つ質問があるのですが、
こちらの場合にストロングパラメーターを設定する必要はないのでしょうか。

この様なコードは見たことないので違うのかもしれませんが、考えてみました。

class UserController < ApplicationController

before_action : update_params

user = User.find(params[:id])

ActiveRecord::Base::transaction do
user.with_lock do
user.increment!(:money, params[:money])
end
end

private

def update_params
params.permit(:money)
end

end

※ スマホから投稿のためインデントがずれていたら申し訳ないです。

[mingos]

ストロングパラメータは使ったほうが良いですけど、使わなくても動きます。
しかもこの例の、update_params内のparams.permit(:money)ってストロングパラメータの仕事をしてないに等しいですよね。

単にparams[:money]を取り出して、{money: 値}というHashを返しているだけです。
この場合、params[:money]を直接使っているのと変わりません。

自分ならparams[:money]が空だったらエラーにして
処理が先に進まないようにしたいのでこうします。

def update_params
{
money: params.require(:money)
}
end

この場合、params[:money]がnilだったら例外を出します。
クライアントは必ずmoneyを送ってくるはずなので、送らない場合はエラーにしたほうがいいという判断です。中途半端に先に処理が進まないほうが安全だと考えています。

[rzz]

mingosさんご丁寧にありがとうございます！　遅くなりすみません。