FIX: Horizon default color scheme must be user selectable (#33428)

Improvements:
- When import Horizon theme, ensure that default color scheme is marked
as `user_selectable`
- Dark version of the theme is `user_selectable` as well
- When merge remote Horizon into system Horizon, also ensure that
default color scheme is marked as `user_selectable`

lib/system_themes_manager.rb

@@ -11,7 +11,21 @@ class SystemThemesManager
 
     theme_dir = "#{Rails.root}/themes/#{theme_name}"
 
-    RemoteTheme.import_theme_from_directory(theme_dir, theme_id: theme_id)
+    remote_theme = RemoteTheme.import_theme_from_directory(theme_dir, theme_id: theme_id)
+    if remote_theme.color_scheme
+      remote_theme.color_scheme.update!(user_selectable: true)
+      alternative_theme_name =
+        if remote_theme.color_scheme.name =~ / Dark$/
+          remote_theme.color_scheme.name.sub(" Dark", "")
+        else
+          "#{remote_theme.color_scheme.name} Dark"
+        end
+      remote_theme
+        .color_schemes
+        .where(name: alternative_theme_name)
+        .first
+        &.update!(user_selectable: true)
+    end
     Stylesheet::Manager.clear_theme_cache!
   end
 end

lib/tasks/themes.rake

@@ -353,13 +353,6 @@ task "themes:deduplicate_horizon" => :environment do |task, args|
           }
           map
         end
-    if remote_horizon_theme.color_scheme.theme_id == remote_horizon_theme.id
-      system_horizon_theme.update!(
-        color_scheme_id: color_schemes_map[remote_horizon_theme.color_scheme_id][:id],
-      )
-    elsif remote_horizon_theme.color_scheme_id
-      system_horizon_theme.update!(color_scheme_id: remote_horizon_theme.color_scheme_id)
-    end
     system_horizon_theme.color_schemes.find_each do |color_scheme|
       color_scheme.update!(
         user_selectable:
@@ -369,6 +362,14 @@ task "themes:deduplicate_horizon" => :environment do |task, args|
             &.dig(:user_selectable),
       )
     end
+    if remote_horizon_theme.color_scheme.theme_id == remote_horizon_theme.id
+      system_horizon_theme.update!(
+        color_scheme_id: color_schemes_map[remote_horizon_theme.color_scheme_id][:id],
+      )
+    elsif remote_horizon_theme.color_scheme_id
+      system_horizon_theme.update!(color_scheme_id: remote_horizon_theme.color_scheme_id)
+    end
+    system_horizon_theme.color_scheme.update!(user_selectable: true)
     puts "Theme color palette is updated"
     UserOption
       .where(color_scheme_id: color_schemes_map.keys)

spec/lib/stylesheet/manager_spec.rb

@@ -979,7 +979,7 @@ RSpec.describe Stylesheet::Manager do
 
       # Ensure we force compile each theme only once
       expect(output.scan(/#{child_theme_with_css.name}/).length).to eq(2) # ltr/rtl
-      expect(StylesheetCache.count).to eq(34) # (1 theme with rtl/ltr) + 32 color schemes (2 themes * 8 color schemes (7 defaults + 1 theme scheme) * 2 (light and dark mode per scheme))
+      expect(StylesheetCache.count).to eq(42) # (1 theme with rtl/ltr) + 32 color schemes (2 themes * 8 color schemes (7 defaults + 1 theme scheme) * 2 (light and dark mode per scheme)) + 8 Horizon
     end
 
     it "generates precompiled CSS - core and themes" do
@@ -987,7 +987,7 @@ RSpec.describe Stylesheet::Manager do
       Stylesheet::Manager.precompile_theme_css
 
       results = StylesheetCache.pluck(:target)
-      expect(results.size).to eq(44) # 10 core targets + 2 theme (ltr/rtl) + 32 color schemes (light and dark mode per scheme)
+      expect(results.size).to eq(52) # 10 core targets + 2 theme (ltr/rtl) + 32 color schemes (light and dark mode per scheme) + 8 Horizon
 
       expect(results.count { |target| target =~ /^common_theme_/ }).to eq(2) # ltr/rtl
     end
@@ -999,7 +999,7 @@ RSpec.describe Stylesheet::Manager do
       Stylesheet::Manager.precompile_theme_css
 
       results = StylesheetCache.pluck(:target)
-      expect(results.size).to eq(58) # 10 core targets + theme rtl/ltr + 32 color schemes (light and dark mode per scheme) + 14 Foundation
+      expect(results.size).to eq(70) # 10 core targets + theme rtl/ltr + 32 color schemes (light and dark mode per scheme) + 14 Foundation + 12 Horizon
 
       expect(results).to include("color_definitions_#{scheme1.name}_#{scheme1.id}_#{user_theme.id}")
       expect(results).to include(

spec/lib/system_themes_manager_spec.rb

@@ -5,5 +5,11 @@ RSpec.describe SystemThemesManager do
     Theme.delete_all
     expect { SystemThemesManager.sync! }.to change { Theme.system.count }.by(2)
     expect { SystemThemesManager.sync! }.not_to change { Theme.count }
+    expect(Theme.horizon_theme.color_scheme.user_selectable).to be true
+    expect(
+      Theme.horizon_theme.color_schemes.where(name: "Horizon Dark").first.user_selectable,
+    ).to be true
+    expect(Theme.horizon_theme.color_schemes.where(user_selectable: true).count).to eq(2)
+    expect(Theme.horizon_theme.color_schemes.where(user_selectable: false).count).to eq(10)
   end
 end

spec/serializers/site_serializer_spec.rb

@@ -90,7 +90,7 @@ RSpec.describe SiteSerializer do
   it "includes user-selectable color schemes" do
     # it includes seeded color schemes
     serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json
-    expect(serialized[:user_color_schemes].count).to eq(6)
+    expect(serialized[:user_color_schemes].count).to eq(8)
 
     scheme_names = serialized[:user_color_schemes].map { |x| x[:name] }
     expect(scheme_names).to include(I18n.t("color_schemes.dark"))
@@ -99,13 +99,15 @@ RSpec.describe SiteSerializer do
     expect(scheme_names).to include(I18n.t("color_schemes.solarized_light"))
     expect(scheme_names).to include(I18n.t("color_schemes.solarized_dark"))
     expect(scheme_names).to include(I18n.t("color_schemes.dracula"))
+    expect(scheme_names).to include("Horizon")
+    expect(scheme_names).to include("Horizon Dark")
 
     dark_scheme = ColorScheme.create_from_base(name: "AnotherDarkScheme", base_scheme_id: "Dark")
     dark_scheme.user_selectable = true
     dark_scheme.save!
 
     serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json
-    expect(serialized[:user_color_schemes].count).to eq(7)
+    expect(serialized[:user_color_schemes].count).to eq(9)
     expect(serialized[:user_color_schemes][0][:is_dark]).to eq(true)
   end
 

spec/tasks/themes_spec.rb

@@ -151,6 +151,13 @@ RSpec.describe "tasks/themes" do
       remote_horizon_theme.color_schemes.create!(
         name: "Lily Dark",
         theme_id: remote_horizon_theme.id,
+        user_selectable: false,
+      )
+    end
+    fab!(:remote_color_scheme_2) do
+      remote_horizon_theme.color_schemes.create!(
+        name: "Violet Dark",
+        theme_id: remote_horizon_theme.id,
         user_selectable: true,
       )
     end
@@ -165,6 +172,9 @@ RSpec.describe "tasks/themes" do
 
     let!(:system_horizon_theme) { Theme.horizon_theme }
     let!(:system_color_scheme) { system_horizon_theme.color_schemes.where(name: "Lily Dark").first }
+    let!(:system_color_scheme_2) do
+      system_horizon_theme.color_schemes.where(name: "Violet Dark").first
+    end
 
     before do
       remote_horizon_theme.update!(color_scheme: remote_color_scheme)
@@ -237,6 +247,7 @@ RSpec.describe "tasks/themes" do
         )
         expect(system_horizon_theme.color_scheme).to eq(system_color_scheme)
         expect(system_color_scheme.reload.user_selectable).to be true
+        expect(system_color_scheme_2.reload.user_selectable).to be true
       end
 
       it "logs that remote theme was deleted" do