Skip to content

Commit

Permalink
Iterate interactors following feedback
Browse files Browse the repository at this point in the history
Following feedback on the first draft of interactors for images this
makes a number of changes:

- Changes the class naming to be suffixed with interactor. As these are
  mostly named with verbs this is a bit weird as it sounds like they
  create an interactor, however it is consistent with most other app
  level naming patterns.
- Makes clearer the properties in play on a context by delegating each
  of them to context in the interactor class.
- Drops using an update_context method to set context at the end of an
  interactor and instead uses an approach where context properties are
  set in the call method.
- Run an interactor in an explicit transaction rather than using the
  `Edition.find_and_lock_current` method. Since we're setting
  context.edition we'd still have a line of code so we don't really
  benefit from the yield.
- Uses a suggestion from Ben to define each instance variable we use
  from a result in a controller, by using values_at method on the result
  object (once converted to a hash)
- Rather than checking on failure of a result check on a particular
  problem, such as issues.
  • Loading branch information
kevindew committed Apr 12, 2019
1 parent 4802ab2 commit adef30a
Show file tree
Hide file tree
Showing 13 changed files with 319 additions and 278 deletions.
79 changes: 43 additions & 36 deletions app/controllers/images_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,24 @@ def index
end

def create
result = Images::Create.call(params: params, user: current_user)
if result.failure?
result = Images::CreateInteractor.call(params: params, user: current_user)
edition, image_revision, issues = result.to_h.values_at(:edition,
:image_revision,
:issues)

if issues
flash.now["alert_with_items"] = {
"title" => I18n.t!("images.index.flashes.upload_requirements"),
"items" => result.issues.items,
"items" => issues.items,
}

render :index,
assigns: { edition: result.edition },
assigns: { edition: edition },
layout: rendering_context,
status: :unprocessable_entity
else
redirect_to crop_image_path(result.edition.document,
result.image_revision.image_id,
redirect_to crop_image_path(edition.document,
image_revision.image_id,
wizard: "upload")
end
end
Expand All @@ -32,9 +36,10 @@ def crop
end

def update_crop
result = Images::UpdateCrop.call(params: params, user: current_user)
redirect_to edit_image_path(result.edition.document,
result.image_revision.image_id,
result = Images::UpdateCropInteractor.call(params: params, user: current_user)
edition, image_revision = result.to_h.values_at(:edition, :image_revision)
redirect_to edit_image_path(edition.document,
image_revision.image_id,
wizard: params[:wizard])
end

Expand All @@ -45,49 +50,51 @@ def edit
end

def update
result = Images::Update.call(params: params, user: current_user)
result = Images::UpdateInteractor.call(params: params, user: current_user)

edition, image_revision, issues, lead_selected, lead_removed =
result.to_h.values_at(:edition,
:image_revision,
:issues,
:selected_lead_image,
:removed_lead_image)

if result.failure?
if issues
flash.now["alert_with_items"] = {
"title" => I18n.t!("images.edit.flashes.requirements"),
"items" => result.issues.items,
"items" => issues.items,
}

render :edit,
assigns: { edition: result.edition,
image_revision: result.image_revision,
issues: result.issues },
assigns: { edition: edition,
image_revision: image_revision,
issues: issues },
layout: rendering_context,
status: :unprocessable_entity
elsif lead_selected
redirect_to document_path(edition.document),
notice: t("documents.show.flashes.lead_image.selected", file: image_revision.filename)
elsif lead_removed
redirect_to images_path(edition.document),
notice: t("images.index.flashes.lead_image.removed", file: image_revision.filename)
else
document = result.edition.document

if result.updater.selected_lead_image?
redirect_to document_path(document),
notice: t("documents.show.flashes.lead_image.selected",
file: result.image_revision.filename)
elsif result.updater.removed_lead_image?
redirect_to images_path(document),
notice: t("images.index.flashes.lead_image.removed",
file: result.image_revision.filename)
else
redirect_to images_path(document)
end
redirect_to images_path(edition.document)
end
end

def destroy
result = Images::Destroy.call(params: params, user: current_user)
document = result.edition.document

if result.updater.removed_lead_image?
redirect_to images_path(document),
result = Images::DestroyInteractor.call(params: params, user: current_user)
edition, image_revision, removed_lead = result.to_h.values_at(:edition,
:image_revision,
:removed_lead_image)
if removed_lead
redirect_to images_path(edition.document),
notice: t("images.index.flashes.lead_image.deleted",
file: result.image_revision.filename)
file: image_revision.filename)
else
redirect_to images_path(document),
redirect_to images_path(edition.document),
notice: t("images.index.flashes.deleted",
file: result.image_revision.filename)
file: image_revision.filename)
end
end

Expand Down
40 changes: 0 additions & 40 deletions app/interactors/images/create.rb

This file was deleted.

43 changes: 43 additions & 0 deletions app/interactors/images/create_interactor.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# frozen_string_literal: true

class Images::CreateInteractor
include Interactor
delegate :params,
:user,
:edition,
:image_revision,
: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])
update_edition
end
end

private

def check_for_issues(image_params)
issues = Requirements::ImageUploadChecker.new(image_params).issues
context.fail!(issues: issues) if issues.any?
end

def upload_image(image_params)
ImageUploadService.new(image_params, 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
end
end
34 changes: 0 additions & 34 deletions app/interactors/images/destroy.rb

This file was deleted.

45 changes: 45 additions & 0 deletions app/interactors/images/destroy_interactor.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# frozen_string_literal: true

class Images::DestroyInteractor
include Interactor
delegate :params,
:user,
:edition,
:image_revision,
: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)
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
end

def create_timeline_entry
TimelineEntry.create_for_revision(entry_type: :image_deleted, edition: edition)
end

def update_preview
PreviewService.new(edition).try_create_preview
end
end
82 changes: 0 additions & 82 deletions app/interactors/images/update.rb

This file was deleted.

Loading

0 comments on commit adef30a

Please sign in to comment.