FIX: font awesome remapping migration should not drop unmapped names from svg_icon_subset (#30058)

* FIX: font awesome remapping migration should not drop unmapped names

db/migrate/20241127072350_remap_fa5_icon_names_to_fa6.rb

@@ -703,25 +703,32 @@ class RemapFa5IconNamesToFa6 < ActiveRecord::Migration[7.1]
       original_setting_value
         .split("|")
         .map do |icon_name|
-          found_icon_name =
+          prefix = nil
-            case icon_name
+
-            when /^fab-/
+          case icon_name
-              "fab-#{FA5_REMAPS[icon_name.sub(/^fab-/, "")]}"
+          when /^fab-/
-            when /^far-/
+            prefix = "fab"
-              "far-#{FA5_REMAPS[icon_name.sub(/^far-/, "")]}"
+            lookup_name = icon_name.sub(/^fab-/, "")
-            when /^fab fa-/
+          when /^far-/
-              "fab-#{FA5_REMAPS[icon_name.sub(/^fab fa-/, "")]}"
+            prefix = "far"
-            when /^far fa-/
+            lookup_name = icon_name.sub(/^far-/, "")
-              "far-#{FA5_REMAPS[icon_name.sub(/^far fa-/, "")]}"
+          when /^fab fa-/
-            when /^fas fa-/
+            prefix = "fab"
-              FA5_REMAPS[icon_name.sub(/^fas fa-/, "")]
+            lookup_name = icon_name.sub(/^fab fa-/, "")
-            when /^fa-/
+          when /^far fa-/
-              FA5_REMAPS[icon_name.sub(/^fa-/, "")]
+            prefix = "far"
-            else
+            lookup_name = icon_name.sub(/^far fa-/, "")
-              FA5_REMAPS[icon_name]
+          when /^fas fa-/
-            end
+            lookup_name = icon_name.sub(/^fas fa-/, "")
-          # if not converted, just return the original icon name
+          when /^fa-/
-          found_icon_name || icon_name
+            lookup_name = icon_name.sub(/^fa-/, "")
+          else
+            lookup_name = icon_name
+          end
+
+          new_icon_name = FA5_REMAPS[lookup_name] || lookup_name
+          new_icon_name = "#{prefix}-#{new_icon_name}" if prefix
+          new_icon_name
         end
         .join("|")
 

spec/migrations/remap_fa5_icon_names_to_fa6_spec.rb

@@ -16,17 +16,32 @@ RSpec.describe RemapFa5IconNamesToFa6 do
   end
 
   context "when svg_icon_subset site setting has values to be remapped" do
+    let(:svg_icon_subset_icon_mapping) do
+      {
+        "fa-ambulance" => "truck-medical",
+        "far-ambulance" => "far-truck-medical",
+        "fab-ambulance" => "fab-truck-medical",
+        "far fa-ambulance" => "far-truck-medical",
+        "far fa-gear" => "far-gear",
+        "gear" => "gear",
+        "fab fa-ambulance" => "fab-truck-medical",
+        "fas fa-ambulance" => "truck-medical",
+      }
+    end
+
     let!(:site_setting) do
       SiteSetting.create!(
         name: "svg_icon_subset",
-        value: icon_mapping.keys.join("|"),
+        value: svg_icon_subset_icon_mapping.keys.join("|"),
         data_type: SiteSettings::TypeSupervisor.types[:list],
       )
     end
 
     it "remaps the values correctly" do
       silence_stdout { migrate }
-      expect(site_setting.reload.value.split("|")).to match_array(icon_mapping.values)
+      expect(site_setting.reload.value.split("|")).to match_array(
+        svg_icon_subset_icon_mapping.values,
+      )
     end
   end
 
@@ -82,7 +97,7 @@ RSpec.describe RemapFa5IconNamesToFa6 do
   end
 
   context "when no icon names can be remapped" do
-    let(:icon_names) { ["fal fa-adjust", "heart"] }
+    let(:icon_names) { ["fal fa-adjust", "heart", "far-heart"] }
     let(:site_setting) do
       SiteSetting.create!(
         name: "svg_icon_subset",
@@ -91,12 +106,19 @@ RSpec.describe RemapFa5IconNamesToFa6 do
       )
     end
     let(:group) { Fabricate(:group, flair_icon: icon_names.first) }
+    let(:group_2) { Fabricate(:group, flair_icon: icon_names.last) }
     let(:post_action_type) { PostActionType.create!(name_key: "foo", icon: icon_names.first) }
+    let(:post_action_type_2) { PostActionType.create!(name_key: "foo", icon: icon_names.last) }
     let(:badge) { Fabricate(:badge, icon: icon_names.first) }
+    let(:badge_2) { Fabricate(:badge, icon: icon_names.last) }
     let(:sidebar_url) { Fabricate(:sidebar_url, icon: icon_names.first) }
+    let(:sidebar_url_2) { Fabricate(:sidebar_url, icon: icon_names.last) }
     let(:directory_column) do
       DirectoryColumn.create!(enabled: true, position: 1, icon: icon_names.first)
     end
+    let(:directory_column_2) do
+      DirectoryColumn.create!(enabled: true, position: 1, icon: icon_names.last)
+    end
 
     it "does not change any icon column values" do
       expect { silence_stdout { migrate } }.not_to change {
@@ -107,6 +129,11 @@ RSpec.describe RemapFa5IconNamesToFa6 do
           badge.reload.icon,
           sidebar_url.reload.icon,
           directory_column.reload.icon,
+          group_2.reload.flair_icon,
+          post_action_type_2.reload.icon,
+          badge_2.reload.icon,
+          sidebar_url_2.reload.icon,
+          directory_column_2.reload.icon,
         ]
       }
     end