From b4176746b48fb0fcfef514c4d750452ba7e869d6 Mon Sep 17 00:00:00 2001
From: James Smith <james@floppy.org.uk>
Date: Fri, 27 Sep 2024 21:15:29 +0100
Subject: [PATCH] avoid creating duplicate problems

resolves #2811
---
 app/models/problem.rb       | 10 +++++-----
 spec/models/problem_spec.rb | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/app/models/problem.rb b/app/models/problem.rb
index fdec49841..fe47052d4 100644
--- a/app/models/problem.rb
+++ b/app/models/problem.rb
@@ -53,13 +53,13 @@ class Problem < ApplicationRecord
     no_tags: :silent
   )
 
-  def self.create_or_clear(problematic, cat, present, options = {})
-    if present
-      problematic.problems.create(options.merge(category: cat))
+  def self.create_or_clear(problematic, category, should_exist, options = {})
+    if should_exist
+      problematic.problems.find_or_create_by(options.merge(category: category))
     else
-      problematic.problems.where(category: cat).destroy_all
+      problematic.problems.where(category: category).destroy_all
     end
-    present
+    should_exist
   end
 
   def self.ransackable_attributes(auth_object = nil)
diff --git a/spec/models/problem_spec.rb b/spec/models/problem_spec.rb
index b5608bda7..aa6a4b971 100644
--- a/spec/models/problem_spec.rb
+++ b/spec/models/problem_spec.rb
@@ -68,4 +68,36 @@
       expect(described_class.count).to eq(1)
     end
   end
+
+  context "when updating problem state" do
+    let(:model) { create(:model, license: nil) }
+
+    it "creates a problem that should exist but doesn't" do
+      expect {
+        described_class.create_or_clear model, :no_license, model.license.blank?
+      }.to change(described_class, :count).from(0).to(1)
+    end
+
+    it "removes a problem that shouldn't exist but does" do
+      described_class.create_or_clear model, :no_license, model.license.blank?
+      model.update!(license: "CC-BY-4.0")
+      expect {
+        described_class.create_or_clear model, :no_license, model.license.blank?
+      }.to change(described_class, :count).from(1).to(0)
+    end
+
+    it "does nothing with a problem that shouldn't exist and doesn't" do
+      model.update!(license: "CC-BY-4.0")
+      expect {
+        described_class.create_or_clear model, :no_license, model.license.blank?
+      }.not_to change(described_class, :count)
+    end
+
+    it "does nothing with a problem that should exist and does" do
+      described_class.create_or_clear model, :no_license, model.license.blank?
+      expect {
+        described_class.create_or_clear model, :no_license, model.license.blank?
+      }.not_to change(described_class, :count)
+    end
+  end
 end