Redirect normal ID to hashid equivalent (joemasilotti#667)

* update sitemap developer page to use hashid

* add en translation for redirection

* handle redirect for normal ID on developers show page

* update test to redirect to show page with hashid

* update sitemap using hashid if Feature is enabled

* use shorthand hash value notation

* extract developer finder functionality to custom object

* use Developers::Finder

* remove duplicate test

* improve logic of @developer and @reDIrect and test

* rename test

* use plain assert and refute

Author: rayhanw

app/controllers/developers_controller.rb

@@ -41,7 +41,13 @@ def update
   end
 
   def show
-    @developer = find_developer!
+    finder = Developers::Finder.new(id: params[:id])
+    @developer = finder.developer
+
+    if finder.should_redirect?
+      redirect_to @developer, status: 302, notice: t(".redirection", url: developer_url(@developer))
+    end
+
     authorize @developer
   end
 

app/models/developers/finder.rb

@@ -0,0 +1,43 @@
+module Developers
+  class Finder
+    NON_ID_PATTERN = /\D+/
+
+    def initialize(id:)
+      @id = id.to_s
+      @redirect = false
+    end
+
+    def developer
+      find_developer unless defined?(@developer)
+      @developer
+    end
+
+    def should_redirect?
+      find_developer unless defined?(@developer)
+      @redirect
+    end
+
+    private
+
+    def find_by_id!
+      @developer = Developer.find(@id)
+    end
+
+    def use_hashid?
+      Feature.enabled?(:obfuscate_developer_urls)
+    end
+
+    def find_developer
+      if use_hashid?
+        if NON_ID_PATTERN.match?(@id)
+          @developer = Developer.find_by_hashid!(@id)
+        else
+          find_by_id!
+          @redirect = true
+        end
+      else
+        find_by_id!
+      end
+    end
+  end
+end

config/locales/en.yml

@@ -393,6 +393,7 @@ en:
       edit: Edit
       hire: Hire me
       message: Message
+      redirection: Links to developer profiles have changed. Please update your bookmark to %{url}.
     update:
       updated: Your profile was updated!
   dir: ltr

config/sitemap.rb

@@ -21,7 +21,8 @@
   add about_path, changefreq: "monthly", priority: 0.9
 
   Developer.visible.newest_first.find_each do |developer|
-    add developer_path(id: developer.id), changefreq: "always", priority: 0.8, lastmod: developer.updated_at
+    id = Feature.enabled?(:obfuscate_developer_urls) ? developer.hashid : developer.id
+    add developer_path(id:), changefreq: "always", priority: 0.8, lastmod: developer.updated_at
   end
 
   add new_user_session_path, changefreq: "weekly", priority: 0.7

test/integration/developers_test.rb

@@ -57,14 +57,8 @@ class DevelopersTest < ActionDispatch::IntegrationTest
     get developer_path(developer.hashid)
     assert_response :ok
 
-    assert_raises ActiveRecord::RecordNotFound do
-      get developer_path(developer.id)
-    end
-
-    sign_in users(:developer)
-    assert_raises ActiveRecord::RecordNotFound do
-      get edit_developer_path(developer.id)
-    end
+    get developer_path(developer.id)
+    assert_redirected_to developer_path(developer.hashid)
 
     sign_in users(:developer)
     assert_raises ActiveRecord::RecordNotFound do

test/models/developers/finder_test.rb

@@ -0,0 +1,34 @@
+require "test_helper"
+
+class Developers::FinderTest < ActiveSupport::TestCase
+  setup do
+    @developer = developers(:one)
+  end
+
+  test "should find a developer by hashid" do
+    finder = Developers::Finder.new(id: @developer.hashid)
+
+    assert_equal finder.developer, @developer
+    refute finder.should_redirect?
+  end
+
+  test "redirect should be true when given an id" do
+    finder = Developers::Finder.new(id: @developer.id)
+
+    assert finder.should_redirect?
+  end
+
+  test "should find a developer if feature is disabled" do
+    Feature.stub(:enabled?, false) do
+      finder = Developers::Finder.new(id: @developer.id)
+
+      assert_equal finder.developer, @developer
+      refute finder.should_redirect?
+
+      finder = Developers::Finder.new(id: @developer.hashid)
+
+      assert_equal finder.developer, @developer
+      refute finder.should_redirect?
+    end
+  end
+end