Refactor Fat Controller to make it look like DDD

Suddenly, let's say you have a Fat Controller action like this.

reports_controller.rb


class ReportsController < ApplicationController
  def create
    @report = Report.new(report_params)
    if @report.status == Report::STATUS_DRAFT
      if @report.save
        redirect_to @report
      else
        render :new
      end
    else
      result = false
      ApplicationRecord.transaction do
        #Report review request
        if @report.report_type == Report::REPORT_TYPE_REVIEW_REQUEST
          #Processing such as notifying the reviewer and sending an email ...
        end
        @report.save!
        result = true
      end
      if result
        redirect_to @report
      else
        render :new
      end
    end
  rescue StanderdError
    render :new
  end
end

It can be improved a little by creating a method in the early return or Report class, but this time I will introduce another approach.

Prepare a test before refactoring?

I think that the class that is Fat Controller is in a state where there is no test or only the part that is easy to do is tested. In light of general principles, I would like to have a test ready before refactoring. However, if the test is easy to write, the test should already exist, and trying to write the test will be painful and heartbreaking. So, write a test only for the code after refactoring, and when the refactoring is finished, go back to the code before refactoring and check that the test passes. It's more risky than writing a test in advance, but I think it's a realistic method.

Prepare Application Service

First, prepare the ApplicationService class and copy the action exactly.

app/services/create_report_service.rb


class CreateReportService
  def create
    @report = Report.new(report_params)
    if @report.status == Report::STATUS_DRAFT
      if @report.save
        redirect_to @report
      else
        render :new
      end
    else
      result = false
      ApplicationRecord.transaction do
        #Report review request
        if @report.report_type == Report::REPORT_TYPE_REVIEW_REQUEST
          #Processing such as notifying the reviewer and sending an email ...
        end
        @report.save!
        result = true
      end
      if result
        redirect_to @report
      else
        render :new
      end
    end
  rescue StanderdError
    render :new
  end
end

Of course it doesn't work, but once you commit this code. Then replace the processing that depends on the Controller for this code to work. Replace redirect_to and render with return, and accept only the required values for session, params, etc. as arguments.

app/services/create_report_service.rb


class CreateReportService
  def create(report_params)
    @report = Report.new(report_params)
    if @report.status == Report::STATUS_DRAFT
      if @report.save
        return { result: true, data: { report: @report }}
      else
        return { result: false, data: { report: @report }}
      end
    else
      result = false
      ApplicationRecord.transaction do
        #Report review request
        if @report.report_type == Report::REPORT_TYPE_REVIEW_REQUEST
          #Processing such as notifying the reviewer and sending an email ...
        end
        @report.save!
        result = true
      end
      return { result: result, data: { report: @report }}
    end
  rescue StanderdError
    return { result: false, data: { report: @report }}
  end
end

Commit here and ask a colleague for a review. Ask them to confirm that the processing that depends on the Controller is being replaced correctly. Since there is no test, the review guarantees the quality. If you get OK in the review, we will start refactoring. If you requested a review with a Pull Request, please close it without merging.

Create a ValueObject

I think it depends on the code where to start. This time, we will start with an easy-to-understand Value Object. status and report_type have fixed conditional expressions, so let's use ValueObjecct.

app/models/report.rb


class Report < ApplicationRecord
  #There is a lot of other processing here...

  def status_value_object
    StatusValueObject.new(status)
  end

  def report_type_value_object
    ReportTypeValueObject.new(report_type)
  end

  class StatusValueObject
    attr_accessor :status
    DRAFT = 1 # Report::STATUS_Moving DRAFT
    def initialize(status)
      self.status = status
    end

    def draft?
      status == DRAFT
    end
  end

  class ReportTypeValueObject
    #Report in the same way_Create a ValueObject of type
  end
end

Temporarily define ValueObject under Report class. Once you've created a ValueObject test, let's commit it.

Separate draft save and post Entity

I think there is often a request to skip validation when saving a draft.

app/models/report.rb


class Report < ApplicationRecord
  STATUS_DRAFT = 1

  validates :status, presence: true
  validates :title, presence: true, unless: :draft?

  def draft?
    status == STATUS_DRAFT
  end
end

Divide this into a class for saving drafts and a class for posting.

app/domains/research_module/draft_report.rb


#I will call it the report submission function of the research survey
module ResearchModule
  class DraftReport
    attr_accessor :report
    def initialize(report)
      self.report = report
    end

    def valid?
      report.valid?
    end
  end
end

app/domains/research_module/publish_report.rb


module ResearchModule
  class PublishReport
    attr_accessor :report
    #Make it possible to call only the ones you use with delegate. It will be easier to understand what column you are using.
    delegate :title, to: :report
    def initialize(report)
      self.report = report
    end

    def valid?
      report.valid?
      report.errors.add(:title, :blank) if title.blank?
      report.errors.blank? # valid?If so, the contents of errors will be reset
    end
  end
end

Create a Report # to_research_module method and associate the DraftReport class and PublishReport class with Report.

app/models/report.rb


class Report < ApplicationRecord
  #Leave the validation here to check whether you save the draft or post
  validates :status, presence: true

  def to_research_module
    if status_value_object.draft?
      return ResearchModule::DraftReport.new(self)
    end
    #If you need the record of the association destination, pass it as a constructor argument
    # report.Calls like user are prohibited in PublishReport
    ResearchModule::PublishReport.new(self)
  end

  #Other processing...
end

Create a validation test to make sure it works and commit [^ 1]. [^ 1]: If there are other parts that use Report validation, it may be broken. Please check if any action is required.

Move ValueObject to Module

Move the ValueObject that was temporarily created in the Report class to the ResearchModule.

app/domains/research_module/report_status_value_object.rb


module ResearchModule
  #Renamed from StatusValueObject
  class ReportStatusValueObject
    #abridgement
  end
end

app/domains/research_module/report_type_value_object.rb


module ResearchModule
  class ReportTypeValueObject
    #abridgement
  end
end

Create a Report parent class in DraftReport and PublishReport. Since it is ResearchModule :: Report, it does not conflict with Report of ActiveRecord class. Move Report # status_value_object etc. to ResearchModule :: Report.

app/domains/research_module/report.rb


module ResearchModule
  class Report
    attr_accessor :report
    delegate :status, :report_type, to: :report
    def initialize(report)
      self.report = report
    end

    def status_value_object
      ReportStatusValueObject.new(status)
    end

    def report_type_value_object
      ReportTypeValueObject.new(report_type)
    end
  end
end

app/domains/research_module/draft_report.rb


module ResearchModule
  class DraftReport < Report
    #Since it is common, the constructor etc. are deleted
    def valid?
      report.valid?
    end
  end
end

app/domains/research_module/publish_report.rb


module ResearchModule
  class PublishReport < Report
    delegate :title, to: :report
    def valid?
      report.valid?
      report.errors.add(:title, :blank) if title.blank?
      report.errors.blank? # valid?If so, the contents of errors will be reset
    end
  end
end

The status_value_object method is called in Report # to_research_module, but replace it with ResearchModule :: ReportStatusValueObject.new (status). The location of the class has changed, so fix the test to make sure it works and commit.

Refactoring review request submission decisions

If it is DraftReport, the review request is not always sent, and if it is PublishReport, if report_type is the review request, it is sent, so create the send_review_request? Method.

app/domains/research_module/draft_report.rb


module ResearchModule
  class DraftReport < Report
    #Others omitted...

    def send_review_request?
      false
    end
  end
end

app/domains/research_module/publish_report.rb


module ResearchModule
  class PublishReport < Report
    #Others omitted...

    def send_review_request?
      report_type_value_object.review_request?
    end
  end
end

Write a test and commit.

Start refactoring Application Service

Before embarking on the refactoring, copy the entire method and leave it with a method name like ʻold_create. This is to replace the method name after the refactoring is done to make sure the test passes even with the old code. First, call Report # to_research_module` to use the DraftReport class and PublishReport class.

app/services/create_report_service.rb


class CreateReportService
  def create(report_params)
    @report = Report.new(report_params)
    report_entity = @report.to_research_module #Added
    if @report.status == Report::STATUS_DRAFT
      if @report.save
        return { result: true, data: { report: @report }}
      else
        return { result: false, data: { report: @report }}
      end
    else
      result = false
      ApplicationRecord.transaction do
        #Report review request
        if @report.report_type == Report::REPORT_TYPE_REVIEW_REQUEST
          #Processing such as notifying the reviewer and sending an email ...
        end
        @report.save!
        result = true
      end
      return { result: result, data: { report: @report }}
    end
  rescue StanderdError
    return { result: false, data: { report: @report }}
  end
end

Then use send_review_request? to remove the conditional branch of the draft because you don't have to check if it's a draft.

app/services/create_report_service.rb


class CreateReportService
  def create(report_params)
    @report = Report.new(report_params)
    report_entity = @report.to_research_module
    result = false
    #Deleted the draft case and unified it with normal posting processing
    ApplicationRecord.transaction do
      #Report review request
      if report_entity.send_review_request? #Change the conditions here
        #Processing such as notifying the reviewer and sending an email ...
      end
      @report.save!
      result = true
    end
    return { result: result, data: { report: @report }}
  rescue StanderdError
    return { result: false, data: { report: @report }}
  end
end

You need to call report_entity.valid? because the validation has changed with the refactoring of the Report class.

app/services/create_report_service.rb


class CreateReportService
  def create(report_params)
    @report = Report.new(report_params)
    report_entity = @report.to_research_module
    result = false
    ApplicationRecord.transaction do
      #Call Entity validation
      unless report_entity.valid?
        return { result: false, data: { report: @report }}
      end
      #Report review request
      if report_entity.send_review_request?
        #Processing such as notifying the reviewer and sending an email ...
      end
      @report.save!
      result = true
    end
    return { result: result, data: { report: @report }}
  rescue StanderdError
    return { result: false, data: { report: @report }}
  end
end

If you are formatting the contents of the report for notification to the reviewer, you may want to move it to the PublishReport class. It is not necessary to store it in an instance variable like @ report, so let's change it to a local variable.

Create a test and make sure it passes. If you are using FactoryBot, it is recommended to define it by associating it with the Entity of Module like factory: research_module_draft_report, class: Report. Commit once at this point (if you don't want to commit a copy of the old code, delete it).

Run tests on old code before refactoring

Make sure that the old code you left will pass. All you have to do is replace the method name and run the test. If it doesn't pass, it's broken by refactoring, so fix it so that it passes the test. Then go back to the refactored code and modify the product code so that the test passes.

Controller refactoring

The controller just calls ApplicationService and switches between calling redirect_to and render on the result.

reports_controller.rb


class ReportsController < ApplicationController
  def create
    service = CreateReportService.new
    service_result = service.create(report_params)
    @report = service_result[:data][:report]
    if service_result[:result]
      redirect_to @report
      return
    end
    render :new
  end
end

I changed the validation method, but it doesn't deviate from the ActiveRecord mechanism, so View doesn't need to be modified at all.

Finally

It is said that Rails is not suitable for DDD, but I think that you can do anything if you prepare a method that corresponds to Entity like Report # to_research_module. In this example, only the Report model is used, but it can be derived in various ways.

app/models/report.rb


class Report < ApplicationRecord
  belongs_to :reporter
  def to_review_module
    #If you want to use the association, pass it in the constructor
    ReviewModule::Report.new(self, reporter)
  end

  def to_review_module2
    #If you want to build as Aggregate, also to Reporter_review_Create a module method
    ReviewModule::Report.new(self, reporter.to_review_module)
  end


  def to_review_module3
    #All you need is the title, body, and reporter's name.
    ReviewModule::Report.new(title, body, reporter.name)
  end
end

Ultimately, ActiveRecord's model should only have associations and validations like belongs_to, scopes, and methods like to_research_module that correspond to Entity. This means that you don't write domain logic in your ActiveRecord model. Also, by limiting the association reference to methods like to_research_module, you only need to check the method like to_research_module to see which table you are using.


This entry is for Use Case (User Story), Swim Lane in Requirements Analysis Driven Design. requirements_analysis_driven_desgin / #% E3% 83% A6% E3% 83% BC% E3% 82% B9% E3% 82% B1% E3% 83% BC% E3% 82% B9% E3% 83% A6% E3% 83% BC% E3% 82% B6% E3% 83% BC% E3% 82% B9% E3% 83% 88% E3% 83% BC% E3% 83% AA% E3% 83% BC% E3% 82% B9% E3% 82% A4% E3% 83% A0% E3% 83% AC% E3% 83% BC% E3% 83% B3) and Data Modeling (https://linyclar.github.io/software_development/requirements_analysis_driven_desgin/ #% E3% 83% 87% E3% 83% BC% E3% 82% BF% E3% 83% A2% E3% 83% 87% E3% 83% AA% E3% 83% B3% E3% 82% B0) Was reorganized and rewritten from the perspective of refactoring the controller and ActiveRecord models. It's a long story, but it's a story about DDD-like things in Rails.

Recommended Posts

Refactor Fat Controller to make it look like DDD
How to make an image posted using css look like an icon
Create assert_equal to make it easy to write tests