trjhtr

I'm working on a feature which user can set frequency of a job. I am using Sidekiq.

My solution is when someone updates the frequency from view, I will delete all old scheduled jobs and create a new job in the action of controller.

I'm looking for a cleaner and more succinct way to do this.

Right now, I have a frequency column on the settings table. There is an update_frequency method in DashboardController to delete all old scheduled jobs and create a new scheduled job.

app/controllers/dashboard_controller.rb

class DashboardController < ApplicationController
 def update_frequency
 setting = Setting.find_by(id: params[:id])
 if setting.blank?
 render json: status: 404, msg: 'Not found.'
 else
 if setting.update_attributes(frequency: params[:frequency])
 # find and delete all scheduled jobs
 scheduler = Sidekiq::ScheduledSet.new
 old_jobs = scheduler.select 
 old_jobs.each(&:delete) if old_jobs.present?

 # add new schedule job
 HardWorker.perform_in(setting.frequency.minutes)

 render json: status: 200, msg: 'update frequency successful.'
 else
 render json: status: 500, msg: 'update frequency failed.'
 end
 end
end

A HardWorker to do something and create a new scheduled job:

app/workers/hard_worker.rb

class HardWorker
 include Sidekiq::Worker

 def perform
 # Do something
 setting = Setting.first
 HardWorker.perform_in(setting.frequency.minutes)
 end
end

I don't think there is a more succinct way to do that with Sidekiq, you will probably need to delete and recreate jobs.

Some comments

You probably don't need to check if old_jobs.present? when deleting. I am not familiar with ScheduledSet#select but I would imagine that it returns an empty array when no job is selected.

You don't need to update your jobs when the frequency is the same as the one already set. Even if you check it in the frontend it is a good idea to check it here to in case the view and the database are out of sync (read: in case someone changed the setting while your view was open).

Instead of using nested ifs, I like to use guards.
I also like to extract local variables into methods (see def setting).

Here is an example of how you could refactor it.

It may seem longer, but it is easier to understand when looking at the update_frequency method. If you need more precise understanding on what is considered as same_frequency, or reschedule_jobs, then you can dive into these methods. Otherwise looking at update_frequency probably gives you everything you need to understand when browsing the code (in 6 months when you will have forgotten of this part)

class DashboardController < ApplicationController
 def update_frequency
 return render_not_found unless setting
 return render_same_frequency if same_frequency?
 return render_update_failed unless update_model

 reschedule_jobs

 render json: status: 200, msg: 'update frequency successful.' 
 end

 def render_not_found
 render json: status: 404, msg: 'Not found.' 
 end

 def render_update_failed
 render json: status: 500, msg: 'update frequency failed.' 
 end

 def render_same_frequency
 render json: status: 200, msg: 'Frequency updated to the same value' 
 end

 def update_model
 setting.update_attributes(frequency: new_frequency)
 end

 def same_frequency?
 setting.frequency == params[:frequency].to_i
 end

 def setting
 @setting ||= Setting.find_by(id: params[:id])
 end

 def reschedule_jobs
 # find and delete all scheduled jobs
 Sidekiq::ScheduledSet.new
 .select work
 .each(&:delete)

 # add new schedule job
 HardWorker.perform_in(setting.frequency.minutes)
 end
end