diff --git a/app/controllers/admin/navigation_links_controller.rb b/app/controllers/admin/navigation_links_controller.rb
index 85942c592..598590dae 100644
--- a/app/controllers/admin/navigation_links_controller.rb
+++ b/app/controllers/admin/navigation_links_controller.rb
@@ -2,7 +2,7 @@ module Admin
class NavigationLinksController < Admin::ApplicationController
after_action :bust_content_change_caches, only: %i[create update destroy]
ALLOWED_PARAMS = %i[
- name url icon display_only_when_signed_in position section
+ name url icon display_to position section
].freeze
layout "admin"
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 3b22823c4..0a179c915 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -36,9 +36,11 @@ module ApplicationHelper
# @return [TrueClass] true when we should render the given link.
# @return [FalseClass] false when we should **not** render the given link.
def display_navigation_link?(link:)
- # This is a quick short-circuit; we already have the link. So don't bother asking the "Is this
- # feature enabled" question if the given link requires a sign in and the user is not signed in.
- return false if link.display_only_when_signed_in? && !user_signed_in?
+ # This is a quick short-circuit; we already have the link. So don't bother asking the "Is this
+ # feature enabled" question if the given link requires a logged in/out state
+ # that doesn't match the user's current state.
+ return false if link.display_to_logged_in? && !user_signed_in?
+ return false if link.display_to_logged_out? && user_signed_in?
return true if navigation_link_is_for_an_enabled_feature?(link: link)
false
diff --git a/app/models/navigation_link.rb b/app/models/navigation_link.rb
index 445526731..97f648fe8 100644
--- a/app/models/navigation_link.rb
+++ b/app/models/navigation_link.rb
@@ -5,6 +5,7 @@ class NavigationLink < ApplicationRecord
before_save :strip_local_hostname, if: :url?
enum section: { default: 0, other: 1 }, _suffix: true
+ enum display_to: { all: 0, logged_in: 1, logged_out: 2 }, _prefix: true
validates :name, :url, :icon, presence: true
validates :url, url: { schemes: %w[https http] }, uniqueness: { scope: :name }
diff --git a/app/views/admin/navigation_links/_form.html.erb b/app/views/admin/navigation_links/_form.html.erb
index e7723d2ef..1e8898571 100644
--- a/app/views/admin/navigation_links/_form.html.erb
+++ b/app/views/admin/navigation_links/_form.html.erb
@@ -49,11 +49,23 @@
Determines the order placement of the links in the sidebar
- <%= form.check_box :display_only_when_signed_in, class: "crayons-checkbox" %>
- <%= form.label :display_only_when_signed_in, class: "crayons-field__label" do %>
- Display only when signed in
-
Determines whether the link should only be shown after the user has signed into their account.
- <% end %>
+
+
diff --git a/db/migrate/20220804135751_add_display_to_to_navigation_links.rb b/db/migrate/20220804135751_add_display_to_to_navigation_links.rb
new file mode 100644
index 000000000..91dc04a17
--- /dev/null
+++ b/db/migrate/20220804135751_add_display_to_to_navigation_links.rb
@@ -0,0 +1,5 @@
+class AddDisplayToToNavigationLinks < ActiveRecord::Migration[7.0]
+ def change
+ add_column :navigation_links, :display_to, :integer, default: 0, null: false
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 328ded446..e75424dff 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema[7.0].define(version: 2022_06_07_133847) do
+ActiveRecord::Schema[7.0].define(version: 2022_08_04_135751) do
# These are extensions that must be enabled in order to support this database
enable_extension "citext"
enable_extension "pg_stat_statements"
@@ -634,6 +634,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_06_07_133847) do
create_table "navigation_links", force: :cascade do |t|
t.datetime "created_at", null: false
t.boolean "display_only_when_signed_in", default: false
+ t.integer "display_to", default: 0, null: false
t.string "icon", null: false
t.string "name", null: false
t.integer "position"
diff --git a/lib/data_update_scripts/20220804135957_backfill_navigation_links_column_display_to.rb b/lib/data_update_scripts/20220804135957_backfill_navigation_links_column_display_to.rb
new file mode 100644
index 000000000..8fa6ea0ef
--- /dev/null
+++ b/lib/data_update_scripts/20220804135957_backfill_navigation_links_column_display_to.rb
@@ -0,0 +1,7 @@
+module DataUpdateScripts
+ class BackfillNavigationLinksColumnDisplayTo
+ def run
+ NavigationLink.where(display_only_when_signed_in: true).update(display_to: "logged_in")
+ end
+ end
+end
diff --git a/lib/tasks/add_navigation_links.rake b/lib/tasks/add_navigation_links.rake
index 19c268e1a..746bba4d0 100644
--- a/lib/tasks/add_navigation_links.rake
+++ b/lib/tasks/add_navigation_links.rake
@@ -59,7 +59,7 @@ namespace :navigation_links do
name: "Home",
url: URL.url("/"),
icon: home_icon,
- display_only_when_signed_in: false,
+ display_to: :all,
position: 1,
section: :default,
)
@@ -70,7 +70,7 @@ namespace :navigation_links do
url: URL.url("readinglist"),
name: "Reading List",
icon: reading_icon,
- display_only_when_signed_in: true,
+ display_to: :logged_in,
position: 2,
section: :default,
)
@@ -81,7 +81,7 @@ namespace :navigation_links do
name: "Contact",
url: URL.url("contact"),
icon: contact_icon,
- display_only_when_signed_in: false,
+ display_to: :all,
position: 3,
section: :default,
)
@@ -92,7 +92,7 @@ namespace :navigation_links do
name: "Code of Conduct",
url: URL.url(Page::CODE_OF_CONDUCT_SLUG),
icon: thumb_up_icon,
- display_only_when_signed_in: false,
+ display_to: :all,
position: 1,
section: :other,
)
@@ -103,7 +103,7 @@ namespace :navigation_links do
name: "Privacy Policy",
url: URL.url(Page::PRIVACY_SLUG),
icon: smart_icon,
- display_only_when_signed_in: false,
+ display_to: :all,
position: 2,
section: :other,
)
@@ -114,7 +114,7 @@ namespace :navigation_links do
name: "Terms of Use",
url: URL.url(Page::TERMS_SLUG),
icon: look_icon,
- display_only_when_signed_in: false,
+ display_to: :all,
position: 3,
section: :other,
)
@@ -131,7 +131,7 @@ namespace :navigation_links do
url: "/",
name: "Home",
icon: home_icon,
- display_only_when_signed_in: false,
+ display_to: :all,
position: 1,
section: :default,
)
@@ -140,7 +140,7 @@ namespace :navigation_links do
url: "#{base_url}/readinglist",
name: "Reading List",
icon: reading_icon,
- display_only_when_signed_in: true,
+ display_to: :logged_in,
position: 2,
section: :default,
)
@@ -148,7 +148,7 @@ namespace :navigation_links do
url: "#{base_url}/listings",
name: "Listings",
icon: listing_icon,
- display_only_when_signed_in: false,
+ display_to: :all,
position: 3,
section: :default,
)
@@ -156,7 +156,7 @@ namespace :navigation_links do
url: "#{base_url}/pod",
name: "Podcasts",
icon: mic_icon,
- display_only_when_signed_in: false,
+ display_to: :all,
position: 4,
section: :default,
)
@@ -164,7 +164,7 @@ namespace :navigation_links do
url: "#{base_url}/videos",
name: "Videos",
icon: camera_icon,
- display_only_when_signed_in: false,
+ display_to: :all,
position: 5,
section: :default,
)
@@ -172,7 +172,7 @@ namespace :navigation_links do
url: "#{base_url}/tags",
name: "Tags",
icon: tag_icon,
- display_only_when_signed_in: false,
+ display_to: :all,
position: 6,
section: :default,
)
@@ -180,7 +180,7 @@ namespace :navigation_links do
url: "#{base_url}/faq",
name: "FAQ",
icon: bulb_icon,
- display_only_when_signed_in: false,
+ display_to: :all,
position: 7,
section: :default,
)
@@ -188,7 +188,7 @@ namespace :navigation_links do
url: "https://shop.dev.to/",
name: "DEV Shop",
icon: shopping_icon,
- display_only_when_signed_in: false,
+ display_to: :all,
position: 6,
section: :default,
)
@@ -196,7 +196,7 @@ namespace :navigation_links do
url: "#{base_url}/sponsors",
name: "Sponsors",
icon: heart_icon,
- display_only_when_signed_in: false,
+ display_to: :all,
position: 7,
section: :default,
)
@@ -204,7 +204,7 @@ namespace :navigation_links do
url: "#{base_url}/about",
name: "About",
icon: rainbowdev,
- display_only_when_signed_in: false,
+ display_to: :all,
position: 8,
section: :default,
)
@@ -212,7 +212,7 @@ namespace :navigation_links do
url: "#{base_url}/contact",
name: "Contact",
icon: contact_icon,
- display_only_when_signed_in: false,
+ display_to: :all,
position: 9,
section: :default,
)
@@ -221,7 +221,7 @@ namespace :navigation_links do
url: "#{base_url}/code-of-conduct",
name: "Code of Conduct",
icon: thumb_up_icon,
- display_only_when_signed_in: false,
+ display_to: :all,
position: 1,
section: :other,
)
@@ -230,7 +230,7 @@ namespace :navigation_links do
url: "#{base_url}/privacy",
name: "Privacy Policy",
icon: smart_icon,
- display_only_when_signed_in: false,
+ display_to: :all,
position: 2,
section: :other,
)
@@ -238,7 +238,7 @@ namespace :navigation_links do
url: "#{base_url}/terms",
name: "Terms of Use",
icon: look_icon,
- display_only_when_signed_in: false,
+ display_to: :all,
position: 3,
section: :other,
)
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index 25608b2ae..6b9ed530e 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -31,7 +31,7 @@ RSpec.describe ApplicationHelper, type: :helper do
describe "#display_navigation_link?" do
subject(:method_call) { helper.display_navigation_link?(link: link) }
- let(:link) { build(:navigation_link, display_only_when_signed_in: display_only_when_signed_in) }
+ let(:link) { build(:navigation_link, display_to: display_to) }
before do
allow(helper).to receive(:user_signed_in?).and_return(user_signed_in)
@@ -42,14 +42,14 @@ RSpec.describe ApplicationHelper, type: :helper do
context "when user signed in and link requires signin and feature enabled" do
let(:navigation_link_is_for_an_enabled_feature) { true }
- let(:display_only_when_signed_in) { true }
+ let(:display_to) { :logged_in }
let(:user_signed_in) { true }
it { is_expected.to be_truthy }
end
context "when user signed in and link requires signin and feature disabled" do
- let(:display_only_when_signed_in) { false }
+ let(:display_to) { :all }
let(:user_signed_in) { true }
let(:navigation_link_is_for_an_enabled_feature) { false }
@@ -58,15 +58,31 @@ RSpec.describe ApplicationHelper, type: :helper do
context "when user signed in and link **does not** require signin and feature enabled" do
let(:navigation_link_is_for_an_enabled_feature) { true }
- let(:display_only_when_signed_in) { false }
+ let(:display_to) { :all }
let(:user_signed_in) { true }
it { is_expected.to be_truthy }
end
+ context "when user signed in and link requires signout and feature enabled" do
+ let(:navigation_link_is_for_an_enabled_feature) { true }
+ let(:display_to) { :logged_out }
+ let(:user_signed_in) { true }
+
+ it { is_expected.to be_falsey }
+ end
+
+ context "when user signed in and link requires signout and feature disabled" do
+ let(:navigation_link_is_for_an_enabled_feature) { false }
+ let(:display_to) { :logged_out }
+ let(:user_signed_in) { true }
+
+ it { is_expected.to be_falsey }
+ end
+
context "when user signed in and link **does not** require signin and feature disabled" do
let(:navigation_link_is_for_an_enabled_feature) { false }
- let(:display_only_when_signed_in) { false }
+ let(:display_to) { :all }
let(:user_signed_in) { true }
it { is_expected.to be_falsey }
@@ -74,7 +90,7 @@ RSpec.describe ApplicationHelper, type: :helper do
context "when user **not** signed in and link requires signin and feature enabled" do
let(:navigation_link_is_for_an_enabled_feature) { true }
- let(:display_only_when_signed_in) { true }
+ let(:display_to) { :logged_in }
let(:user_signed_in) { false }
it { is_expected.to be_falsey }
@@ -82,7 +98,7 @@ RSpec.describe ApplicationHelper, type: :helper do
context "when user **not** signed in and link **does not** require signin and feature enabled" do
let(:navigation_link_is_for_an_enabled_feature) { true }
- let(:display_only_when_signed_in) { false }
+ let(:display_to) { :all }
let(:user_signed_in) { false }
it { is_expected.to be_truthy }
@@ -90,7 +106,23 @@ RSpec.describe ApplicationHelper, type: :helper do
context "when user **not** signed in and link **does not** require signin and feature disabled" do
let(:navigation_link_is_for_an_enabled_feature) { false }
- let(:display_only_when_signed_in) { false }
+ let(:display_to) { :all }
+ let(:user_signed_in) { false }
+
+ it { is_expected.to be_falsey }
+ end
+
+ context "when user **not** signed in and link requires signout and feature enabled" do
+ let(:navigation_link_is_for_an_enabled_feature) { true }
+ let(:display_to) { :logged_out }
+ let(:user_signed_in) { false }
+
+ it { is_expected.to be_truthy }
+ end
+
+ context "when user **not** signed in and link requires signout and feature disabled" do
+ let(:navigation_link_is_for_an_enabled_feature) { false }
+ let(:display_to) { :logged_out }
let(:user_signed_in) { false }
it { is_expected.to be_falsey }
diff --git a/spec/lib/data_update_scripts/backfill_navigation_links_column_display_to_spec.rb b/spec/lib/data_update_scripts/backfill_navigation_links_column_display_to_spec.rb
new file mode 100644
index 000000000..bea2ea406
--- /dev/null
+++ b/spec/lib/data_update_scripts/backfill_navigation_links_column_display_to_spec.rb
@@ -0,0 +1,24 @@
+require "rails_helper"
+require Rails.root.join(
+ "lib/data_update_scripts/20220804135957_backfill_navigation_links_column_display_to.rb",
+)
+
+describe DataUpdateScripts::BackfillNavigationLinksColumnDisplayTo do
+ it "backfills relevant navigation links with 'logged_in' value for display_to" do
+ logged_in_navigation_link = create(:navigation_link, url: "/privacy", display_only_when_signed_in: true,
+ display_to: "all")
+
+ expect do
+ described_class.new.run
+ end.to change { logged_in_navigation_link.reload.display_to }.from("all").to("logged_in")
+ end
+
+ it "leaves visible-to-all navigation links unchanged" do
+ visible_to_all_navigation_link = create(:navigation_link, url: "/readinglist", display_only_when_signed_in: false,
+ display_to: "all")
+
+ expect do
+ described_class.new.run
+ end.not_to change { visible_to_all_navigation_link.reload.display_to }
+ end
+end
diff --git a/spec/support/seeds/seeds_e2e.rb b/spec/support/seeds/seeds_e2e.rb
index c9bd1a826..64b3e8434 100644
--- a/spec/support/seeds/seeds_e2e.rb
+++ b/spec/support/seeds/seeds_e2e.rb
@@ -530,7 +530,7 @@ seeder.create_if_none(NavigationLink) do
name: "Reading List",
url: "#{base_url}/readinglist",
icon: reading_icon,
- display_only_when_signed_in: true,
+ display_to: :logged_in,
position: 0,
section: :default,
)
diff --git a/spec/system/homepage/user_visits_homepage_spec.rb b/spec/system/homepage/user_visits_homepage_spec.rb
index fdc7d1ec9..1dae84a70 100644
--- a/spec/system/homepage/user_visits_homepage_spec.rb
+++ b/spec/system/homepage/user_visits_homepage_spec.rb
@@ -39,24 +39,24 @@ RSpec.describe "User visits a homepage", type: :system do
create(:navigation_link,
name: "Listings",
icon: "",
- display_only_when_signed_in: true,
+ display_to: :logged_in,
position: 1)
create(:navigation_link,
name: "Shop",
icon: "",
- display_only_when_signed_in: false,
+ display_to: :all,
position: 2)
create(:navigation_link,
:other_section_link,
name: "Podcasts",
icon: "",
- display_only_when_signed_in: false,
+ display_to: :all,
position: nil)
create(:navigation_link,
:other_section_link,
name: "Privacy Policy",
icon: "",
- display_only_when_signed_in: false,
+ display_to: :all,
position: 1)
visit "/"
end
@@ -82,7 +82,7 @@ RSpec.describe "User visits a homepage", type: :system do
expect(page).not_to have_selector("nav[data-testid='other-nav']")
end
- it "hides link when display_only_when_signed_in is true" do
+ it "hides link when display_to is set to logged in users only" do
within("nav[data-testid='main-nav']", match: :first) do
expect(page).to have_selector(".default-navigation-links .sidebar-navigation-link", count: 1)
end
@@ -92,7 +92,7 @@ RSpec.describe "User visits a homepage", type: :system do
create(:navigation_link,
name: "Mock",
icon: "",
- display_only_when_signed_in: false,
+ display_to: :all,
position: 3)
visit "/"
@@ -167,7 +167,7 @@ RSpec.describe "User visits a homepage", type: :system do
name: "Reading List",
url: app_url("readinglist").to_s,
icon: "",
- display_only_when_signed_in: false,
+ display_to: :all,
position: 1)
end
let!(:navigation_link_2) do
@@ -175,14 +175,14 @@ RSpec.describe "User visits a homepage", type: :system do
:other_section_link,
name: "Podcasts",
icon: "",
- display_only_when_signed_in: false,
+ display_to: :all,
position: nil)
end
let!(:navigation_link_3) do
create(:navigation_link,
name: "Beauty",
icon: "",
- display_only_when_signed_in: true,
+ display_to: :logged_in,
position: nil)
end
@@ -218,7 +218,7 @@ RSpec.describe "User visits a homepage", type: :system do
end
end
- it "shows link when display_only_when_signed_in is true" do
+ it "shows link when display_to is set to logged_in" do
within("nav[data-testid='main-nav']", match: :first) do
expect(page).to have_selector(".default-navigation-links li:nth-child(2)", text: "Beauty")
end
diff --git a/spec/tasks/add_navigation_links_spec.rb b/spec/tasks/add_navigation_links_spec.rb
index acd957fdd..4b603ee13 100644
--- a/spec/tasks/add_navigation_links_spec.rb
+++ b/spec/tasks/add_navigation_links_spec.rb
@@ -17,7 +17,7 @@ RSpec.describe "Navigation Links tasks", type: :task do
create(:navigation_link,
name: "Reading List",
url: URL.url("readinglist"),
- display_only_when_signed_in: true,
+ display_to: :logged_in,
position: 0)
end