From af8848bbb078a279a0ddba85902c092e780efeb3 Mon Sep 17 00:00:00 2001
From: Elliot Crosby-McCullough <elliot.cm@gmail.com>
Date: Tue, 5 Mar 2019 11:11:57 +0000
Subject: [PATCH] Fix a bug where specialist sectors wouldn't stick

Because this code was using `defined?` to check for the presence of the `specialist_sectors` method on the edition, and the edition is wrapped in a `LocalisedModel` which technically doesn't have that method defined, the code was replacing existing specialist sectors with recommended ones.

Using `respond_to?` provides the correct behaviour since it accounts for `method_missing` in `LocalisedModel` and therefore uses duck typing to figure out whether it's safe to call `specialist_sectors` on the object.
---
 .../taxons_to_legacy_associations_tagging.rb  |  2 +-
 ...ons_to_legacy_associations_tagging_test.rb | 56 +++++++++++++++----
 2 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/app/services/taxons_to_legacy_associations_tagging.rb b/app/services/taxons_to_legacy_associations_tagging.rb
index 20d2a687b1f4..f4b9178532e6 100644
--- a/app/services/taxons_to_legacy_associations_tagging.rb
+++ b/app/services/taxons_to_legacy_associations_tagging.rb
@@ -49,6 +49,6 @@ def legacy_mapping_for_taxons(selected_taxons)
   end
 
   def tagged_to_specialist_sectors?(edition)
-    defined?(edition.specialist_sectors) && edition.specialist_sectors.any?
+    edition.respond_to?(:specialist_sectors) && edition.specialist_sectors.any?
   end
 end
diff --git a/test/unit/services/taxons_to_legacy_associations_tagging_test.rb b/test/unit/services/taxons_to_legacy_associations_tagging_test.rb
index 2d63d6ca4f47..7b9e6240e683 100644
--- a/test/unit/services/taxons_to_legacy_associations_tagging_test.rb
+++ b/test/unit/services/taxons_to_legacy_associations_tagging_test.rb
@@ -1,28 +1,62 @@
 require 'test_helper'
 
 class TaxonsToLegacyAssociationsTaggingTest < ActiveSupport::TestCase
-  test "handles specialist sectors" do
-    specialist_sector_content_id = SecureRandom.uuid
-    taxon = taxon_mapped_to_specialist_sector(
-      specialist_sector_content_id
+  setup do
+    @specialist_sector_content_id = SecureRandom.uuid
+    @taxon = taxon_mapped_to_specialist_sector(
+      @specialist_sector_content_id
     )
 
     Taxonomy::TopicTaxonomy
       .any_instance
       .stubs(:all_taxons)
-      .returns([taxon])
+      .returns([@taxon])
+
+    @edition = create(:publication)
+    @edition.topics.delete_all
+  end
+
+  test "handles specialist sectors" do
+    TaxonsToLegacyAssociationsTagging.new.call(
+      edition: @edition,
+      user: create(:user),
+      selected_taxons: [@taxon.content_id]
+    )
+
+    assert @edition.specialist_sectors.count, 1
+    assert @edition.specialist_sectors.first.topic_content_id, @specialist_sector_content_id
+  end
 
-    edition = create(:publication)
-    edition.topics.delete_all
+  test "it doesn't set specialist sectors if they're already set" do
+    specialist_sector = FactoryBot.create(:specialist_sector,
+      edition: @edition,
+      topic_content_id: SecureRandom.uuid,
+    )
+
+    TaxonsToLegacyAssociationsTagging.new.call(
+      edition: @edition,
+      user: create(:user),
+      selected_taxons: [@taxon.content_id]
+    )
+
+    assert_equal(1, @edition.specialist_sectors.count)
+    assert_equal(specialist_sector, @edition.specialist_sectors.first)
+  end
+
+  test "it doesn't set specialist sectors if they're already set and the edition is wrapped in a LocalisedModel" do
+    specialist_sector = FactoryBot.create(:specialist_sector,
+      edition: @edition,
+      topic_content_id: SecureRandom.uuid,
+    )
 
     TaxonsToLegacyAssociationsTagging.new.call(
-      edition: edition,
+      edition: LocalisedModel.new(@edition, @edition.primary_locale),
       user: create(:user),
-      selected_taxons: [taxon.content_id]
+      selected_taxons: [@taxon.content_id]
     )
 
-    assert edition.specialist_sectors.count, 1
-    assert edition.specialist_sectors.first.topic_content_id, specialist_sector_content_id
+    assert_equal(1, @edition.specialist_sectors.count)
+    assert_equal(specialist_sector, @edition.specialist_sectors.first)
   end
 
   def taxon_mapped_to_specialist_sector(specialist_sector_content_id)