Skip to content

Commit

Permalink
FIX: Correctly clear theme stylesheet cache when changing color scheme
Browse files Browse the repository at this point in the history
  • Loading branch information
davidtaylorhq committed May 8, 2019
1 parent 54c2f24 commit e84531a
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 4 deletions.
6 changes: 2 additions & 4 deletions app/models/theme.rb
Expand Up @@ -52,6 +52,7 @@ def notify_color_change(color)
changed_fields.clear

Theme.expire_site_cache! if saved_change_to_user_selectable? || saved_change_to_name?
notify_with_scheme = saved_change_to_color_scheme_id?

reload
settings_field&.ensure_baked! # Other fields require setting to be **baked**
Expand All @@ -60,6 +61,7 @@ def notify_color_change(color)
remove_from_cache!
clear_cached_settings!
ColorScheme.hex_cache.clear
notify_theme_change(with_scheme: notify_with_scheme)
end

after_destroy do
Expand All @@ -86,10 +88,6 @@ def notify_color_change(color)
SvgSprite.expire_cache
end

after_commit ->(theme) do
theme.notify_theme_change(with_scheme: theme.saved_change_to_color_scheme_id?)
end, on: [:create, :update]

def self.get_set_cache(key, &blk)
if val = @cache[key]
return val
Expand Down
25 changes: 25 additions & 0 deletions spec/models/theme_spec.rb
Expand Up @@ -481,6 +481,31 @@ def cached_settings(id)
expect(ColorScheme.hex_for_name('header_primary')).to eq('333333')
end

it "correctly notifies about theme changes" do
cs1 = Fabricate(:color_scheme)
cs2 = Fabricate(:color_scheme)

theme = Fabricate(:theme,
user_selectable: true,
user: user,
color_scheme_id: cs1.id
)

messages = MessageBus.track_publish do
theme.save!
end.filter { |m| m.channel == "/file-change" }
expect(messages.count).to eq(1)
expect(messages.first.data.map { |d| d[:target] }).to contain_exactly(:desktop_theme, :mobile_theme)

# With color scheme change:
messages = MessageBus.track_publish do
theme.color_scheme_id = cs2.id
theme.save!
end.filter { |m| m.channel == "/file-change" }
expect(messages.count).to eq(1)
expect(messages.first.data.map { |d| d[:target] }).to contain_exactly(:admin, :desktop, :desktop_theme, :mobile, :mobile_theme)
end

it 'handles settings cache correctly' do
Theme.destroy_all

Expand Down

1 comment on commit e84531a

@discoursebot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/discourse-theme-cli-console-app-to-help-you-build-themes/82950/97

Please sign in to comment.