Merge pull request #9765 from alphagov/remove-important-board-member-constraint

Remove "important board members" image constraint

Author: ChrisBAshton

app/components/admin/organisations/show/summary_list_component.rb

@@ -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,
     ]
@@ -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
 

app/controllers/admin/organisations_controller.rb

@@ -107,7 +107,6 @@ def organisation_params
       :public_meetings,
       :public_minutes,
       :regulatory_function,
-      :important_board_members,
       :custom_jobs_url,
       :homepage_type,
       :political,

app/presenters/publishing_api/organisation_presenter.rb

@@ -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:,
@@ -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

app/views/admin/organisations/_form.html.erb

@@ -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: (1..@organisation.management_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",

db/data_migration/20230518160200_reinstate_independent_agricultural_appeals_panel_organisation.rb

@@ -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,

db/migrate/20241220143730_remove_important_board_members_from_organisations.rb

@@ -0,0 +1,5 @@
+class RemoveImportantBoardMembersFromOrganisations < ActiveRecord::Migration[7.1]
+  def change
+    remove_column :organisations, :important_board_members, :integer
+  end
+end

db/schema.rb

@@ -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
@@ -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"

lib/whitehall/authority/rules/organisation_rules.rb

@@ -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

test/components/admin/organisations/show/summary_list_component_test.rb

@@ -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"
@@ -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
@@ -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
@@ -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
@@ -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/
@@ -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
@@ -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"
@@ -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/
@@ -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
@@ -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/
@@ -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
@@ -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/
@@ -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/
@@ -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/
@@ -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/
@@ -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/
@@ -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/
@@ -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

test/functional/admin/organisations_controller_test.rb

@@ -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
 
@@ -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)
 

test/unit/app/presenters/publishing_api/organisation_presenter_test.rb

@@ -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(
@@ -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",