Activity import: prevent duplicates

Pipe-delimited values were allows to have duplicate values on import,
meaning for example that countries would appear twice in an activity's
details, e.g. "Brazil, Egypt and Egypt".

This was the only visible duplication on the surface; however, it
applied to any pipe-delimited field on import and they were stored with
dupes (even though some views elided that duplication).

Update the converter in all the places required. There *could* have
been a method for this, but for convert_country_partner_organisations
that requires a `strip`. Kept the duplication. Ironically.

app/services/activity/import/converter.rb

@@ -117,7 +117,7 @@ def convert_gcrf_challenge_area(gcrf_challenge_area)
       end
 
       def convert_gcrf_strategic_area(gcrf_strategic_area)
-        gcrf_strategic_area.split("|").map do |code|
+        gcrf_strategic_area.split("|").uniq.map do |code|
           valid_codes = gcrf_strategic_area_options.map { |area| area.code.to_s }
           raise I18n.t("importer.errors.activity.invalid_gcrf_strategic_area") unless valid_codes.include?(code)
           code
@@ -134,7 +134,7 @@ def convert_sustainable_development_goal(goal)
       alias_method :convert_sdg_3, :convert_sustainable_development_goal
 
       def convert_benefitting_countries(benefitting_countries)
-        benefitting_countries.split("|").map do |code|
+        benefitting_countries.split("|").uniq.map do |code|
           validate_country(
             code,
             I18n.t("importer.errors.activity.invalid_benefitting_countries")
@@ -248,14 +248,14 @@ def convert_actual_end_date(actual_end_date)
       end
 
       def convert_country_partner_organisations(partner_orgs)
-        partner_orgs.split("|").map(&:strip).reject(&:blank?)
+        partner_orgs.split("|").map(&:strip).uniq.reject(&:blank?)
       end
 
       def convert_ispf_themes(ispf_themes)
         return [] if ispf_themes.blank?
 
         valid_codes = ispf_themes_options.map { |theme| theme.code.to_s }
-        ispf_themes.split("|").map do |ispf_theme|
+        ispf_themes.split("|").uniq.map do |ispf_theme|
           raise I18n.t("importer.errors.activity.invalid_ispf_themes", code: ispf_theme) unless valid_codes.include?(ispf_theme)
 
           Integer(ispf_theme)
@@ -265,7 +265,7 @@ def convert_ispf_themes(ispf_themes)
       def convert_ispf_oda_partner_countries(ispf_oda_partner_countries)
         valid_codes = ispf_partner_countries_options(oda: true).map { |country| country.code.to_s }
 
-        ispf_oda_partner_countries.split("|").map do |code|
+        ispf_oda_partner_countries.split("|").uniq.map do |code|
           unless valid_codes.include?(code)
             raise I18n.t("importer.errors.activity.invalid_ispf_oda_partner_countries", code: code)
           end
@@ -277,7 +277,7 @@ def convert_ispf_oda_partner_countries(ispf_oda_partner_countries)
       def convert_ispf_non_oda_partner_countries(ispf_non_oda_partner_countries)
         valid_codes = ispf_partner_countries_options(oda: false).map { |country| country.code.to_s }
 
-        ispf_non_oda_partner_countries.split("|").map do |code|
+        ispf_non_oda_partner_countries.split("|").uniq.map do |code|
           unless valid_codes.include?(code)
             raise I18n.t("importer.errors.activity.invalid_ispf_non_oda_partner_countries", code: code)
           end
@@ -304,7 +304,7 @@ def convert_tags(tags)
         return [] if tags.blank?
 
         valid_codes = tags_options.map { |tag| tag.code.to_s }
-        tags.split("|").map do |tag|
+        tags.split("|").uniq.map do |tag|
           raise I18n.t("importer.errors.activity.invalid_tags", code: tag) unless valid_codes.include?(tag)
 
           Integer(tag)

spec/features/users_can_upload_activities_spec.rb

@@ -401,6 +401,42 @@
       expect(page).to have_content("Original commitment figure £1,000.00")
     end
 
+    scenario "uploading a valid set of activities with duplicates in pipe fields" do
+      old_count = Activity.count
+
+      # When I upload a valid Activity CSV with duplicates in pipe-delimited fields
+      within ".upload-form--ispf-oda" do
+        attach_file "report[activity_csv_ispf_oda]",
+          File.new("spec/fixtures/csv/valid_ispf_oda_activities_upload_with_duplicate_pipe_items.csv").path
+        click_button t("action.activity.upload.button")
+      end
+
+      expect(Activity.count - old_count).to eq(1)
+      # Then I should see confirmation that I have uploaded a new activity
+      expect(page).to have_text(t("action.activity.upload.success"))
+      expect(page).to have_table(t("table.caption.activity.new_activities"))
+
+      within "//tbody/tr[1]" do
+        expect(page).to have_xpath("td[2]", text: "A title")
+      end
+
+      activity_link = within("tbody") { page.find(:css, "a")["href"] }
+
+      # When I visit an activity with a comment
+      visit activity_link
+
+      # When I visit the details page
+      click_on "Details"
+
+      # Then I should see no duplicated countries
+      aggregate_failures do
+        expect(page).not_to have_content("Brazil, Egypt, and Egypt")
+        expect(page).to have_content("Brazil and Egypt")
+        expect(page).not_to have_content("Angola and Angola")
+        expect(page).to have_content("Angola")
+      end
+    end
+
     scenario "linking an activity to a non-ODA activity via the bulk upload" do
       within ".upload-form--ispf-oda" do
         attach_file "report[activity_csv_ispf_oda]", File.new("spec/fixtures/csv/valid_ispf_oda_activities_upload_with_linked_non_oda_activity.csv").path

spec/fixtures/csv/valid_ispf_oda_activities_upload_with_duplicate_pipe_items.csv

@@ -0,0 +1,2 @@
+RODA ID,Parent RODA ID,Linked activity RODA ID,Transparency identifier,Title,Description,Benefitting Countries,Partner organisation identifier,GDI,SDG 1,SDG 2,SDG 3,Covid-19 related research,ODA Eligibility,ODA Eligibility Lead,Activity Status,Call open date,Call close date,Total applications,Total awards,Planned start date,Planned end date,Actual start date,Actual end date,Sector,Channel of delivery code,Collaboration type (Bi/Multi Marker),DFID policy marker - Gender,DFID policy marker - Climate Change - Adaptation,DFID policy marker - Climate Change - Mitigation,DFID policy marker - Biodiversity,DFID policy marker - Desertification,DFID policy marker - Disability,DFID policy marker - Disaster Risk Reduction,DFID policy marker - Nutrition,Aid type,Free Standing Technical Cooperation,Aims/Objectives,UK PO Named Contact,ISPF themes,ISPF ODA partner countries,ISPF non-ODA partner countries,Comments,Implementing organisation names,Tags,Original commitment figure
+,ISPF-ODA-PROGRAMME-ID,,1234,A title,A description,AO|AO,example-id-1arn,4,,,,,1,ODA lead 1,7,11/01/2019,11/05/2019,14,5,10/10/2020,10/10/2021,,,12182,11000,1,,,,,,,,,D02,0,Freetext objectives,Someone Somebody,1|2|1,BR|EG|EG,CA|CA,A comment,,1|2|2|1|1,1000

spec/services/activity/import_spec.rb

@@ -295,6 +295,32 @@
       expect(existing_activity.implementing_organisations.count).to equal(1)
     end
 
+    it 'eliminates duplication in pipe-delimited fields' do
+      existing_activity_attributes["Benefitting Countries"] = "KH|KP|ID|ID"
+      existing_activity_attributes["GCRF Strategic Area"] = "17A|RF|RF"
+      existing_activity_attributes["ISPF themes"] = "2|3|3"
+      existing_activity_attributes["ISPF ODA partner countries"] = "BR|BR"
+      existing_activity_attributes["ISPF non-ODA partner countries"] = "AU|AU"
+      existing_activity_attributes["Tags"] = "1|2|2"
+      existing_activity_attributes["NF Partner Country PO"] =
+        "Board of Sample Organisations (BSO) | Board of Sample Organisations (BSO)"
+
+      subject.import([existing_activity_attributes])
+
+      expect(subject.errors).to be_empty
+
+      activity = subject.updated.first
+      aggregate_failures do
+        expect(activity.benefitting_countries).to eq(%w[KH KP ID])
+        expect(activity.gcrf_strategic_area).to eq(%w[17A RF])
+        expect(activity.ispf_oda_partner_countries).to eq(%w[BR])
+        expect(activity.ispf_non_oda_partner_countries).to eq(%w[AU])
+        expect(activity.country_partner_organisations).to eq(["Board of Sample Organisations (BSO)"])
+        expect(activity.ispf_themes).to eq([2, 3])
+        expect(activity.tags).to eq([1, 2])
+      end
+    end
+
     describe "setting the commitment" do
       context "when there is no commitment present on the activity" do
         it "allows users to update the original commitment figure" do
@@ -586,6 +612,32 @@
       allow(ActivityPolicy).to receive(:new).with(uploader, parent_activity).and_return(activity_policy_double)
     end
 
+    it 'eliminates duplication in pipe-delimited fields' do
+      new_activity_attributes["Benefitting Countries"] = "KH|KP|ID|ID"
+      new_activity_attributes["GCRF Strategic Area"] = "17A|RF|RF"
+      new_activity_attributes["ISPF themes"] = "2|3|3"
+      new_activity_attributes["ISPF ODA partner countries"] = "BR|BR"
+      new_activity_attributes["ISPF non-ODA partner countries"] = "AU|AU"
+      new_activity_attributes["Tags"] = "1|2|2"
+      new_activity_attributes["NF Partner Country PO"] =
+        "Board of Sample Organisations (BSO) | Board of Sample Organisations (BSO)"
+
+      subject.import([new_activity_attributes])
+
+      expect(subject.errors).to be_empty
+
+      activity = subject.created.first
+      aggregate_failures do
+        expect(activity.benefitting_countries).to eq(%w[KH KP ID])
+        expect(activity.gcrf_strategic_area).to eq(%w[17A RF])
+        expect(activity.ispf_oda_partner_countries).to eq(%w[BR])
+        expect(activity.ispf_non_oda_partner_countries).to eq(%w[AU])
+        expect(activity.country_partner_organisations).to eq(["Board of Sample Organisations (BSO)"])
+        expect(activity.ispf_themes).to eq([2, 3])
+        expect(activity.tags).to eq([1, 2])
+      end
+    end
+
     it "returns an error when the ID and fragments are not present" do
       existing_activity_attributes["RODA ID"] = ""