From 0cd2dac40757a0d3f5440e55dcfa3ac84c219fc0 Mon Sep 17 00:00:00 2001
From: natdeanlewissoftwire <nat.dean-lewis@softwire.com>
Date: Tue, 14 Nov 2023 11:35:36 +0000
Subject: [PATCH] feat:tidy up routing and fix scheme tab current bug

---
 app/helpers/navigation_items_helper.rb | 18 +++++++++---------
 app/helpers/tab_nav_helper.rb          |  2 +-
 app/views/organisations/logs.html.erb  | 16 ++++++++--------
 config/routes.rb                       |  1 -
 4 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/app/helpers/navigation_items_helper.rb b/app/helpers/navigation_items_helper.rb
index adf109f15..e3183efe0 100644
--- a/app/helpers/navigation_items_helper.rb
+++ b/app/helpers/navigation_items_helper.rb
@@ -5,18 +5,18 @@ module NavigationItemsHelper
     if current_user.support?
       [
         NavigationItem.new("Organisations", organisations_path, organisations_current?(path)),
-        NavigationItem.new("Users", "/users", users_current?(path)),
+        NavigationItem.new("Users", users_path, users_current?(path)),
         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", supported_housing_schemes_current?(path)),
+        NavigationItem.new("Schemes", schemes_path, supported_housing_schemes_current?(path)),
       ].compact
     else
       [
         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", supported_housing_schemes_current?(path)) if current_user.organisation.holds_own_stock? || current_user.organisation.stock_owners.present?),
+        (NavigationItem.new("Schemes", schemes_organisation_path(current_user.organisation.id), subnav_supported_housing_schemes_path?(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("About your organisation", details_organisation_path(current_user.organisation.id), subnav_details_path?(path)),
         NavigationItem.new("Stock owners", stock_owners_organisation_path(current_user.organisation), stock_owners_path?(path)),
         NavigationItem.new("Managing agents", managing_agents_organisation_path(current_user.organisation), managing_agents_path?(path)),
       ].compact
@@ -25,11 +25,11 @@ module NavigationItemsHelper
 
   def secondary_items(path, current_organisation_id)
     [
-      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("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("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("Lettings logs", lettings_logs_organisation_path(current_organisation_id), subnav_lettings_logs_path?(path)),
+      NavigationItem.new("Sales logs", sales_logs_organisation_path(current_organisation_id), subnav_sales_logs_path?(path)),
+      (NavigationItem.new("Schemes", schemes_organisation_path(current_organisation_id), subnav_supported_housing_schemes_path?(path)) if Organisation.find(current_organisation_id).holds_own_stock? || Organisation.find(current_organisation_id).stock_owners.present?),
+      NavigationItem.new("Users", users_organisation_path(current_organisation_id), subnav_users_path?(path)),
+      NavigationItem.new("About this organisation", details_organisation_path(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
diff --git a/app/helpers/tab_nav_helper.rb b/app/helpers/tab_nav_helper.rb
index 9d441bdfe..1e9ae7b42 100644
--- a/app/helpers/tab_nav_helper.rb
+++ b/app/helpers/tab_nav_helper.rb
@@ -3,7 +3,7 @@ module TabNavHelper
 
   def user_cell(user)
     link_text = user.name.presence || user.email
-    link = current_user.support? ? govuk_link_to(link_text, organisation_user_path(user.organisation, user)) : govuk_link_to(link_text, user)
+    link = govuk_link_to(link_text, user)
     [link, "<span class=\"govuk-visually-hidden\">User </span><span class=\"govuk-!-font-weight-regular app-!-colour-muted\">#{user.email}</span>"].join("\n")
   end
 
diff --git a/app/views/organisations/logs.html.erb b/app/views/organisations/logs.html.erb
index c4f897832..81b32c9d1 100644
--- a/app/views/organisations/logs.html.erb
+++ b/app/views/organisations/logs.html.erb
@@ -5,10 +5,17 @@
 
 <%= render partial: "organisations/headings", locals: { main: @organisation.name, sub: nil } %>
 
+<% if current_user.support? %>
+  <%= render SubNavigationComponent.new(
+    items: secondary_items(request.path, @organisation.id),
+  ) %>
+  <h2 class="govuk-visually-hidden">Logs</h2>
+<% end %>
+
 <%= render DataProtectionConfirmationBannerComponent.new(
   user: current_user,
   organisation: @organisation,
-) %>
+  ) %>
 
 <% if @duplicate_sets_count&.positive? %>
   <%= govuk_notification_banner(title_text: "Important", text: govuk_link_to("Review logs", organisation_duplicates_path(@organisation, referrer: "duplicate_logs_banner"))) do |banner| %>
@@ -16,13 +23,6 @@
   <% end %>
 <% end %>
 
-<% if current_user.support? %>
-  <%= render SubNavigationComponent.new(
-    items: secondary_items(request.path, @organisation.id),
-  ) %>
-  <h2 class="govuk-visually-hidden">Logs</h2>
-<% end %>
-
 <div class="app-filter-layout" data-controller="filter-layout">
   <%= render partial: "logs/create_for_org_actions" %>
 
diff --git a/config/routes.rb b/config/routes.rb
index de9c071b9..052570cb1 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -122,7 +122,6 @@ Rails.application.routes.draw do
   end
 
   resources :organisations do
-    resources :users
     get "duplicates", to: "duplicate_logs#index"
 
     member do