diff --git a/app/models/navigation_link.rb b/app/models/navigation_link.rb index f5f66a546..b12c5f05e 100644 --- a/app/models/navigation_link.rb +++ b/app/models/navigation_link.rb @@ -13,6 +13,41 @@ class NavigationLink < ApplicationRecord scope :ordered, -> { order(position: :asc, name: :asc) } + # With the given :url either create a NavigationLink or update an existing NavigationLink with the + # given :attributes. + # + # @param url [String] the URL of the navigation link. + # @param name [String] the name of the navigation link. + # + # @param attributes [Hash] the attributes to create or update the given :url. + # Note, this assumes hash keys. + # + # @return [NavigationLink] + # + # @note In constructing this function, the named args (e.g., :url and :name) are what we check for + # "equality" with other NavigationLinks. We can say that two NavigationLink object's that + # have the same :name and :url are the same. This way, when we update the NavigationLink's + # position we ensure that we're not create new records (as per the past `first_or_create` + # methodology.) + def self.create_or_update_by_identity(url:, name:, **attributes) + find_or_initialize_by(url: normalize_url(url), name: name).update(attributes) + end + + # A helper function to ensure that we're normalizing URLs that we store (or query from storage). + # + # @param url [String] + # + # @return [String] + # + # @see NavigationLink.create_or_update_by_identity + # @see NavigationLink#strip_local_hostname + def self.normalize_url(url) + parsed_url = URI.parse(url) + return url unless url.match?(/^#{URL.url}/i) + + parsed_url.path + end + private # We want to allow relative URLs (e.g. /contact) for navigation links while @@ -27,9 +62,6 @@ class NavigationLink < ApplicationRecord # When persisting to the database we store local links as relative URLs which # makes it easier to switch from a forem.cloud subdomain to the live domain. def strip_local_hostname - parsed_url = URI.parse(url) - return unless url.match?(/^#{URL.url}/i) - - self.url = parsed_url.path + self.url = self.class.normalize_url(url) end end diff --git a/lib/tasks/add_navigation_links.rake b/lib/tasks/add_navigation_links.rake index 8dbd1c347..7b78b3143 100644 --- a/lib/tasks/add_navigation_links.rake +++ b/lib/tasks/add_navigation_links.rake @@ -53,56 +53,56 @@ namespace :navigation_links do namespace :find_or_create do task readinglist: :environment do - NavigationLink.where(url: "/readinglist").first_or_create( - name: "Reading List", + NavigationLink.create_or_update_by_identity( url: URL.url("readinglist"), + name: "Reading List", icon: reading_icon, display_only_when_signed_in: true, - position: 0, + position: 1, section: :default, ) end task contact: :environment do - NavigationLink.where(url: "/contact").first_or_create( + NavigationLink.create_or_update_by_identity( name: "Contact", url: URL.url("contact"), icon: contact_icon, display_only_when_signed_in: false, - position: 1, + position: 2, section: :default, ) end task code_of_conduct: :environment do - NavigationLink.where(url: "/code-of-conduct").first_or_create( + NavigationLink.create_or_update_by_identity( name: "Code of Conduct", url: URL.url(Page::CODE_OF_CONDUCT_SLUG), icon: thumb_up_icon, display_only_when_signed_in: false, - position: 0, - section: :other, - ) - end - - task privacy: :environment do - NavigationLink.where(url: "/privacy").first_or_create( - name: "Privacy Policy", - url: URL.url(Page::PRIVACY_SLUG), - icon: smart_icon, - display_only_when_signed_in: false, position: 1, section: :other, ) end + task privacy: :environment do + NavigationLink.create_or_update_by_identity( + name: "Privacy Policy", + url: URL.url(Page::PRIVACY_SLUG), + icon: smart_icon, + display_only_when_signed_in: false, + position: 2, + section: :other, + ) + end + task terms: :environment do - NavigationLink.where(url: "/terms").first_or_create( + NavigationLink.create_or_update_by_identity( name: "Terms of Use", url: URL.url(Page::TERMS_SLUG), icon: look_icon, display_only_when_signed_in: false, - position: 2, + position: 3, section: :other, ) end @@ -114,97 +114,112 @@ namespace :navigation_links do domain = Rails.application&.initialized? ? Settings::General.app_domain : ApplicationConfig["APP_DOMAIN"] base_url = "#{protocol}#{domain}".freeze - NavigationLink.where(url: "#{base_url}/readinglist").first_or_create( + NavigationLink.create_or_update_by_identity( + url: "#{base_url}/readinglist", name: "Reading List", icon: reading_icon, display_only_when_signed_in: true, - position: 0, - section: :default, - ) - NavigationLink.where(url: "#{base_url}/listings").first_or_create( - name: "Listings", - icon: listing_icon, - display_only_when_signed_in: false, - position: 1, - section: :default, - ) - NavigationLink.where(url: "#{base_url}/pod").first_or_create( - name: "Podcasts", - icon: mic_icon, - display_only_when_signed_in: false, position: 2, section: :default, ) - NavigationLink.where(url: "#{base_url}/videos").first_or_create( - name: "Videos", - icon: camera_icon, + NavigationLink.create_or_update_by_identity( + url: "#{base_url}/listings", + name: "Listings", + icon: listing_icon, display_only_when_signed_in: false, position: 3, section: :default, ) - NavigationLink.where(url: "#{base_url}/tags").first_or_create( - name: "Tags", - icon: tag_icon, + NavigationLink.create_or_update_by_identity( + url: "#{base_url}/pod", + name: "Podcasts", + icon: mic_icon, display_only_when_signed_in: false, position: 4, section: :default, ) - NavigationLink.where(url: "#{base_url}/code-of-conduct").first_or_create( - name: "Code of Conduct", - icon: thumb_up_icon, - display_only_when_signed_in: false, - position: 0, - section: :other, - ) - NavigationLink.where(url: "#{base_url}/faq").first_or_create( - name: "FAQ", - icon: bulb_icon, + NavigationLink.create_or_update_by_identity( + url: "#{base_url}/videos", + name: "Videos", + icon: camera_icon, display_only_when_signed_in: false, position: 5, section: :default, ) - NavigationLink.where(url: "https://shop.dev.to/").first_or_create( + NavigationLink.create_or_update_by_identity( + url: "#{base_url}/tags", + name: "Tags", + icon: tag_icon, + display_only_when_signed_in: false, + position: 6, + section: :default, + ) + NavigationLink.create_or_update_by_identity( + url: "#{base_url}/faq", + name: "FAQ", + icon: bulb_icon, + display_only_when_signed_in: false, + position: 7, + section: :default, + ) + NavigationLink.create_or_update_by_identity( + url: "https://shop.dev.to/", name: "DEV Shop", icon: shopping_icon, display_only_when_signed_in: false, position: 6, section: :default, ) - NavigationLink.where(url: "#{base_url}/sponsors").first_or_create( + NavigationLink.create_or_update_by_identity( + url: "#{base_url}/sponsors", name: "Sponsors", icon: heart_icon, display_only_when_signed_in: false, position: 7, section: :default, ) - NavigationLink.where(url: "#{base_url}/about").first_or_create( + NavigationLink.create_or_update_by_identity( + url: "#{base_url}/about", name: "About", icon: rainbowdev, display_only_when_signed_in: false, position: 8, section: :default, ) - NavigationLink.where(url: "#{base_url}/privacy").first_or_create( - name: "Privacy Policy", - icon: smart_icon, - display_only_when_signed_in: false, - position: 1, - section: :other, - ) - NavigationLink.where(url: "#{base_url}/terms").first_or_create( - name: "Terms of Use", - icon: look_icon, - display_only_when_signed_in: false, - position: 2, - section: :other, - ) - NavigationLink.where(url: "#{base_url}/contact").first_or_create( + NavigationLink.create_or_update_by_identity( + url: "#{base_url}/contact", name: "Contact", icon: contact_icon, display_only_when_signed_in: false, position: 9, section: :default, ) + + NavigationLink.create_or_update_by_identity( + url: "#{base_url}/code-of-conduct", + name: "Code of Conduct", + icon: thumb_up_icon, + display_only_when_signed_in: false, + position: 1, + section: :other, + ) + + NavigationLink.create_or_update_by_identity( + url: "#{base_url}/privacy", + name: "Privacy Policy", + icon: smart_icon, + display_only_when_signed_in: false, + position: 2, + section: :other, + ) + NavigationLink.create_or_update_by_identity( + url: "#{base_url}/terms", + name: "Terms of Use", + icon: look_icon, + display_only_when_signed_in: false, + position: 3, + section: :other, + ) end end # rubocop:enable Metrics/BlockLength diff --git a/spec/models/navigation_link_spec.rb b/spec/models/navigation_link_spec.rb index 9cd815ac9..89bc67d99 100644 --- a/spec/models/navigation_link_spec.rb +++ b/spec/models/navigation_link_spec.rb @@ -3,6 +3,38 @@ require "rails_helper" RSpec.describe NavigationLink, type: :model do let(:navigation_link) { create(:navigation_link) } + describe ".create_or_update_by_identity" do + let(:attributes) { attributes_for(:navigation_link).except(:url, :id, :name).stringify_keys } + let(:name) { navigation_link.name } + + # I want an existing navigation link, but don't want to apply the `let!` to the declaration as + # that impacts tests in other describe blocks. + before { navigation_link } + + context "when the url already exists" do + let(:url) { navigation_link.url } + + it "updates the existing NavigationLink" do + expect do + described_class.create_or_update_by_identity(url: url, name: name, **attributes.symbolize_keys) + end.not_to change(described_class, :count) + + expect(navigation_link.reload.attributes.slice(*attributes.keys)).to eq(attributes) + end + end + + context "when the url does not exist" do + # Creating a different URL + let(:url) { "#{navigation_link.url}-404" } + + it "creates a new NavigationLink" do + expect do + described_class.create_or_update_by_identity(url: url, name: name, **attributes) + end.to change(described_class, :count).by(1) + end + end + end + describe "validations" do describe "presence validations" do it { is_expected.to validate_presence_of(:name) }