From e708c99e126e7d116766ca7324a1f1e6860aa050 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Tue, 26 Nov 2024 13:53:10 +1000
Subject: [PATCH] FIX: Hide broken theme about/license URLs (#29930)

At the top of the theme show page we have a link
to the theme About and License, which are supposed
to be URLs. However some themes have left placeholder
text in these metadata fields, which leads to a wonky
experience.

Instead, we can just not serialize these fields if they
are not valid URLs, then they will not show as links
in the UI.
---
 app/serializers/remote_theme_serializer.rb    | 10 +++--
 spec/fabricators/theme_fabricator.rb          |  4 ++
 .../remote_theme_serializer_spec.rb           | 37 +++++++++++++++++++
 3 files changed, 48 insertions(+), 3 deletions(-)
 create mode 100644 spec/serializers/remote_theme_serializer_spec.rb

diff --git a/app/serializers/remote_theme_serializer.rb b/app/serializers/remote_theme_serializer.rb
index 4ba798ca479..3f7d4e7ef22 100644
--- a/app/serializers/remote_theme_serializer.rb
+++ b/app/serializers/remote_theme_serializer.rb
@@ -19,10 +19,14 @@ class RemoteThemeSerializer < ApplicationSerializer
              :minimum_discourse_version,
              :maximum_discourse_version
 
-  # wow, AMS has some pretty nutty logic where it tries to find the path here
-  # from action dispatch, tell it not to
+  # ActiveModelSerializer has some pretty nutty logic where it tries to find
+  # the path here from action dispatch, tell it not to
   def about_url
-    object.about_url
+    object.about_url if UrlHelper.is_valid_url?(object.about_url)
+  end
+
+  def license_url
+    object.license_url if UrlHelper.is_valid_url?(object.license_url)
   end
 
   def include_github_diff_link?
diff --git a/spec/fabricators/theme_fabricator.rb b/spec/fabricators/theme_fabricator.rb
index e1d4d5af91a..bfc110a5ff3 100644
--- a/spec/fabricators/theme_fabricator.rb
+++ b/spec/fabricators/theme_fabricator.rb
@@ -4,3 +4,7 @@ Fabricator(:theme) do
   name { sequence(:name) { |i| "Cool theme #{i + 1}" } }
   user
 end
+
+Fabricator(:remote_theme) { remote_url { "https://github.com/org/testtheme.git" } }
+
+Fabricator(:theme_with_remote_url, from: :theme) { remote_theme }
diff --git a/spec/serializers/remote_theme_serializer_spec.rb b/spec/serializers/remote_theme_serializer_spec.rb
new file mode 100644
index 00000000000..bee4cfd0b5a
--- /dev/null
+++ b/spec/serializers/remote_theme_serializer_spec.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+RSpec.describe RemoteThemeSerializer do
+  fab!(:remote_theme) do
+    Fabricate(
+      :remote_theme,
+      about_url: "https://meta.discourse.org/t/some-theme/123",
+      license_url: "https://github.com/repo/repo/LICENSE.md",
+    )
+  end
+
+  describe "about_url" do
+    it "returns the about_url" do
+      serialized = RemoteThemeSerializer.new(remote_theme).as_json[:remote_theme]
+      expect(serialized[:about_url]).to eq("https://meta.discourse.org/t/some-theme/123")
+    end
+
+    it "returns nil if the URL is not a valid URL" do
+      remote_theme.update!(about_url: "todo: Put your theme's public repo or Meta topic URL here")
+      serialized = RemoteThemeSerializer.new(remote_theme).as_json[:remote_theme]
+      expect(serialized[:about_url]).to be_nil
+    end
+  end
+
+  describe "license_url" do
+    it "returns the license_url" do
+      serialized = RemoteThemeSerializer.new(remote_theme).as_json[:remote_theme]
+      expect(serialized[:license_url]).to eq("https://github.com/repo/repo/LICENSE.md")
+    end
+
+    it "returns nil if the URL is not a valid URL" do
+      remote_theme.update!(license_url: "todo: Put your theme's LICENSE URL here")
+      serialized = RemoteThemeSerializer.new(remote_theme).as_json[:remote_theme]
+      expect(serialized[:license_url]).to be_nil
+    end
+  end
+end