From 10e559c403ada9bb7a3bf008cfd7fac5dcc012b6 Mon Sep 17 00:00:00 2001 From: Joseph Kempster Date: Wed, 23 Aug 2023 16:57:10 +0100 Subject: [PATCH] Filter people roles by organisation People linked to a worldwide organisation may have multiple roles, some of which are related to other organsaitions. Currently, we are incorrectly displaying these roles on the worldwide organisation pages. This copies the approach taken in alphagov/collections#1719. We link to all roles from the worldwide organisation, then filter out the incorrect ones in the rendering app. See also alphagov/publishing-api#2478 and alphagov/whitehall#8162. --- .../worldwide_organisation_presenter.rb | 16 ++++++++++++++-- .../worldwide_organisation_presenter_test.rb | 14 ++++++++++++-- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/app/presenters/worldwide_organisation_presenter.rb b/app/presenters/worldwide_organisation_presenter.rb index 14d8fa291..7d116f2f1 100644 --- a/app/presenters/worldwide_organisation_presenter.rb +++ b/app/presenters/worldwide_organisation_presenter.rb @@ -51,7 +51,7 @@ def person_in_primary_role href: person["web_url"], image_url: person["details"]["image"]["url"], image_alt: person["details"]["image"]["alt_text"], - description: current_roles.map { |role_app| role_app.dig("links", "role").first["title"] }.join(", "), + description: organisation_roles_for(current_roles), } end @@ -67,7 +67,7 @@ def people_in_non_primary_roles { name: person["title"], href: person["web_url"], - description: current_roles.map { |role_app| role_app.dig("links", "role").first["title"] }.join(", "), + description: organisation_roles_for(current_roles), } end end @@ -124,6 +124,18 @@ def sponsoring_organisations private + def organisation_roles_for(current_appointments) + current_appointments + .map { |role_appointment| role_appointment.dig("links", "role").first } + .select { |role| organisation_role_ids.include?(role["content_id"]) } + .map { |role| role["title"] } + .compact.join(", ") + end + + def organisation_role_ids + content_item.dig("links", "roles")&.map { |role| role["content_id"] } || [] + end + def world_locations content_item.dig("links", "world_locations") || [] end diff --git a/test/presenters/worldwide_organisation_presenter_test.rb b/test/presenters/worldwide_organisation_presenter_test.rb index 6a2e91222..cb5e93ee7 100644 --- a/test/presenters/worldwide_organisation_presenter_test.rb +++ b/test/presenters/worldwide_organisation_presenter_test.rb @@ -6,15 +6,25 @@ def schema_name end test "description of primary_role_person should have spaces between roles" do - presenter = create_presenter(WorldwideOrganisationPresenter, content_item: { "links" => { "primary_role_person" => [{ "details" => { "image" => {} }, "links" => { "role_appointments" => [{ "details" => { "current" => true }, "links" => { "role" => [{ "title" => "Example Role 1" }] } }, { "details" => { "current" => true }, "links" => { "role" => [{ "title" => "Example Role 2" }] } }] } }] } }) + presenter = create_presenter(WorldwideOrganisationPresenter, content_item: { "links" => { "primary_role_person" => [{ "details" => { "image" => {} }, "links" => { "role_appointments" => [{ "details" => { "current" => true }, "links" => { "role" => [{ "content_id" => "1", "title" => "Example Role 1" }] } }, { "details" => { "current" => true }, "links" => { "role" => [{ "content_id" => "2", "title" => "Example Role 2" }] } }] } }], "roles" => [{ "content_id" => "1" }, { "content_id" => "2" }] } }) assert_equal "Example Role 1, Example Role 2", presenter.person_in_primary_role[:description] end + test "description of primary_role_person should only show roles that are associated with the organisation" do + presenter = create_presenter(WorldwideOrganisationPresenter, content_item: { "links" => { "primary_role_person" => [{ "details" => { "image" => {} }, "links" => { "role_appointments" => [{ "details" => { "current" => true }, "links" => { "role" => [{ "content_id" => "1", "title" => "Example Role 1" }] } }, { "details" => { "current" => true }, "links" => { "role" => [{ "content_id" => "2", "title" => "Example Role 2" }] } }] } }], "roles" => [{ "content_id" => "1" }] } }) + assert_equal "Example Role 1", presenter.person_in_primary_role[:description] + end + test "description of people_in_non_primary_roles should have spaces between roles" do - presenter = create_presenter(WorldwideOrganisationPresenter, content_item: { "links" => { "secondary_role_person" => [{ "details" => { "image" => {} }, "links" => { "role_appointments" => [{ "details" => { "current" => true }, "links" => { "role" => [{ "title" => "Example Role 1" }] } }, { "details" => { "current" => true }, "links" => { "role" => [{ "title" => "Example Role 2" }] } }] } }] } }) + presenter = create_presenter(WorldwideOrganisationPresenter, content_item: { "links" => { "secondary_role_person" => [{ "details" => { "image" => {} }, "links" => { "role_appointments" => [{ "details" => { "current" => true }, "links" => { "role" => [{ "content_id" => "1", "title" => "Example Role 1" }] } }, { "details" => { "current" => true }, "links" => { "role" => [{ "content_id" => "2", "title" => "Example Role 2" }] } }] } }], "roles" => [{ "content_id" => "1" }, { "content_id" => "2" }] } }) assert_equal "Example Role 1, Example Role 2", presenter.people_in_non_primary_roles.first[:description] end + test "description of people_in_non_primary_roles should only show roles that are associated with the organisation" do + presenter = create_presenter(WorldwideOrganisationPresenter, content_item: { "links" => { "secondary_role_person" => [{ "details" => { "image" => {} }, "links" => { "role_appointments" => [{ "details" => { "current" => true }, "links" => { "role" => [{ "content_id" => "1", "title" => "Example Role 1" }] } }, { "details" => { "current" => true }, "links" => { "role" => [{ "content_id" => "2", "title" => "Example Role 2" }] } }] } }], "roles" => [{ "content_id" => "1" }] } }) + assert_equal "Example Role 1", presenter.people_in_non_primary_roles.first[:description] + end + test "#title returns the title" do assert_equal schema_item["title"], presented_item.title end