Skip to content

Commit

Permalink
Filter people roles by organisation
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jkempster34 committed Aug 24, 2023
1 parent 6c0ec4f commit 10e559c
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 4 deletions.
16 changes: 14 additions & 2 deletions app/presenters/worldwide_organisation_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
14 changes: 12 additions & 2 deletions test/presenters/worldwide_organisation_presenter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 10e559c

Please sign in to comment.