FIX: color schemes not updating when remote saves

app/models/color_scheme.rb

@@ -43,12 +43,7 @@ class ColorScheme < ActiveRecord::Base
 
   alias_method :colors, :color_scheme_colors
 
-  before_save do
+  before_save :bump_version
-    if self.id
-      self.version += 1
-    end
-  end
-
   after_save :publish_discourse_stylesheet
   after_save :dump_hex_cache
   after_destroy :dump_hex_cache
@@ -180,6 +175,12 @@ class ColorScheme < ActiveRecord::Base
     self.class.hex_cache.clear
   end
 
+  def bump_version
+    if self.id
+      self.version += 1
+    end
+  end
+
 end
 
 # == Schema Information

app/models/remote_theme.rb

@@ -154,9 +154,10 @@ class RemoteTheme < ActiveRecord::Base
   end
 
   def update_theme_color_schemes(theme, schemes)
-    return if schemes.blank?
+    missing_scheme_names = Hash[*theme.color_schemes.pluck(:name, :id).flatten]
 
     schemes.each do |name, colors|
+      missing_scheme_names.delete(name)
       existing = theme.color_schemes.find_by(name: name)
       if existing
         existing.colors.each do |c|
@@ -174,9 +175,15 @@ class RemoteTheme < ActiveRecord::Base
         end
       end
     end
+
+    if missing_scheme_names.length > 0
+      ColorScheme.where(id: missing_scheme_names.values).delete_all
+      # we may have stuff pointed at the incorrect scheme?
     end
   end
 
+end
+
 # == Schema Information
 #
 # Table name: remote_themes

app/models/theme.rb

@@ -26,8 +26,16 @@ class Theme < ActiveRecord::Base
   end
 
   after_save do
-    changed_colors.each(&:save!)
+    color_schemes = {}
+    changed_colors.each do |color|
+      color.save!
+      color_schemes[color.color_scheme_id] ||= color.color_scheme
+    end
+
+    color_schemes.values.each(&:save!)
+
     changed_colors.clear
+
     changed_fields.each(&:save!)
     changed_fields.clear
 

spec/models/remote_theme_spec.rb

@@ -18,10 +18,8 @@ describe RemoteTheme do
       repo_dir
     end
 
-    def about_json(options = {})
+    def about_json(love_color: "FAFAFA", color_scheme_name: "Amazing")
-      options[:love] ||= "FAFAFA"
+      <<~JSON
-
-<<JSON
         {
           "name": "awesome theme",
           "about_url": "https://www.site.com/about",
@@ -38,8 +36,8 @@ describe RemoteTheme do
             "name": "sam"
           },
           "color_schemes": {
-            "Amazing": {
+            "#{color_scheme_name}": {
-              "love": "#{options[:love]}"
+              "love": "#{love_color}"
             }
           }
         }
@@ -108,7 +106,7 @@ JSON
       expect(scheme.colors.find_by(name: 'love').hex).to eq('fafafa')
 
       File.write("#{initial_repo}/common/header.html", "I AM UPDATED")
-      File.write("#{initial_repo}/about.json", about_json(love: "EAEAEA"))
+      File.write("#{initial_repo}/about.json", about_json(love_color: "EAEAEA"))
 
       File.write("#{initial_repo}/settings.yml", "integer_setting: 32")
       `cd #{initial_repo} && git add settings.yml`
@@ -140,6 +138,17 @@ JSON
       expect(@theme.settings.first.value).to eq(32)
 
       expect(remote.remote_updated_at).to eq(time)
+
+      # It should be able to remove old colors as well
+      File.write("#{initial_repo}/about.json", about_json(love_color: "BABABA", color_scheme_name: "Amazing 2"))
+      `cd #{initial_repo} && git commit -am "update"`
+
+      remote.update_from_remote
+      @theme.save
+      @theme.reload
+
+      scheme_count = ColorScheme.where(theme_id: @theme.id).count
+      expect(scheme_count).to eq(1)
     end
   end
 end