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