davydovanton · February 21, 2022 08:51 · Jul 16, 2017 · Jul 16, 2017 · Jul 16, 2017 · Jul 16, 2017
diff --git a/transaction_refactoring.md b/transaction_refactoring.md
@@ -93,8 +93,8 @@ module Web::Controllers::Tasks
           redirect_to routes.tasks_path
         end
 
-        m.failure do |error|
-          @task = Task.new(result.value)
+        m.failure do |task_params|
+          @task = Task.new(task_params)
           self.body = Web::Views::Tasks::New.render(format: format, task: @task,
             current_user: current_user, params: params, updated_csrf_token: set_csrf_token)
         end

diff --git a/transaction_refactoring.md b/transaction_refactoring.md
@@ -31,7 +31,7 @@ class CreateTask
     params[:body] = Markdown.parse(hash[:md_body])
     params[:status] = Task::VALID_STATUSES[:in_progress]
     params[:approved] = nil
-    Right(TaskRepository.new.create(params))
+    Right(TaskRepository.new.create(params)) # если все хорошо - возвращаем task энтити для notificate шага
   end
 
   def notificate(task)

diff --git a/transaction_refactoring.md b/transaction_refactoring.md
@@ -15,15 +15,15 @@ require "dry/transaction"
 class CreateTask
   include Dry::Transaction
 
-  step :validate
-  try  :persist
-  tee  :notificate
+  step :validate # первый шаг
+  try  :persist # второй шаг
+  tee  :notificate # третий шаг
 
   def validate(params)
     if paams.valid?
-      Right(paams.to_h)
+      Right(parms.to_h) # параметры нужны для persist метода
     else
-      Left(paams.to_h)
+      Left(parms.to_h) # параметры нужны для создания энтити в экшене
     end
   end
 
@@ -93,7 +93,7 @@ module Web::Controllers::Tasks
           redirect_to routes.tasks_path
         end
 
-        m.failure :validate do |error|
+        m.failure do |error|
           @task = Task.new(result.value)
           self.body = Web::Views::Tasks::New.render(format: format, task: @task,
             current_user: current_user, params: params, updated_csrf_token: set_csrf_token)

diff --git a/transaction_refactoring.md b/transaction_refactoring.md
@@ -28,11 +28,14 @@ class CreateTask
   end
 
   def persist(params)
+    params[:body] = Markdown.parse(hash[:md_body])
+    params[:status] = Task::VALID_STATUSES[:in_progress]
+    params[:approved] = nil
     Right(TaskRepository.new.create(params))
   end
 
   def notificate(task)
-		NewTaskNotificationWorker.perform_async(task.id)
+    NewTaskNotificationWorker.perform_async(task.id)
   end
 end
 ```

diff --git a/transaction_refactoring.md b/transaction_refactoring.md
@@ -53,7 +53,7 @@ module Web::Controllers::Tasks
     def call(params)
       return unless authenticated?
 
-		result = CreateTask.new.call(params)
+      result = CreateTask.new.call(params)
 
       if result.success?
         flash[:info] = INFO_MESSAGE
@@ -83,8 +83,8 @@ module Web::Controllers::Tasks
 
     def call(params)
       return unless authenticated?
-
-		  CreateTask.new.call(params) do |m|
+      
+      CreateTask.new.call(params) do |m|
         m.success do
           flash[:info] = INFO_MESSAGE
           redirect_to routes.tasks_path

diff --git a/transaction_refactoring.md b/transaction_refactoring.md
@@ -3,11 +3,11 @@
 Опять рассмотрим [`tasks#create` экшен](https://github.com/ossboard-org/ossboard/blob/d4a32af7e3f885792a67e2d8142c8dbdaccf047b/apps/web/controllers/tasks/create.rb).
 
 В экшене 3 разных логики, которые выполняются последовательно:
-1. валидация данных - необходима
-2. сохраниение таска - необходима
-3. отправка нотификаций - мы не хотим, что бы наша транзакия не выполнялась, если отправка нотификации не выполнится
+1. валидация данных - **необходимый шаг**;
+2. сохраниение таска - **необходимый шаг**, если какая-то ошибка, необходимо возвращать failed значение;
+3. отправка нотификаций - мы не хотим, что бы наша транзакия не выполнялась, если отправка нотификации не выполнится;
 
-Поэтому напишем нашу транзакцию. Так же мы будем использовать Either монаду для возвращения статуса шага транзакции. Right для успешного, Left - не успешного:
+Поэтому напишем нашу транзакцию. Так же мы будем использовать Either монаду для возвращения статуса шага транзакции. `Right` для успешного, `Left` - не успешного:
 
 ```ruby
 require "dry/transaction"

diff --git a/transaction_refactoring.md b/transaction_refactoring.md
@@ -0,0 +1,108 @@
+# Пример рефакторинга с использованием транзакций
+
+Опять рассмотрим [`tasks#create` экшен](https://github.com/ossboard-org/ossboard/blob/d4a32af7e3f885792a67e2d8142c8dbdaccf047b/apps/web/controllers/tasks/create.rb).
+
+В экшене 3 разных логики, которые выполняются последовательно:
+1. валидация данных - необходима
+2. сохраниение таска - необходима
+3. отправка нотификаций - мы не хотим, что бы наша транзакия не выполнялась, если отправка нотификации не выполнится
+
+Поэтому напишем нашу транзакцию. Так же мы будем использовать Either монаду для возвращения статуса шага транзакции. Right для успешного, Left - не успешного:
+
+```ruby
+require "dry/transaction"
+
+class CreateTask
+  include Dry::Transaction
+
+  step :validate
+  try  :persist
+  tee  :notificate
+
+  def validate(params)
+    if paams.valid?
+      Right(paams.to_h)
+    else
+      Left(paams.to_h)
+    end
+  end
+
+  def persist(params)
+    Right(TaskRepository.new.create(params))
+  end
+
+  def notificate(task)
+		NewTaskNotificationWorker.perform_async(task.id)
+  end
+end
+```
+
+Обновим контроллер:
+
+```ruby
+module Web::Controllers::Tasks
+  class Create
+    include Web::Action
+
+    expose :task
+
+    params do
+      # ...
+    end
+
+    def call(params)
+      return unless authenticated?
+
+		result = CreateTask.new.call(params)
+
+      if result.success?
+        flash[:info] = INFO_MESSAGE
+        redirect_to routes.tasks_path
+      else
+        @task = Task.new(result.value)
+        self.body = Web::Views::Tasks::New.render(format: format, task: @task,
+          current_user: current_user, params: params, updated_csrf_token: set_csrf_token)
+      end
+    end
+  end
+end
+```
+
+Экшен опять стал чище и вся лишняя логика теперь в транзакции. Давайте воспользуемся матчером, вместо лишнего условия:
+
+```ruby
+module Web::Controllers::Tasks
+  class Create
+    include Web::Action
+
+    expose :task
+
+    params do
+      # ...
+    end
+
+    def call(params)
+      return unless authenticated?
+
+		  CreateTask.new.call(params) do |m|
+        m.success do
+          flash[:info] = INFO_MESSAGE
+          redirect_to routes.tasks_path
+        end
+
+        m.failure :validate do |error|
+          @task = Task.new(result.value)
+          self.body = Web::Views::Tasks::New.render(format: format, task: @task,
+            current_user: current_user, params: params, updated_csrf_token: set_csrf_token)
+        end
+      end
+    end
+  end
+end
+```
+
+Вот и все. Что мы получили:
+1. больше не нужно переживать из-за сессии, что бы проверить создание таска в тестах;
+2. можно воспользоваться DI и протестировать экшен с `NullTransaction` который будет возвращать нужный результат без вызова бизнес логики и работы с BD;
+3. убрав лишние методы в экшене, нужно думать, зачем нужен метод `task_params` и почему было именно так;
+3. каждый из шагов транзакции можно вынести в отдельный класс, что бы изолированно протестировать и легко контролировать логику в этом классе;