Refactor updating question

To be more consistent with how we usually do updates, move updating
the page when submitting the edit question page to the `submit` method
on the QuestionInput.

Previously we would have wanted this to be in the controller as it was
calling forms-api using ActiveResource. But now the Page is updated in
the local database instead.

Improve the tests around this.

Author: stephencdaly

app/controllers/pages/questions_controller.rb

@@ -35,8 +35,6 @@ def edit
   end
 
   def update
-    page.assign_attributes(page_params_for_update)
-
     @question_input = Pages::QuestionInput.new(page_params_for_form_object)
 
     if @question_input.update_page(@page)
@@ -62,14 +60,6 @@ def page_params_for_form_object
                       guidance_markdown: draft_question.guidance_markdown)
   end
 
-  def page_params_for_update
-    page_params.merge(form_id: current_form.id,
-                      answer_settings: draft_question.answer_settings,
-                      page_heading: draft_question.page_heading,
-                      guidance_markdown: draft_question.guidance_markdown,
-                      answer_type: draft_question.answer_type)
-  end
-
   def check_draft_question
     render "errors/missing_draft_question", status: :unprocessable_content, formats: :html if draft_question.answer_type.blank?
   end

app/input_objects/pages/question_input.rb

@@ -17,14 +17,7 @@ class Pages::QuestionInput < BaseInput
   def submit
     return false if invalid?
 
-    draft_question.assign_attributes(
-      question_text:,
-      hint_text:,
-      is_optional:,
-      is_repeatable:,
-    )
-
-    draft_question.save!(validate: false)
+    update_draft_question
 
     Page.create_and_update_form!(form_id:,
                                  question_text:,
@@ -40,15 +33,16 @@ def submit
   def update_page(page)
     return false if invalid?
 
-    draft_question.assign_attributes(
-      question_text:,
+    update_draft_question
+
+    page.assign_attributes(question_text:,
       hint_text:,
       is_optional:,
       is_repeatable:,
-    )
-
-    draft_question.save!(validate: false)
-
+      answer_settings:,
+      page_heading:,
+      guidance_markdown:,
+      answer_type:)
     page.save_and_update_form
   end
 
@@ -82,4 +76,17 @@ def validate_question_text_length
     translation_key = answer_type == "file" ? :too_long_file : :too_long
     errors.add(:question_text, translation_key, count: QuestionTextValidation::QUESTION_TEXT_MAX_LENGTH)
   end
+
+private
+
+  def update_draft_question
+    draft_question.assign_attributes(
+      question_text:,
+      hint_text:,
+      is_optional:,
+      is_repeatable:,
+    )
+
+    draft_question.save!(validate: false)
+  end
 end

spec/input_objects/pages/question_input_spec.rb

@@ -2,12 +2,16 @@
 
 RSpec.describe Pages::QuestionInput, type: :model do
   let(:form) { create :form }
-  let(:question_input) { build :question_input, answer_type:, question_text:, draft_question:, is_optional:, is_repeatable:, form_id: form.id }
-  let(:draft_question) { build :draft_question, question_text:, form_id: form.id }
+  let(:question_input) do
+    build(:question_input, answer_type:, question_text:, draft_question:, is_optional:,
+      is_repeatable:, form_id: form.id, answer_settings: draft_question.answer_settings,
+      page_heading: draft_question.page_heading, guidance_markdown: draft_question.guidance_markdown)
+  end
+  let(:draft_question) { build :address_draft_question, :with_guidance, question_text:, form_id: form.id }
   let(:question_text) { "What is your full name?" }
   let(:is_optional) { "false" }
   let(:is_repeatable) { "false" }
-  let(:answer_type) { "email" }
+  let(:answer_type) { draft_question.answer_type }
 
   it "has a valid factory" do
     expect(build(:question_input)).to be_valid
@@ -315,6 +319,17 @@
       it "sets a draft_question is_repeatable" do
         expect(question_input.draft_question.is_repeatable.to_s).to eq question_input.is_repeatable
       end
+
+      it "updates the page attributes" do
+        expect(page.question_text).to eq question_input.question_text
+        expect(page.hint_text).to eq question_input.hint_text
+        expect(page.is_optional.to_s).to eq question_input.is_optional
+        expect(page.is_repeatable.to_s).to eq question_input.is_repeatable
+        expect(page.answer_settings).to eq DataStruct.recursive_new(question_input.answer_settings)
+        expect(page.page_heading).to eq question_input.page_heading
+        expect(page.guidance_markdown).to eq question_input.guidance_markdown
+        expect(page.answer_type).to eq question_input.answer_type
+      end
     end
   end
 end

spec/requests/pages/questions_controller_spec.rb

@@ -176,24 +176,19 @@
 
   describe "#update" do
     let(:draft_question) do
-      record = create :draft_question, user: standard_user, form_id: form.id
-      record.question_text = nil
-      record.save!(validate: false)
-      record.reload
+      create(:address_draft_question, :with_guidance, page_id: page.id, user: standard_user, form_id: form.id)
     end
 
     let(:page) do
       create(
         :page,
         form_id: form.id,
-        question_text: "What is your work address?",
-        hint_text: "This should be the location stated in your contract.",
-        answer_type: "address",
+        question_text: "Old question text",
+        hint_text: "Old hint text",
+        answer_type: "email",
         answer_settings: nil,
         is_optional: false,
         is_repeatable: false,
-        page_heading: "New page heading",
-        guidance_markdown: "## Heading level 2",
       )
     end
 
@@ -203,20 +198,27 @@
           form_id: form.id,
           question_text: "What is your home address?",
           hint_text: "This should be the location stated in your contract.",
-          answer_type: "address",
-          is_optional: "false",
-          is_repeatable: "false",
-          page_heading: "New page heading",
-          guidance_markdown: "## Heading level 2",
+          is_optional: "true",
+          is_repeatable: "true",
         } }
       end
 
       before do
+        draft_question
         post update_question_path(form_id: form.id, page_id: page.id), params:
       end
 
-      it "Updates the page through the page repository" do
-        expect(page.reload.question_text).to eq("What is your home address?")
+      it "Updates the page" do
+        expect(page.reload).to have_attributes(
+          question_text: "What is your home address?",
+          hint_text: "This should be the location stated in your contract.",
+          is_optional: true,
+          is_repeatable: true,
+          answer_type: "address",
+          answer_settings: DataStruct.recursive_new(draft_question.answer_settings),
+          page_heading: draft_question.page_heading,
+          guidance_markdown: draft_question.guidance_markdown,
+        )
       end
 
       it "Redirects you to edit page for question that was updated" do
@@ -233,7 +235,7 @@
       end
 
       it "logs the answer type from the page", :capture_logging do
-        expect(log_line["answer_type"]).to eq(page.answer_type)
+        expect(log_line["answer_type"]).to eq(page.reload.answer_type)
       end
 
       context "when question being updated has a question after it" do