Clean up the controller

bensheldon · Jan 22, 2025 · 96f5270 · 96f5270
1 parent 109deb1
commit 96f5270
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 21 deletions.
diff --git a/app/controllers/good_job/pauses_controller.rb b/app/controllers/good_job/pauses_controller.rb
@@ -2,22 +2,33 @@
 
 module GoodJob
   class PausesController < ApplicationController
+    before_action :validate_type, only: [:create, :destroy]
     def index
       @paused = GoodJob::Setting.paused
     end
 
     def create
-      return redirect_to({ action: :index }) unless params[:type].in?(%w[queue job_class label]) && params[:value].present?
+      pause_type = params[:type].to_sym
+      pause_value = params[:value].to_s
 
-      GoodJob::Setting.pause(params[:type].to_sym => params[:value])
-      redirect_to(good_job.pauses_path, notice: "Successfully paused #{params[:type]} '#{params[:value]}'")
+      GoodJob::Setting.pause(pause_type => pause_value)
+      redirect_to({ action: :index }, notice: "Successfully paused #{params[:type]} '#{params[:value]}'")
     end
 
     def destroy
-      return redirect_to({ action: :index }) unless params[:type].in?(%w[queue job_class label]) && params[:value].present?
+      pause_type = params[:type].to_sym
+      pause_value = params[:value].to_s
 
-      GoodJob::Setting.unpause(params[:type].to_sym => params[:value].to_s)
-      redirect_to(good_job.pauses_path, notice: "Successfully unpaused #{params[:type]} '#{params[:value]}'")
+      GoodJob::Setting.unpause(pause_type => pause_value)
+      redirect_to({ action: :index }, notice: "Successfully unpaused #{params[:type]} '#{params[:value]}'")
+    end
+
+    private
+
+    def validate_type
+      return if params[:type].in?(%w[queue job_class label]) && params[:value].to_s.present?
+
+      raise ActionController::BadRequest, "Invalid type"
     end
   end
 end
diff --git a/app/views/good_job/pauses/_group.html.erb b/app/views/good_job/pauses/_group.html.erb
@@ -14,12 +14,12 @@
           <%= value %>
           <%= button_to(
                 { action: :destroy, type: type, value: value },
-            method: :delete,
-            class: 'btn btn-sm btn-outline-primary',
-            data: { confirm: t('good_job.pauses.index.confirm_unpause', value: value) }
+                method: :delete,
+                class: 'btn btn-sm btn-outline-primary',
+                data: { confirm: t('good_job.pauses.index.confirm_unpause', value: value) }
               ) do %>
-              <%= render_icon "play" %>
-              <%= t("good_job.pauses.index.unpause") %>
+                  <%= render_icon "play" %>
+                  <%= t("good_job.pauses.index.unpause") %>
           <% end %>
         </li>
       <% end %>

diff --git a/app/views/good_job/pauses/index.html.erb b/app/views/good_job/pauses/index.html.erb
@@ -9,7 +9,7 @@
 <% if GoodJob.configuration.enable_pauses %>
   <div class="card mb-4">
     <div class="card-body">
-      <%= form_tag({ action: :destroy }, method: :post, class: 'row g-3 align-items-end') do %>
+      <%= form_tag({ action: :create }, method: :post, class: 'row g-3 align-items-end') do %>
         <div class="col-3">
           <%= label_tag :type, t('good_job.pauses.index.type'), class: 'form-label' %>
           <%= select_tag :type,
@@ -26,7 +26,10 @@
           <%= text_field_tag :value, nil, class: 'form-control', autocomplete: "off", required: true %>
         </div>
         <div class="col-3">
-          <%= submit_tag t('good_job.pauses.index.pause', value: ''), class: 'btn btn-primary' %>
+          <%= button_tag type: "submit", class: 'btn btn-primary' do %>
+            <%= render_icon "pause" %>
+            <%= t('good_job.pauses.index.pause', value: '') %>
+          <% end %>
         </div>
       <% end %>
     </div>

diff --git a/spec/app/controllers/good_job/pauses_controller_spec.rb b/spec/app/controllers/good_job/pauses_controller_spec.rb
@@ -34,11 +34,13 @@
     end
 
     it 'redirects with no action for invalid parameters' do
-      post :create, params: { type: 'invalid', value: 'something' }
-      expect(response).to redirect_to action: :index
+      expect do
+        post :create, params: { type: 'invalid', value: 'something' }
+      end.to raise_error(ActionController::BadRequest)
 
-      post :create, params: { type: 'queue', value: '' }
-      expect(response).to redirect_to action: :index
+      expect do
+        post :create, params: { type: 'queue', value: '' }
+      end.to raise_error(ActionController::BadRequest)
     end
   end
 
@@ -60,11 +62,13 @@
     end
 
     it 'redirects with no action for invalid parameters' do
-      delete :destroy, params: { type: 'invalid', value: 'something' }
-      expect(response).to redirect_to action: :index
+      expect do
+        delete :destroy, params: { type: 'invalid', value: 'something' }
+      end.to raise_error(ActionController::BadRequest)
 
-      delete :destroy, params: { type: 'queue', value: '' }
-      expect(response).to redirect_to action: :index
+      expect do
+        delete :destroy, params: { type: 'queue', value: '' }
+      end.to raise_error(ActionController::BadRequest)
     end
   end
 end