Skip to content

Commit

Permalink
Use resourceful route for running a Task
Browse files Browse the repository at this point in the history
Instead of using a custom tasks#run route, we should move this
action to the RunsController as runs#create, taking advantage
of conventional CRUD operations.
  • Loading branch information
adrianna-chang-shopify committed Sep 8, 2022
1 parent 5b0e233 commit def5477
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 25 deletions.
20 changes: 19 additions & 1 deletion app/controllers/maintenance_tasks/runs_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,25 @@ module MaintenanceTasks
#
# @api private
class RunsController < ApplicationController
before_action :set_run
before_action :set_run, except: :create

# Creates a Run for a given Task and redirects to the Task page.
def create(&block)
task = Runner.run(
name: params.fetch(:task_id),
csv_file: params[:csv_file],
arguments: params.fetch(:task_arguments, {}).permit!.to_h,
&block
)
redirect_to(task_path(task))
rescue ActiveRecord::RecordInvalid => error
redirect_to(task_path(error.record.task_name), alert: error.message)
rescue ActiveRecord::ValueTooLong => error
task_name = params.fetch(:id)
redirect_to(task_path(task_name), alert: error.message)
rescue Runner::EnqueuingError => error
redirect_to(task_path(error.run.task_name), alert: error.message)
end

# Updates a Run status to paused.
def pause
Expand Down
18 changes: 0 additions & 18 deletions app/controllers/maintenance_tasks/tasks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,24 +24,6 @@ def show
@runs_page = RunsPage.new(@task.completed_runs, params[:cursor])
end

# Runs a given Task and redirects to the Task page.
def run(&block)
task = Runner.run(
name: params.fetch(:id),
csv_file: params[:csv_file],
arguments: params.fetch(:task_arguments, {}).permit!.to_h,
&block
)
redirect_to(task_path(task))
rescue ActiveRecord::RecordInvalid => error
redirect_to(task_path(error.record.task_name), alert: error.message)
rescue ActiveRecord::ValueTooLong => error
task_name = params.fetch(:id)
redirect_to(task_path(task_name), alert: error.message)
rescue Runner::EnqueuingError => error
redirect_to(task_path(error.run.task_name), alert: error.message)
end

private

def set_refresh
Expand Down
2 changes: 1 addition & 1 deletion app/views/maintenance_tasks/tasks/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
</h1>

<div class="buttons">
<%= form_with url: run_task_path(@task), method: :put do |form| %>
<%= form_with url: task_runs_path(@task), method: :post do |form| %>
<% if @task.csv_task? %>
<div class="block">
<%= form.label :csv_file %>
Expand Down
6 changes: 1 addition & 5 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,7 @@

MaintenanceTasks::Engine.routes.draw do
resources :tasks, only: [:index, :show], format: false do
member do
put "run"
end

resources :runs, only: [], format: false do
resources :runs, only: [:create], format: false do
member do
put "pause"
put "cancel"
Expand Down

0 comments on commit def5477

Please sign in to comment.