Introduction

We write web development software for ~20 years, we used Perl, PHP, Python, C#, and Java during the first 13 years of our existence. Now for the last 7 years, we decided to focus on our favorite language which is Ruby. A language that has one of the greatest MVC frameworks, which was a pattern for all other newer MVC frameworks in PHP, Java or C#. We are talking about Ruby on Rails of course.

MVC or not to MVC?

Is Ruby on Rails, or even the MVC pattern, a silver bullet for each web application? Of course not. It is an excellent tool for starting new applications, that have to be delivered fast and reliably. And that is great. Ruby community has grown a lot because of that. As a community we shipped thousands of applications in a really short period. And as a community, that has a lot of successful applications, we encountered another challenge. Develop those applications, add a lot of complex features, model long business process and handle all its edge-cases. Maintain that, and write it in a really explicit way so new developers can be easily introduced to our big legacy projects. And then our community was like:

"Man, I don't like rails way anymore".

A lot of great conferences was only about "how we should change rails way so we can still have fun and joy during developing our applications?".

Few people were like:

“Forget it, let's switch to functional programming”.

Well, probably with applications where each millisecond is really important they were mostly right.

When and how to replace MVC?

But there was also a lot of people who were like:

"Yea rails way to have its disadvantages, but we know which one. Let's try to change some approaches to convert those parts of our frameworks to advantages".

Those people created a few new "frameworks", "libraries" or "approaches". Name it as you like. The thing is, after those years and testing a lot of solutions, we found Trailblazer a great tool for complex applications. Why? That series won't be about it but there you have some great articles about Trailblazer, and why you should consider it in some of your projects:

Problem with Trailblazer

But when you know that you want to use TRB and you know why, you will encounter quite an important problem. There are not a lot of detailed tutorials about how to start with TRB. Even though it doesn't have to be a big deal - we can refactor our existing application in the 'step by step' strategy without diving into all Trailblazer details at once.

Solution

So in a co-operation with Trailblazer core-team, we are starting a tutorial called "Refactor your legacy Rails app into shiny Trailblazer application". If you are interested in all free content that we will provide during the next weeks and months,

button subscribe

so you won't miss any post full of trailblazer-related knowledge.

Refactoring - part 1

In this and all next posts we will focus on refactoring below application, to have more explicit and well organized code using Trailblazer.

https://github.com/rubycentral/cfp-app

Let’s talk about core steps of our refactor:

  1. Select one of the core features that we want to refactor
  2. Identify where logic is implemented and analyze it
  3. Describe our new Operation by integration tests
  4. Implement Operation so our tests will pass (like TDD)
  5. Update old fat controller
  6. Commit better code

Identify core feature

First feature that we want to refactor, is creating proposal- since application is about creating and accepting proposals - that sounds to be a reasonable choice.

Analyze business logic

def create

  if @event.closed? && @event.closes_at < 1.hour.ago 
    redirect_to event_path(@event)
    flash[:danger] = 'The CFP is closed for proposal submission.'
    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

So as we can see, we have old-fashioned rails way caused by poor MVC architecture approach here. Fat controller, with whole business logic inside controller and/or model. Let’s define what business logic we have here:

  • Checking if event is closed or almost closed
@event.closed? && @event.closes_at < 1.hour.ago
  • Initializing proposal with given data
@proposal = @event.proposals.new(proposal_params)
  • Setting speaker data
speaker = @proposal.speakers[0]
speaker.user_id = current_user.id
speaker.event_id = @event.id
  • Saving proposal
if @proposal.save
  • Updating bio of current user based on speaker from proposal bio (that is User model logic )
current_user.update_bio

Since controller is responsible to just (considering MVC) route the HTTP calls, receive the request, run something and make some decisions based on its result... (render some view, set cookie, http code) , what we want to have in controller is only:

  • Calling proper Operation Objects that are handling some logic
  • Render proper response depends on Operation result, and none of above do that. So that is code smell that tells us we need to move that logic to Operation class.

Refactoring

(Iteration 1): Tests:

  • Creating proposal - basic case
require 'rails_helper'
describe Proposal::Operation::Create do
 subject(:result) {described_class.trace(params: params) }
 context "with valid params" do
   let(:params) {
    {
       proposal: {
             title: 'Foo',
             abstract: 'Abstract bar',
             details: 'About proposal',
             pitch: 'Elevator'
       }
     }
   }

   context "with open event" do
     let(:current_user) { FactoryGirl.create(:user) }
     let!(:session_format) { SessionFormat.create(name: 'FooBar', event: event)}
     it "creates a proposal" do
       expect(result).to be_success
       expect(result[:model].title).to eq('Foo')
       expect(result[:model].persisted?).to be_truthy
     end
   end
  end
end

Setup is:

  • Create Session format which is necessary to create proposal

  • Fill params with proper data

What test does:

  • Checks if our result is success

  • Checks if result returned model with saved title (so we know params was passed correctly)

  • Checks if our model was saved

(Iteration 1) Implement Operation & Contract

  • Basic Operation ( add link to description of Present class )
module Proposal::Operation
  class Create < Trailblazer::Operation
    class Present < Trailblazer::Operation
     step :model
     # we have to define which contract we use to build validation
     step Contract::Build(constant: Proposal::Contract::Create)
     # even though we don't vlaidate anything this step is
     # necessary to fill model with params (double-check it)
     step Contract::Validate(key: :proposal)

     def model(ctx, **)
       ctx[:model] = proposal.new
     end
    end
    step Nested(Present)
    step Contract::Persist()
  end
end
  • Basic Contract - we need to define which fields our proposal will receive and accept
    module Proposal::Contract
      class Create < Reform::Form
        property :title
        property :abstract
        property :details
        property :pitch
      end
    end

At this point we have test and operation which would pass, if we won't have model validation that checks if session_format_id is presence. So since we are trying to follow TDD rules, let's make another iteration.

(Iteration 2) Update our spec, by setup-ing all needed data

Creating proposal - basic case with necessary data:

require 'rails_helper'
describe Proposal::Operation::Create do
 subject(:result) {described_class.trace(params: params) }

  context "with valid params" do
    let(:params) {
      {
         event_slug: 'Lorem',
         proposal: {
           title: 'Foo',
           abstract: 'Abstract bar',
           details: 'About proposal',
           pitch: 'Elevator'
        }
      }
    }

    context "with open event" do
      let(:current_user) { FactoryGirl.create(:user) }
      let!(:event) {
        Event.create(state: 'open', slug: 'lorem', name: 'Ipsum', url: 'http://www.c.url')
      }
      let!(:session_format) { SessionFormat.create(name: 'FooBar', event: event)}

      it "creates a proposal" do
        expect(result).to be_success
        expect(result[:model].title).to eq('Foo')
        expect(result[:model].persisted?).to be_truthy
      end
    end
  end
end

(Iteration 2) Update contract so it will accept session_format_id by adding below line

property :session_format_id

Now our test is passing:

Finished in 0.2853 seconds (files took 6.91 seconds to load)
1 example, 0 failures

But that is only basic creating. Lets iterate again, to handle assigning proposal to event.

(Iteration 3) Update success flow test

Change description and add expectation that checks if we assigned proposal to proper event:

context with open event do
  let!(:event) { 
    event.create(state: 'open', slug: 'Lorem', name: 'Ipsum', url: 'http://www.c.url')
  }
  let!(:session_format) { SessionFormat.create(name: 'FooBar', event: event) }

  it "creates a proposal assigned to event identified by slug" do
    expect(result).to be_truthy
    expect(result[:model].title).to eq('Foo')
    expect(result[:model].persisted?).to be_truthy
    expect(result[:model].event).to eq(event)
  end

Since we don’t have any logic responsible for assigning proposal to event, lets update Operation

(Iteration 3) Update Operation class

We have to find and set event, and then assign it to proposal

module Proposal::Operation
  class Create < Trailblazer::Operation
    class Present < Trailblazer::Operation
      step :model
      step Contract::Build(constant: Proposal::Contract::Create)
      step Contract::Validate(key: :proposal)

      def model(ctx, **)
        ctx[:model] = Proposal.new
      end
    end

    step Nested(Present)
    #find event
    step :event
    #assign event to our model
    step :assign_event
    # save model
    step Contract::Persist()

    def event(ctx, params:, **)
      ctx[:event] = Event.find_by(slug: params[:event_slig])
    end

    def assign_event(ctx, **)
      ctx[:model].event = ctx[:event]
    end
  end
end

And that would be it ( notice: test is 3x times slower thanks to querying DB but for the sake of explicity of the code and having Operation test as a "integration test", we will avoid mocking and stubbing).

Finished in 0.70979 ( files took 8.12 seconds to load)
1 example, 0 failures

Since we are looking for an event, we should test edge-case when we won't find any event.

(Iteration 4) Add test checking case when we didn't find any event

Since there will be no event, we don't need to create real session_format, because code won't event try to find it (but we need variable so params let won’t throw exception). Also we need to check if our :errors will store error message:

context 'without event' do
 let(:session_format) { instance_double(SessionFormat, id: 53) }
 it 'returns result object with an error about closed event without saving the proposal' do
   expect(result[:errors]).to eq('Event not found')
   expect(result).to be_failure
 end
end

Lets run our tests:

$ rspec spec/concepts/proposal/operation/create_spec.rb
.F
Failures:
  1) Proposal::Operation::Create with valid params without event returns result object with an error about closed event without saving the proposal
     Failure/Error: expect(result[:errors]).to eq('Event not found')
       expected: 'Event not found'
            got: nil
       (compared using ==)

Finished in 0.34319 seconds (files took 6.09 seconds to load)
2 examples, 1 failure

Since we only added test, and didn't update Operation logic to handle this case, lets update Operation _ (Iteration 4)_ Implement handling "no event" case. We need to add fail step which will be called if any of the previous methods will return nil/false/StandardError ancestor.

module Proposal::Operation
 class Create < Trailblazer::Operation
   class Present < Trailblazer::Operation
     step Model(Proposal, :new)
     step Contract::Build(constant: Proposal::Contract::Create)
     step Contract::Validate(key: :proposal)
   end
   step Nested(Present)
   step :event
   fail :event_not_found
   step :assign_event
   step Contract::Persist()

   def event(ctx, params:, **)
     ctx[:event] = Event.find_by(slug: params[:event_slug])
   end

   def assign_event(ctx, **)
     ctx[:model].event = ctx[:event]
   end

  # -- bad stuff handled there --
   def event_not_found(ctx, **)
     # we set error message as a string ( of course it can be anything, f.e. Array of strings )
     ctx[:errors] =  'Event not found'
   end
 end
end

What is happening there? If a step returns value that evaulatues to false (nil/false) then operation flow goes to the fail track ( closest fail step ).
So in our case since event method returns nil - operation calls for fail :event_not_found. Method itself sets ctx[:errors] value, and Trailblazer Operation itself returns result which will be marked as failure (Result object responds to failure? method and returns true).

$ rspec spec/concepts/proposal/operation/create_spec.rb
..
Finished in 0.48725 seconds (files took 6.93 seconds to load)
2 examples, 0 failures