From 9f6e34b34a530039e5cf88505ed63cbe3479b30a Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Tue, 26 Mar 2019 17:50:54 +0000 Subject: [PATCH] A different take on the Interactor pattern The Interactor pattern is quite simple, but trying to use it leaves two issues unresolved: the verbose passing of variables, either with context or between methods; and the structure of the actions we're trying to encapsulate, most which have a somewhat hidden lock/release mechanism. Re-implementing the Interactor pattern locally gives the opportunity to address some of these issues, but also to go completely off-the-rails! This is really another example of what the end-result could look like, which has some good and bad attributes over vanilla Interactors. --- app/beneractors/images/create_actor.rb | 31 +++++++ app/beneractors/images/update_actor.rb | 51 +++++++++++ app/controllers/images_controller.rb | 120 ++++++++++--------------- lib/beneractors/beneractor.rb | 47 ++++++++++ lib/beneractors/context.rb | 26 ++++++ 5 files changed, 203 insertions(+), 72 deletions(-) create mode 100644 app/beneractors/images/create_actor.rb create mode 100644 app/beneractors/images/update_actor.rb create mode 100644 lib/beneractors/beneractor.rb create mode 100644 lib/beneractors/context.rb diff --git a/app/beneractors/images/create_actor.rb b/app/beneractors/images/create_actor.rb new file mode 100644 index 0000000000..3017a204c9 --- /dev/null +++ b/app/beneractors/images/create_actor.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require "beneractors/beneractor" + +module Images + class CreateActor < Beneractors::Beneractor + def pre_op + export :edition, Edition.find_current(document: document) + edition.lock! + + export :issues, ::Requirements::ImageUploadChecker.new(image).issues + abort!(:issues) if issues.any? + end + + def op + export :image_revision, ImageUploadService + .new(image, edition.revision) + .call(user) + + updater = Versioning::RevisionUpdater.new(edition.revision, user) + updater.update_image(image_revision) + + edition.assign_revision(updater.next_revision, user) + edition.save! + end + + def post_op + PreviewService.new(edition).try_create_preview + end + end +end diff --git a/app/beneractors/images/update_actor.rb b/app/beneractors/images/update_actor.rb new file mode 100644 index 0000000000..2783e14930 --- /dev/null +++ b/app/beneractors/images/update_actor.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require "beneractors/beneractor" + +module Images + class UpdateActor < Beneractors::Beneractor + def pre_op + export :edition, Edition.find_current(document: document) + edition.lock! + + export :image_revision, edition.image_revisions.find_by!(image_id: image_id) + image_updater = Versioning::ImageRevisionUpdater.new(image_revision, user) + + image_updater.assign(update_params) + export :next_image_revision, image_updater.next_revision + + export :issues, Requirements::ImageRevisionChecker.new(next_image_revision) + .pre_preview_metadata_issues + + abort!(:issues) if issues.any? + + export :updater, Versioning::RevisionUpdater.new(edition.revision, user) + updater.update_image(next_image_revision, lead_image == "on") + abort! unless updater.changed? + end + + def op + export :timeline_entry_type, + if updater.selected_lead_image? + :lead_image_selected + elsif updater.removed_lead_image? + :lead_image_removed + else + :image_updated + end + + TimelineEntry.create_for_revision( + entry_type: timeline_entry_type, + edition: edition, + ) + + edition.assign_revision(updater.next_revision, user) + edition.save! + success!(timeline_entry_type) + end + + def post_op + PreviewService.new(edition).try_create_preview + end + end +end diff --git a/app/controllers/images_controller.rb b/app/controllers/images_controller.rb index ae55256e1c..bfc5bef8f4 100644 --- a/app/controllers/images_controller.rb +++ b/app/controllers/images_controller.rb @@ -7,29 +7,27 @@ def index end def create - Edition.find_and_lock_current(document: params[:document]) do |edition| - @issues = ::Requirements::ImageUploadChecker.new(params[:image]).issues - - if @issues.any? - flash.now["alert_with_items"] = { - "title" => I18n.t!("images.index.flashes.upload_requirements"), - "items" => @issues.items, - } - - render :index, - assigns: { edition: edition }, - layout: rendering_context, - status: :unprocessable_entity - next - end - - image_revision = ImageUploadService.new(params[:image], edition.revision).call(current_user) - updater = Versioning::RevisionUpdater.new(edition.revision, current_user) - updater.update_image(image_revision) - edition.assign_revision(updater.next_revision, current_user).save! - PreviewService.new(edition).try_create_preview - redirect_to crop_image_path(params[:document], image_revision.image_id, wizard: "upload") + result = Images::CreateActor.call(document: params[:document], + image: params[:image], + user: current_user) + + if result.aborted?(:issues) + flash.now["alert_with_items"] = { + "title" => I18n.t!("images.index.flashes.upload_requirements"), + "items" => result.issues.items, + } + + render :index, + assigns: { issues: result.issues, edition: result.edition }, + layout: rendering_context, + status: :unprocessable_entity + + return end + + redirect_to crop_image_path(params[:document], + result.image_revision.image_id, + wizard: "upload") end def crop @@ -63,57 +61,35 @@ def edit end def update - Edition.find_and_lock_current(document: params[:document]) do |edition| # rubocop:disable Metrics/BlockLength - image_revision = edition.image_revisions.find_by!(image_id: params[:image_id]) - image_updater = Versioning::ImageRevisionUpdater.new(image_revision, current_user) - - image_updater.assign(update_params) - next_image_revision = image_updater.next_revision - - issues = Requirements::ImageRevisionChecker.new(next_image_revision) - .pre_preview_metadata_issues - - if issues.any? - flash.now["alert_with_items"] = { - "title" => I18n.t!("images.edit.flashes.requirements"), - "items" => issues.items, - } - - render :edit, - assigns: { edition: edition, image_revision: next_image_revision, issues: issues }, - layout: rendering_context, - status: :unprocessable_entity - next - end - - updater = Versioning::RevisionUpdater.new(edition.revision, current_user) - updater.update_image(next_image_revision, params[:lead_image] == "on") - - if updater.changed? - timeline_entry_type = if updater.selected_lead_image? - :lead_image_selected - elsif updater.removed_lead_image? - :lead_image_removed - else - :image_updated - end - - TimelineEntry.create_for_revision(entry_type: timeline_entry_type, edition: edition) - edition.assign_revision(updater.next_revision, current_user).save! - PreviewService.new(edition).try_create_preview - end + result = Images::UpdateActor.call(document: params[:document], + image_id: params[:image_id], + lead_image: params[:lead_image], + user: current_user, + update_params: update_params) + + if result.aborted?(:issues) + flash.now["alert_with_items"] = { + "title" => I18n.t!("images.edit.flashes.requirements"), + "items" => result.issues.items, + } + + render :edit, + assigns: { edition: result.edition, image_revision: result.next_image_revision, issues: result.issues }, + layout: rendering_context, + status: :unprocessable_entity + return + end - if updater.selected_lead_image? - redirect_to document_path(edition.document), - notice: t("documents.show.flashes.lead_image.selected", - file: image_revision.filename) - elsif updater.removed_lead_image? - redirect_to images_path(edition.document), - notice: t("images.index.flashes.lead_image.removed", - file: image_revision.filename) - else - redirect_to images_path(edition.document) - end + if result.success?(:lead_image_selected) + redirect_to document_path(params[:document]), + notice: t("documents.show.flashes.lead_image.selected", + file: result.image_revision.filename) + elsif result.success?(:lead_image_removed) + redirect_to images_path(params[:document]), + notice: t("images.index.flashes.lead_image.removed", + file: result.image_revision.filename) + else + redirect_to images_path(params[:document]) end end diff --git a/lib/beneractors/beneractor.rb b/lib/beneractors/beneractor.rb new file mode 100644 index 0000000000..c548e0902a --- /dev/null +++ b/lib/beneractors/beneractor.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require "beneractors/context" + +module Beneractors + class Beneractor + attr_reader :context + delegate_missing_to :@context + + def self.call(context = {}) + context = Context.new(context) + + ActiveRecord::Base.transaction do + new(context).call + context + end + rescue Context::FailError + context + end + + private_class_method :new + + def export(name, value) + context[name] = value + end + + def initialize(context) + @context = context + end + + def call + pre_op + op + post_op + end + + def pre_op + context.fail! + end + + def op + raise "not implemented" + end + + def post_op; end + end +end diff --git a/lib/beneractors/context.rb b/lib/beneractors/context.rb new file mode 100644 index 0000000000..b1f87972e1 --- /dev/null +++ b/lib/beneractors/context.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Beneractors + class Context < OpenStruct + class FailError < RuntimeError; end + + def abort!(failure_type) + @failure_type = failure_type + @success_type = false + raise FailError + end + + def success!(success_type = true) + @success_type = success_type + @failure_type = false + end + + def success?(success_type = true) + @success_type == success_type + end + + def aborted?(failure_type = true) + @failure_type == failure_type + end + end +end