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

Remove "important board members" image constraint #9765

Merged
merged 2 commits into from
Jan 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions app/components/admin/organisations/show/summary_list_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ def rows
topical_events_row,
featured_links_position_row,
featured_links_row,
management_team_row,
foi_exempt_row,
analytics_identifier_row,
]
Expand Down Expand Up @@ -224,15 +223,6 @@ def featured_links_row
end
end

def management_team_row
return if organisation.important_board_members.blank?

{
field: "Management team images on homepage",
value: organisation.important_board_members,
}
end

def foi_exempt_row
return unless organisation.foi_exempt

Expand Down
1 change: 0 additions & 1 deletion app/controllers/admin/organisations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ def organisation_params
:public_meetings,
:public_minutes,
:regulatory_function,
:important_board_members,
:custom_jobs_url,
:homepage_type,
:political,
Expand Down
5 changes: 0 additions & 5 deletions app/presenters/publishing_api/organisation_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ def details
ordered_featured_links: featured_links,
ordered_featured_documents: featured_documents(item, item.featured_documents_display_limit),
ordered_promotional_features: promotional_features,
important_board_members:,
organisation_featuring_priority:,
organisation_govuk_status:,
organisation_type:,
Expand Down Expand Up @@ -335,10 +334,6 @@ def people_content_ids(role:)
.map(&:content_id)
end

def important_board_members
item.important_board_members
end

def organisation_featuring_priority
item.homepage_type
end
Expand Down
19 changes: 0 additions & 19 deletions app/views/admin/organisations/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -319,25 +319,6 @@
</div>
<% end %>

<div class="govuk-!-margin-top-6">
<% if can?(:manage_important_board_members, @organisation) && @organisation.management_roles.any? %>
<%= render "govuk_publishing_components/components/select", {
name: "organisation[important_board_members]",
id: "organisation_important_board_members",
label: "Display management team images (required)",
heading_size: "l",
error_message: errors_for_input(@organisation.errors, :important_board_members),
options: ([email protected]_roles.count).map do |n|
{
text: n,
value: n,
selected: @organisation.important_board_members == n,
}
end,
} %>
<% end %>
</div>

<div class="app-view-organisation__form__non-departmental-public-body-field js-view-organisation__form__non-departmental-public-body-fields <%= "app-view-organisation__form__non-departmental-public-body-fields--hidden" unless organisation.type&.non_departmental_public_body? %>">
<%= render "govuk_publishing_components/components/fieldset", {
legend_text: "Non-Departmental Public Body Information",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ def recreate_organisation_and_dependencies
organisation_logo_type_id: 2,
analytics_identifier: "PB210",
handles_fatalities: false,
important_board_members: 1,
default_news_organisation_image_data_id: nil,
closed_at: nil,
organisation_brand_colour_id: 7,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class RemoveImportantBoardMembersFromOrganisations < ActiveRecord::Migration[7.1]
def change
remove_column :organisations, :important_board_members, :integer
end
end
3 changes: 1 addition & 2 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.1].define(version: 2024_11_11_142527) do
ActiveRecord::Schema[7.1].define(version: 2024_12_20_143730) do
create_table "assets", charset: "utf8mb3", force: :cascade do |t|
t.string "asset_manager_id", null: false
t.string "variant", null: false
Expand Down Expand Up @@ -788,7 +788,6 @@
t.integer "organisation_logo_type_id", default: 2
t.string "analytics_identifier"
t.boolean "handles_fatalities", default: false
t.integer "important_board_members", default: 1
t.datetime "closed_at", precision: nil
t.integer "organisation_brand_colour_id"
t.boolean "ocpa_regulated"
Expand Down
2 changes: 0 additions & 2 deletions lib/whitehall/authority/rules/organisation_rules.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ def can?(action)
actor.gds_admin? || actor_is_from_organisation_or_parent?(actor, subject)
when :manage_featured_links
actor.gds_admin? || actor.gds_editor? || managing_editor_for_org?(actor, subject)
when :manage_important_board_members
actor.gds_admin? || actor.gds_editor? || managing_editor_for_org?(actor, subject) || departmental_editor_for_org?(actor, subject)
else
false
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class Admin::Organisations::Show::SummaryListComponentTest < ViewComponent::Test

render_inline(Admin::Organisations::Show::SummaryListComponent.new(organisation:))

assert_selector ".govuk-summary-list__row", count: 8
assert_selector ".govuk-summary-list__row", count: 7
assert_selector ".govuk-summary-list__row:nth-child(1) .govuk-summary-list__key", text: "Name"
assert_selector ".govuk-summary-list__row:nth-child(1) .govuk-summary-list__value", text: organisation.name
assert_selector ".govuk-summary-list__row:nth-child(2) .govuk-summary-list__key", text: "Logo formatted name"
Expand All @@ -38,18 +38,16 @@ class Admin::Organisations::Show::SummaryListComponentTest < ViewComponent::Test
assert_selector ".govuk-summary-list__row:nth-child(5) .govuk-summary-list__value", text: organisation.govuk_status.titleize
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__key", text: "Featured link position"
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__value", text: "News priority"
assert_selector ".govuk-summary-list__row:nth-child(7) .govuk-summary-list__key", text: "Management team images on homepage"
assert_selector ".govuk-summary-list__row:nth-child(7) .govuk-summary-list__value", text: organisation.important_board_members
assert_selector ".govuk-summary-list__row:nth-child(8) .govuk-summary-list__key", text: "Analytics identifier"
assert_selector ".govuk-summary-list__row:nth-child(8) .govuk-summary-list__value", text: organisation.analytics_identifier
assert_selector ".govuk-summary-list__row:nth-child(7) .govuk-summary-list__key", text: "Analytics identifier"
assert_selector ".govuk-summary-list__row:nth-child(7) .govuk-summary-list__value", text: organisation.analytics_identifier
end

test "renders acronym_row if the organisation has an acronym" do
organisation = build_stubbed(:ministerial_department, acronym: "ACRO")

render_inline(Admin::Organisations::Show::SummaryListComponent.new(organisation:))

assert_selector ".govuk-summary-list__row", count: 9
assert_selector ".govuk-summary-list__row", count: 8
assert_selector ".govuk-summary-list__row:nth-child(2) .govuk-summary-list__key", text: "Acronym"
assert_selector ".govuk-summary-list__row:nth-child(2) .govuk-summary-list__value", text: organisation.acronym
end
Expand All @@ -59,7 +57,7 @@ class Admin::Organisations::Show::SummaryListComponentTest < ViewComponent::Test

render_inline(Admin::Organisations::Show::SummaryListComponent.new(organisation:))

assert_selector ".govuk-summary-list__row", count: 9
assert_selector ".govuk-summary-list__row", count: 8
assert_selector ".govuk-summary-list__row:nth-child(4) .govuk-summary-list__key", text: "Brand colour"
assert_selector ".govuk-summary-list__row:nth-child(4) .govuk-summary-list__value", text: organisation.organisation_brand_colour.title
end
Expand All @@ -70,7 +68,7 @@ class Admin::Organisations::Show::SummaryListComponentTest < ViewComponent::Test

render_inline(Admin::Organisations::Show::SummaryListComponent.new(organisation:))

assert_selector ".govuk-summary-list__row", count: 9
assert_selector ".govuk-summary-list__row", count: 8
assert_selector ".govuk-summary-list__row:nth-child(4) .govuk-summary-list__key", text: "Default news image"
assert_selector ".govuk-summary-list__row:nth-child(4) .govuk-summary-list__value img[src='#{news_image.file.url(:s300)}']"
end
Expand All @@ -80,7 +78,7 @@ class Admin::Organisations::Show::SummaryListComponentTest < ViewComponent::Test

render_inline(Admin::Organisations::Show::SummaryListComponent.new(organisation:))

assert_selector ".govuk-summary-list__row", count: 9
assert_selector ".govuk-summary-list__row", count: 8
assert_selector ".govuk-summary-list__row:nth-child(4) .govuk-summary-list__key", text: "Organisation’s URL"
assert_selector ".govuk-summary-list__row:nth-child(4) .govuk-summary-list__value", text: "http://parrot.org"
assert_selector ".govuk-summary-list__row:nth-child(4) .govuk-summary-list__actions a[href='#{organisation.url}']", text: /View/
Expand All @@ -91,7 +89,7 @@ class Admin::Organisations::Show::SummaryListComponentTest < ViewComponent::Test

render_inline(Admin::Organisations::Show::SummaryListComponent.new(organisation:))

assert_selector ".govuk-summary-list__row", count: 9
assert_selector ".govuk-summary-list__row", count: 8
assert_selector ".govuk-summary-list__row:nth-child(5) .govuk-summary-list__key", text: "Accessible formats request email"
assert_selector ".govuk-summary-list__row:nth-child(5) .govuk-summary-list__value", text: organisation.alternative_format_contact_email
end
Expand All @@ -103,7 +101,7 @@ class Admin::Organisations::Show::SummaryListComponentTest < ViewComponent::Test

render_inline(Admin::Organisations::Show::SummaryListComponent.new(organisation:))

assert_selector ".govuk-summary-list__row", count: 11
assert_selector ".govuk-summary-list__row", count: 10
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__key", text: "Reason for closure"
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__value", text: organisation.govuk_closed_status
assert_selector ".govuk-summary-list__row:nth-child(7) .govuk-summary-list__key", text: "Organisation closed on"
Expand All @@ -121,7 +119,7 @@ class Admin::Organisations::Show::SummaryListComponentTest < ViewComponent::Test

render_inline(Admin::Organisations::Show::SummaryListComponent.new(organisation:))

assert_selector ".govuk-summary-list__row", count: 12
assert_selector ".govuk-summary-list__row", count: 11
assert_selector ".govuk-summary-list__row:nth-child(8) .govuk-summary-list__key", text: "Superseding organisation 1"
assert_selector ".govuk-summary-list__row:nth-child(8) .govuk-summary-list__value", text: superseding_organisation1.name
assert_selector ".govuk-summary-list__row:nth-child(8) .govuk-summary-list__actions a[href='#{superseding_organisation1.public_url}']", text: /View/
Expand All @@ -135,7 +133,7 @@ class Admin::Organisations::Show::SummaryListComponentTest < ViewComponent::Test

render_inline(Admin::Organisations::Show::SummaryListComponent.new(organisation:))

assert_selector ".govuk-summary-list__row", count: 9
assert_selector ".govuk-summary-list__row", count: 8
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__key", text: "Organisation chart URL"
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__value", text: organisation.organisation_chart_url
end
Expand All @@ -145,7 +143,7 @@ class Admin::Organisations::Show::SummaryListComponentTest < ViewComponent::Test

render_inline(Admin::Organisations::Show::SummaryListComponent.new(organisation:))

assert_selector ".govuk-summary-list__row", count: 9
assert_selector ".govuk-summary-list__row", count: 8
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__key", text: "Recruitment URL"
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__value", text: organisation.custom_jobs_url
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__actions a[href='#{organisation.custom_jobs_url}']", text: /View/
Expand All @@ -156,7 +154,7 @@ class Admin::Organisations::Show::SummaryListComponentTest < ViewComponent::Test

render_inline(Admin::Organisations::Show::SummaryListComponent.new(organisation:))

assert_selector ".govuk-summary-list__row", count: 9
assert_selector ".govuk-summary-list__row", count: 8
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__key", text: "Publishes content associated with the current government"
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__value", text: "Yes"
end
Expand All @@ -168,7 +166,7 @@ class Admin::Organisations::Show::SummaryListComponentTest < ViewComponent::Test

render_inline(Admin::Organisations::Show::SummaryListComponent.new(organisation:))

assert_selector ".govuk-summary-list__row", count: 9
assert_selector ".govuk-summary-list__row", count: 8
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__key", text: "Sponsoring organisation"
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__value", text: parent_organisation.name
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__actions a[href='#{parent_organisation.public_url}']", text: /View/
Expand All @@ -182,7 +180,7 @@ class Admin::Organisations::Show::SummaryListComponentTest < ViewComponent::Test

render_inline(Admin::Organisations::Show::SummaryListComponent.new(organisation:))

assert_selector ".govuk-summary-list__row", count: 10
assert_selector ".govuk-summary-list__row", count: 9
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__key", text: "Sponsoring organisation 1"
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__value", text: parent_organisation1.name
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__actions a[href='#{parent_organisation1.public_url}']", text: /View/
Expand All @@ -198,7 +196,7 @@ class Admin::Organisations::Show::SummaryListComponentTest < ViewComponent::Test

render_inline(Admin::Organisations::Show::SummaryListComponent.new(organisation:))

assert_selector ".govuk-summary-list__row", count: 9
assert_selector ".govuk-summary-list__row", count: 8
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__key", text: "Topical event"
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__value", text: topical_event.name
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__actions a[href='#{topical_event.public_url}']", text: /View/
Expand All @@ -212,7 +210,7 @@ class Admin::Organisations::Show::SummaryListComponentTest < ViewComponent::Test

render_inline(Admin::Organisations::Show::SummaryListComponent.new(organisation:))

assert_selector ".govuk-summary-list__row", count: 10
assert_selector ".govuk-summary-list__row", count: 9
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__key", text: "Topical event 1"
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__value", text: topical_event1.name
assert_selector ".govuk-summary-list__row:nth-child(6) .govuk-summary-list__actions a[href='#{topical_event1.public_url}']", text: /View/
Expand All @@ -228,7 +226,7 @@ class Admin::Organisations::Show::SummaryListComponentTest < ViewComponent::Test

render_inline(Admin::Organisations::Show::SummaryListComponent.new(organisation:))

assert_selector ".govuk-summary-list__row", count: 9
assert_selector ".govuk-summary-list__row", count: 8
assert_selector ".govuk-summary-list__row:nth-child(7) .govuk-summary-list__key", text: "Featured link"
assert_selector ".govuk-summary-list__row:nth-child(7) .govuk-summary-list__value", text: featured_link.title
assert_selector ".govuk-summary-list__row:nth-child(7) .govuk-summary-list__actions a[href='#{featured_link.url}']", text: /View/
Expand All @@ -242,7 +240,7 @@ class Admin::Organisations::Show::SummaryListComponentTest < ViewComponent::Test

render_inline(Admin::Organisations::Show::SummaryListComponent.new(organisation:))

assert_selector ".govuk-summary-list__row", count: 10
assert_selector ".govuk-summary-list__row", count: 9
assert_selector ".govuk-summary-list__row:nth-child(7) .govuk-summary-list__key", text: "Featured link 1"
assert_selector ".govuk-summary-list__row:nth-child(7) .govuk-summary-list__value", text: featured_link1.title
assert_selector ".govuk-summary-list__row:nth-child(7) .govuk-summary-list__actions a[href='#{featured_link1.url}']", text: /View/
Expand All @@ -256,8 +254,8 @@ class Admin::Organisations::Show::SummaryListComponentTest < ViewComponent::Test

render_inline(Admin::Organisations::Show::SummaryListComponent.new(organisation:))

assert_selector ".govuk-summary-list__row", count: 9
assert_selector ".govuk-summary-list__row:nth-child(8) .govuk-summary-list__key", text: "Exempt from Freedom of Information requests"
assert_selector ".govuk-summary-list__row:nth-child(8) .govuk-summary-list__value", text: "Yes"
assert_selector ".govuk-summary-list__row", count: 8
assert_selector ".govuk-summary-list__row:nth-child(7) .govuk-summary-list__key", text: "Exempt from Freedom of Information requests"
assert_selector ".govuk-summary-list__row:nth-child(7) .govuk-summary-list__value", text: "Yes"
end
end
38 changes: 0 additions & 38 deletions test/functional/admin/organisations_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,16 +91,6 @@ def example_organisation_attributes
assert_match %r{logo.png}, Organisation.last.logo.file.filename
end

test "POST create can set number of important board members" do
post :create,
params: {
organisation: example_organisation_attributes
.merge(important_board_members: 1),
}

assert_equal 1, Organisation.last.important_board_members
end

test "POST on :create with invalid data re-renders the new form" do
attributes = example_organisation_attributes

Expand Down Expand Up @@ -138,34 +128,6 @@ def example_organisation_attributes
assert_equal organisation, assigns(:organisation)
end

view_test "GET on :edit allows entry of important board members only data to Editors and above" do
organisation = create(:organisation)
junior_board_member_role = create(:board_member_role)
senior_board_member_role = create(:board_member_role)

create(:organisation_role, organisation:, role: senior_board_member_role)
create(:organisation_role, organisation:, role: junior_board_member_role)

managing_editor = create(:managing_editor, organisation:)
departmental_editor = create(:departmental_editor, organisation:)
world_editor = create(:world_editor, organisation:)

get :edit, params: { id: organisation }
assert_select "select#organisation_important_board_members option", count: 2

login_as(departmental_editor)
get :edit, params: { id: organisation }
assert_select "select#organisation_important_board_members option", count: 2

login_as(managing_editor)
get :edit, params: { id: organisation }
assert_select "select#organisation_important_board_members option", count: 2

login_as(world_editor)
get :edit, params: { id: organisation }
assert_select "select#organisation_important_board_members option", count: 0
end

view_test "GET :edit renders hidden id field for default news image" do
organisation = create(:organisation, :with_default_news_image)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ def govspeak_to_html(govspeak)
analytics_identifier: "O123",
parent_organisations: [parent_organisation],
url: "https://www.gov.uk/oot",
important_board_members: 5,
default_news_image: news_image,
)
create(
Expand Down Expand Up @@ -75,7 +74,6 @@ def govspeak_to_html(govspeak)
ordered_featured_links: [],
ordered_featured_documents: [],
ordered_promotional_features: [],
important_board_members: 5,
organisation_featuring_priority: "news",
organisation_govuk_status: {
status: "live",
Expand Down
Loading
Loading