Since we prepared and tested Operation and Contract to create Proposal with Trailblazer way, it is time to clean the mess in the controller. Let's remind how our controller #create action looks like right now:

def create  
  if @event.closed? && @event.closes_at < 1.hour.ago  
    redirect_to event_path (@event)  
    flash[:danger] = "The CFP is closed for proposal submissions."  
  return  
 end  @proposal = @event.proposals.new(proposal_params)  
  speaker = @proposal.speakers[0]  
  speaker.user_id = current_user.id  
  speaker.event_id = @event.id  

  if @proposal.save  
    current_user.update_bio  
    flash[:info] = setup_flash_message  
    redirect_to event_proposal_url(event_slug: @event.slug, uuid: @proposal)  
  else  
  flash[:danger] = "There was a problem saving your proposal."  
  render :new  
  end  
end

Since we have all business logic moved to Operation, and all context-validation rules applied into the controller let's use it. But before, since we are refactoring our controller, let us improve test-coverage for all cases in this action, so we will be able to sleep well after deploying refactored code to production:

# spec/controllers/proposals_controller_spec.rb
require 'rails_helper'  

describe ProposalsController, type: :controller do  
  let(:event) { FactoryBot.create(:event, state: 'open') }
  # ...
  describe 'POST #create' do  
    let(:proposal) { build(:proposal, uuid: 'abc123') }  
    let(:user) { create(:user) }  
    let(:params) {  
      {  
        event_slug: event.slug,  
        proposal: {  
          title: proposal.title,  
          abstract: proposal.abstract,  
          details: proposal.details,  
          pitch: proposal.pitch,  
          session_format_id: proposal.session_format.id,  
          speakers: [  
            {  
              bio: 'my bio'  
            }  
          ]  
        }  
      }  
    }  

    before { allow(controller).to receive(:current_user).and_return(user) }  

    it "create Proposal" do  
      expect{  
        post :create, params: params  
      }.to change{Proposal.count}.by(1)  
    end

    it "sets the user's bio if not is present" do  
      user.bio = nil  
      post :create, params: params  
      expect(user.bio).to eq('my bio')  
    end  

    context "event closed" do  
      let!(:event) { FactoryBot.create(:event, state: "closed") }  
      it "redirects to event show page with 'The CFP is closed for proposal submissions.'  message" do  
        post :create, params: params  
        expect(response).to redirect_to(event_path(event))  
        expect(flash[:danger]).to match(/The CFP is closed for proposal submissions.*/)  
      end
    end  

    context "wrong slug passed" do  
      let(:wrong_params) { params.merge(event_slug: 'Non-existing-slug')}  
        it "re-render new form with 'Your event could not be found, please check the url.' message" do  
        post :create, params: wrong_params  
        expect(response).to redirect_to(events_path)  
        expect(flash[:danger]).to match(/Your event could not be found, please check the url.*/)  
      end  
    end
  end
end

Before we will start refactoring controller code, let's take a look for what as Rails developers we love so much:

class ProposalsController < ApplicationController  
  before_action :require_event, except: :index  
  before_action :require_user  
  before_action :require_proposal, except: [ :index, :create, :new, :parse_edit_field ]  
  before_action :require_invite_or_speaker, only: [:show]  
  before_action :require_speaker, except: [ :index, :create, :new, :parse_edit_field ]
  #a lot of actions

  private  
  def proposal_params  
    params.require(:proposal).permit(:title, {tags: []}, :session_format_id,
      :track_id, :abstract, :details, :pitch, custom_fields: @event.custom_fields,  
      comments_attributes: [:body, :proposal_id, :user_id],  
      speakers_attributes: [:bio, :id])  
  end
end

That is not a tutorial which goal is to convince you that having a lot of before_action callback will bite you sooner than later. I also don't want to try to list all OOP rules that this approach breaks. I just strongly believe that you understand why we will try to avoid using callbacks, and if you don't - then you can google it :) Meantime we will focus on our step-by-step tutorial.

So what we are planning to do with our controller is:

  • replace require_event as a callback into subprocess called from Operations that needs event (what we already have in our Operation),
  • change setup_flash_message to expect receive event argument, instead of using instance variable inside,
  • stop using strong params since contract using reform are taking care of it better than whole controller ( which doesn't know the context of each action, if it has one strong params method for the whole set of actions ),
  • use Proposal::Operation::Create and remove all business logic from controller,
  • ensure that our controller is only responsible for rendering proper response.

So how our new action would look like after refactor:

def create  
  result = Proposal::Operation::Create.call(params: params, current_user: current_user)  
  if result.success?  
   flash[:info] = setup_flash_message(result[:model].event)  
   redirect_to event_proposal_url(event_slug: result[:model].event.slug, uuid: result[:model])  
  elsif result[:errors] == "Event not found"  
    flash[:danger] = "Your event could not be found, please check the url."  
    redirect_to events_path  
  elsif result[:errors] == "Event is closed"  
    flash[:danger] = "The CFP is closed for proposal submissions."  
    redirect_to event_path(slug: result[:model].event.slug)  
  end
end

We also can exclude our action from before_action:

before_action :require_event, except: [:index, :create]

After we run all tests for #create context we get:

Run options: include {:locations=>{"./spec/controllers/proposals_controller_spec.rb"=>[35]}}
....

Finished in 1.37 seconds (files took 8.54 seconds to load)
4 examples, 0 failures

So that was it. Refactoring of #create action is finished. So let's slow down a bit, and think about what we have done, and what we achieved: we created 2 new abstract layers (Operation and Contract) that are more explicit and readable - they also have their own responsibilities. Both Operation and Contract are easier to test, they are isolated and easier to reuse.

Operation:
> It’s a simple service object that encapsulates and orchestrates all the business logic necessary to accomplish a certain task, such as creating a blog post or updating a user.

Contract:
> A contract is an abstraction to handle the validation of arbitrary data or object state. It is a fully self-contained object that is orchestrated by the operation.

We cleared controller so:
- it is not an example of how to break all SOLID rules at one time,
- it is easier to test - we can mock the whole Operation result instead of running integration test if we care about speed and isolation,
- we don't use before_action callback anymore so it is more explicit what is happening there and when,
- we get rid of strong params, so we can validate params that we receive on a contract layer in the context of a given domain process.

This was the first step in refactoring. Meantime we added few #ToDos, so now - we will focus on moving forward and clearing those parts. Let us know if you have any questions/feedback in comments or just join the Trailblazer community on gitter ( https://gitter.im/trailblazer/chat ).

If you still want to see some code, there is PR with our changes:
https://github.com/panSarin/cfp-app/pull/1/files.