From f1864847e7435281152f44170553425f39d1c524 Mon Sep 17 00:00:00 2001
From: Suzanne Aitchison <suzanne@forem.com>
Date: Tue, 1 Feb 2022 09:27:09 +0000
Subject: [PATCH] Make home link an admin-customisable navigation link (#16268)

* move home link to a customisable navlink

* add trailing slash for path

* update specs

* replace positions, set home link to -1

* add update script for adding home navigation link

* update data update script
---
 app/views/layouts/_main_nav.html.erb          |  6 ----
 ...20126102900_create_home_navigation_link.rb | 20 ++++++++++++
 lib/tasks/add_navigation_links.rake           | 25 +++++++++++++--
 .../create_home_navigation_link_spec.rb       | 31 +++++++++++++++++++
 .../homepage/user_visits_homepage_spec.rb     |  6 ++--
 spec/tasks/add_navigation_links_spec.rb       |  4 +--
 6 files changed, 79 insertions(+), 13 deletions(-)
 create mode 100644 lib/data_update_scripts/20220126102900_create_home_navigation_link.rb
 create mode 100644 spec/lib/data_update_scripts/create_home_navigation_link_spec.rb

diff --git a/app/views/layouts/_main_nav.html.erb b/app/views/layouts/_main_nav.html.erb
index 8e032c0a1..71135488a 100644
--- a/app/views/layouts/_main_nav.html.erb
+++ b/app/views/layouts/_main_nav.html.erb
@@ -3,12 +3,6 @@
 
 <nav class="mb-4 <% unless user_signed_in? %>mt-4<% end %>" id="main-navigation" aria-label="<%= community_name %>">
   <ul class="default-navigation-links sidebar-navigation-links spec-sidebar-navigation-links">
-    <li>
-      <a href="<%= root_path %>" class="c-link c-link--block c-link--icon-left">
-        <%= crayons_icon_tag("twemoji/house", class: "c-link__icon", native: true, aria_hidden: true) %>
-        <%= t("views.main.home") %>
-      </a>
-    </li>
     <% default_nav_links.each do |link| %>
       <%= render "layouts/sidebar_nav_link", link: link %>
     <% end %>
diff --git a/lib/data_update_scripts/20220126102900_create_home_navigation_link.rb b/lib/data_update_scripts/20220126102900_create_home_navigation_link.rb
new file mode 100644
index 000000000..07fe220a3
--- /dev/null
+++ b/lib/data_update_scripts/20220126102900_create_home_navigation_link.rb
@@ -0,0 +1,20 @@
+module DataUpdateScripts
+  class CreateHomeNavigationLink
+    def run
+      NavigationLink.create_or_update_by_identity(
+        name: "Home",
+        url: URL.url("/"),
+        icon: File.read(Rails.root.join("app/assets/images/twemoji/house.svg")),
+        display_only_when_signed_in: false,
+        position: 1,
+        section: :default,
+      )
+
+      # Increment all other default section links to account for new home link, and update to be 1-based
+      NavigationLink.where(section: :default).find_each do |link|
+        new_position = link.position + 2
+        link.update_columns(position: new_position)
+      end
+    end
+  end
+end
diff --git a/lib/tasks/add_navigation_links.rake b/lib/tasks/add_navigation_links.rake
index 7b78b3143..ba6b1b45e 100644
--- a/lib/tasks/add_navigation_links.rake
+++ b/lib/tasks/add_navigation_links.rake
@@ -8,6 +8,7 @@ namespace :navigation_links do
     image_path("twemoji", name)
   end
 
+  home_icon = twemoji_path("house.svg")
   reading_icon = twemoji_path("drawer.svg")
   contact_icon = twemoji_path("contact.svg")
   thumb_up_icon = twemoji_path("thumb-up.svg")
@@ -40,6 +41,7 @@ namespace :navigation_links do
       # [@jeremyf] I went ahead and atomized these tasks so we _could_ call them individually if
       #            desired.  I did not add descriptions so those tasks will not show up in the task
       #            list.
+      Rake::Task["navigation_links:find_or_create:home"].invoke
       Rake::Task["navigation_links:find_or_create:readinglist"].invoke
       Rake::Task["navigation_links:find_or_create:contact"].invoke
       Rake::Task["navigation_links:find_or_create:code_of_conduct"].invoke
@@ -52,13 +54,24 @@ namespace :navigation_links do
   end
 
   namespace :find_or_create do
+    task home: :environment do
+      NavigationLink.create_or_update_by_identity(
+        name: "Home",
+        url: URL.url("/"),
+        icon: home_icon,
+        display_only_when_signed_in: false,
+        position: 1,
+        section: :default,
+      )
+    end
+
     task readinglist: :environment do
       NavigationLink.create_or_update_by_identity(
         url: URL.url("readinglist"),
         name: "Reading List",
         icon: reading_icon,
         display_only_when_signed_in: true,
-        position: 1,
+        position: 2,
         section: :default,
       )
     end
@@ -69,7 +82,7 @@ namespace :navigation_links do
         url: URL.url("contact"),
         icon: contact_icon,
         display_only_when_signed_in: false,
-        position: 2,
+        position: 3,
         section: :default,
       )
     end
@@ -114,6 +127,14 @@ namespace :navigation_links do
     domain = Rails.application&.initialized? ? Settings::General.app_domain : ApplicationConfig["APP_DOMAIN"]
     base_url = "#{protocol}#{domain}".freeze
 
+    NavigationLink.create_or_update_by_identity(
+      name: "Home",
+      icon: home_icon,
+      display_only_when_signed_in: false,
+      position: 1,
+      section: :default,
+    )
+
     NavigationLink.create_or_update_by_identity(
       url: "#{base_url}/readinglist",
       name: "Reading List",
diff --git a/spec/lib/data_update_scripts/create_home_navigation_link_spec.rb b/spec/lib/data_update_scripts/create_home_navigation_link_spec.rb
new file mode 100644
index 000000000..21ce5c27c
--- /dev/null
+++ b/spec/lib/data_update_scripts/create_home_navigation_link_spec.rb
@@ -0,0 +1,31 @@
+require "rails_helper"
+require Rails.root.join(
+  "lib/data_update_scripts/20220126102900_create_home_navigation_link.rb",
+)
+
+describe DataUpdateScripts::CreateHomeNavigationLink do
+  it "creates a home navigation link when it doesn't already exist" do
+    expect do
+      described_class.new.run
+    end.to change(NavigationLink, :count).by(1)
+  end
+
+  it "skips home navigation link creation if already exists" do
+    create(:navigation_link, url: "/", name: "Home")
+    expect do
+      described_class.new.run
+    end.not_to change(NavigationLink, :count)
+  end
+
+  it "updates the position of other default navigation links" do
+    link = create(:navigation_link, url: "/example", name: "Example", section: :default, position: 0)
+    described_class.new.run
+    expect(link.reload.position).to eq(2)
+  end
+
+  it "doesn't update the position of other navigation links" do
+    link = create(:navigation_link, url: "/example", name: "Example", section: :other, position: 0)
+    described_class.new.run
+    expect(link.reload.position).to eq(0)
+  end
+end
diff --git a/spec/system/homepage/user_visits_homepage_spec.rb b/spec/system/homepage/user_visits_homepage_spec.rb
index aa05d0200..b13bf3f0a 100644
--- a/spec/system/homepage/user_visits_homepage_spec.rb
+++ b/spec/system/homepage/user_visits_homepage_spec.rb
@@ -97,8 +97,8 @@ RSpec.describe "User visits a homepage", type: :system do
         visit "/"
 
         within("#main-navigation", match: :first) do
-          expect(page).to have_selector(".default-navigation-links li:nth-child(2)", text: "Shop")
-          expect(page).to have_selector(".default-navigation-links li:nth-child(3)", text: "Mock")
+          expect(page).to have_selector(".default-navigation-links li:nth-child(1)", text: "Shop")
+          expect(page).to have_selector(".default-navigation-links li:nth-child(2)", text: "Mock")
         end
       end
     end
@@ -220,7 +220,7 @@ RSpec.describe "User visits a homepage", type: :system do
 
       it "shows link when display_only_when_signed_in is true" do
         within("#main-navigation", match: :first) do
-          expect(page).to have_selector(".default-navigation-links li:nth-child(3)", text: "Beauty")
+          expect(page).to have_selector(".default-navigation-links li:nth-child(2)", text: "Beauty")
         end
       end
     end
diff --git a/spec/tasks/add_navigation_links_spec.rb b/spec/tasks/add_navigation_links_spec.rb
index 758fa1201..acd957fdd 100644
--- a/spec/tasks/add_navigation_links_spec.rb
+++ b/spec/tasks/add_navigation_links_spec.rb
@@ -8,7 +8,7 @@ RSpec.describe "Navigation Links tasks", type: :task do
 
   describe "#create" do
     it "creates navigation links for new forem if nonexistent" do
-      expect { Rake::Task["navigation_links:create"].invoke }.to change(NavigationLink, :count).by(5)
+      expect { Rake::Task["navigation_links:create"].invoke }.to change(NavigationLink, :count).by(6)
     end
 
     context "when navigation links exist" do
@@ -22,7 +22,7 @@ RSpec.describe "Navigation Links tasks", type: :task do
       end
 
       it "does not create nav links if they already exist" do
-        expect { Rake::Task["navigation_links:create"].invoke }.to change(NavigationLink, :count).from(1).to(5)
+        expect { Rake::Task["navigation_links:create"].invoke }.to change(NavigationLink, :count).from(1).to(6)
       end
     end
   end