Какой хороший подход для очистки (СУХОГО) этого контроллера?

Question

У меня есть действие, выполняемое с каждым методом create, update и update_status на моем контроллере, но я чувствую, что повторяюсь, и буду очень признателен за помощь в улучшении подхода к написанию этого.

Я абстрагировал логику обновления для службы, но параметры разные для каждого метода моего контроллера.

def create
    @story = Story.new(story_params)
    @story.creator = current_user

    if @story.save

      next_state = StoryStateService.new(@story, current_user, nil).call

      if next_state 
        @story.update_column(:status_id, next_state) 
      end 

      redirect_to stories_path
    else
      render 'stories/new'
    end
  end

  def update
    @story = Story.find(params[:id])   

    if @story.update(story_params)

      next_state = StoryStateService.new(@story, current_user, nil).call

      if next_state 
        @story.update_column(:status_id, next_state) 
      end  

      redirect_to stories_path
    else
      render 'edit'
    end
  end

  def update_story_status_event
    story = Story.find(params['story_id'])
    sub_action = params['sub_action']

    next_state = StoryStateService.new(story, current_user, sub_action).call

    if next_state 
      story.update_column(:status_id, next_state) 
    end

    redirect_to stories_path
  end

Как видите, у меня

   next_state = StoryStateService.new(story, current_user, sub_action).call

    if next_state 
      story.update_column(:status_id, next_state) 
    end

повторяется для трех методов, но при обычном создании и обновлении мне не нужно отправлять параметр sub_action (строку).

Answer 1

Я бы просто извлек продвижение истории, что-то большее, чем это было бы для меня излишним, учитывая простоту всего остального.

def create
  @story = Story.new(story_params)
  @story.creator = current_user

  if @story.save
    advance_story @story

    redirect_to stories_path
  else
    render 'stories/new'
  end
end

def update
  @story = Story.find(params[:id])

  if @story.update(story_params)
    advance_story @story

    redirect_to stories_path
  else
    render 'edit'
  end
end

def update_story_status_event
  story = Story.find(params['story_id'])
  sub_action = params['sub_action']

  advance_story story, sub_action

  redirect_to stories_path
end

private 

def advance_story(story, sub_action = nil)
  next_state = StoryStateService.new(story, current_user, sub_action).call

  if next_state
    story.update_column(:status_id, next_state)
  end
end

Answer 2

Я не уверен, что делает StoryStateService, но если он имеет дело с обновлением ряда моделей, то вы также можете создать модуль или класс и поместить его в каталог lib.

Другой вариант (это может быть более предпочтительным для Story.find()) - использовать before_action обратный вызов.(https://apidock.com/rails/v4.0.2/AbstractController/Callbacks/ClassMethods/before_action)

например

Поскольку вы используете Story.find() в update и update_story_status_event, вы можете сделать следующее:

before_action :find_story, :only => [:update, :update_story_status_event]

   def find_story
     @story = Story.find(params[:id])
   rescue ActiveRecord::RecordNotFound
     # handle error here
   end