Simplify theme and color scheme seeding (#10872)

Simplify theme and color scheme seeding (#10872)

Now that we have support for user-selectable color schemes, it makes sense to simplify seeding and theme updates in the wizard.

We now:

  • seed only one theme, named “Default” (previously “Light”)
  • seed a user-selectable Dark color scheme
  • rename the “Themes” wizard step to “Colors”
  • update the default theme’s color scheme if a default is set (a new theme is created if there is no default)
diff --git a/app/models/color_scheme.rb b/app/models/color_scheme.rb
index ab82c11..6632872 100644
--- a/app/models/color_scheme.rb
+++ b/app/models/color_scheme.rb
@@ -196,6 +196,7 @@ class ColorScheme < ActiveRecord::Base
     new_color_scheme = new(name: params[:name])
     new_color_scheme.via_wizard = true if params[:via_wizard]
     new_color_scheme.base_scheme_id = params[:base_scheme_id]
+    new_color_scheme.user_selectable = true if params[:user_selectable]
 
     colors = CUSTOM_SCHEMES[params[:base_scheme_id].to_sym]&.map do |name, hex|
       { name: name, hex: hex }
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index d885ac3..01859dc 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -3923,7 +3923,7 @@ en:
     latte: "Latte"
     summer: "Summer"
     dark_rose: "Dark Rose"
-    default_theme_name: "Light"
+    default_theme_name: "Default"
     light_theme_name: "Light"
     dark_theme_name: "Dark"
     neutral_theme_name: "Neutral"
@@ -4709,7 +4709,7 @@ en:
             placeholder: "San Francisco, California"
 
       colors:
-        title: "Theme"
+        title: "Colors"
 
       fonts:
         title: "Fonts"
diff --git a/db/fixtures/600_themes.rb b/db/fixtures/600_themes.rb
index 23cc993..718f650 100644
--- a/db/fixtures/600_themes.rb
+++ b/db/fixtures/600_themes.rb
@@ -2,22 +2,13 @@
 
 # we can not guess what to do if customization already started, so skip it
 if !Theme.exists?
-  STDERR.puts "> Seeding dark and light themes"
+  STDERR.puts "> Seeding theme and color schemes"
 
   name = I18n.t("color_schemes.dark_theme_name")
   dark_scheme = ColorScheme.find_by(base_scheme_id: "Dark")
-  dark_scheme ||= ColorScheme.create_from_base(name: name, via_wizard: true, base_scheme_id: "Dark")
-
-  name = I18n.t('color_schemes.dark_theme_name')
-
-  _dark_theme = Theme.create!(
-    name: name, user_id: -1,
-    color_scheme_id: dark_scheme.id,
-    user_selectable: true
-  )
+  dark_scheme ||= ColorScheme.create_from_base(name: name, via_wizard: true, base_scheme_id: "Dark", user_selectable: true)
 
   name = I18n.t('color_schemes.default_theme_name')
-  default_theme = Theme.create!(name: name, user_id: -1, user_selectable: true)
-
+  default_theme = Theme.create!(name: name, user_id: -1)
   default_theme.set_default!
 end
diff --git a/lib/wizard/builder.rb b/lib/wizard/builder.rb
index a18af57..b6f3b41 100644
--- a/lib/wizard/builder.rb
+++ b/lib/wizard/builder.rb
@@ -181,26 +181,22 @@ class Wizard
           next unless scheme_name.present? && ColorScheme.is_base?(scheme_name)
 
           name = I18n.t("color_schemes.#{scheme_name.downcase.gsub(' ', '_')}_theme_name")
-          theme = nil
+
           scheme = ColorScheme.find_by(base_scheme_id: scheme_name, via_wizard: true)
-          is_light_theme = (scheme_name == ColorScheme::LIGHT_THEME_ID)
           scheme ||= ColorScheme.create_from_base(name: name, via_wizard: true, base_scheme_id: scheme_name)
 
-          themes = Theme.where(color_scheme_id: scheme.id).order(:id).to_a
-          if is_light_theme
-            themes = (themes || []).concat(Theme.where(color_scheme_id: nil).order(:id).to_a)
-            themes.sort_by(&:id)
-          end
-          theme = themes.find(&:default?)
-          theme ||= themes.first
-
-          theme ||= Theme.create!(
-            name: name,
-            user_id: @wizard.user.id,
-            color_scheme_id: scheme.id
-          )
+          if default_theme
+            default_theme.color_scheme_id = scheme.id
+            default_theme.save!
+          else
+            theme = Theme.create!(
+              name: name,
+              user_id: @wizard.user.id,
+              color_scheme_id: scheme.id
+            )
 
-          theme.set_default!
+            theme.set_default!
+          end
         end
       end
 
diff --git a/spec/components/wizard/step_updater_spec.rb b/spec/components/wizard/step_updater_spec.rb
index d06dd24..a4ea41c 100644
--- a/spec/components/wizard/step_updater_spec.rb
+++ b/spec/components/wizard/step_updater_spec.rb
@@ -204,6 +204,13 @@ describe Wizard::StepUpdater do
           wizard.create_updater('colors', {}).update
         end.to_not change { SiteSetting.default_theme_id }
       end
+
+      it "should update the color scheme of the default theme" do
+        updater = wizard.create_updater('colors', theme_previews: 'Neutral')
+        expect { updater.update }.not_to change { Theme.count }
+        theme.reload
+        expect(theme.color_scheme.base_scheme_id).to eq('Neutral')
+      end
     end
 
     context "without an existing theme" do
@@ -257,8 +264,6 @@ describe Wizard::StepUpdater do
 
         theme = Theme.find_by(id: SiteSetting.default_theme_id)
         expect(theme.color_scheme_id).to eq(color_scheme.id)
-
-        expect(Theme.where(user_selectable: true).count).to eq(2)
       end
     end
   end
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index d2ff47b..394ebaa 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -450,12 +450,16 @@ describe ApplicationHelper do
   end
 
   describe "dark_color_scheme?" do
-    it 'returns nil for the base color scheme' do
+    it 'returns false for the base color scheme' do
       expect(helper.dark_color_scheme?).to eq(false)
     end
 
     it 'works correctly for a dark scheme' do
-      dark_theme = Theme.where(name: "Dark").first
+      dark_theme = Theme.create(
+        name: "Dark",
+        user_id: -1,
+        color_scheme_id: ColorScheme.find_by(base_scheme_id: "Dark").id
+      )
       helper.request.env[:resolved_theme_ids] = [dark_theme.id]
 
       expect(helper.dark_color_scheme?).to eq(true)
diff --git a/spec/serializers/site_serializer_spec.rb b/spec/serializers/site_serializer_spec.rb
index bc9787a..bfa3793 100644
--- a/spec/serializers/site_serializer_spec.rb
+++ b/spec/serializers/site_serializer_spec.rb
@@ -30,12 +30,9 @@ describe SiteSerializer do
   end
 
   it "includes user-selectable color schemes" do
-    scheme = ColorScheme.create_from_base(name: "Neutral", base_scheme_id: "Neutral")
-    scheme.user_selectable = true
-    scheme.save!
-
+    # 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 (1)
+    expect(serialized[:user_color_schemes].count).to eq(1)
 
     dark_scheme = ColorScheme.create_from_base(name: "ADarkScheme", base_scheme_id: "Dark")
     dark_scheme.user_selectable = true
diff --git a/spec/services/site_settings_spec.rb b/spec/services/site_settings_spec.rb
index 906a2ac..a9a540b 100644
--- a/spec/services/site_settings_spec.rb
+++ b/spec/services/site_settings_spec.rb
@@ -32,9 +32,10 @@ describe SiteSettingsTask do
     end
 
     it "updates hidden settings" do
+      original_default_theme_id = SiteSetting.default_theme_id.inspect
       yml = "default_theme_id: 999999999"
       log, counts = SiteSettingsTask.import(yml)
-      expect(log[0]).to eq "Changed default_theme_id FROM: 2 TO: 999999999"
+      expect(log[0]).to eq "Changed default_theme_id FROM: #{original_default_theme_id} TO: 999999999"
       expect(counts[:updated]).to eq(1)
       expect(SiteSetting.default_theme_id).to eq(999999999)
     end

GitHub sha: 74de7a49

This commit appears in #10872 which was approved by danielwaterworth. It was merged by pmusaraj.