FIX: Theme import error handling needs to happen inside the hijack block (#18866)

Otherwise the errors don't get caught.

app/controllers/admin/themes_controller.rb

@@ -97,32 +97,32 @@ class Admin::ThemesController < Admin::AdminController
         return
       end
 
-      begin
-        branch = params[:branch] ? params[:branch] : nil
-        private_key = params[:public_key] ? Discourse.redis.get("ssh_key_#{params[:public_key]}") : nil
-        return render_json_error I18n.t("themes.import_error.ssh_key_gone") if params[:public_key].present? && private_key.blank?
+      hijack do
+        begin
+          branch = params[:branch] ? params[:branch] : nil
+          private_key = params[:public_key] ? Discourse.redis.get("ssh_key_#{params[:public_key]}") : nil
+          return render_json_error I18n.t("themes.import_error.ssh_key_gone") if params[:public_key].present? && private_key.blank?
 
-        hijack do
           @theme = RemoteTheme.import_theme(remote, theme_user, private_key: private_key, branch: branch)
           render json: @theme, status: :created
-        end
-      rescue RemoteTheme::ImportError => e
-        if params[:force]
-          theme_name = params[:remote].gsub(/.git$/, "").split("/").last
+        rescue RemoteTheme::ImportError => e
+          if params[:force]
+            theme_name = params[:remote].gsub(/.git$/, "").split("/").last
 
-          remote_theme = RemoteTheme.new
-          remote_theme.private_key = private_key
-          remote_theme.branch = params[:branch] ? params[:branch] : nil
-          remote_theme.remote_url = params[:remote]
-          remote_theme.save!
+            remote_theme = RemoteTheme.new
+            remote_theme.private_key = private_key
+            remote_theme.branch = params[:branch] ? params[:branch] : nil
+            remote_theme.remote_url = params[:remote]
+            remote_theme.save!
 
-          @theme = Theme.new(user_id: theme_user&.id || -1, name: theme_name)
-          @theme.remote_theme = remote_theme
-          @theme.save!
+            @theme = Theme.new(user_id: theme_user&.id || -1, name: theme_name)
+            @theme.remote_theme = remote_theme
+            @theme.save!
 
-          render json: @theme, status: :created
-        else
-          render_json_error e.message
+            render json: @theme, status: :created
+          else
+            render_json_error e.message
+          end
         end
       end
     elsif params[:bundle] || (params[:theme] && THEME_CONTENT_TYPES.include?(params[:theme].content_type))

spec/requests/admin/themes_controller_spec.rb

@@ -230,6 +230,14 @@ RSpec.describe Admin::ThemesController do
         expect(response.status).to eq(201)
       end
 
+      it 'fails to import with a failing status' do
+        post "/admin/themes/import.json", params: {
+          remote: 'non-existant'
+        }
+
+        expect(response.status).to eq(422)
+      end
+
       it 'can lookup a private key by public key' do
         Discourse.redis.setex('ssh_key_abcdef', 1.hour, 'rsa private key')