feat: add schemes to navbar when org has stock owners and code refactoring (#1980)

app/helpers/navigation_items_helper.rb

@@ -14,7 +14,7 @@ module NavigationItemsHelper
       [
         NavigationItem.new("Lettings logs", lettings_logs_path, lettings_logs_current?(path)),
         NavigationItem.new("Sales logs", sales_logs_path, sales_logs_current?(path)),
-        (NavigationItem.new("Schemes", "/schemes", subnav_supported_housing_schemes_path?(path)) if current_user.organisation.holds_own_stock?),
+        (NavigationItem.new("Schemes", "/schemes", supported_housing_schemes_current?(path)) if current_user.organisation.holds_own_stock? || current_user.organisation.stock_owners.present?),
         NavigationItem.new("Users", users_organisation_path(current_user.organisation), subnav_users_path?(path)),
         NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", subnav_details_path?(path)),
         NavigationItem.new("Stock owners", stock_owners_organisation_path(current_user.organisation), stock_owners_path?(path)),
@@ -24,26 +24,15 @@ module NavigationItemsHelper
   end
 
   def secondary_items(path, current_organisation_id)
-    if current_user.organisation.holds_own_stock?
+    [
-      [
+      NavigationItem.new("Lettings logs", "/organisations/#{current_organisation_id}/lettings-logs", subnav_lettings_logs_path?(path)),
-        NavigationItem.new("Lettings logs", "/organisations/#{current_organisation_id}/lettings-logs", subnav_lettings_logs_path?(path)),
+      NavigationItem.new("Sales logs", "/organisations/#{current_organisation_id}/sales-logs", subnav_sales_logs_path?(path)),
-        NavigationItem.new("Sales logs", "/organisations/#{current_organisation_id}/sales-logs", subnav_sales_logs_path?(path)),
+      (NavigationItem.new("Schemes", "/organisations/#{current_organisation_id}/schemes", subnav_supported_housing_schemes_path?(path)) if current_user.organisation.holds_own_stock? || current_user.organisation.stock_owners.present?),
-        NavigationItem.new("Schemes", "/organisations/#{current_organisation_id}/schemes", subnav_supported_housing_schemes_path?(path)),
+      NavigationItem.new("Users", "/organisations/#{current_organisation_id}/users", subnav_users_path?(path)),
-        NavigationItem.new("Users", "/organisations/#{current_organisation_id}/users", subnav_users_path?(path)),
+      NavigationItem.new("About this organisation", "/organisations/#{current_organisation_id}", subnav_details_path?(path)),
-        NavigationItem.new("About this organisation", "/organisations/#{current_organisation_id}", subnav_details_path?(path)),
+      NavigationItem.new("Stock owners", stock_owners_organisation_path(current_organisation_id), stock_owners_path?(path)),
-        NavigationItem.new("Stock owners", stock_owners_organisation_path(current_organisation_id), stock_owners_path?(path)),
+      NavigationItem.new("Managing agents", managing_agents_organisation_path(current_organisation_id), managing_agents_path?(path)),
-        NavigationItem.new("Managing agents", managing_agents_organisation_path(current_organisation_id), managing_agents_path?(path)),
+    ].compact
-      ].compact
-    else
-      [
-        NavigationItem.new("Lettings logs", "/organisations/#{current_organisation_id}/lettings-logs", subnav_lettings_logs_path?(path)),
-        NavigationItem.new("Sales logs", "/organisations/#{current_organisation_id}/sales-logs", sales_logs_current?(path)),
-        NavigationItem.new("Users", "/organisations/#{current_organisation_id}/users", subnav_users_path?(path)),
-        NavigationItem.new("About this organisation", "/organisations/#{current_organisation_id}", subnav_details_path?(path)),
-        NavigationItem.new("Stock owners", stock_owners_organisation_path(current_organisation_id), stock_owners_path?(path)),
-        NavigationItem.new("Managing agents", managing_agents_organisation_path(current_organisation_id), managing_agents_path?(path)),
-      ].compact
-    end
   end
 
   def scheme_items(path, current_scheme_id, title)

spec/helpers/navigation_items_helper_spec.rb

@@ -8,6 +8,51 @@ RSpec.describe NavigationItemsHelper do
 
   describe "#primary_items" do
     context "when the user is a data coordinator" do
+      context "when the user's org does not own stock" do
+        before do
+          current_user.organisation.update!(holds_own_stock: false)
+        end
+
+        let(:expected_navigation_items) do
+          [
+            NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", true),
+            NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false),
+            NavigationItemsHelper::NavigationItem.new("Users", users_path, false),
+            NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false),
+            NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false),
+            NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", false),
+          ]
+        end
+
+        it "returns navigation items with the users item set as current" do
+          expect(primary_items("/lettings-logs", current_user)).to eq(expected_navigation_items)
+        end
+
+        context "when the user's org has a stock owner" do
+          before do
+            current_user.organisation.update!(holds_own_stock: false)
+            create(:organisation_relationship, child_organisation: current_user.organisation, parent_organisation: stock_owner)
+          end
+
+          let(:stock_owner) { create(:organisation) }
+          let(:expected_navigation_items) do
+            [
+              NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", true),
+              NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false),
+              NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false),
+              NavigationItemsHelper::NavigationItem.new("Users", users_path, false),
+              NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false),
+              NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false),
+              NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", false),
+            ]
+          end
+
+          it "returns navigation items with the users item set as current" do
+            expect(primary_items("/lettings-logs", current_user)).to eq(expected_navigation_items)
+          end
+        end
+      end
+
       context "when the user is on the lettings logs page" do
         let(:expected_navigation_items) do
           [
@@ -141,6 +186,51 @@ RSpec.describe NavigationItemsHelper do
       it "includes schemes" do
         expect(primary_items("/", current_user)).to include(NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false))
       end
+
+      context "when the user's org does not own stock" do
+        before do
+          current_user.organisation.update!(holds_own_stock: false)
+        end
+
+        let(:expected_navigation_items) do
+          [
+            NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", true),
+            NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false),
+            NavigationItemsHelper::NavigationItem.new("Users", users_path, false),
+            NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false),
+            NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false),
+            NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", false),
+          ]
+        end
+
+        it "returns navigation items with the users item set as current" do
+          expect(primary_items("/lettings-logs", current_user)).to eq(expected_navigation_items)
+        end
+
+        context "when the user's org has a stock owner" do
+          before do
+            current_user.organisation.update!(holds_own_stock: false)
+            create(:organisation_relationship, child_organisation: current_user.organisation, parent_organisation: stock_owner)
+          end
+
+          let(:stock_owner) { create(:organisation) }
+          let(:expected_navigation_items) do
+            [
+              NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", true),
+              NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false),
+              NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false),
+              NavigationItemsHelper::NavigationItem.new("Users", users_path, false),
+              NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false),
+              NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false),
+              NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", false),
+            ]
+          end
+
+          it "returns navigation items with the users item set as current" do
+            expect(primary_items("/lettings-logs", current_user)).to eq(expected_navigation_items)
+          end
+        end
+      end
     end
 
     context "when the user is a support user" do