Merge branch 'main' into add-user-research-contact-preference

Author: alice-carr

app/helpers/report_helper.rb

@@ -73,7 +73,7 @@ def report_questions_table_rows(questions)
   def report_forms_table_row(form)
     [
       form_link(form),
-      form.dig("group", "organisation", "name") || "",
+      form["organisation_name"],
     ]
   end
 

app/services/reports/feature_report_service.rb

@@ -81,48 +81,29 @@ def forms_with_branch_routes
   def forms_with_payments
     form_documents
       .select { |form| Reports::FormDocumentsService.has_payments?(form) }
-      .map { |form| form_details(form) }
   end
 
   def forms_with_exit_pages
     form_documents
       .select { |form| Reports::FormDocumentsService.has_exit_pages?(form) }
-      .map { |form| form_details(form) }
   end
 
   def forms_with_csv_submission_enabled
     form_documents
       .select { |form| Reports::FormDocumentsService.has_csv_submission_enabled?(form) }
-      .map { |form| form_details(form) }
   end
 
 private
 
   def questions_details(form, step)
-    form = form_details(form)
     step.dup.merge("form" => form)
   end
 
   def form_with_routes_details(form)
-    form = form_details(form)
     form["metadata"] = {
       "number_of_routes" => form["content"]["steps"].count { |step| step["routing_conditions"].present? },
       "number_of_branch_routes" => Reports::FormDocumentsService.count_secondary_skip_routes(form),
     }
     form
   end
-
-  def form_details(form)
-    form = form.dup
-    form["group"] = {
-      "organisation" => {
-        "name" => organisation_name(form["form_id"]),
-      },
-    }
-    form
-  end
-
-  def organisation_name(form_id)
-    GroupForm.find_by_form_id(form_id)&.group&.organisation&.name
-  end
 end

app/services/reports/form_documents_service.rb

@@ -48,15 +48,33 @@ def has_exit_pages?(form_document)
   private
 
     def draft_form_documents
-      draft_forms = Form.joins(group_form: { group: :organisation })
-        .where(state: %w[draft live_with_draft archived_with_draft])
-          .where.not(organisation: { "internal": true })
+      draft_forms = Form
+                      .joins(group_form: { group: :organisation })
+                      .where(state: %w[draft live_with_draft archived_with_draft])
+                      .where.not(organisation: { "internal": true })
+                      .select("forms.*",
+                              "organisation.name AS organisation_name",
+                              "organisation.id AS organisation_id",
+                              "groups.external_id AS group_external_id",
+                              "groups.name AS group_name")
 
-      draft_forms.map { |form| FormDocument.new(form:, tag: "draft", content: form.as_form_document).as_json }
+      draft_forms.map do |form|
+        form_details = FormDocument.new(form:, tag: "draft", content: form.as_form_document).as_json
+        form_details.merge({
+          "organisation_name" => form.organisation_name,
+          "organisation_id" => form.organisation_id,
+          "group_external_id" => form.group_external_id,
+          "group_name" => form.group_name,
+        })
+      end
     end
 
     def live_form_documents
-      FormDocument.joins(form: { group_form: { group: :organisation } }).where(tag: "live").where.not(organisation: { "internal": true }).map(&:as_json)
+      FormDocument.joins(form: { group_form: { group: :organisation } })
+                  .where(tag: "live")
+                  .where.not(organisation: { "internal": true })
+                  .select("form_documents.*", "organisation.name AS organisation_name", "organisation.id AS organisation_id", "groups.external_id AS group_external_id", "groups.name AS group_name")
+                  .map(&:as_json)
     end
 
     def secondary_skip_conditions(form_document)

app/services/reports/forms_csv_report_service.rb

@@ -47,16 +47,15 @@ def csv
 
   def form_row(form)
     form_id = form["form_id"]
-    group = GroupForm.find_by_form_id(form_id)&.group
     [
       form_id,
       form["tag"],
       form["content"]["name"],
       form["content"]["form_slug"],
-      group&.organisation&.name,
-      group&.organisation&.id,
-      group&.name,
-      group&.external_id,
+      form["organisation_name"],
+      form["organisation_id"],
+      form["group_name"],
+      form["group_external_id"],
       form["content"]["created_at"],
       form["content"]["updated_at"],
       form["content"]["steps"].length,

app/services/reports/questions_csv_report_service.rb

@@ -50,16 +50,16 @@ def csv
   def question_row(step)
     form = step["form"]
     form_id = form["form_id"]
-    group = GroupForm.find_by_form_id(form_id)&.group
+    GroupForm.find_by_form_id(form_id)&.group
 
     [
       form_id,
       form["tag"],
       form["content"]["name"],
-      group&.organisation&.name,
-      group&.organisation&.id,
-      group&.name,
-      group&.external_id,
+      form["organisation_name"],
+      form["organisation_id"],
+      form["group_name"],
+      form["group_external_id"],
       step["position"],
       step["data"]["question_text"],
       step["data"]["answer_type"],

spec/helpers/report_helpers_spec.rb

@@ -3,23 +3,23 @@
 RSpec.describe ReportHelper, type: :helper do
   let(:forms) do
     [
-      { "form_id" => 1, "tag" => "live", "content" => { "name" => "All question types form" }, "group" => { "organisation" => { "name" => "Government Digital Service" } } },
-      { "form_id" => 3, "tag" => "live", "content" => { "name" => "Branch route form" }, "group" => { "organisation" => { "name" => "Ministry of Tests" } } },
-      { "form_id" => 4, "tag" => "live", "content" => { "name" => "Skip route form" }, "group" => { "organisation" => { "name" => "Department for Testing" } } },
+      { "form_id" => 1, "tag" => "live", "content" => { "name" => "All question types form" }, "organisation_name" => "Government Digital Service" },
+      { "form_id" => 3, "tag" => "live", "content" => { "name" => "Branch route form" }, "organisation_name" => "Ministry of Tests" },
+      { "form_id" => 4, "tag" => "live", "content" => { "name" => "Skip route form" }, "organisation_name" => "Department for Testing" },
     ]
   end
 
   let(:forms_with_routes) do
     [
-      { "form_id" => 3, "tag" => "live", "content" => { "name" => "Branch route form" }, "group" => { "organisation" => { "name" => "Ministry of Tests" } }, "metadata" => { "number_of_routes" => 2, "number_of_branch_routes" => 1 } },
-      { "form_id" => 4, "tag" => "live", "content" => { "name" => "Skip route form" }, "group" => { "organisation" => { "name" => "Department for Testing" } }, "metadata" => { "number_of_routes" => 1, "number_of_branch_routes" => 0 } },
+      { "form_id" => 3, "tag" => "live", "content" => { "name" => "Branch route form" }, "organisation_name" => "Ministry of Tests", "metadata" => { "number_of_routes" => 2, "number_of_branch_routes" => 1 } },
+      { "form_id" => 4, "tag" => "live", "content" => { "name" => "Skip route form" }, "organisation_name" => "Department for Testing", "metadata" => { "number_of_routes" => 1, "number_of_branch_routes" => 0 } },
     ]
   end
 
   let(:questions) do
     [
-      { "type" => "question_page", "data" => { "question_text" => "Email address" }, "form" => { "form_id" => 1, "tag" => "live", "content" => { "name" => "All question types form" }, "group" => { "organisation" => { "name" => "Government Digital Service" } } } },
-      { "type" => "question_page", "data" => { "question_text" => "What’s your email address?" }, "form" => { "form_id" => 3, "tag" => "live", "content" => { "name" => "Branch route form" }, "group" => { "organisation" => { "name" => "Ministry of Tests" } } } },
+      { "type" => "question_page", "data" => { "question_text" => "Email address" }, "form" => { "form_id" => 1, "tag" => "live", "content" => { "name" => "All question types form" }, "organisation_name" => "Government Digital Service" } },
+      { "type" => "question_page", "data" => { "question_text" => "What’s your email address?" }, "form" => { "form_id" => 3, "tag" => "live", "content" => { "name" => "Branch route form" }, "organisation_name" => "Ministry of Tests" } },
     ]
   end
 
@@ -154,20 +154,6 @@
         "Department for Testing",
       ]
     end
-
-    context "when form is not in a group" do
-      let(:forms) do
-        [
-          { "form_id" => 1, "tag" => "live", "content" => { "name" => "All question types form" }, "group" => nil },
-        ]
-      end
-
-      it "returns the empty string for the organisation name" do
-        expect(helper.report_forms_table_rows(forms).map(&:second)).to eq [
-          "",
-        ]
-      end
-    end
   end
 
   describe "#report_forms_with_routes_table_head" do
@@ -228,20 +214,6 @@
         0
       ]
     end
-
-    context "when form is not in a group" do
-      let(:forms) do
-        [
-          { "form_id" => 1, "tag" => "live", "content" => { "name" => "All question types form" }, "group" => nil },
-        ]
-      end
-
-      it "returns the empty string for the organisation name" do
-        expect(helper.report_forms_table_rows(forms).map(&:second)).to eq [
-          "",
-        ]
-      end
-    end
   end
 
   describe "#report_questions_table_head" do
@@ -282,19 +254,5 @@
         "What’s your email address?",
       ]
     end
-
-    context "when form is not in a group" do
-      let(:questions) do
-        [
-          { "type" => "question_page", "data" => { "question_text" => "Email address" }, "form" => { "form_id" => 1, "tag" => "live", "content" => { "name" => "All question types form" }, "group" => nil } },
-        ]
-      end
-
-      it "returns the empty string for the organisation name" do
-        expect(helper.report_questions_table_rows(questions).map(&:second)).to eq [
-          "",
-        ]
-      end
-    end
   end
 end