DEV Community: Adam Piotrowski

Refinements in ruby

Adam Piotrowski — Tue, 22 Mar 2022 09:07:39 +0000

Today I had the opportunity to use https://docs.ruby-lang.org/en/2.4.0/syntax/refinements_rdoc.html for the first time in my almost 8 years of Ruby programming. So in general it works in the way that you can override some classes in Ruby but the change is not globally. It creates the module, applies monkey patching, and using ModuleName. That makes things overridden only in the context of the runtime of that using.

Let's describe it with my use case example. In application, we had an evaluator for formulas. Implementation details are not important, what we should focus on is that after parsing data we had a simple eval like that:

max( nil * 10, 20 )

Normally any operation like * on a nil object raises an error and that was ok. But in this case, we have a proper value of 20 which should be the result of that code since it is the maximum value from passed values.

So my solution is to expand basic arithmetic methods in NilClass, and thanks to using refinement i did it only in the context of our evaluator without causing any changes in the rest of application.

  class Evaluator
    Refinement = Module.new do
      refine NilClass do
        %i[- + / *].each do |operator|
          define_method(operator) { |_| nil }
        end
      end
    end

    using Refinement

    def initialize
      (...)
     end
  end

Graphql-ruby and passing ISO8601 DateTimes without timezone

Adam Piotrowski — Fri, 25 Jun 2021 10:00:00 +0000

If you are writing web apps for some time, you had to encounter at least a few issues/situations when you had to deal with Timezones. If you are a Ruby developer, and you use Ruby on Rails then you are probably familiar with the difference of Time.now, Time.current and Time.zone.now. (if not, that is quick blogpost for one of those teams that are setting the standard of how to write Ruby code : https://thoughtbot.com/blog/its-about-time-zones)

Conclusions of that blogpost (and many others) are simple: use those methods that are referring to your app timezone and do not mess with that if you want to have smooth to maintain time-related code.

Now, let’s focus for a moment on a gem that helped a lot of us to have a nice time during working with our graphl API: https://github.com/rmosolgo/graphql-ruby .

What is easy to forget, that this gem is for all ruby applications. Not only for rails-based.

If you are thinking about that gem as an addition to your rails app that is following “standard” approaches: remember that this gem doesn’t have access to Rails DateAndTime::Zones class for example.

That is Time class ancestors from gem:

$: Time.ancestors
=> [Time, Comparable, Object, ErrorBubblingHelpers, Minitest::Expectations, PP::ObjectMixin,
 JSON::Ext::Generator::GeneratorMethods::Object, Kernel, BasicObject]

and that one is how Time ancestors looks like if we load default rails console:

$: Time.ancestors

=> [Time, DateAndTime::Compatibility, DateAndTime::Calculations, DateAndTime::Zones, Comparable, ActiveSupport::ToJsonWithActiveSupportEncoder, Object, ActiveSupport::Dependencies::Loadable, PP::ObjectMixin, JSON::Ext::Generator::GeneratorMethods::Object, ActiveSupport::Tryable, Kernel, BasicObject]

Why is it important? Because when you realize that, you can assume and check it in the gem codebase to be sure how exactly it works if it comes to ISO8601DateTime types when they don't have a TimeZone passed from client side.

As you probably already suspect - it parse it using server timezone, not application timezone (because it doesn’t have access to it by default).

Conslusion:

Remember that graphql-ruby is a gem prepared not only for rails environment, so do not expect it will be operating on your rails app timezone. Ensure that your system have the same timezone as your app, and enjoy graphl-ruby gem, as you were before!

P.S. (few tips for setup that we mostly work with)

App: To change timezone in app, go to config/environments/.rb and set:

config.time_zone = "UTC"

System To change linux based system timezone just run

 sudo ln -fs /usr/share/zoneinfo/UTC /etc/localtime

Postgres Database :
- Run psql
- show config_file;
- because SET TIME ZONE 'UTC; -from psql will work only until next restart of postgres to change timezone open config file and set

  timezone = 'UTC'

Trailblazer tutorial: updating old fat controller - part 4.

Adam Piotrowski — Thu, 31 Oct 2019 10:02:42 +0000

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.

Trailblazer tutorial: refactoring fat controller - part 3.

Adam Piotrowski — Wed, 23 Oct 2019 10:28:18 +0000

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.

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,

@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

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.

#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.

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

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

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

we can also call it by:

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)

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

step Contract::Validate(key: :proposal)

we can also call it by:

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

this step call .save method on our object

step Contract::Persist(method: :save)

we can also call it by:

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:

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:

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.

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).

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,

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

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

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).

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)*.

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:

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:

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:

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

And a step definition:

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.

Trailblazer tutorial: move business logic from controller - part 2.

Adam Piotrowski — Tue, 15 Oct 2019 11:49:06 +0000

Since we have basic cases and success flow tested and implemented, it is time to focus on our business logic which we want to move from controller to Operation.

Our first identified logic is to care about if event is closed or is about to be closed in 1 hour. So let's add tests to check if we will not be able to add a proposal in any of those cases (we already have a test for a success flow in which an event is opened, as described by the context name).

@event.closed? && @event.closes_at < 1.hour.ago

So lets add 2 tests and update “open case”

context 'with open event' do
  let!(:event) {
    Event.create(
      state: 'open', slug: 'Lorem', name: 'Ipsum',
      url: 'http://www.c.url', closes_at: Time.current + 1.day
    ) }
  ...
end

context 'with closed event' do
  let(:session_format) { instance_double(SessionFormat, id: 53) }
  let!(:event) { Event.create(state: 'closed', slug: 'Lorem', name: 'Ipsum', url: 'http://www.c.url') }
  it 'returns result object with an error about closed event without saving the proposal' do
    expect(result[:errors]).to eq('Event is closed')
    expect(result).to be_failure
  end
end


context 'with event that is about to be closed' do
  let(:session_format) { instance_double(SessionFormat, id: 53) }
  let!(:event) {
    Event.create(
      state: 'open', slug: 'Lorem', name: 'Ipsum',
      url: 'http://www.c.url', closes_at: Time.current + 40.minutes
    ) }

  it 'returns result object with an error about closed event without saving the proposal' do
    expect(result[:errors]).to eq('Event is closed')
    expect(result).to be_failure
  end
end

And check if they pass:

$ rspec spec/concepts/proposal/operation/create_spec.rb
..FF
Failures:

1) Proposal::Operation::Create with valid params with closed event returns result object with an error about closed event without saving the proposal
Failure/Error: expect(result[:errors]).to eq("Event is closed")
expected: 'Event is closed'
got: nil

2) Proposal::Operation::Create with valid params with event that is about to be closed returns result object with an error about closed event without saving the proposal
Failure/Error: expect(result[:errors]).to eq('Event is closed')
expected: 'Event is closed'
got: nil

Finished in 0.43801 seconds (files took 8.64 seconds to load)
4 examples, 2 failures

That wasn’t unexpected since we didn’t implement handling this case. Let’s do that.

(Iteration 5) Add handling “event closed or about to be closed” case.

We need to add a step checking if the event is opened, and a method to handle failure flow.

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 for checking if event is opened
    step :event_open?
    # step for handling failure
    fail :event_not_open_error
    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

    # we check if event.open? Method is true, and if event will be open for next hour
    def event_open?(ctx, **)
      ctx[:event].open? && ctx[:event].closes_at >= 1.hour.since
    end

    # -- bad stuff handled there --
    def event_not_found(ctx, **)
      ctx[:errors] = "Event not found"
    end

    def event_not_open_error(ctx, **)
      ctx[:errors] = "Event is closed"
    end
  end
end

Should all of our tests be green now? Let's check:

$ rspec spec/concepts/proposal/operation/create_spec.rb
FF..
Failures:

1) Proposal::Operation::Create with valid params with open event creates a proposal assigned to event identified by slug
Failure/Error: ctx[:event].open? && ctx[:event].closes_at >= 1.hour.since

NoMethodError:
undefined method `>=' for nil:NilClass

2) 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: 'Event is closed'

Finished in 1.37 seconds (files took 12.98 seconds to load)
4 examples, 2 failures

So what can we see, is regression - newly implemented feature, broke our previous tests? Why is that? Let's try to debug “no event” case by one of the coolest TRB features which are #wtf? method:

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
    binding.pry
    expect(result[:errors]).to eq('Event not found')
    expect(result).to be_failure
  end
end

pry(...)> result.wtf?

`-- Proposal::Operation::Create
|-- Start.default
|-- Nested(Proposal::Operation::Create::Present)
| |-- Start.default
| |-- model.build
| |-- contract.build
| |-- contract.default.validate
| | |-- Start.default
| | |-- contract.default.params_extract
| | |-- contract.default.call
| | `-- End.success
| `-- End.success
|-- event
|-- event_not_found
|-- event_not_open_error
`-- End.failure

As you can see, #wtf? method called on our result, gave us a really explicit tree of what was called when and how it was nested. If we will track what happened, we can see that after the method event was called, fail step event_not_found was called, and then every other (in our case it's just event_not_open_error) fail step was also called.

To understand it we need to fully understand what keyword trail in Trailblazer means, and how it works. And that will be the most important knowledge from this blogpost. http://trailblazer.to/gems/operation/2.0/index.html - Flow Control section should explain basics.
But what else we have in our case? We have another fail step in the failure trail. So after calling event_not_found which calls:

ctx[:errors] = 'Event not found'

Then we call event_not_open_error which calls

ctx[:errors] = 'Event is closed'

and that overwrite our [:errors] value.

Since sometimes we need to exit from failure trail and just stop operation, and sometimes we want to run all further fail steps, TRB comes with handy method:

fail :event_not_found, fail_fast: true

After we change fail_fast value to true, whole operation logic will be stopped on the fail trail after calling failing method event_not_found in this case.

So our Operation flow will look like:

step Nested(Present)
step :event
fail :event_not_found, fail_fast: true
step :event_open?
fail :event_not_open_error
step :assign_event
step Contract::Persist()

And running the tests will result in:

$ rspec spec/concepts/proposal/operation/create_spec.rb
....

Finished in 0.48573 seconds (files took 8.7 seconds to load)
4 examples, 0 failures

Trailblazer tutorial: fat controller - part 1.

Adam Piotrowski — Wed, 09 Oct 2019 15:36:09 +0000

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?

But if Ruby on Rails or even whole MVC pattern is a silver bullet for each web application? Of course not. That is an excellent tool for starting new applications, that have to be delivered fast and be reliable. 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 implemented into 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:

Trailblazer home page,
a quick overview of Trailblazer main features,
review of Trailblazer book,
trailblazer book,
a short article from the creator of Trailblazer about what Trailblazer is and why it was invented (the year 2014),
presentation about Trailblazer coolest features.

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, subscribe to our newsletter or follow us here on dev.to 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:

Select one of the core features that we want to refactor
Identify where logic is implemented and analyze it
Describe our new Operation by integration tests
Implement Operation so our tests will pass (like TDD)
Update old fat controller
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',
           session_format_id: session_format.id
        }
      }
    }

    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

Should we even care about the code?

Adam Piotrowski — Fri, 27 Sep 2019 12:12:35 +0000

Context

Before I start this little controversial blog post, I would like to give a little context of the situation, which made me come with conclusions below. About 7 years ago I started to a co-organize conference called wroclove_rb https://wroclove.rb. About 5 years ago together with a few of my friends, we started to organize conference Programistok https://programistok.org.

Since I started to help with wroclove_rb - I perfectly remember that all the tickets were sold in one minute despite that we didn’t do any marketing actions - to be honest, I started to help with organizing to get the conference pass, because it was the 3rd year in a row when I wasn’t fast enough to buy a ticket. After first two editions of Programistok, where we ( well mostly Mateusz Andrzejewski and Maciej Korsan) did great marketing job, following editions didn’t need any marketing like wroclove_rb. Tickets were sold out in one, maximum of two minutes.

Question

For a long time, one question was coming to my mind. “Why people want to go across the whole country to spend a few hours at our conferences so much?”. The first obvious answer was “because as organizers we did everything perfectly”. But let's be honest - we knew that is not the answer. The organization was professional, but that is not a reason to press “refresh” 70 times per minute on the ticket-selling system page when we release tickets. Today, finally I think I know the answer, but we will come back to that.

Doubts

In 2N our small but experienced team is getting bigger and stronger. We deliver more and more projects, the amount of satisfied clients is growing and in the ruby community, we are more and more recognizable as reliable partners. Therefore, as CEO, I have less time for coding, and I have to think more about sales. I read more and more about marketing and how important it is to be able to sell your services well. It is hard to disagree with this from the CEO or CFO perspective. It's hard to disagree with this from the perspective of what the employee market looks like and how hard it is to keep a good developer in the company.

However, with each subsequent article describing how the most important element of a software house is sales, my doubts about that started to grow. Doubt fueled by information about subsequent safety-critical issues like those:

Revelation

Today's revelation came when I was viewing the landing page of one of the largest software houses on the Polish market. Landing page contained a beautiful promo-movie showing how work in this idyllic corporation looks like. There was almost everything in this film:

ping pong,
coffee,
scrum with colorful cards,
smiling team,
very fancy office,
even dancing!

However, something was missing in all of this. Programming. None of the frames of the film presented a programmer writing the code. Yes, I know - it doesn't sell. Yes I know - it can look boring from the outside. But that's what we do. It is this work that lies at the heart of our companies. It depends on whether the project succeeds or not - and in extreme cases human life depends on it. That's what the best programmers want to do - code.

Two days after writing this post, Programistok begins. And I understand why I get excited about it because:

I will meet programmers interested in raising their competences and skills,
I will be able to hear technical presentations,
no one will try to sell anything - everyone appearing at these types of conferences crave for knowledge and exchange of experience,
I will not waste a single hour at the presentation which should really be intended for the HR, marketing or finance department.

Conclusion

So here are my three thoughts I would like to share today:

Let's focus on IT when organizing an IT conference.
Let's be fair to people who more than once finance their own flight, stay and ticket.
If we promise them an IT conference, let's give them the best speakers we can afford (from this place I would like to greet all the speakers present at wroclove_rb and Programistok - now I am sure that the organization of the conference is only the icing on the cake, which you prepared by your fleshy presentations).
If the conference has a small budget - cut the refreshments, do not buy t-shirts, and pay for editing a promotional film. Let's focus on technology - because this is our main responsibility. If our goal is not to spread knowledge among fellow developers, then we don't really organize IT conferences.
As IT project managers, remember that without money and sales, the company will not survive on the market, but at the same time, remember to care for our developers. Let's think if it is worth spending part of the marketing budget on development of developers. It can give them more time to write Open Source projects in new technologies - in the end, we don't want them to learn them on the production of our client application.
Let's focus not only on the quality of marketing but also on the quality of the code we produce - let subsequent developers do not have to swear at the code they found after the team that an excellent marketer sold as "seniors".