Trailblazer tutorial: refactoring fat controller - part 3. 
Tags: ruby, tutorial, beginners, programming
Published: 2019-10-23 10:28:18

---
title: Trailblazer tutorial: refactoring fat controller - part 3. 
published: true
description: 
tags: ruby, tutorial, beginners, programming
cover_image: https://thepracticaldev.s3.amazonaws.com/i/cq3a26651rjaew7lyc7t.png
series: Trailblazer tutorial step by step
canonical_url: https://www.2n.pl/blog/trailblazer-tutorial-refactoring-fat-controller-part-3
---
Before we move our business logic to Operation class we still need to handle:

- Setting the speaker data,
- Updating the bio of current user based on the speaker from proposal bio. 

**_(Iteration 6)_** Describe handling speaker data by tests.

```ruby
context "with open event" do
  let(:current_user) { FactoryBot.create(:user) }
  let!(:event) {
    FactoryBot.create(:event, state: 'open', slug: 'Lorem')
  }
  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

  it 'assign current user as a speaker assigned to current event and filled with passed bio' do
    expect(result[:model].speakers[0].user).to eq(current_user)
    expect(result[:model].speakers[0].bio).to eq('my bio')
    expect(result[:model].speakers[0].event).to eq(event)
  end

  it 'update speaker bio from passed params' do
    expect(result[:model].speakers.first.bio).to eq("my bio")
  end
end
```

Since we have the whole expected behavior described by tests, we can investigate how current code works and figure out about what we want to re-use and what we want to avoid.

As we can see in controller #create,

```ruby
@proposal = @event.proposals.new(proposal_params)
speaker = @proposal.speakers[0]
speaker.user_id = current_user.id
speaker.event_id = @event.id
```

our current code is using nested_attributes_for to handle initializing data for speakers. So part of "initializing speakers data" is handled by rails magic, and second part like assigning user and event is handled by the controller, which (as we already know) should be responsible for that.

Since our logic is more complex than just assigning nested data from the form, let's avoid using nested_attributes and try to avoid handling this kind of logic in the controller action altogether. We shall stick to the rules of SOLID and OOP and write as much explicit code as we can.

Also, we can investigate the whole codebase to check if we use

```ruby 
accepts_nested_attributes_for :speakers
``` 

from proposal.rb model anywhere else except #create action in our controller. Since it is used in proposals/_form.html.haml partial which is also used in edit action, we will not remove it from model yet as we don’t want to break the edit/update actions. But it would be great to comment out this code in the model to remember we need to remove it after refactoring the edit action.

```ruby
#toDo remove when refactor #edit proposal 
accepts_nested_attributes_for :speakers
 ```

Let's focus on handling that data the trailblazer way. Trailblazer contracts (which are using reform under the hood) give us a really simple way to handle that. As we can see in the documentation, to handle nested resources inside the contract we can use **_collection_**, which enables us to define collection of associated resources we will initialize as we fill the contract with data.

```ruby      
collection :speakers, populate_if_empty: Speaker do
  property :bio
end
```

The above code will search for *speakers* key in params we received and use them to populate all speakers, passing *bio* attribute to each new instance of **Speaker**.

After we added this code, let's run all tests and check whether they fail or pass. And as we can see, the tests within "with open event", including the ones that had previously passed, all have failed.
**What is the reason** for that and **how to debug** it? The reason is quite obvious, but also quite hard to track before we understand how each step works. Now we can slow down a bit, and discuss each TRB Macro step:

- initialize our proposal without filling it with data

```ruby
step Model(Proposal, :new) 
```

- that initialized contract will be used to validate data that we want to pass to our model through Contract rules. In this step, we already filled 

```ruby
step Contract::Build(constant: Proposal::Contract::Create) 
```

we can also call it by:

```ruby
step :build_contract

def build_contract(ctx, model:, **)  
  ctx["result.contract.default"] = User::Contract::Create.new(model)  
end
```

- that step redirects the flow to the Present operation. After finishing the flow returns back to the outer operation (Proposal::Create in our case)

```ruby
step Nested(Present)
```

- *IMPORTANT*: First it fills the model with data from params based on contract (and members/collection if there are some) and only then it checks if data in [:proposal] hash from params have proper values and format based on initialized Contract

```ruby
step Contract::Validate(key: :proposal)
```

we can also call it by:

```ruby
step :validate
def validate(ctx, params:, **)  
  ctx["result.contract.default"].validate(params)  
end
```

- this step call .save method on our object

```ruby
step Contract::Persist(method: :save) 
```

we can also call it by:

```ruby
def persist(ctx, model:, **)  
  ctx["result.contract.default"].save  
end
```

Since our Proposal class is AR class, and we didn’t refactor it yet, we still have one of the biggest issues with standard MVC approach there - we have all validations in the model that doesn’t care about the context of where we try to save the object. Because of that if we call  ctx[:model].errors after this step, we will receive: 

```ruby  
step Contract::Persist(method: :save)
fail ->(ctx, **) { binding.pry; true}

pry(Proposal::Operation::Create)> ctx[:model].errors
=> #<ActiveModel::Errors:0x007ff9c50cfe38
@base=
#<Proposal:0x007ff9c2799de8
id: nil,
event_id: 1,
state: "submitted",
uuid: nil,
title: "Foo",
session_format_id: 1,
track_id: nil,
abstract: "Abstract bar",
details: "About proposal",
pitch: "Elevator",
last_change: nil,
confirmation_notes: nil,
proposal_data: {},
updated_by_speaker_at: nil,
confirmed_at: nil,
created_at: nil,
updated_at: nil>,
@details=
{:"speakers.event"=>[{:error=>:blank}], :"speakers.name"=>[{:error=>:blank}], :"speakers.email"=>[{:error=>:blank}, {:error=>:invalid, :value=>nil}]},
@messages={:"speakers.event"=>["can't be blank"], :"speakers.name"=>["can't be blank"], :"speakers.email"=>["can't be blank", "is invalid"]}>
```

Since we know about those validations, before we focus on refactoring Speaker model, we should check if other places are using those validations.

If we comment on all AR validation and run specs one will fail:

```bash
rspec ./spec/features/staff/speakers_spec.rb:101 
# Organizers can manage speakers for Program Sessions 
# An organizer Can't update a speaker with a bad email .
```

In this case lets comment validation, add ToDo comment that will indicate that we need to handle that validation for other actions & contexts in our application and go back to our current code.

```ruby
class Speaker < ApplicationRecord
...
#ToDo: move all validations to proper contracts, that will distinguish which validation is necessary when 
# validates :event, presence: true
# validates :bio, length: {maximum: 500}
# validates :name, :email, presence: true, unless: :skip_name_email_validation
# validates_format_of :email, with: Devise.email_regexp
```

So let's move validation to contract, and handle validation errors from the contract in operation (like a gentleman, not a barbarian which uses one set of AR validations definition disregarding the context).

```ruby
module Proposal::Contract
  class Create < Reform::Form
    property :event_id
    ...
    collection :speakers, populate_if_empty: Speaker do
      property :bio  
      property :event_id  
      property :user_id  
  
      validates :event_id, presence: true  
      validates :user_id, presence: true  
      validates :bio, length: {maximum: 500}  
    end
  end
end
```

Then let's update our Operation to handle validation errors,

```ruby
  step  Contract::Validate(key: :proposal)
  fail :validation_failed, fail_fast: true
```

and run our "with open event" tests which will give us:


```bash
Failure/Error: expect(result).to be_success

expected `#<Trailblazer::Operation::Trace::Result(<Result:false #<Trailblazer::Context:0x007fcb32946498 @wrappe..., 
:errors=>{:"speakers.event_id"=>["can't be blank"],
:"speakers.user_id"=>["can't be blank"]}}> >)>.success?`
to return true, got false
```

Since we don’t pass event_id or user_id we shouldn’t be surprised. Starting with doing that, user_id will be taken from current_user, so we need to pass it to contract ([http://trailblazer.to/gems/operation/2.0/contract.html#manual-build](http://trailblazer.to/gems/operation/2.0/contract.html#manual-build)).

```ruby
module Proposal::Contract
  class Create < Reform::Form
    property :title
    property :abstract
    property :details
    property :pitch
    property :session_format_id
    property :event_id
    property :current_user, virtual: true
    validates :current_user, presence: true

    collection :speakers, populate_if_empty: Speaker do
      property :bio
      property :event_id
      property :user_id

      validates :event_id, presence: true
      validates :user_id, presence: true
      validates :bio, length: {maximum: 500}
    end
  end
end
```

That is how we prepared our contract to receive current_user which is not a field of Proposal.

We shall focus on how to pass event_id (which we have in Contract object), and how to pass user_id ( taken from current_user) to initialized speaker. Since the contract is filled by data during building it, we are supposed to have an event initialized to have it in the contract. So we have to change the operation steps order*(in the sake of visibility, for now, we skip using Nested Present class)*.

```ruby
module Proposal::Operation
  class Create < Trailblazer::Operation      
    step :event
    fail :event_not_found, fail_fast: true
    step Model(Proposal, :new)
    step :assign_event
    step Contract::Build(constant: Proposal::Contract::Create,
      builder: -> (ctx, constant:, model:, **){
        constant.new(model, current_user: ctx[:current_user])
      }
    )
    
    step :event_open?
    fail :event_not_open_error, fail_fast: true
    step Contract::Validate(key: :proposal)
    fail :validation_failed, fail_fast: true
    step Contract::Persist(method: :save)
    #our definitions ...    
  end  
end
```    

As we can see we moved the event step to the beginning of the operation since **event** object will be useful during the next steps like *building contract* and *eventually validating* it. Since we have our event, the second issue was to pass **current_user**. We did by using *builder* which is an indication of how we will initialize Proposal::Contract::Create. “**constant**” there is just Proposal::Contract::Create (we could replace constant with Proposal::Contract::Create, but it will be longer and less readable). Thanks to using builder, we can not only pass model but also additional arguments, like current_user. 

Operation initializes the contract with all the necessary data, so let's focus on how to initialize speakers based on passed data in our **contract**:

```ruby
collection :speakers, populator: ->(model:, index:, **args) {    
  model.insert(index,
    Speaker.find_or_initialize_by(user_id: current_user.id, event_id: self.event_id) )
  } do
      property :bio  
      property :event_id  
      property :user_id  

      validates :event_id, presence: true  
      validates :user_id, presence: true  
      validates :bio, length: {maximum: 500}  
    end
```

We can check our tests:

```bash
1) Proposal::Operation::Create with valid params with open event
 update speaker user bio from passed params
Failure/Error: expect(result[:model].speakers[0].user.bio).to eq("my bio")

expected: "my bio"
got: "A great Bio"

(compared using ==)
#./spec/concepts/proposal/operation/create_spec.rb:48:in `block (4 levels) in <top (required)>'

  
Finished in 1.37 seconds (files took 7.3 seconds to load)
6 examples, 1 failure
```

This is not surprising since we didn’t implement updating user bio-based on a new speaker bio. All other tests are green, so that is cool, and we can focus on making the last one to pass. Updating user bio-based on bio from a saved speaker is business logic of the whole operation, so we just add another step:

```ruby
  step Contract::Persist(method: :save)  
  step :update_user_bio  
```  

And a step definition:

```ruby    
def update_user_bio(ctx, **)    
  ctx[:current_user].update(bio: ctx[:model].speakers[0][:bio])  
end
```    


And that's it. The test is passing, everyone is happy. Of course, we should also test and handle all invalid scenarios, like validations in the user model, or errors during saving our contract - and I suggest you to try to implement it on your own.