maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Add "only display when logged out" as an option on Navigation Links (#18275)

Browse source 
* add new column with enum value

* refactor to use display_to column

* add some more specs to application_helper

* test for the backfill DUS

* fix line length

* refactor
This commit is contained in:
Suzanne Aitchison 2022-08-08 10:55:24 +01:00 • committed by GitHub
parent 6ff7e26ea8
commit f1df37f431
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 135 additions and 51 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/controllers/admin/navigation_links_controller.rb
View file

@ -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"

8
app/helpers/application_helper.rb
View file

@ -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

1
app/models/navigation_link.rb
View file

@ -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 }

24
app/views/admin/navigation_links/_form.html.erb
View file

@ -49,11 +49,23 @@
<p class="crayons-field__description">Determines the order placement of the links in the sidebar</p>
<%= form.number_field :position, class: "crayons-textfield" %>
</div>
<div class="crayons-field crayons-field--checkbox">
<%= 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
<p class="crayons-field__description">Determines whether the link should only be shown after the user has signed into their account.</p>
<% end %>
<div class="crayons-field mb-2">
<fieldset aria-describedby="section-description" aria-describedby="display-to-description">
<legend class="crayons-field__label pl-0">User group</legend>
<p id="display-to-description" class="crayons-field__description mb-2">Determines which users will be able to see the link</p>
<label class="rich-selector crayons-field crayons-field--radio">
<%= form.radio_button :display_to, "all", class: "crayons-radio" %>
<div class="crayons-field__label">All users</div>
</label>
<label class="rich-selector crayons-field crayons-field--radio">
<%= form.radio_button :display_to, "logged_in", class: "crayons-radio" %>
<div class="crayons-field__label">Only logged in users</div>
</label>
<label class="rich-selector crayons-field crayons-field--radio">
<%= form.radio_button :display_to, "logged_out", class: "crayons-radio" %>
<div class="crayons-field__label">Only logged out users</div>
</label>
</fieldset>
</div>
</div>

5
db/migrate/20220804135751_add_display_to_to_navigation_links.rb Normal file
View file

@ -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

3
db/schema.rb
View file

@ -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"

7
lib/data_update_scripts/20220804135957_backfill_navigation_links_column_display_to.rb Normal file
View file

@ -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

40
lib/tasks/add_navigation_links.rake
View file

@ -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,
)

48
spec/helpers/application_helper_spec.rb
View file

@ -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 }

24
spec/lib/data_update_scripts/backfill_navigation_links_column_display_to_spec.rb Normal file
View file

@ -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

2
spec/support/seeds/seeds_e2e.rb
View file

@ -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,
)

20
spec/system/homepage/user_visits_homepage_spec.rb
View file

@ -39,24 +39,24 @@ RSpec.describe "User visits a homepage", type: :system do
create(:navigation_link,
name: "Listings",
icon: "<svg xmlns='http://www.w3.org/2000/svg'/></svg>",
display_only_when_signed_in: true,
display_to: :logged_in,
position: 1)
create(:navigation_link,
name: "Shop",
icon: "<svg xmlns='http://www.w3.org/2000/svg'/></svg>",
display_only_when_signed_in: false,
display_to: :all,
position: 2)
create(:navigation_link,
:other_section_link,
name: "Podcasts",
icon: "<svg xmlns='http://www.w3.org/2000/svg'/></svg>",
display_only_when_signed_in: false,
display_to: :all,
position: nil)
create(:navigation_link,
:other_section_link,
name: "Privacy Policy",
icon: "<svg xmlns='http://www.w3.org/2000/svg'/></svg>",
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: "<svg xmlns='http://www.w3.org/2000/svg'/></svg>",
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: "<svg xmlns='http://www.w3.org/2000/svg'/></svg>",
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: "<svg xmlns='http://www.w3.org/2000/svg'/></svg>",
display_only_when_signed_in: false,
display_to: :all,
position: nil)
end
let!(:navigation_link_3) do
create(:navigation_link,
name: "Beauty",
icon: "<svg xmlns='http://www.w3.org/2000/svg'/></svg>",
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

2
spec/tasks/add_navigation_links_spec.rb
View file

@ -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