From 4916049e18449fa2b1ecd42951c9456d3a46dd56 Mon Sep 17 00:00:00 2001
From: Arthur Campbell <51094020+arfacamble@users.noreply.github.com>
Date: Mon, 13 Mar 2023 17:37:21 +0000
Subject: [PATCH] CLDC-2014 remove text field for buyer 1 nationality other
 (#1388)

* remove question no longer needed and associated test file

* amend page and the other question in that page that determined whether the deleted question should be shown
update associated test files

* createa a migration to remove the column for the deleted question

* update test broken by changes

* fix test broken by changes
---
 app/models/form/sales/pages/nationality1.rb   |  7 +---
 .../form/sales/questions/nationality1.rb      | 10 -----
 .../sales/questions/other_nationality1.rb     | 10 -----
 .../imports/sales_logs_import_service.rb      |  1 -
 ...43_remove_othernational_from_sales_logs.rb |  5 +++
 db/schema.rb                                  |  1 -
 .../form/conditional_questions_spec.rb        | 12 +++---
 .../form/sales/pages/nationality1_spec.rb     | 12 +++---
 .../form/sales/questions/nationality1_spec.rb | 30 +++++---------
 .../questions/other_nationality1_spec.rb      | 41 -------------------
 10 files changed, 28 insertions(+), 101 deletions(-)
 delete mode 100644 app/models/form/sales/questions/other_nationality1.rb
 create mode 100644 db/migrate/20230307111943_remove_othernational_from_sales_logs.rb
 delete mode 100644 spec/models/form/sales/questions/other_nationality1_spec.rb

diff --git a/app/models/form/sales/pages/nationality1.rb b/app/models/form/sales/pages/nationality1.rb
index 96723e857..8db4a64ea 100644
--- a/app/models/form/sales/pages/nationality1.rb
+++ b/app/models/form/sales/pages/nationality1.rb
@@ -7,15 +7,12 @@ class Form::Sales::Pages::Nationality1 < ::Form::Page
         "privacynotice" => 1,
       },
       {
-        "noint" => 1,
+        "buyer_not_interviewed?" => true,
       },
     ]
   end
 
   def questions
-    @questions ||= [
-      Form::Sales::Questions::Nationality1.new(nil, nil, self),
-      Form::Sales::Questions::OtherNationality1.new(nil, nil, self),
-    ]
+    @questions ||= [Form::Sales::Questions::Nationality1.new(nil, nil, self)]
   end
 end
diff --git a/app/models/form/sales/questions/nationality1.rb b/app/models/form/sales/questions/nationality1.rb
index 53e9edc5f..765489f27 100644
--- a/app/models/form/sales/questions/nationality1.rb
+++ b/app/models/form/sales/questions/nationality1.rb
@@ -7,16 +7,6 @@ class Form::Sales::Questions::Nationality1 < ::Form::Question
     @type = "radio"
     @hint_text = "Buyer 1 is the person in the household who does the most paid work. If it’s a joint purchase and the buyers do the same amount of paid work, buyer 1 is whoever is the oldest."
     @answer_options = ANSWER_OPTIONS
-    @conditional_for = {
-      "othernational" => [12],
-    }
-    @hidden_in_check_answers = {
-      "depends_on" => [
-        {
-          "national" => 12,
-        },
-      ],
-    }
     @check_answers_card_number = 1
     @inferred_check_answers_value = [{
       "condition" => {
diff --git a/app/models/form/sales/questions/other_nationality1.rb b/app/models/form/sales/questions/other_nationality1.rb
deleted file mode 100644
index 3eb6a48d7..000000000
--- a/app/models/form/sales/questions/other_nationality1.rb
+++ /dev/null
@@ -1,10 +0,0 @@
-class Form::Sales::Questions::OtherNationality1 < ::Form::Question
-  def initialize(id, hsh, page)
-    super
-    @id = "othernational"
-    @check_answer_label = "Buyer 1’s nationality"
-    @header = "Nationality"
-    @type = "text"
-    @check_answers_card_number = 1
-  end
-end
diff --git a/app/services/imports/sales_logs_import_service.rb b/app/services/imports/sales_logs_import_service.rb
index ed127bff2..577e37e5f 100644
--- a/app/services/imports/sales_logs_import_service.rb
+++ b/app/services/imports/sales_logs_import_service.rb
@@ -54,7 +54,6 @@ module Imports
         attributes["details_known_#{index}"] = details_known(index, attributes)
       end
       attributes["national"] = unsafe_string_as_integer(xml_doc, "P1Nat")
-      attributes["othernational"] = nil
       attributes["ethnic"] = unsafe_string_as_integer(xml_doc, "P1Eth")
       attributes["ethnic_group"] = ethnic_group(attributes["ethnic"])
       attributes["buy1livein"] = unsafe_string_as_integer(xml_doc, "LiveInBuyer1")
diff --git a/db/migrate/20230307111943_remove_othernational_from_sales_logs.rb b/db/migrate/20230307111943_remove_othernational_from_sales_logs.rb
new file mode 100644
index 000000000..534962773
--- /dev/null
+++ b/db/migrate/20230307111943_remove_othernational_from_sales_logs.rb
@@ -0,0 +1,5 @@
+class RemoveOthernationalFromSalesLogs < ActiveRecord::Migration[7.0]
+  def change
+    remove_column :sales_logs, :othernational, :string
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 385d565c7..bea2209c3 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -403,7 +403,6 @@ ActiveRecord::Schema[7.0].define(version: 2023_03_08_101826) do
     t.integer "age1_known"
     t.string "sex1"
     t.integer "national"
-    t.string "othernational"
     t.integer "ethnic"
     t.integer "ethnic_group"
     t.integer "buy1livein"
diff --git a/spec/features/form/conditional_questions_spec.rb b/spec/features/form/conditional_questions_spec.rb
index 4fc9387da..52d8886e0 100644
--- a/spec/features/form/conditional_questions_spec.rb
+++ b/spec/features/form/conditional_questions_spec.rb
@@ -54,13 +54,13 @@ RSpec.describe "Form Conditional Questions" do
     end
 
     it "gets cleared if the conditional question is hidden after editing the answer" do
-      sales_log.update!(national: 12, othernational: "other")
-      visit("/sales-logs/#{sales_log.id}/buyer-1-nationality")
-      expect(page).to have_field("sales-log-othernational-field", with: "other")
+      sales_log.update!(age1_known: 0, age1: 50)
+      visit("/sales-logs/#{sales_log.id}/buyer-1-age")
+      expect(page).to have_field("sales-log-age1-field", with: 50)
 
-      choose("sales-log-national-18-field", allow_label_click: true)
-      choose("sales-log-national-12-field", allow_label_click: true)
-      expect(page).to have_field("sales-log-othernational-field", with: "")
+      choose("sales-log-age1-known-1-field", allow_label_click: true)
+      choose("sales-log-age1-known-0-field", allow_label_click: true)
+      expect(page).to have_field("sales-log-age1-field", with: "")
     end
   end
 end
diff --git a/spec/models/form/sales/pages/nationality1_spec.rb b/spec/models/form/sales/pages/nationality1_spec.rb
index da5a5f802..37b58a89f 100644
--- a/spec/models/form/sales/pages/nationality1_spec.rb
+++ b/spec/models/form/sales/pages/nationality1_spec.rb
@@ -1,22 +1,20 @@
 require "rails_helper"
 
 RSpec.describe Form::Sales::Pages::Nationality1, type: :model do
-  subject(:page) { described_class.new(page_id, page_definition, subsection) }
+  subject(:page) { described_class.new(nil, nil, subsection) }
 
-  let(:page_id) { nil }
-  let(:page_definition) { nil }
   let(:subsection) { instance_double(Form::Subsection) }
 
   it "has correct subsection" do
-    expect(page.subsection).to eq(subsection)
+    expect(page.subsection).to be subsection
   end
 
   it "has correct questions" do
-    expect(page.questions.map(&:id)).to eq(%w[national othernational])
+    expect(page.questions.map(&:id)).to eq %w[national]
   end
 
   it "has the correct id" do
-    expect(page.id).to eq("buyer_1_nationality")
+    expect(page.id).to eq "buyer_1_nationality"
   end
 
   it "has the correct header" do
@@ -28,6 +26,6 @@ RSpec.describe Form::Sales::Pages::Nationality1, type: :model do
   end
 
   it "has correct depends_on" do
-    expect(page.depends_on).to eq([{ "privacynotice" => 1 }, { "noint" => 1 }])
+    expect(page.depends_on).to eq [{ "privacynotice" => 1 }, { "buyer_not_interviewed?" => true }]
   end
 end
diff --git a/spec/models/form/sales/questions/nationality1_spec.rb b/spec/models/form/sales/questions/nationality1_spec.rb
index 18f659e86..4b8816dbf 100644
--- a/spec/models/form/sales/questions/nationality1_spec.rb
+++ b/spec/models/form/sales/questions/nationality1_spec.rb
@@ -1,30 +1,28 @@
 require "rails_helper"
 
 RSpec.describe Form::Sales::Questions::Nationality1, type: :model do
-  subject(:question) { described_class.new(question_id, question_definition, page) }
+  subject(:question) { described_class.new(nil, nil, page) }
 
-  let(:question_id) { nil }
-  let(:question_definition) { nil }
   let(:page) { instance_double(Form::Page) }
 
   it "has correct page" do
-    expect(question.page).to eq(page)
+    expect(question.page).to be page
   end
 
   it "has the correct id" do
-    expect(question.id).to eq("national")
+    expect(question.id).to eq "national"
   end
 
   it "has the correct header" do
-    expect(question.header).to eq("What is buyer 1’s nationality?")
+    expect(question.header).to eq "What is buyer 1’s nationality?"
   end
 
   it "has the correct check_answer_label" do
-    expect(question.check_answer_label).to eq("Buyer 1’s nationality")
+    expect(question.check_answer_label).to eq "Buyer 1’s nationality"
   end
 
   it "has the correct type" do
-    expect(question.type).to eq("radio")
+    expect(question.type).to eq "radio"
   end
 
   it "is not marked as derived" do
@@ -32,7 +30,7 @@ RSpec.describe Form::Sales::Questions::Nationality1, type: :model do
   end
 
   it "has the correct hint" do
-    expect(question.hint_text).to eq("Buyer 1 is the person in the household who does the most paid work. If it’s a joint purchase and the buyers do the same amount of paid work, buyer 1 is whoever is the oldest.")
+    expect(question.hint_text).to eq "Buyer 1 is the person in the household who does the most paid work. If it’s a joint purchase and the buyers do the same amount of paid work, buyer 1 is whoever is the oldest."
   end
 
   it "has the correct answer_options" do
@@ -46,23 +44,15 @@ RSpec.describe Form::Sales::Questions::Nationality1, type: :model do
   end
 
   it "has correct conditional for" do
-    expect(question.conditional_for).to eq({
-      "othernational" => [12],
-    })
+    expect(question.conditional_for).to be_nil
   end
 
   it "has correct hidden in check answers" do
-    expect(question.hidden_in_check_answers).to eq({
-      "depends_on" => [
-        {
-          "national" => 12,
-        },
-      ],
-    })
+    expect(question.hidden_in_check_answers).to be_nil
   end
 
   it "has the correct check_answers_card_number" do
-    expect(question.check_answers_card_number).to eq(1)
+    expect(question.check_answers_card_number).to be 1
   end
 
   it "has the correct inferred_check_answers_value" do
diff --git a/spec/models/form/sales/questions/other_nationality1_spec.rb b/spec/models/form/sales/questions/other_nationality1_spec.rb
deleted file mode 100644
index c3faa73dc..000000000
--- a/spec/models/form/sales/questions/other_nationality1_spec.rb
+++ /dev/null
@@ -1,41 +0,0 @@
-require "rails_helper"
-
-RSpec.describe Form::Sales::Questions::OtherNationality1, type: :model do
-  subject(:question) { described_class.new(question_id, question_definition, page) }
-
-  let(:question_id) { nil }
-  let(:question_definition) { nil }
-  let(:page) { instance_double(Form::Page) }
-
-  it "has correct page" do
-    expect(question.page).to eq(page)
-  end
-
-  it "has the correct id" do
-    expect(question.id).to eq("othernational")
-  end
-
-  it "has the correct header" do
-    expect(question.header).to eq("Nationality")
-  end
-
-  it "has the correct check_answer_label" do
-    expect(question.check_answer_label).to eq("Buyer 1’s nationality")
-  end
-
-  it "has the correct type" do
-    expect(question.type).to eq("text")
-  end
-
-  it "is not marked as derived" do
-    expect(question.derived?).to be false
-  end
-
-  it "has the correct hint" do
-    expect(question.hint_text).to be_nil
-  end
-
-  it "has the correct check_answers_card_number" do
-    expect(question.check_answers_card_number).to eq(1)
-  end
-end