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.

Sign up for GitHub

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

Use formatted name for worldwide organisations #3029

Merged
merged 1 commit into from
Jan 3, 2024
Merged

Use formatted name for worldwide organisations #3029

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions app/presenters/worldwide_corporate_information_page_presenter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ class WorldwideCorporateInformationPagePresenter < ContentItemPresenter
include ContentItem::ContentsList
include WorldwideOrganisation::Branding

def formatted_title
worldwide_organisation&.formatted_title
end

def show_default_breadcrumbs?
false
end
Expand All @@ -17,10 +21,6 @@ def sponsoring_organisations
worldwide_organisation&.sponsoring_organisations
end

def organisation_logo
super.merge!(name: worldwide_organisation&.title)
end

private

def show_contents_list?
Expand Down
4 changes: 4 additions & 0 deletions app/presenters/worldwide_office_presenter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ class WorldwideOfficePresenter < ContentItemPresenter
include ContentItem::ContentsList
include WorldwideOrganisation::Branding

def formatted_title
worldwide_organisation&.formatted_title
end

def body
content_item.dig("details", "access_and_opening_times")
end
Expand Down
2 changes: 1 addition & 1 deletion app/presenters/worldwide_organisation/branding.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ def organisation_logo

sponsoring_organisation = sponsoring_organisations&.first
{
name: content_item["title"],
name: formatted_title.html_safe,
url: link_to_organisation ? worldwide_organisation.base_path : nil,
crest: sponsoring_organisation&.dig("details", "logo", "crest") || DEFAULT_ORGANISATION_LOGO,
brand: sponsoring_organisation&.dig("details", "brand") || DEFAULT_ORGANISATION_LOGO,
Expand Down
4 changes: 4 additions & 0 deletions app/presenters/worldwide_organisation_presenter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ class WorldwideOrganisationPresenter < ContentItemPresenter
include WorldwideOrganisation::Branding
include ActionView::Helpers::UrlHelper

def formatted_title
content_item.dig("details", "logo", "formatted_title")
end

def sponsoring_organisation_links
return if sponsoring_organisations.empty?

Expand Down
3 changes: 2 additions & 1 deletion test/integration/worldwide_corporate_information_page_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ class WorldwideCorporateInformationPageTest < ActionDispatch::IntegrationTest
setup_and_visit_content_item("worldwide_corporate_information_page")

assert_has_component_organisation_logo
assert page.has_link? "British Embassy Manila", href: "/world/organisations/british-embassy-manila"
assert_has_component_title("British Embassy\nManila")
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand why this works with a \n and not a <br>😕

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that's because we're matching on text here, rather than the raw HTML, so the line break has already been parsed.

assert page.has_link? "British EmbassyManila", href: "/world/organisations/british-embassy-manila"
Copy link
Contributor

Choose a reason for hiding this comment

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

Shame this doesn't also work with the \n

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed. There seems to be some inconsistency in the way these matchers work.

end

test "includes the world locations and sponsoring organisations" do
Expand Down
5 changes: 3 additions & 2 deletions test/integration/worldwide_office_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,12 @@ class WorldwideOfficeTest < ActionDispatch::IntegrationTest
assert page.has_content? "24/7 consular support is available by telephone for all routine enquiries and emergencies."
end

test "includes the logo and name of the worldwide organisation as a link" do
test "includes the logo and formatted name of the worldwide organisation as a link" do
setup_and_visit_content_item("worldwide_office")

assert_has_component_organisation_logo
assert page.has_link? "British Embassy Manila", href: "/world/organisations/british-embassy-manila"
assert_has_component_title("British Embassy\nManila")
assert page.has_link? "British EmbassyManila", href: "/world/organisations/british-embassy-manila"
end

test "includes the world locations and sponsoring organisations" do
Expand Down
2 changes: 1 addition & 1 deletion test/integration/worldwide_organisation_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
class WorldwideOrganisationTest < ActionDispatch::IntegrationTest
test "renders basic worldwide organisation page" do
setup_and_visit_content_item("worldwide_organisation")
assert_has_component_title(@content_item["title"])
assert_has_component_title("British Deputy High Commission\nHyderabad")
assert page.has_text?(@content_item["description"])
end

Expand Down
6 changes: 3 additions & 3 deletions test/presenters/worldwide_corporate_information_page_presenter_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def schema_name

presented = create_presenter(WorldwideCorporateInformationPagePresenter, content_item: with_non_default_crest)

expected = { name: "British Embassy Manila", url: "/world/organisations/british-embassy-manila", crest: "dbt", brand: "foreign-commonwealth-development-office" }
expected = { name: "British Embassy<br/>Manila", url: "/world/organisations/british-embassy-manila", crest: "dbt", brand: "foreign-commonwealth-development-office" }
assert_equal expected, presented.organisation_logo
end

Expand All @@ -32,7 +32,7 @@ def schema_name

presented = create_presenter(WorldwideCorporateInformationPagePresenter, content_item: with_empty_logo)

expected = { name: "British Embassy Manila", url: "/world/organisations/british-embassy-manila", crest: "single-identity", brand: "single-identity" }
expected = { name: "British Embassy<br/>Manila", url: "/world/organisations/british-embassy-manila", crest: "single-identity", brand: "single-identity" }
assert_equal expected, presented.organisation_logo
end

Expand All @@ -42,7 +42,7 @@ def schema_name

presented = create_presenter(WorldwideCorporateInformationPagePresenter, content_item: without_sponsoring_organisations)

expected = { name: "British Embassy Manila", url: "/world/organisations/british-embassy-manila", crest: "single-identity", brand: "single-identity" }
expected = { name: "British Embassy<br/>Manila", url: "/world/organisations/british-embassy-manila", crest: "single-identity", brand: "single-identity" }
Copy link
Contributor

@hannako hannako Jan 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't understand where this <br/> is coming from. I can see the title in the schema example...

Copy link
Member Author

Choose a reason for hiding this comment

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

It comes from the formatted_title.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh different field! OK gottit!

assert_equal expected, presented.organisation_logo
end

Expand Down
6 changes: 3 additions & 3 deletions test/presenters/worldwide_office_presenter_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def schema_name

presented = create_presenter(WorldwideOfficePresenter, content_item: with_non_default_crest)

expected = { name: "British Embassy Manila", url: "/world/organisations/british-embassy-manila", crest: "dbt", brand: "foreign-commonwealth-development-office" }
expected = { name: "British Embassy<br/>Manila", url: "/world/organisations/british-embassy-manila", crest: "dbt", brand: "foreign-commonwealth-development-office" }
assert_equal expected, presented.organisation_logo
end

Expand All @@ -36,7 +36,7 @@ def schema_name

presented = create_presenter(WorldwideOfficePresenter, content_item: with_empty_logo)

expected = { name: "British Embassy Manila", url: "/world/organisations/british-embassy-manila", crest: "single-identity", brand: "single-identity" }
expected = { name: "British Embassy<br/>Manila", url: "/world/organisations/british-embassy-manila", crest: "single-identity", brand: "single-identity" }
assert_equal expected, presented.organisation_logo
end

Expand All @@ -46,7 +46,7 @@ def schema_name

presented = create_presenter(WorldwideOfficePresenter, content_item: without_sponsoring_organisations)

expected = { name: "British Embassy Manila", url: "/world/organisations/british-embassy-manila", crest: "single-identity", brand: "single-identity" }
expected = { name: "British Embassy<br/>Manila", url: "/world/organisations/british-embassy-manila", crest: "single-identity", brand: "single-identity" }
assert_equal expected, presented.organisation_logo
end

Expand Down