Skip to content

Commit

Permalink
Abstract republishing_all controller action
Browse files Browse the repository at this point in the history
In writing the next bulk republishing 'all' action (all documents), it
became evident that only two lines needed changing. We'll have six or
seven of these, so this abstracts the action and related code so we can
keep things DRY

A minor change has been applied to the (already abstract) `confirm_all`
action to keep the code and naming more similar between the two actions
for easy comparison

This change makes the `metadata` name feel more wrong: having a
lambda/method in metadata feels a bit wrong. One to think about
  • Loading branch information
yndajas committed May 30, 2024
1 parent e41151f commit 83d87e6
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 16 deletions.
16 changes: 9 additions & 7 deletions app/controllers/admin/bulk_republishing_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,24 @@ class Admin::BulkRepublishingController < Admin::BaseController
before_action :enforce_permissions!

def confirm_all
@bulk_content_type_metadata = bulk_content_type_metadata[params[:bulk_content_type].underscore.to_sym]
bulk_content_type_key = params[:bulk_content_type].underscore.to_sym
@bulk_content_type_metadata = bulk_content_type_metadata[bulk_content_type_key]
return render "admin/errors/not_found", status: :not_found unless @bulk_content_type_metadata

@republishing_event = RepublishingEvent.new
end

def republish_all_organisation_about_us_pages
bulk_content_type_key = :all_organisation_about_us_pages
bulk_content_type_value = RepublishingEvent.bulk_content_types.fetch(bulk_content_type_key)
@bulk_content_type_metadata = bulk_content_type_metadata.fetch(bulk_content_type_key)
action = "#{@bulk_content_type_metadata[:name].upcase_first} have been queued for republishing"
def republish_all
bulk_content_type_key = params[:bulk_content_type].underscore.to_sym
@bulk_content_type_metadata = bulk_content_type_metadata.fetch(bulk_content_type_key, nil)
return render "admin/errors/not_found", status: :not_found unless @bulk_content_type_metadata

action = "#{@bulk_content_type_metadata[:name].upcase_first} have been queued for republishing"
bulk_content_type_value = RepublishingEvent.bulk_content_types.fetch(bulk_content_type_key)
@republishing_event = build_republishing_event(action:, bulk_content_type: bulk_content_type_value)

if @republishing_event.save
BulkRepublisher.new.republish_all_organisation_about_us_pages
@bulk_content_type_metadata[:republish_method].call

flash[:notice] = action
redirect_to(admin_republishing_index_path)
Expand Down
3 changes: 2 additions & 1 deletion app/helpers/admin/republishing_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ def bulk_content_type_metadata
all_organisation_about_us_pages: {
id: "all-organisation-about-us-pages",
name: "all organisation 'About Us' pages",
republishing_path: admin_bulk_republishing_all_organisation_about_us_pages_republish_path,
republishing_path: admin_bulk_republishing_all_republish_path("all-organisation-about-us-pages"),
confirmation_path: admin_bulk_republishing_all_confirm_path("all-organisation-about-us-pages"),
republish_method: -> { BulkRepublisher.new.republish_all_organisation_about_us_pages },
},
}
end
Expand Down
2 changes: 1 addition & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def redirect(path, options = { prefix: Whitehall.router_prefix })
end
scope :bulk do
get "/:bulk_content_type/confirm" => "bulk_republishing#confirm_all", as: :bulk_republishing_all_confirm
post "/all-organisation-about-us-pages/republish" => "bulk_republishing#republish_all_organisation_about_us_pages", as: :bulk_republishing_all_organisation_about_us_pages_republish
post "/:bulk_content_type/republish" => "bulk_republishing#republish_all", as: :bulk_republishing_all_republish
end
end

Expand Down
23 changes: 16 additions & 7 deletions test/functional/admin/bulk_republishing_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ class Admin::BulkRepublishingControllerTest < ActionController::TestCase
assert_response :forbidden
end

test "GDS Admin users should be able to POST :republish_all_organisation_about_us_pages, creating a RepublishingEvent for the current user" do
test "GDS Admin users should be able to POST :republish_all with a valid bulk content type and a reason, creating a RepublishingEvent for the current user" do
BulkRepublisher.any_instance.expects(:republish_all_organisation_about_us_pages).once

post :republish_all_organisation_about_us_pages, params: { reason: "this needs republishing" }
post :republish_all, params: { bulk_content_type: "all-organisation-about-us-pages", reason: "this needs republishing" }

newly_created_event = RepublishingEvent.last
assert_equal newly_created_event.user, current_user
Expand All @@ -40,19 +40,28 @@ class Admin::BulkRepublishingControllerTest < ActionController::TestCase
assert_equal "All organisation 'About Us' pages have been queued for republishing", flash[:notice]
end

test "GDS Admin users should encounter an error on POST :republish_all_organisation_about_us_pages without a `reason` and be sent back to the confirm_all page" do
BulkRepublisher.any_instance.expects(:republish_all_organisation_about_us_pages).never
test "GDS Admin users should encounter an error on POST :republish_all without a `reason` and be sent back to the confirm_all page" do
BulkRepublisher.expects(:new).never

post :republish_all_organisation_about_us_pages, params: { reason: "" }
post :republish_all, params: { bulk_content_type: "all-organisation-about-us-pages", reason: "" }

assert_equal ["Reason can't be blank"], assigns(:republishing_event).errors.full_messages
assert_template "confirm_all"
end

test "Non-GDS Admin users should not be able to POST :republish_all_organisation_about_us_pages" do
test "GDS Admin users should see a 404 page when trying to POST :republish_all with an invalid bulk content type" do
BulkRepublisher.expects(:new).never

post :republish_all, params: { bulk_content_type: "not-a-bulk-content-type", reason: "this needs republishing" }
assert_response :not_found
end

test "Non-GDS Admin users should not be able to POST :republish_all" do
BulkRepublisher.expects(:new).never

login_as :writer

post :republish_all_organisation_about_us_pages, params: { reason: "this needs republishing" }
post :republish_all, params: { bulk_content_type: "all-organisation-about-us-pages", reason: "this needs republishing" }
assert_response :forbidden
end
end

0 comments on commit 83d87e6

Please sign in to comment.