From 6c286709f837b5eb89a8a938b50b4a7b7101bae6 Mon Sep 17 00:00:00 2001
From: natdeanlewissoftwire <nat.dean-lewis@softwire.com>
Date: Tue, 20 Jun 2023 09:39:25 +0100
Subject: [PATCH] refactor: from tech review

---
 app/helpers/schemes_helper.rb | 24 +++++-------------------
 spec/features/schemes_spec.rb |  8 ++------
 2 files changed, 7 insertions(+), 25 deletions(-)

diff --git a/app/helpers/schemes_helper.rb b/app/helpers/schemes_helper.rb
index 0656d556e..610d33af5 100644
--- a/app/helpers/schemes_helper.rb
+++ b/app/helpers/schemes_helper.rb
@@ -1,35 +1,21 @@
 module SchemesHelper
   def display_scheme_attributes(scheme, user)
-    base_attributes = [
+    [
       { name: "Scheme code", value: scheme.id_to_display },
       { name: "Name", value: scheme.service_name, edit: true },
       { name: "Confidential information", value: scheme.sensitive, edit: true },
       { name: "Type of scheme", value: scheme.scheme_type },
       { name: "Registered under Care Standards Act 2000", value: scheme.registered_under_care_act },
-      { name: "Housing stock owned by", value: scheme.owning_organisation.name, edit: true },
+      user.data_coordinator? ? nil : { name: "Housing stock owned by", value: scheme.owning_organisation.name, edit: true },
       { name: "Support services provided by", value: scheme.arrangement_type },
       { name: "Primary client group", value: scheme.primary_client_group },
       { name: "Has another client group", value: scheme.has_other_client_group },
+      scheme.has_other_client_group == "Yes" ? { name: "Secondary client group", value: scheme.secondary_client_group } : nil,
       { name: "Level of support given", value: scheme.support_type },
       { name: "Intended length of stay", value: scheme.intended_stay },
       { name: "Availability", value: scheme_availability(scheme) },
-    ]
-
-    if FeatureToggle.scheme_toggle_enabled?
-      base_attributes.append({ name: "Status", value: status_tag(scheme.status) })
-    end
-
-    if user.data_coordinator?
-      base_attributes.delete_if { |item| item[:name] == "Housing stock owned by" }
-    end
-
-    if scheme.has_other_client_group == "Yes"
-      insertion_index = base_attributes.index { |attribute| attribute[:name] == "Has another client group" } + 1
-      base_attributes.insert(insertion_index, { name: "Secondary client group", value: scheme.secondary_client_group })
-
-    end
-
-    base_attributes
+      FeatureToggle.scheme_toggle_enabled? ? { name: "Status", value: status_tag(scheme.status) } : nil,
+    ].compact
   end
 
   def scheme_availability(scheme)
diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb
index f9559778b..fd1c49d7e 100644
--- a/spec/features/schemes_spec.rb
+++ b/spec/features/schemes_spec.rb
@@ -170,9 +170,7 @@ RSpec.describe "Schemes scheme Features" do
 
           context "when there is no secondary client group" do
             before do
-              scheme.has_other_client_group = "No"
-              scheme.secondary_client_group = nil
-              scheme.save!
+              scheme.update!(has_other_client_group: "No", secondary_client_group: nil)
               click_link(scheme.service_name)
             end
 
@@ -733,9 +731,7 @@ RSpec.describe "Schemes scheme Features" do
 
           context "when there is no secondary client group" do
             before do
-              scheme.has_other_client_group = "No"
-              scheme.secondary_client_group = nil
-              scheme.save!
+              scheme.update!(has_other_client_group: "No", secondary_client_group: nil)
               click_link(scheme.service_name)
             end