ActiveRecord::RecordNotUniqueのエラーが出ないように修正 by su-su-su-su · Pull Request #8006 · fjordllc/bootcamp

su-su-su-su · 2024-08-10T11:18:48Z

2つPRをcloseしている理由なのですが、push前にgit pull --rebase origin mainしておらず、自分のコミットでないものが入ってしまいました。1度目はそこから修正しようとしましたが余計に分からなくなった為closeしました。2度目は解決方法が分かったので、closeしました。

Issue

ActiveRecord::RecordNotUnique: PG::UniqueViolation: ERROR: duplicate key value violates unique constraint "index_reports_on_user_id_and_reported_on" DETAIL: Key (user_id, reported_on)=(2018, 2024-06-24) already exists. #7888

概要

ActiveRecord::RecordNotUnique: PG::UniqueViolation: ERROR: duplicate key value violates unique constraint "index_reports_on_user_id_and_reported_on" DETAIL: Key (user_id, reported_on)=(2018, 2024-06-24) already exists.

とエラーが出るのを回避するための修正を行いました。

変更確認方法

ブランチ bug/fix-unique-violation-cleanup をローカルに取り込む
foreman start -f Procfile.devでローカル環境を立ち上げる
hajime でログイン
http://localhost:3000/reports/new にアクセスし、日報作成画面を2つのタブで表示
タイトルはそれぞれ別のタイトルにする。日付は同じにして同時に提出ボタンをクリック。
片方の日報は保存され、もう片方の日報はバリデーションが表示されることを確認。

変更前

重複が起こった際にActiveRecord::RecordNotUniqueのエラーが表示され、エラーハンドリングが行われていませんでした。

変更後

変更後は、バリデーションエラーが画面上に表示されるようになりました。

su-su-su-su · 2024-08-11T11:33:00Z

@reckyy
お疲れ様です。
可能でしたらお手隙の際に、こちらのPRのレビューをお願いできますでしょうか？
ご都合が合わない場合はおっしゃってください。
どうぞよろしくお願いいたします 🙇

reckyy

@su-su-su-su
お疲れ様です！
何点かコメントしていますので、ご確認よろしくお願いいたします 🙇

以下、コードとは関係ない部分です。

PRの最初のコメント上部に、不要な文字列があります。
PRを2つすでにcloseされていますが、何故closeしたかの理由を記載しておいたほうがいいと思います〜。
変更前後のコードも、写真ではなくコードブロックで記載していただきたかったです〜！
そちらのほうがみやすいので☺️

reckyy · 2024-08-12T03:17:12Z

app/models/report.rb

+
+  def save_with_uniqueness_check
+    ActiveRecord::Base.transaction do
+      if new_record? && Report.where(user_id:, reported_on:).exists?


[ask]このメソッドって、createアクションでしか呼ばれないので新しいレコードなのは自明だと思いますが、いかがでしょうか。👀

ご指摘ありがとうございます。
おっしゃる通り通りで不要だと感じたので削除いたしました。

reckyy · 2024-08-12T03:21:23Z

app/models/report.rb

  end
+
+  def save_with_uniqueness_check
+    ActiveRecord::Base.transaction do


[must]transactionは一連の操作を「全成功」か「全失敗」させることにより、矛盾を防ぐものです。
そのため、失敗した場合にはRollbackしないといけませんがこのメソッドではコミットされてしまっています。（内部で何も行なっていないのでDB上は違いはありませんが、期待される結果ではありません）
Rollbackされるように修正をお願いいたします！

元のコードを見るとRollbackされていないことを確認しました。
こちら

def save_with_uniqueness_check ActiveRecord::Base.transaction do if Report.where(user_id:, reported_on:).exists? errors.add(:reported_on, 'はすでに存在します') raise ActiveRecord::Rollback end save end end

と明示的にRollbackするように修正いたしました。

[must]rollbackされるのが同じ日の日報が存在してた時のみになっています〜。
存在しなかった場合、もし入力内容に不備があってもコミットされてしまっています！

ありがとうございます！
こちらも確認したところRollbackされていませんでした。

def save_with_uniqueness_check ActiveRecord::Base.transaction do if Report.where(user_id:, reported_on:).exists? errors.add(:reported_on, 'はすでに存在します') raise ActiveRecord::Rollback end raise ActiveRecord::Rollback unless save true end end

としてsaveされない場合はRollbackするように修正しました。

同じタイトルが存在した場合：

report2 = Report.new( user: user, reported_on: '2022-06-12', title: 'test55', description: 'test', emotion: 'happy' ) => #<Report:0x00007f0dcf0f4e50 ... if report2.save_with_uniqueness_check puts "レポート2: 保存" else puts "レポート2: エラー #{report2.errors.full_messages.join(', ')}" end TRANSACTION (0.6ms) BEGIN Report Exists? (1.5ms) SELECT 1 AS one FROM "reports" WHERE "reports"."user_id" = $1 AND "reports"."reported_on" = $2 LIMIT $3 [["user_id", 253826460], ["reported_on", "2022-06-12"], ["LIMIT", 1]] Report Exists? (1.0ms) SELECT 1 AS one FROM "reports" WHERE "reports"."title" = $1 AND "reports"."user_id" = $2 LIMIT $3 [["title", "test55"], ["user_id", 253826460], ["LIMIT", 1]] Report Exists? (0.9ms) SELECT 1 AS one FROM "reports" WHERE "reports"."reported_on" = $1 AND "reports"."user_id" = $2 LIMIT $3 [["reported_on", "2022-06-12"], ["user_id", 253826460], ["LIMIT", 1]] TRANSACTION (0.6ms) ROLLBACK レポート2: エラータイトルはすでに存在します => nil

また、

raise ActiveRecord::Rollback unless save

あとのtrueなのですが、true返さない形でもデータの保存自体は問題なく行われていました。しかし、以下のコードの実行結果では、保存が成功しているにもかかわらず、elseのレポート2: エラーが表示されてしまいました。

if report2.save_with_uniqueness_check puts "レポート2: 保存" else puts "レポート2: エラー #{report2.errors.full_messages.join(', ')}" end TRANSACTION (0.4ms) BEGIN Report Exists? (1.4ms) SELECT 1 AS one FROM "reports" WHERE "reports"."user_id" = $1 AND "reports"."reported_on" = $2 LIMIT $3 [["user_id", 253826460], ["reported_on", "2022-06-05"], ["LIMIT", 1]] Report Exists? (1.0ms) SELECT 1 AS one FROM "reports" WHERE "reports"."title" = $1 AND "reports"."user_id" = $2 LIMIT $3 [["title", "test123"], ["user_id", 253826460], ["LIMIT", 1]] Report Exists? (0.7ms) SELECT 1 AS one FROM "reports" WHERE "reports"."reported_on" = $1 AND "reports"."user_id" = $2 LIMIT $3 [["reported_on", "2022-06-05"], ["user_id", 253826460], ["LIMIT", 1]] Report Create (0.9ms) INSERT INTO "reports" ("user_id", "title", "description", "created_at", "updated_at", "reported_on", "emotion") VALUES ($1, $2, $3, $4, $5, $6, $7) RETURNING "id" [["user_id", 253826460], ["title", "test123"], ["description", "test"], ["created_at", "2024-08-19 02:44:09.909464"], ["updated_at", "2024-08-19 02:44:09.909464"], ["reported_on", "2022-06-05"], ["emotion", 2]] Watch Exists? (1.3ms) SELECT 1 AS one FROM "watches" WHERE "watches"."user_id" = $1 AND "watches"."watchable_id" = $2 AND "watches"."watchable_type" = $3 LIMIT $4 [["user_id", 253826460], ["watchable_id", 1066585839], ["watchable_type", "Report"], ["LIMIT", 1]] Watch Create (0.9ms) INSERT INTO "watches" ("watchable_type", "watchable_id", "created_at", "updated_at", "user_id") VALUES ($1, $2, $3, $4, $5) RETURNING "id" [["watchable_type", "Report"], ["watchable_id", 1066585839], ["created_at", "2024-08-19 02:44:09.938174"], ["updated_at", "2024-08-19 02:44:09.938174"], ["user_id", 253826460]] User Update (1.1ms) UPDATE "users" SET "updated_at" = $1 WHERE "users"."id" = $2 [["updated_at", "2024-08-19 02:44:09.940861"], ["id", 253826460]] TRANSACTION (15.1ms) COMMIT レポート2: エラー => nil

これを修正するために正常に保存が完了した場合にtrueを返すようにしました。

[ask]save!を使うのではなく、明示的にtrueを返すようにした理由は何かありますか？
トランザクションは例外をキャッチしたら自動でRollbackを行なってくれます。

Exceptions will force a ROLLBACK that returns the database to the state before the transaction began.
https://api.rubyonrails.org/classes/ActiveRecord/Transactions/ClassMethods.html

Suggested change

ActiveRecord::Base.transaction do

ActiveRecord::Base.transaction do

if Report.where(user_id:, reported_on:).exists?

errors.add(:reported_on, 'はすでに存在します')

raise ActiveRecord::Rollback

end

save!

end

@reckyy

お疲れ様です。
save_with_uniqueness_checkメソッドを以下のようにsave!に修正した際の挙動についてなのですが、

ActiveRecord::Base.transaction do if Report.where(user_id:, reported_on:).exists? errors.add(:reported_on, 'はすでに存在します') raise ActiveRecord::Rollback end save! end

app/controllers/reports_controller.rbのcreateメソッドが if @report.save_with_uniqueness_checkで条件分岐をしています。

def create @report = Report.new(report_params) @report.user = current_user set_wip canonicalize_learning_times(@report) if @report.save_with_uniqueness_check Newspaper.publish(:report_save, { report: @report }) redirect_to redirect_url(@report), notice: notice_message(@report), flash: flash_contents(@report) else render :new end end

この場合save_with_uniqueness_checkメソッドがtrueまたはfalseを返すことが期待されています。
しかし、save!を使って例外を発生させた場合、トランザクションはロールバックされますが、falseは返さないようです。
その結果、

if @report.save_with_uniqueness_check

の部分が処理することができず、エラーになってしまうことを確認しました。

そこで次にどう修正をしていくかなのですが、
save! を使わずに元のコードに戻す方が良いか。
引き続きsave!を使って他を修正する方が良いか。
どちらの方法が良いか、ご意見いただけますと幸いです。

@su-su-su-su
お疲れ様です。

createメソッドの分岐、失念していました。。申し訳ないです😭
となると、unless文でRollbackさせていたコードが良いかもです。
不適切な提案してしまい、申し訳ないです。

@reckyy
トランザクションでsave!を使う意味なども知れたのでむしろ勉強になりました🙇
ありがとうございます！
こちら再度コードを戻すように修正しました。
再度ご確認いただけますでしょうか。
よろしくお願いいたします。

ありがとうございます！
確認しました〜。

ありがとうございます🙇‍♂️

reckyy · 2024-08-12T03:26:01Z

test/models/report_test.rb

    assert_equal 10, reports(:report32).interval
  end
+
+  test '#reported_on_uniqueness_check' do


[must]テストの名前と実際のメソッド名が異なっています。

ご指摘ありがとうございます。
モデルのメソッド名に合わせて名前を統一しました。

reckyy · 2024-08-12T03:28:25Z

test/models/report_test.rb

+    report1.title = 'test1'
+    report1.description = 'test1'
+    report1.emotion = 'happy'
+    report1.save_with_uniqueness_check


[ask]ここで既存のfixturesを使用しなかった理由はありますか？👀

fixturesを分かっていなかったです。もし前に勉強していたら忘れてしまったいました。
fixturesを使うように修正いたしました。

test '#save_with_uniqueness_check' do report1 = reports(:report1) report1.save_with_uniqueness_check report2 = report1.dup report2.title = 'test2' report2.save_with_uniqueness_check assert_includes report2.errors.full_messages, '学習日はすでに存在します' end

[imo]以下のコードは必要でしょうか？
report1.save_with_uniqueness_check
report2.title = 'test2'

ご指摘ありがとうございます！
再度確認したところ、
report1.save_with_uniqueness_check
は既に保存しているデータなので不要でした。

またreport2.title = 'test2'はタイトルが同じだとそのバリデーションも実行されてしまうと思っていたのですが、save_with_uniqueness_checkメソッドで

if Report.where(user_id:, reported_on:).exists?

を先にしてからsaveしていたのでこちらも不要でした。

su-su-su-su · 2024-08-13T03:07:50Z

@reckyy
お疲れ様です！
ご確認、コメントありがとうございます。

トランザクションを使って失敗すると勝手にロールバックするものだと勘違いしていました。
ご指摘ありがとうございました！勉強になりました。

fixturesも使うことでコードが短くなり、こちらも勉強になりました！

コードと関係ない部分も合わせて修正いたしましたのでお手隙の際に再度ご確認をお願いします 🙇

reckyy

@su-su-su-su
遅くなり申し訳ございません！

2点コメントしていますので、ご確認よろしくお願いいたします。
個人的にお話ししたいのですが、Discordアカウントはお持ちではないですか？チャンネルが見当たらなかったので。

またPRをcloseした理由は、各PRに記載すべきだと思います。closeされたPRを見てから、本PRに辿り着くとは限らないからです。
（closeされた理由を書くというのは、自分が気にしすぎなのかもしれないのでご対応はお任せします。）

reckyy · 2024-08-17T12:45:49Z

app/models/report.rb

  end
+
+  def save_with_uniqueness_check
+    ActiveRecord::Base.transaction do


[must]rollbackされるのが同じ日の日報が存在してた時のみになっています〜。
存在しなかった場合、もし入力内容に不備があってもコミットされてしまっています！

reckyy · 2024-08-17T12:57:02Z

test/models/report_test.rb

+    report1.title = 'test1'
+    report1.description = 'test1'
+    report1.emotion = 'happy'
+    report1.save_with_uniqueness_check


[imo]以下のコードは必要でしょうか？
report1.save_with_uniqueness_check
report2.title = 'test2'

su-su-su-su · 2024-08-18T12:02:18Z

@reckyy
ご確認、コメントありがとうございます！

個人的にお話ししたいのですが、Discordアカウントはお持ちではないですか？チャンネルが見当たらなかったので。

すみません。確認したところチャンネルがなかったので作成しました！

またPRをcloseした理由は、各PRに記載すべきだと思います。closeされたPRを見てから、本PRに辿り着くとは限らないからです。

アドバイスありがとうございます。とても納得しました。closeしたPRにも理由を書きました。

コードの修正はまだ終えていないので改めてご連絡させていただきます。

su-su-su-su · 2024-08-19T02:56:58Z

@reckyy
お疲れ様です！
ご指摘頂いた点を修正いたしました。
お手隙の際にご確認いただけますと幸いです。
よろしくお願いいたします。

reckyy · 2024-08-19T03:51:00Z

@su-su-su-su
修正ありがとうございます〜！
一点だけ疑問をコメントしていますので、ご確認お願いいたします。

また、Discordでも昨日メッセージを送信しているため、ご返信お待ちしております。

reckyy

@su-su-su-su
お疲れ様です。
返信が遅くなり申し訳ないです。

私からはOKです🙆

su-su-su-su · 2024-08-26T02:25:10Z

@reckyy
お疲れ様です。
ご確認ありがとうございます！
レビュー頂いてありがとうございました🙇

su-su-su-su · 2024-08-26T02:25:57Z

@komagata
お疲れ様です。
approveいただきましたのでレビューをお願いできますでしょうか。
よろしくお願いいたします。

komagata · 2024-09-18T05:01:31Z

@su-su-su-su

まず、「変更確認方法」がおかしいように思います。
実際にActiveRecord::RecordNotUniqueが起きているのはどこのコードでしょうか？
原因がわかる前に、現状存在しないコードで確かめても意味がないように思います。

su-su-su-su · 2024-09-24T09:29:16Z

@komagata
アドバイスありがとうございます。

ActiveRecord::RecordNotUnique: PG::UniqueViolation: ERROR: duplicate key value violates unique constraint "index_reports_on_user_id_and_reported_on" DETAIL: Key (user_id, reported_on)=(2018, 2024-06-24) already exists.
の原因となるコードを確認しました。
まずは日報の存在を確認しました。

Report.where(user_id: 2018, reported_on: '2024-06-24')
Report Load (0.5ms) SELECT "reports".* FROM "reports" WHERE "reports"."user_id" = $1 AND "reports"."reported_on" = $2 [["user_id", 2018], ["reported_on", "2024-06-24"]]

次にこの日報と同じuser_id、同じ日付でエラーを再現しようとしました。

report1 = Report.new(
  user_id: 2018,
  reported_on: '2024-06-24',
  title: 'test1',
  description: 'test',
  emotion: 'happy'
)

begin
  report1.save!(validate: false)
  puts "レポート1: 保存"
rescue => e
  puts "レポート1: エラー #{e.message}"
end

すると以下の結果になりました。

TRANSACTION (0.6ms) BEGIN
Report Create (0.8ms) INSERT INTO "reports" ("user_id", "title", "description", "created_at", "updated_at", "reported_on", "emotion") VALUES ($1, $2, $3, $4, $5, $6, $7) RETURNING "id" [["user_id", 2018], ["title", "test1"], ["description", "test"], ["created_at", "2024-09-24 08:09:20.844418"], ["updated_at", "2024-09-24 08:09:20.844418"], ["reported_on", "2024-06-24"], ["emotion", 2]]
User Load (0.8ms) SELECT "users".* FROM "users" WHERE "users"."id" = $1 LIMIT $2 [["id", 2018], ["LIMIT", 1]]
Watch Exists? (0.7ms) SELECT 1 AS one FROM "watches" WHERE "watches"."user_id" IS NULL AND "watches"."watchable_id" = $1 AND "watches"."watchable_type" = $2 LIMIT $3 [["watchable_id", 1066585821], ["watchable_type", "Report"], ["LIMIT", 1]]
TRANSACTION (0.5ms) ROLLBACK
レポート1: エラー バリデーションに失敗しました: Userを入力してください, Userを入力してください

その後User.pluck(:id, :name)で確認したところuser_id: 2018というユーザーは存在していませんでした。

退会したかも調べたのですが、

user = User.find_by(id: 2018)
if user&.retired_on
  puts "#{user.name}."
else
  puts "ユーザーはいません"
end

ユーザーはいませんとなり、こちらもuser_id: 2018というユーザーは存在していませんでした。

ご指摘頂いた「原因がわかる前に、現状存在しないコードで確かめても意味がない」というのは、このuser_id: 2018が存在しない状態を確認せずで進めてしまったことを指しているのでしょうか？
他のイシューでも質問していて恐縮なのですが、アドバイス頂けますと幸いです。
よろしくお願いいたします。

komagata · 2024-09-30T07:57:34Z

@su-su-su-su

ご指摘頂いた「原因がわかる前に、現状存在しないコードで確かめても意味がない」というのは、このuser_id: 2018が存在しない状態を確認せずで進めてしまったことを指しているのでしょうか？

いいえです。

report1 = Report.new(
  user_id: 2018,
  reported_on: '2024-06-24',
  title: 'test1',
  description: 'test',
  emotion: 'happy'
)

begin
  report1.save!(validate: false)
  puts "レポート1: 保存"
rescue => e
  puts "レポート1: エラー #{e.message}"
end

このコードが現状のbootcampのコードに存在しないとしたら、それで確かめるのは意味がないということになります。
（特にsave!メソッドを使っている箇所、validate: falseを使っている箇所）

su-su-su-su · 2024-10-03T08:16:57Z

@komagata
お疲れ様です。コメントありがとうございます。

#7888 にて
View details in Rollbar: https://app.rollbar.com/a/komagata/fix/item/Bootcamp/1690
と表示されていたので、Rollbarというものでエラーの詳細を確認しようとしました。しかし、ActiveRecord::RecordNotUniqueエラーに関する情報が表示されず、別のエラーが表示されています。

そのため、詳細を確認することが難しい状況です。
ActiveRecord::RecordNotUniqueエラーの詳細を確認したいのですが、どのようにすればよいかアドバイスをいただけますでしょうか？
これを見ていなかったせいなのか「変更確認方法」がずれていたのかもしれないです。
よろしくお願いいたします。

komagata · 2024-10-03T09:12:39Z

@su-su-su-su

該当のRollbarのデータは下記ですが、どこのコードというのは残ってないのでコードからどこで起きているのか少し考えて探してみてください〜

https://app.rollbar.com/a/komagata/fix/item/Bootcamp/1690

su-su-su-su · 2024-10-03T10:14:00Z

@komagata

返信ありがとうございます。

Rollbarのリンクhttps://app.rollbar.com/a/komagata/fix/item/Bootcamp/1690をにアクセスしましたが、ActiveRecord::RecordNotUniqueエラーに関する情報が表示されず、先ほどと変わらず別のエラーが表示されています。

これはRollbarへのアクセス権限に問題があるのでしょうか？

それとも

どこのコードというのは残ってないので

ということから、この挙動で問題ないのでしょうか。

komagata · 2024-10-13T21:59:19Z

@su-su-su-su

Rollbarのリンクhttps://app.rollbar.com/a/komagata/fix/item/Bootcamp/1690をにアクセスしましたが、ActiveRecord::RecordNotUniqueエラーに関する情報が表示されず、先ほどと変わらず別のエラーが表示されています。

こちらはこの情報しか残ってないので気にしないでください。

su-su-su-su · 2025-01-07T13:52:22Z

@komagata

お疲れ様です。
save!メソッド、validate: falseをしないでエラーを起こすように「変更確認方法」を修正しました。

お手すきの際にご確認お願い致します。

user = User.find_by(name: 'Hajime Tayo')

threads = []

2.times do |i|
  threads << Thread.new do
    ActiveRecord::Base.connection_pool.with_connection do
      report = Report.new(
        user: user,
        reported_on: '2024-06-24',
        title: "title#{i+1}",
        description: 'test',
        emotion: 'happy'
      )
      begin
        if report.save
          puts "レポート#{i+1}: 保存成功"
        else
          puts "レポート#{i+1}: 保存失敗 - #{report.errors.full_messages.join(', ')}"
        end
      rescue ActiveRecord::RecordNotUnique => e
        puts "レポート#{i+1}: エラー #{e.message}"
      rescue => e
        puts "レポート#{i+1}: その他のエラー #{e.message}"
      end
    end
  end
end

以下が実行結果です：

TRANSACTION (4.2ms)  BEGIN
  TRANSACTION (1.0ms)  BEGIN
    Report Exists? (0.9ms)  SELECT 1 AS one FROM "reports" WHERE "reports"."title" = $1 AND "reports"."user_id" = $2 LIMIT $3  [["title", "title1"], ["user_id", 253826460], ["LIMIT", 1]]
    Report Exists? (1.0ms)  SELECT 1 AS one FROM "reports" WHERE "reports"."title" = $1 AND "reports"."user_id" = $2 LIMIT $3  [["title", "title2"], ["user_id", 253826460], ["LIMIT", 1]]
    Report Exists? (2.8ms)  SELECT 1 AS one FROM "reports" WHERE "reports"."reported_on" = $1 AND "reports"."user_id" = $2 LIMIT $3  [["reported_on", "2024-06-24"], ["user_id", 253826460], ["LIMIT", 1]]
    Report Exists? (0.9ms)  SELECT 1 AS one FROM "reports" WHERE "reports"."reported_on" = $1 AND "reports"."user_id" = $2 LIMIT $3  [["reported_on", "2024-06-24"], ["user_id", 253826460], ["LIMIT", 1]]
    Report Create (3.0ms)  INSERT INTO "reports" ("user_id", "title", "description", "created_at", "updated_at", "reported_on", "emotion") VALUES ($1, $2, $3, $4, $5, $6, $7) RETURNING "id"  [["user_id", 253826460], ["title", "title1"], ["description", "test"], ["created_at", "2025-01-07 13:44:40.403909"], ["updated_at", "2025-01-07 13:44:40.403909"], ["reported_on", "2024-06-24"], ["emotion", 2]]
    Watch Exists? (1.4ms)  SELECT 1 AS one FROM "watches" WHERE "watches"."user_id" = $1 AND "watches"."watchable_id" = $2 AND "watches"."watchable_type" = $3 LIMIT $4  [["user_id", 253826460], ["watchable_id", 1066585859], ["watchable_type", "Report"], ["LIMIT", 1]]
    Watch Create (1.1ms)  INSERT INTO "watches" ("watchable_type", "watchable_id", "created_at", "updated_at", "user_id") VALUES ($1, $2, $3, $4, $5) RETURNING "id"  [["watchable_type", "Report"], ["watchable_id", 1066585859], ["created_at", "2025-01-07 13:44:40.416216"], ["updated_at", "2025-01-07 13:44:40.416216"], ["user_id", 253826460]]
    User Update (3.7ms)  UPDATE "users" SET "updated_at" = $1 WHERE "users"."id" = $2  [["updated_at", "2025-01-07 13:44:40.420204"], ["id", 253826460]]
  TRANSACTION (3.9ms)  COMMIT
  Report Create (23.2ms)  INSERT INTO "reports" ("user_id", "title", "description", "created_at", "updated_at", "reported_on", "emotion") VALUES ($1, $2, $3, $4, $5, $6, $7) RETURNING "id"  [["user_id", 253826460], ["title", "title2"], ["description", "test"], ["created_at", "2025-01-07 13:44:40.407142"], ["updated_at", "2025-01-07 13:44:40.407142"], ["reported_on", "2024-06-24"], ["emotion", 2]]
レポート1: 保存成功
  TRANSACTION (0.4ms)  ROLLBACK
レポート2: エラー PG::UniqueViolation: ERROR:  duplicate key value violates unique constraint "index_reports_on_user_id_and_reported_on"
DETAIL:  Key (user_id, reported_on)=(253826460, 2024-06-24) already exists.

komagata · 2025-01-20T05:56:35Z

@su-su-su-su

save!メソッド、validate: falseをしないでエラーを起こすように「変更確認方法」を修正しました。

すみません、僕の説明の仕方が悪かったです。
お互いに基本的な認識が間違っているかもです。

「変更の確認方法」というのはbootcampのアプリを使って、「こういうアクセス方法ではエラーが出ていたのが、今回のPRで修正したのでエラーが出なくなりました」

というのをレビューワーに確認してもらうものです。

bootcampと関係無いコードを「書いてエラーが出ました」というのは意味がない感じです。

具体的に「変更方法の例」を書くと、

ブラウザを2個開いて、同時に「日報投稿ボタン」を押すと〜のエラーが出ます。
このPRではそれが修正されています。

みたいな感じになるはずです。

su-su-su-su · 2025-01-27T04:07:30Z

@komagata

ご説明ありがとうございます。
おっしゃる通り認識が間違っておりました。
ご提示して頂いた変更方法に従い、ブラウザを二つ開いてほぼ同時に投稿ボタンをクリックしたところ、
ActiveRecord::RecordNotUniqueエラーを発生させることができました。
その上で、コードを修正いたしましたので、ご確認をお願いいたします。

komagata · 2025-01-28T03:01:21Z

app/controllers/reports_controller.rb

+        render :new
+      end
+    rescue ActiveRecord::RecordNotUnique
+      @report.errors.add(:reported_on, 'はすでに存在します')


@su-su-su-su こういうValidationはReportクラス自体が持つものになっています。
Railsガイドなどでエラー処理・バリデーションについて基本的な部分から調べてみると良いかもです。

su-su-su-su · 2025-02-18T04:24:45Z

@komagata
ご指摘ありがとうございます。
モデル側にバリデーションやビジネスロジックを定義し、コントローラではその結果に基づいて分岐処理をするように修正しました。
お手すきの際にご確認いただけますと幸いです。

komagata · 2025-02-21T05:31:17Z

app/models/report.rb


+  def save_with_uniqueness_handling(*args, &block)
+    save(*args, &block)
+  rescue ActiveRecord::RecordNotUnique


Validationで回避できると思うので、そもそもこのエラーが発生しないようにできると思います。

ご指摘ありがとうございます。

def save_with_lock Report.transaction do if user.reports.lock.find_by(reported_on:) valid? false else save end end end

と修正しました。

su-su-su-su · 2025-02-22T16:10:56Z

@komagata
レビューありがとうございます。
再度修正いたしました。
お手隙の際にご確認お願いします。

komagata · 2025-03-08T19:10:33Z

@su-su-su-su こんな感じはいかがでしょうか。
3da67d1

su-su-su-su · 2025-03-29T02:51:56Z

@komagata
修正案ありがとうございました。
テストも修正いたしました。
お手隙の際に再度ご確認お願いいたします。

komagata

確認させて頂きました。OKです〜🙆‍♂️

su-su-su-su requested a review from reckyy August 11, 2024 11:33

su-su-su-su self-assigned this Aug 11, 2024

reckyy reviewed Aug 12, 2024

View reviewed changes

reckyy reviewed Aug 17, 2024

View reviewed changes

reckyy approved these changes Aug 23, 2024

View reviewed changes

su-su-su-su force-pushed the bug/fix-unique-violation-cleanup branch from 7a4f02f to dddc7b0 Compare January 22, 2025 15:26

komagata reviewed Jan 28, 2025

View reviewed changes

su-su-su-su force-pushed the bug/fix-unique-violation-cleanup branch from dddc7b0 to 5a44281 Compare February 13, 2025 01:07

komagata reviewed Feb 21, 2025

View reviewed changes

su-su-su-su force-pushed the bug/fix-unique-violation-cleanup branch from e9f842c to 105e04e Compare February 22, 2025 13:26

su-su-su-su force-pushed the bug/fix-unique-violation-cleanup branch from 3da67d1 to 6f30682 Compare March 25, 2025 15:20

su-su-su-su and others added 17 commits March 27, 2025 22:24

ActiveRecord::RecordNotUniqueのエラーが出ないように修正

be162b1

テストのメソッド名を修正

576d50e

モデルのメソッド名とテストのメソッド名を統一

684380f

fixturesを使用したテストに修正

949cf30

テストに不要なコードを削除

3255568

不要なコードを削除

eece825

日報の保存時に重複エラーを処理する例外ハンドリングを追加

b6c17e7

ReportsControllerのshowメソッドに@recent_reportsを追加

89bd89d

コントローラの例外処理を削除し、save_with_uniqueness_handlingを作成してユニーク違反をモデル側に集約

f1faf6b

全レポートからnot_wipカウントに戻す

93a4028

Newspaper.publish呼び出しを復元

82be01b

reportの保存処理を修正

bad4e03

save_with_lockの重複登録防止テストを追加

02c5b15

２ブラウザで同時に日報保存した時にActiveRecord::RecordNotUniqueが出るのを修正

fcb341f

save_uniquelyメソッドに合わせてテストを修正

676eb94

既存のエラーメッセージに合わせた

69d2d04

コンフリクトで誤って削除したコードを復元

3346e70

su-su-su-su force-pushed the bug/fix-unique-violation-cleanup branch from 042fdf3 to 3346e70 Compare March 27, 2025 13:48

komagata approved these changes Apr 2, 2025

View reviewed changes

komagata merged commit 6401a39 into main Apr 2, 2025
2 checks passed

komagata deleted the bug/fix-unique-violation-cleanup branch April 2, 2025 05:05

github-actions bot mentioned this pull request Apr 2, 2025

Release 2025-04-02 13:22:58 +0000 #8478

Merged

15 tasks

This was referenced May 8, 2025

ActiveRecord::RecordNotUniqueのエラーが出ないように修正 #8617

Closed

ActiveRecord::RecordNotUniqueのエラーが出ないように修正 #8618

Merged

komagata mentioned this pull request Jun 19, 2025

ActiveRecord::RecordNotUnique: PG::UniqueViolation: ERROR: duplicate key value violates unique constraint "index_footprintable" DETAIL: Key (user_id, footprintable_id, footprintable_type)=(1466, 1134, Announcement) already exists. #8517

Closed

Conversation

su-su-su-su commented Aug 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

概要

変更確認方法

変更前

変更後

Uh oh!

su-su-su-su commented Aug 11, 2024

Uh oh!

reckyy left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reckyy Aug 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reckyy Aug 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

su-su-su-su Aug 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

su-su-su-su commented Aug 13, 2024

Uh oh!

reckyy left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

su-su-su-su commented Aug 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

su-su-su-su commented Aug 19, 2024

Uh oh!

reckyy commented Aug 19, 2024

Uh oh!

reckyy left a comment

Choose a reason for hiding this comment

Uh oh!

su-su-su-su commented Aug 26, 2024

Uh oh!

su-su-su-su commented Aug 26, 2024

su-su-su-su commented Aug 10, 2024 •

edited

Loading

reckyy left a comment •

edited

Loading

reckyy Aug 19, 2024 •

edited

Loading

reckyy Aug 12, 2024 •

edited

Loading

su-su-su-su Aug 19, 2024 •

edited

Loading

reckyy left a comment •

edited

Loading

su-su-su-su commented Aug 18, 2024 •

edited

Loading

komagata commented Jan 20, 2025 •

edited

Loading

komagata Jan 28, 2025 •

edited

Loading