REFACTOR: All kinds of permalinks should return relative URLs

Mixing relative and absolute URLs is unnecessary and confusing.

app/models/category.rb

@@ -946,6 +946,8 @@ class Category < ActiveRecord::Base
     end
   end
 
+  alias_method :relative_url, :url
+
   # If the name changes, try and update the category definition topic too if it's an exact match
   def rename_category_definition
     return if topic.blank?

app/models/permalink.rb

@@ -72,11 +72,11 @@ class Permalink < ActiveRecord::Base
 
   def target_url
     return external_url if external_url
-    return "#{Discourse.base_path}#{post.url}" if post
+    return post.relative_url if post
     return topic.relative_url if topic
-    return category.url if category
-    return tag.full_url if tag
-    return user.full_url if user
+    return category.relative_url if category
+    return tag.relative_url if tag
+    return user.relative_url if user
     nil
   end
 

app/models/tag.rb

@@ -210,6 +210,8 @@ class Tag < ActiveRecord::Base
     "#{Discourse.base_path}/tag/#{UrlHelper.encode_component(self.name)}"
   end
 
+  alias_method :relative_url, :url
+
   def full_url
     "#{Discourse.base_url}/tag/#{UrlHelper.encode_component(self.name)}"
   end

app/models/user.rb

@@ -1779,6 +1779,10 @@ class User < ActiveRecord::Base
     username_lower == User.normalize_username(another_username)
   end
 
+  def relative_url
+    "#{Discourse.base_path}/u/#{encoded_username}"
+  end
+
   def full_url
     "#{Discourse.base_url}/u/#{encoded_username}"
   end

spec/models/permalink_spec.rb

@@ -33,79 +33,124 @@ RSpec.describe Permalink do
     let(:tag) { Fabricate(:tag) }
     let(:user) { Fabricate(:user) }
 
-    it "returns a topic url when topic_id is set" do
-      permalink.topic_id = topic.id
-      expect(target_url).to eq(topic.relative_url)
-    end
-
-    it "returns nil when topic_id is set but topic is not found" do
-      permalink.topic_id = 99_999
-      expect(target_url).to eq(nil)
-    end
-
-    it "returns a post url when post_id is set" do
-      permalink.post_id = post.id
-      expect(target_url).to eq(post.url)
-    end
-
-    it "returns nil when post_id is set but post is not found" do
-      permalink.post_id = 99_999
-      expect(target_url).to eq(nil)
-    end
-
-    it "returns a post url when post_id and topic_id are both set" do
-      permalink.post_id = post.id
-      permalink.topic_id = topic.id
-      expect(target_url).to eq(post.url)
-    end
-
-    it "returns a category url when category_id is set" do
-      permalink.category_id = category.id
-      expect(target_url).to eq("#{category.url}")
-    end
-
-    it "returns nil when category_id is set but category is not found" do
-      permalink.category_id = 99_999
-      expect(target_url).to eq(nil)
-    end
-
-    it "returns a post url when topic_id, post_id, and category_id are all set for some reason" do
-      permalink.post_id = post.id
-      permalink.topic_id = topic.id
-      permalink.category_id = category.id
-      expect(target_url).to eq(post.url)
-    end
-
-    it "returns a tag url when tag_id is set" do
-      permalink.tag_id = tag.id
-      expect(target_url).to eq(tag.full_url)
-    end
-
-    it "returns nil when tag_id is set but tag is not found" do
-      permalink.tag_id = 99_999
-      expect(target_url).to eq(nil)
-    end
-
-    it "returns a post url when topic_id, post_id, category_id and tag_id are all set for some reason" do
-      permalink.post_id = post.id
-      permalink.topic_id = topic.id
-      permalink.category_id = category.id
-      permalink.tag_id = tag.id
-      expect(target_url).to eq(post.url)
-    end
-
     it "returns nil when nothing is set" do
       expect(target_url).to eq(nil)
     end
 
-    it "returns a user url when user_id is set" do
-      permalink.user_id = user.id
-      expect(target_url).to eq(user.full_url)
+    context "when `topic_id` is set" do
+      it "returns an absolute path" do
+        permalink.topic_id = topic.id
+        expect(target_url).to eq(topic.relative_url)
+        expect(target_url).not_to start_with("http")
+      end
+
+      it "returns nil when topic is not found" do
+        permalink.topic_id = 99_999
+        expect(target_url).to eq(nil)
+      end
     end
 
-    it "returns nil when user_id is set but user is not found" do
-      permalink.user_id = 99_999
-      expect(target_url).to eq(nil)
+    context "when `post_id` is set" do
+      it "returns an absolute path" do
+        permalink.post_id = post.id
+        expect(target_url).to eq(post.relative_url)
+        expect(target_url).not_to start_with("http")
+      end
+
+      it "returns nil when post is not found" do
+        permalink.post_id = 99_999
+        expect(target_url).to eq(nil)
+      end
+    end
+
+    context "when `category_id` is set" do
+      it "returns an absolute path" do
+        permalink.category_id = category.id
+        expect(target_url).to eq(category.url)
+        expect(target_url).not_to start_with("http")
+      end
+
+      it "returns nil when category is not found" do
+        permalink.category_id = 99_999
+        expect(target_url).to eq(nil)
+      end
+    end
+
+    context "when `tag_id` is set" do
+      it "returns an absolute path" do
+        permalink.tag_id = tag.id
+        expect(target_url).to eq(tag.relative_url)
+        expect(target_url).not_to start_with("http")
+      end
+
+      it "returns nil when tag is not found" do
+        permalink.tag_id = 99_999
+        expect(target_url).to eq(nil)
+      end
+    end
+
+    context "when `user_id` is set" do
+      it "returns an absolute path" do
+        permalink.user_id = user.id
+        expect(target_url).to eq(user.relative_url)
+        expect(target_url).not_to start_with("http")
+      end
+
+      it "returns nil when user is not found" do
+        permalink.user_id = 99_999
+        expect(target_url).to eq(nil)
+      end
+    end
+
+    context "when `external_url` is set" do
+      it "returns a URL when an absolute URL is set" do
+        permalink.external_url = "https://example.com"
+        expect(target_url).to eq("https://example.com")
+      end
+
+      it "returns a protocol-relative URL when a PRURL is set" do
+        permalink.external_url = "//example.com"
+        expect(target_url).to eq("//example.com")
+      end
+
+      it "returns an absolute path when an absolute path is set" do
+        permalink.external_url = "/my/preferences"
+        expect(target_url).to eq("/my/preferences")
+      end
+
+      it "returns a relative path when a relative path is set" do
+        permalink.external_url = "foo/bar"
+        expect(target_url).to eq("foo/bar")
+      end
+    end
+
+    it "returns the highest priority url when multiple attributes are set" do
+      permalink.external_url = "/my/preferences"
+      permalink.post = post
+      permalink.topic = topic
+      permalink.category = category
+      permalink.tag = tag
+      permalink.user = user
+
+      expect(permalink.target_url).to eq("/my/preferences")
+
+      permalink.external_url = nil
+      expect(permalink.target_url).to eq(post.relative_url)
+
+      permalink.post = nil
+      expect(permalink.target_url).to eq(topic.relative_url)
+
+      permalink.topic = nil
+      expect(permalink.target_url).to eq(category.relative_url)
+
+      permalink.category = nil
+      expect(permalink.target_url).to eq(tag.relative_url)
+
+      permalink.tag = nil
+      expect(permalink.target_url).to eq(user.relative_url)
+
+      permalink.user = nil
+      expect(permalink.target_url).to be_nil
     end
   end
 end