Prevent users from converting the default theme to a component (PR #11688)

[riking]

GitHub

[markvanlan]

app/controllers/admin/themes_controller.rb

@@ -401,8 +401,10 @@ def log_theme_component_enabled
   def handle_switch
     param = theme_params[:component]
     if param.to_s == "false" && @theme.component?
+      raise Discourse::InvalidParameters.new(:component) if @theme.id == SiteSetting.default_theme_id

It looks like this line is repeated and you could move it out of the if

[riking]

We need to NOT raise the error if no change needs to happen – so it would run if (param.to_s == "true") == (@theme.component?)

But as written above it doesn’t properly handle unspecified, so I put it inside both ifs.

[pmusaraj]

app/assets/javascripts/admin/addon/templates/customize-themes-show.hbs

@@ -290,7 +290,7 @@
     <a href={{previewUrl}} title={{i18n "admin.customize.explain_preview"}} rel="noopener noreferrer" target="_blank" class="btn btn-default">{{d-icon "desktop"}}{{i18n "admin.customize.theme.preview"}}</a>
     <a class="btn btn-default export" rel="noopener noreferrer" target="_blank" href={{downloadUrl}}>{{d-icon "download"}} {{i18n "admin.export_json.button_text"}}</a>
 
-    {{d-button action=(action "switchType") label="admin.customize.theme.convert" icon=convertIcon class="btn-default btn-normal" title=convertTooltip}}
+    {{d-button action=(action "switchType") label="admin.customize.theme.convert" icon=convertIcon class="btn-default btn-normal" disabled=model.default title=convertTooltip}}

I’m sorry, I’m late on this, but… I think it would be better to remove the “Convert” button for the default theme. It’s a very rare action, anyway.

[eviltrout]

@riking can you address @pmusaraj’s feedback and get this merged today?

[riking]

Done