From 2b4f2672698e0b077387954baf905cedd3d23d35 Mon Sep 17 00:00:00 2001 From: Kevin Dew Date: Tue, 9 Apr 2019 16:38:17 +0100 Subject: [PATCH] Make interactors more step focused Following a workshop with Ben we've been able to agree on an approach to them that is quite focused on declarative steps. This increases the readability of the call method, but does mean context is set as a side effect of other methods. Hopefully that isn't too confusing. We've kept the delegates to context, they serve the purpose of reducing context verbosity in the class and defining the available properties that can be used when operating on the result. We dropped the initialize with params and user. These were used as a means to formalise the class interface but this is quite repetitive. --- app/interactors/images/create_interactor.rb | 31 +++++----- app/interactors/images/destroy_interactor.rb | 29 +++++---- .../images/update_crop_interactor.rb | 49 +++++++-------- app/interactors/images/update_interactor.rb | 59 +++++++++---------- 4 files changed, 84 insertions(+), 84 deletions(-) diff --git a/app/interactors/images/create_interactor.rb b/app/interactors/images/create_interactor.rb index 219de566cc..d3419f3627 100644 --- a/app/interactors/images/create_interactor.rb +++ b/app/interactors/images/create_interactor.rb @@ -9,35 +9,34 @@ class Images::CreateInteractor :issues, to: :context - def initialize(params:, user:) - super - end - def call Edition.transaction do - context.edition = Edition.lock.find_current(document: params[:document]) - check_for_issues(params[:image]) - - context.image_revision = upload_image(params[:image]) + find_and_lock_edition + check_for_issues + create_image_revision update_edition end end private - def check_for_issues(image_params) - issues = Requirements::ImageUploadChecker.new(image_params).issues + def find_and_lock_edition + context.edition = Edition.lock.find_current(document: params[:document]) + end + + def check_for_issues + issues = Requirements::ImageUploadChecker.new(params[:image]).issues context.fail!(issues: issues) if issues.any? end - def upload_image(image_params) - ImageUploadService.new(image_params, edition.revision).call(user) + def create_image_revision + context.image_revision = ImageUploadService.new(params[:image], edition.revision) + .call(user) end def update_edition - Versioning::RevisionUpdater.new(edition.revision, user).tap do |updater| - updater.update_image(image_revision, false) - edition.assign_revision(updater.next_revision, user).save! - end + updater = Versioning::RevisionUpdater.new(edition.revision, user) + updater.update_image(image_revision, false) + edition.assign_revision(updater.next_revision, user).save! end end diff --git a/app/interactors/images/destroy_interactor.rb b/app/interactors/images/destroy_interactor.rb index 9de2e03c90..6c3e4601f4 100644 --- a/app/interactors/images/destroy_interactor.rb +++ b/app/interactors/images/destroy_interactor.rb @@ -9,30 +9,29 @@ class Images::DestroyInteractor :removed_lead_image, to: :context - def initialize(params:, user:) - super - end - def call Edition.transaction do - context.edition = Edition.lock.find_current(document: params[:document]) - context.image_revision = edition.image_revisions.find_by!(image_id: params[:image_id]) - - updater = remove_image(image_revision) + find_and_lock_edition + find_and_remove_image create_timeline_entry update_preview - - context.removed_lead_image = updater.removed_lead_image? end end private - def remove_image(image_revision) - Versioning::RevisionUpdater.new(edition.revision, user).tap do |updater| - updater.remove_image(image_revision) - edition.assign_revision(updater.next_revision, user).save! - end + def find_and_lock_edition + context.edition = Edition.lock.find_current(document: params[:document]) + end + + def find_and_remove_image + image_revision = edition.image_revisions.find_by!(image_id: params[:image_id]) + updater = Versioning::RevisionUpdater.new(edition.revision, user) + updater.remove_image(image_revision) + edition.assign_revision(updater.next_revision, user).save! + + context.image_revision = image_revision + context.removed_lead_image = updater.removed_lead_image? end def create_timeline_entry diff --git a/app/interactors/images/update_crop_interactor.rb b/app/interactors/images/update_crop_interactor.rb index 056cade261..5d0a8a4899 100644 --- a/app/interactors/images/update_crop_interactor.rb +++ b/app/interactors/images/update_crop_interactor.rb @@ -6,31 +6,34 @@ class Images::UpdateCropInteractor :user, :edition, :image_revision, + :edition_updated, to: :context - def initialize(params:, user:) - super - end - def call Edition.transaction do - context.edition = Edition.lock.find_current(document: params[:document]) + find_and_lock_edition + find_and_update_image - updater = update_image(find_image_revision(params[:image_id]), crop_params) + update_edition - if updater.changed? - create_timeline_entry - update_preview - end - - context.image_revision = updater.next_revision + create_timeline_entry + update_preview end end private - def find_image_revision(image_id) - edition.image_revisions.find_by!(image_id: image_id) + def find_and_lock_edition + context.edition = Edition.lock.find_current(document: params[:document]) + end + + def find_and_update_image + image_revision = edition.image_revisions.find_by!(image_id: params[:image_id]) + + updater = Versioning::ImageRevisionUpdater.new(image_revision, user) + updater.assign(crop_params) + + context.image_revision = updater.next_revision end def crop_params @@ -42,24 +45,24 @@ def crop_params .tap { |p| p[:crop_height] = (p[:crop_width].to_i * image_aspect_ratio).round } end - def update_image(image_revision, params) - Versioning::ImageRevisionUpdater.new(image_revision, user).tap do |updater| - updater.assign(params) - update_edition(updater.next_revision) if updater.changed? - end - end - - def update_edition(image_revision) + def update_edition updater = Versioning::RevisionUpdater.new(edition.revision, user) + updater.update_image(image_revision, false) - edition.assign_revision(updater.next_revision, user).save! + edition.assign_revision(updater.next_revision, user).save! if updater.changed? + + context.edition_updated = updater.changed? end def create_timeline_entry + return unless edition_updated + TimelineEntry.create_for_revision(entry_type: :image_updated, edition: edition) end def update_preview + return unless edition_updated + PreviewService.new(edition).try_create_preview end end diff --git a/app/interactors/images/update_interactor.rb b/app/interactors/images/update_interactor.rb index 335f321d9a..0d35de10ac 100644 --- a/app/interactors/images/update_interactor.rb +++ b/app/interactors/images/update_interactor.rb @@ -7,65 +7,62 @@ class Images::UpdateInteractor :edition, :image_revision, :issues, + :edition_updated, :selected_lead_image, :removed_lead_image, to: :context - def initialize(params:, user:) - super - end - def call Edition.transaction do - context.edition = Edition.lock.find_current(document: params[:document]) - context.image_revision = update_image_revision(image_params) + find_and_lock_edition + find_and_update_image check_for_issues + update_edition - updater = update_image(params[:lead_image] == "on") - - context.selected_lead_image = updater.selected_lead_image? - context.removed_lead_image = updater.removed_lead_image? - - if updater.changed? - create_timeline_entry - update_preview - end + create_timeline_entry + update_preview end end private - def update_image_revision(update_params) + def find_and_lock_edition + context.edition = Edition.lock.find_current(document: params[:document]) + end + + def find_and_update_image image_revision = edition.image_revisions.find_by!(image_id: params[:image_id]) + image_params = params.require(:image_revision).permit(:caption, :alt_text, :credit) + updater = Versioning::ImageRevisionUpdater.new(image_revision, user) - updater.assign(update_params) - updater.next_revision - end + updater.assign(image_params) - def image_params - params.require(:image_revision).permit(:caption, :alt_text, :credit) + context.image_revision = updater.next_revision end def check_for_issues checker = Requirements::ImageRevisionChecker.new(image_revision) issues = checker.pre_preview_metadata_issues + context.fail!(issues: issues) if issues.any? end - def update_image(lead_image) - Versioning::RevisionUpdater.new(edition.revision, user).tap do |updater| - updater.update_image(image_revision, lead_image) - edition.assign_revision(updater.next_revision, user).save! if updater.changed? - end - end + def update_edition + is_lead_image = params[:lead_image] == "on" + updater = Versioning::RevisionUpdater.new(edition.revision, user) - def upload_image(image_params) - upload_service = ImageUploadService.new(image_params, context.edition.revision) - context.image_revision = upload_service.call(context.user) + updater.update_image(image_revision, is_lead_image) + edition.assign_revision(updater.next_revision, user).save! if updater.changed? + + context.edition_updated = updater.changed? + context.selected_lead_image = updater.selected_lead_image? + context.removed_lead_image = updater.removed_lead_image? end def create_timeline_entry + return unless edition_updated + timeline_entry_type = if selected_lead_image :lead_image_selected elsif removed_lead_image @@ -78,6 +75,8 @@ def create_timeline_entry end def update_preview + return unless edition_updated + PreviewService.new(edition).try_create_preview end end