Skip to content

Commit

Permalink
Introduce Whitehall.attachment_notifier
Browse files Browse the repository at this point in the history
This is intended to work in a similar way to the notifier in
EditionServiceCoordinator, except that it's used to publish events
concerning the Attachment class. My plan is to subscribe an
instance of ServiceListeners::AttachmentDraftStatusUpdater to this
notifier in order to keep the draft status of attachments associated
with policy groups up-to-date in Asset Manager.

Note that the visibility of attachments on a policy group is
determined simply by the existence of the policy group. This logic is
already encapsulated in the AttachmentVisibility class which is used in
the ServiceListeners::AttachmentDraftStatusUpdater.

I did consider introducing a service for creating attachments, but there
doesn't seem to be any precedent for using services in that way and in
any case it felt a bit like overkill.

I also considered wiring an instance of
ServiceListeners::AttachmentDraftStatusUpdater directly into the
Admin::AttachmentsController, but that felt at odds with the approach
taken elsewhere and would've introduced unnecessary coupling and made
testing of the controller harder.

Note that there's no need to publish 'update' events as well as 'create'
events, because updating an attachment should not have any effect on the
draft status of its corresponding asset. We might need to introduce a
'destroy' event at some point, but we're planning to tackle that
separately in this issue [1].

[1]: alphagov/asset-manager#469
  • Loading branch information
floehopper committed Feb 15, 2018
1 parent 46e170d commit ce51eb1
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 0 deletions.
1 change: 1 addition & 0 deletions app/controllers/admin/attachments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def new; end

def create
if save_attachment
Whitehall.attachment_notifier.publish('create', attachment)
redirect_to attachable_attachments_path(attachable), notice: "Attachment '#{attachment.title}' uploaded"
else
render :new
Expand Down
4 changes: 4 additions & 0 deletions lib/whitehall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,10 @@ def self.edition_services
@edition_actions ||= EditionServiceCoordinator.new
end

def self.attachment_notifier
@attachment_notifier ||= ActiveSupport::Notifications::Fanout.new
end

def self.organisations_in_tagging_beta
@taggable_organisations ||=
YAML.load_file(Rails.root + "config/organisations_in_tagging_beta.yml")["organisations_in_tagging_beta"]
Expand Down
15 changes: 15 additions & 0 deletions test/functional/admin/attachments_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def valid_external_attachment_params
setup do
login_as :gds_editor
@edition = create(:consultation)
Whitehall.attachment_notifier.stubs(:publish)
end

def self.supported_attachable_types
Expand Down Expand Up @@ -93,6 +94,20 @@ def self.supported_attachable_types
assert_redirected_to admin_edition_attachments_url(attachable)
end

test "POST :create publishes event to attachment notifier on success" do
Whitehall.attachment_notifier.expects(:publish).with('create', is_a(Attachment))
attachable = create(:edition)

post :create, params: { edition_id: attachable.id, attachment: valid_file_attachment_params }
end

test "POST :create does not publish event to attachment notifier on failure" do
Whitehall.attachment_notifier.expects(:publish).never
attachable = create(:edition)

post :create, params: { edition_id: attachable.id, attachment: {} }
end

view_test 'GET :index shows html attachments' do
create(:html_attachment, title: 'An HTML attachment', attachable: @edition)

Expand Down

0 comments on commit ce51eb1

Please sign in to comment.