Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preview mode for Ministers page when in "reshuffle mode" #9137

Merged
merged 10 commits into from
Jun 13, 2024
Merged
11 changes: 7 additions & 4 deletions app/controllers/admin/cabinet_ministers_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
class Admin::CabinetMinistersController < Admin::BaseController
include ReshuffleMode
before_action :enforce_permissions!

helper_method :reshuffle_in_progress?

def show
@cabinet_minister_roles = MinisterialRole.includes(:translations).where(cabinet_member: true).order(:seniority)
@also_attends_cabinet_roles = MinisterialRole.includes(:translations).also_attends_cabinet.order(:seniority)
Expand All @@ -14,7 +17,7 @@ def reorder_cabinet_minister_roles

def order_cabinet_minister_roles
MinisterialRole.reorder_without_callbacks!(order_ministerial_roles_params, :seniority)
PresentPageToPublishingApiWorker.perform_async("PublishingApi::MinistersIndexPresenter")
republish_ministers_index_page_to_publishing_api

redirect_to admin_cabinet_ministers_path(anchor: "cabinet_minister")
end
Expand All @@ -25,7 +28,7 @@ def reorder_also_attends_cabinet_roles

def order_also_attends_cabinet_roles
MinisterialRole.reorder_without_callbacks!(order_ministerial_roles_params, :seniority)
PresentPageToPublishingApiWorker.perform_async("PublishingApi::MinistersIndexPresenter")
republish_ministers_index_page_to_publishing_api

redirect_to admin_cabinet_ministers_path(anchor: "also_attends_cabinet")
end
Expand All @@ -36,7 +39,7 @@ def reorder_whip_roles

def order_whip_roles
MinisterialRole.reorder_without_callbacks!(order_ministerial_roles_params, :whip_ordering)
PresentPageToPublishingApiWorker.perform_async("PublishingApi::MinistersIndexPresenter")
republish_ministers_index_page_to_publishing_api

redirect_to admin_cabinet_ministers_path(anchor: "whips")
end
Expand All @@ -47,7 +50,7 @@ def reorder_ministerial_organisations

def order_ministerial_organisations
Organisation.reorder_without_callbacks!(order_ministerial_organisations_params, :ministerial_ordering)
PresentPageToPublishingApiWorker.perform_async("PublishingApi::MinistersIndexPresenter")
republish_ministers_index_page_to_publishing_api

redirect_to admin_cabinet_ministers_path(anchor: "organisations")
end
Expand Down
6 changes: 6 additions & 0 deletions app/controllers/admin/republishing_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
class Admin::RepublishingController < Admin::BaseController
include Admin::RepublishingHelper
include ReshuffleMode
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using this model concern in a controller feels a tad weird. On the other hand it's not really a model concern arguably, we just don't have anywhere more logical to put it 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realised I missed this first time around, sorry for commenting at second time of asking!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it felt a bit odd. I think we could probably find a way of removing it from the RepublishingController, as it's only used to make reshuffle_in_progress? available (could probably extract that to a helper class we can require_relative or something).

But the other controllers make use of the republish_ministers_index_page_to_publishing_api callbacks which I don't think we can move to a helper class whilst still allowing neatly referencing it in an after_save. And rather than spread the code across both a concern and a helper, I like that we've encapsulated this mess in one place 😅 So think this is good to go for now. Thanks!


before_action :enforce_permissions!

Expand All @@ -21,6 +22,11 @@ def republish_page
page_to_republish = republishable_pages.find { |page| page[:slug] == params[:page_slug] }
return render "admin/errors/not_found", status: :not_found unless page_to_republish

if reshuffle_in_progress? && %w[how-government-works ministers].include?(params[:page_slug])
flash[:alert] = "Cannot republish #{params[:page_slug]} page while in reshuffle mode"
return redirect_to(admin_republishing_index_path)
end

action = "The page '#{page_to_republish[:title]}' has been scheduled for republishing"

@republishing_event = build_republishing_event(action:, content_id: page_to_republish[:presenter].constantize.new.content_id)
Expand Down
26 changes: 26 additions & 0 deletions app/models/concerns/reshuffle_mode.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
module ReshuffleMode
extend ActiveSupport::Concern

included do
def republish_ministerial_pages_to_publishing_api
PresentPageToPublishingApiWorker.perform_async("PublishingApi::HowGovernmentWorksPresenter", update_live?)
PresentPageToPublishingApiWorker.perform_async("PublishingApi::MinistersIndexPresenter", update_live?)
end

def republish_ministers_index_page_to_publishing_api
PresentPageToPublishingApiWorker.perform_async("PublishingApi::MinistersIndexPresenter", update_live?)
end

def republish_how_government_works_page_to_publishing_api
PresentPageToPublishingApiWorker.perform_async("PublishingApi::HowGovernmentWorksPresenter", update_live?)
end

def reshuffle_in_progress?
SitewideSetting.find_by(key: :minister_reshuffle_mode)&.on || false
end
end

def update_live?
!reshuffle_in_progress?
end
end
6 changes: 1 addition & 5 deletions app/models/ministerial_role.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
class MinisterialRole < Role
include ReshuffleMode
include UserOrderable

has_many :editions, -> { distinct }, through: :role_appointments
Expand Down Expand Up @@ -39,9 +40,4 @@ def destroyable?
def default_person_name
name
end

def republish_ministerial_pages_to_publishing_api
PresentPageToPublishingApiWorker.perform_async("PublishingApi::HowGovernmentWorksPresenter")
PresentPageToPublishingApiWorker.perform_async("PublishingApi::MinistersIndexPresenter")
end
end
15 changes: 5 additions & 10 deletions app/models/organisation.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
class Organisation < ApplicationRecord
include DateValidation
include PublishesToPublishingApi
include ReshuffleMode
include Searchable
include Organisation::OrganisationSearchIndexConcern
include Organisation::OrganisationTypeConcern
Expand Down Expand Up @@ -206,8 +207,10 @@ def foi_contacts

before_destroy { |r| throw :abort unless r.destroyable? }
after_save :ensure_analytics_identifier
after_save :republish_how_government_works_page_to_publishing_api, :republish_ministers_index_page_to_publishing_api, :republish_organisations_index_page_to_publishing_api
after_destroy :republish_ministers_index_page_to_publishing_api, :republish_organisations_index_page_to_publishing_api
after_save :republish_how_government_works_page_to_publishing_api, :republish_organisations_index_page_to_publishing_api
after_save :republish_ministers_index_page_to_publishing_api, if: :ministerial_department?
after_destroy :republish_organisations_index_page_to_publishing_api
after_destroy :republish_ministers_index_page_to_publishing_api, if: :ministerial_department?

after_save do
# If the organisation has an about us page and the chart URL changes we need
Expand All @@ -225,14 +228,6 @@ def foi_contacts
end
end

def republish_how_government_works_page_to_publishing_api
PresentPageToPublishingApiWorker.perform_async("PublishingApi::HowGovernmentWorksPresenter")
end

def republish_ministers_index_page_to_publishing_api
PresentPageToPublishingApiWorker.perform_async("PublishingApi::MinistersIndexPresenter") if ministerial_department?
end

def republish_organisations_index_page_to_publishing_api
PresentPageToPublishingApiWorker.perform_async("PublishingApi::OrganisationsIndexPresenter")
end
Expand Down
13 changes: 7 additions & 6 deletions app/models/person.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
class Person < ApplicationRecord
include PublishesToPublishingApi
include ReshuffleMode

has_many :role_appointments,
lambda {
Expand Down Expand Up @@ -37,7 +38,8 @@ class Person < ApplicationRecord
translates :biography

before_destroy :prevent_destruction_if_appointed
after_update :touch_role_appointments, :republish_ministerial_pages_to_publishing_api
after_update :touch_role_appointments, :republish_past_prime_ministers_page_to_publishing_api
after_update :republish_ministerial_pages_to_publishing_api, if: :has_ministerial_appointments?

def biography_without_markup
Govspeak::Document.new(biography).to_text
Expand Down Expand Up @@ -156,12 +158,11 @@ def truncated_biography
biography&.split(/\n/)&.first
end

def republish_ministerial_pages_to_publishing_api
if role_appointments.any?(&:ministerial?)
PresentPageToPublishingApiWorker.perform_async("PublishingApi::HowGovernmentWorksPresenter")
PresentPageToPublishingApiWorker.perform_async("PublishingApi::MinistersIndexPresenter")
end
def has_ministerial_appointments?
role_appointments.any?(&:ministerial?)
end

def republish_past_prime_ministers_page_to_publishing_api
if current_or_previous_prime_minister?
historical_account.republish_to_publishing_api_async if historical_account.present?
PresentPageToPublishingApiWorker.perform_async("PublishingApi::HistoricalAccountsIndexPresenter")
Expand Down
11 changes: 3 additions & 8 deletions app/models/role_appointment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ class RoleAppointment < ApplicationRecord
include DateValidation
include HasContentId
include PublishesToPublishingApi
include ReshuffleMode

date_attributes(:started_at, :ended_at)

Expand Down Expand Up @@ -73,7 +74,8 @@ def validate(record)
after_create :make_other_current_appointments_non_current
before_destroy :prevent_destruction_unless_destroyable

after_save :republish_associated_editions_to_publishing_api, :republish_organisation_to_publishing_api, :republish_worldwide_organisations_to_publishing_api, :republish_prime_ministers_index_page_to_publishing_api, :republish_ministerial_pages_to_publishing_api, :republish_role_to_publishing_api
after_save :republish_ministerial_pages_to_publishing_api, if: :ministerial?
after_save :republish_associated_editions_to_publishing_api, :republish_organisation_to_publishing_api, :republish_worldwide_organisations_to_publishing_api, :republish_prime_ministers_index_page_to_publishing_api, :republish_role_to_publishing_api
after_destroy :republish_associated_editions_to_publishing_api, :republish_organisation_to_publishing_api, :republish_worldwide_organisations_to_publishing_api, :republish_prime_ministers_index_page_to_publishing_api, :republish_role_to_publishing_api

def republish_associated_editions_to_publishing_api
Expand All @@ -94,13 +96,6 @@ def republish_worldwide_organisations_to_publishing_api
end
end

def republish_ministerial_pages_to_publishing_api
if ministerial?
PresentPageToPublishingApiWorker.perform_async("PublishingApi::HowGovernmentWorksPresenter")
PresentPageToPublishingApiWorker.perform_async("PublishingApi::MinistersIndexPresenter")
end
end

def republish_prime_ministers_index_page_to_publishing_api
PresentPageToPublishingApiWorker.perform_async("PublishingApi::HistoricalAccountsIndexPresenter") unless current? || role.slug != "prime-minister" || has_historical_account?
end
Expand Down
18 changes: 12 additions & 6 deletions app/models/sitewide_setting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,17 @@ def name
def republish_downstream_if_reshuffle
return unless key == "minister_reshuffle_mode"

payload = PublishingApi::MinistersIndexPresenter.new

Services.publishing_api.put_content(payload.content_id, payload.content)
Services.publishing_api.publish(payload.content_id, nil, locale: "en")

PresentPageToPublishingApiWorker.perform_async("PublishingApi::HowGovernmentWorksPresenter")
update_live = true
if on
# These have to be sent synchronously so we can guarantee the order in which they're processed.
# First, send 'page is currently being updated' message to Draft and promote to Live
PresentPageToPublishingApiWorker.new.perform("PublishingApi::HowGovernmentWorksEnableReshufflePresenter", update_live)
PresentPageToPublishingApiWorker.new.perform("PublishingApi::MinistersIndexEnableReshufflePresenter", update_live)
# Finally, send normal ministers index payload to draft so that we can use it as a preview
PresentPageToPublishingApiWorker.new.perform("PublishingApi::MinistersIndexPresenter", false)
else
PresentPageToPublishingApiWorker.perform_async("PublishingApi::HowGovernmentWorksPresenter", update_live)
PresentPageToPublishingApiWorker.perform_async("PublishingApi::MinistersIndexPresenter", update_live)
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module PublishingApi
class HowGovernmentWorksEnableReshufflePresenter < HowGovernmentWorksPresenter
def links
{
current_prime_minister: [],
}
end

def details
{
reshuffle_in_progress: true,
}
end
end
end
23 changes: 6 additions & 17 deletions app/presenters/publishing_api/how_government_works_presenter.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
module PublishingApi
class HowGovernmentWorksPresenter
include ReshuffleMode
attr_accessor :update_type

def initialize(update_type: nil)
Expand Down Expand Up @@ -39,25 +40,17 @@ def base_path
end

def links
return {} if reshuffle_in_progress?

{
current_prime_minister: [MinisterialRole.find_by(slug: "prime-minister")&.current_person&.content_id],
}
end

def details
if reshuffle_in_progress?
{
reshuffle_in_progress: reshuffle_in_progress?,
}
else
{
department_counts:,
ministerial_role_counts:,
reshuffle_in_progress: reshuffle_in_progress?,
}
end
{
department_counts:,
ministerial_role_counts:,
reshuffle_in_progress: reshuffle_in_progress?,
}
end

def department_counts
Expand All @@ -79,10 +72,6 @@ def ministerial_role_counts

private

def reshuffle_in_progress?
SitewideSetting.find_by(key: :minister_reshuffle_mode)&.on || false
end

def prime_minister
1
end
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
module PublishingApi
class MinistersIndexEnableReshufflePresenter < MinistersIndexPresenter
def links
{
ordered_cabinet_ministers: [],
ordered_also_attends_cabinet: [],
ordered_ministerial_departments: [],
ordered_house_of_commons_whips: [],
ordered_junior_lords_of_the_treasury_whips: [],
ordered_assistant_whips: [],
ordered_house_lords_whips: [],
ordered_baronesses_and_lords_in_waiting_whips: [],
}
end

private

def details
{
reshuffle: { message: bare_govspeak_to_html(SitewideSetting.find_by(key: :minister_reshuffle_mode).govspeak) },
}
end
end
end
49 changes: 13 additions & 36 deletions app/presenters/publishing_api/ministers_index_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,29 +32,16 @@ def content
end

def links
if reshuffle_in_progress?
{
ordered_cabinet_ministers: [],
ordered_also_attends_cabinet: [],
ordered_ministerial_departments: [],
ordered_house_of_commons_whips: [],
ordered_junior_lords_of_the_treasury_whips: [],
ordered_assistant_whips: [],
ordered_house_lords_whips: [],
ordered_baronesses_and_lords_in_waiting_whips: [],
}
else
{
ordered_cabinet_ministers: ordered_cabinet_ministers_content_ids,
ordered_also_attends_cabinet: ordered_also_attends_cabinet_content_ids,
ordered_ministerial_departments: ordered_ministerial_departments_content_ids,
ordered_house_of_commons_whips: ordered_whips_content_ids(Whitehall::WhipOrganisation::WhipsHouseOfCommons),
ordered_junior_lords_of_the_treasury_whips: ordered_whips_content_ids(Whitehall::WhipOrganisation::JuniorLordsoftheTreasury),
ordered_assistant_whips: ordered_whips_content_ids(Whitehall::WhipOrganisation::AssistantWhips),
ordered_house_lords_whips: ordered_whips_content_ids(Whitehall::WhipOrganisation::WhipsHouseofLords),
ordered_baronesses_and_lords_in_waiting_whips: ordered_whips_content_ids(Whitehall::WhipOrganisation::BaronessAndLordsInWaiting),
}
end
{
ordered_cabinet_ministers: ordered_cabinet_ministers_content_ids,
ordered_also_attends_cabinet: ordered_also_attends_cabinet_content_ids,
ordered_ministerial_departments: ordered_ministerial_departments_content_ids,
ordered_house_of_commons_whips: ordered_whips_content_ids(Whitehall::WhipOrganisation::WhipsHouseOfCommons),
ordered_junior_lords_of_the_treasury_whips: ordered_whips_content_ids(Whitehall::WhipOrganisation::JuniorLordsoftheTreasury),
ordered_assistant_whips: ordered_whips_content_ids(Whitehall::WhipOrganisation::AssistantWhips),
ordered_house_lords_whips: ordered_whips_content_ids(Whitehall::WhipOrganisation::WhipsHouseofLords),
ordered_baronesses_and_lords_in_waiting_whips: ordered_whips_content_ids(Whitehall::WhipOrganisation::BaronessAndLordsInWaiting),
}
end

def title
Expand All @@ -68,19 +55,9 @@ def base_path
private

def details
if reshuffle_in_progress?
{
reshuffle: { message: bare_govspeak_to_html(SitewideSetting.find_by(key: :minister_reshuffle_mode).govspeak) },
}
else
{
body: "Read biographies and responsibilities of <a href=\"#cabinet-ministers\" class=\"govuk-link\">Cabinet ministers</a> and all <a href=\"#ministers-by-department\" class=\"govuk-link\">ministers by department</a>, as well as the <a href=\"#whips\" class=\"govuk-link\">whips</a> who help co-ordinate parliamentary business.",
}
end
end

def reshuffle_in_progress?
SitewideSetting.find_by(key: :minister_reshuffle_mode)&.on || false
{
body: "Read biographies and responsibilities of <a href=\"#cabinet-ministers\" class=\"govuk-link\">Cabinet ministers</a> and all <a href=\"#ministers-by-department\" class=\"govuk-link\">ministers by department</a>, as well as the <a href=\"#whips\" class=\"govuk-link\">whips</a> who help co-ordinate parliamentary business.",
}
end

def ordered_cabinet_ministers_content_ids
Expand Down
Loading