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

Editionable worldwide organisation logo formatted name #8684

Merged
merged 6 commits into from
Jan 2, 2024
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
1 change: 1 addition & 0 deletions app/controllers/admin/editions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ def permitted_edition_attributes
:read_consultation_principles,
:all_nation_applicability,
:speaker_radios,
:logo_formatted_name,
{
all_nation_applicability: [],
secondary_specialist_sector_tags: [],
Expand Down
8 changes: 8 additions & 0 deletions app/helpers/organisation_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ def organisation_logo_name(organisation, stacked: true)
end
end

def worldwide_organisation_logo_name(organisation)
if I18n.locale == :en && organisation.logo_formatted_name.present?
format_with_html_line_breaks(ERB::Util.html_escape(organisation.logo_formatted_name))
else
organisation.title
end
end

def organisation_type_name(organisation)
ActiveSupport::Inflector.singularize(organisation.organisation_type.name.downcase)
end
Expand Down
1 change: 1 addition & 0 deletions app/models/worldwide_organisation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class WorldwideOrganisation < ApplicationRecord
translates :name

alias_method :original_main_office, :main_office
alias_method :title, :name

validates_with SafeHtmlValidator
validates :name, presence: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ module PublishingApi
class EditionableWorldwideOrganisationPresenter
include Rails.application.routes.url_helpers
include ActionView::Helpers::UrlHelper
include ApplicationHelper
include OrganisationHelper

attr_accessor :item, :update_type, :state

Expand All @@ -23,9 +25,10 @@ def content
details: {
logo: {
crest: "single-identity",
formatted_title: worldwide_organisation_logo_name(item),
},
},
document_type: item.class.name.underscore,
document_type: "worldwide_organisation",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this okay? I was in the middle of changing Publishing API, and I wondering if we actually needed these to be different? We can distinguish the old ones from new through the base path instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could revert this if this is okay alphagov/publishing-api#2578

If not, happy to add in the editionable_worldwide_organisation to the schema types.

Copy link
Member

Choose a reason for hiding this comment

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

It's probably fine to keep both editionable and non-editionable as having the same document type, since the same code will be rendering both versions.

public_updated_at: item.updated_at,
rendering_app: Whitehall::RenderingApp::GOVERNMENT_FRONTEND,
schema_name: "worldwide_organisation",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ module PublishingApi
class WorldwideOrganisationPresenter
include Rails.application.routes.url_helpers
include ActionView::Helpers::UrlHelper
include ApplicationHelper
include OrganisationHelper

attr_accessor :item, :update_type, :state

Expand All @@ -26,7 +28,7 @@ def content
body:,
logo: {
crest: "single-identity",
formatted_title: item.logo_formatted_name,
formatted_title: worldwide_organisation_logo_name(item),
},
ordered_corporate_information_pages:,
secondary_corporate_information_pages:,
Expand Down
12 changes: 12 additions & 0 deletions app/views/admin/editionable_worldwide_organisations/_form.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,16 @@
<%= standard_edition_form(edition) do |form| %>
<%= render "govuk_publishing_components/components/textarea", {
label: {
text: "Logo formatted name",
heading_size: "l",
},
name: "edition[logo_formatted_name]",
id: "edition_logo_formatted_name",
value: edition.logo_formatted_name,
error_items: errors_for(edition.errors, :logo_formatted_name),
rows: 4,
} %>

<%= render "govuk_publishing_components/components/fieldset", {
legend_text: "Associations",
heading_level: 2,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddLogoFormattedNameToEditions < ActiveRecord::Migration[7.0]
def change
add_column :editions, :logo_formatted_name, :string
end
end
3 changes: 2 additions & 1 deletion 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.0].define(version: 2023_12_19_154010) do
ActiveRecord::Schema[7.0].define(version: 2023_12_28_163205) 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 @@ -395,6 +395,7 @@
t.string "auth_bypass_id", null: false
t.string "mapped_specialist_topic_content_id"
t.string "taxonomy_topic_email_override"
t.string "logo_formatted_name"
t.index ["alternative_format_provider_id"], name: "index_editions_on_alternative_format_provider_id"
t.index ["closing_at"], name: "index_editions_on_closing_at"
t.index ["document_id"], name: "index_editions_on_document_id"
Expand Down
2 changes: 2 additions & 0 deletions features/editionable-worldwide-organisations.feature
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ Feature: Editionable worldwide organisations
When I draft a new worldwide organisation "Test Worldwide Organisation" assigned to world location "United Kingdom"
Then the worldwide organisation "Test Worldwide Organisation" should have been created
And I should see it has been assigned to the "United Kingdom" world location
And I should see the editionable worldwide organisation "Test Worldwide Organisation" in the list of draft documents

Scenario Outline: Assigning a role to a worldwide organisation
Given a role "Prime Minister" exists
When I draft a new worldwide organisation "Test Worldwide Organisation" assigned to world location "United Kingdom"
And I edit the worldwide organisation "Test Worldwide Organisation" adding the role of "Prime Minister"
Then I should see the "Prime Minister" role has been assigned to the worldwide organisation "Test Worldwide Organisation"

Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@

Then(/^the worldwide organisation "([^"]*)" should have been created$/) do |title|
@worldwide_organisation = EditionableWorldwideOrganisation.find_by(title:)

expect(@worldwide_organisation).to be_present
expect(@worldwide_organisation.logo_formatted_name).to eq("Logo\r\nformatted\r\nname\r\n")
end

And(/^I should see it has been assigned to the "([^"]*)" world location$/) do |title|
Expand Down
1 change: 1 addition & 0 deletions features/support/document_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ def pdf_attachment

def fill_in_worldwide_organisation_fields(world_location: "United Kingdom")
select world_location, from: "World locations"
fill_in "Logo formatted name", with: "Logo\r\nformatted\r\nname\r\n"
end

def fill_in_news_article_fields(first_published: "2010-01-01", announcement_type: "News story")
Expand Down
3 changes: 2 additions & 1 deletion test/factories/editionable_worldwide_organisations.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
FactoryBot.define do
factory :editionable_worldwide_organisation, class: EditionableWorldwideOrganisation, parent: :edition_with_organisations do
title { "editionable-worldwide-organisation-title" }
title { "Editionable worldwide organisation title" }
logo_formatted_name { title.to_s.split.join("\n") }

after :build do |news_article, evaluator|
if evaluator.world_locations.empty?
Expand Down
6 changes: 6 additions & 0 deletions test/unit/app/models/worldwide_organisation_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -382,4 +382,10 @@ class WorldwideOrganisationTest < ActiveSupport::TestCase

organisation.update!(name: "new name")
end

test "#title returns the name of the organisation" do
organisation = create(:worldwide_organisation)

assert_equal organisation.name, organisation.title
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def present(...)
base_path: public_path,
title: worldwide_org.title,
schema_name: "worldwide_organisation",
document_type: "editionable_worldwide_organisation",
document_type: "worldwide_organisation",
locale: "en",
publishing_app: Whitehall::PublishingApp::WHITEHALL,
rendering_app: Whitehall::RenderingApp::GOVERNMENT_FRONTEND,
Expand All @@ -24,6 +24,7 @@ def present(...)
details: {
logo: {
crest: "single-identity",
formatted_title: "Editionable<br/>worldwide<br/>organisation<br/>title",
},
},
update_type: "major",
Expand All @@ -41,11 +42,27 @@ def present(...)
assert_equal "major", presented_item.update_type
assert_equal worldwide_org.content_id, presented_item.content_id

# TODO: uncomment the below assertion when the editionable_worldwide_organisation model is
# finished and all content can be added to this presenter.
# assert_valid_against_publisher_schema(presented_item.content, "worldwide_organisation")
assert_valid_against_publisher_schema(presented_item.content, "worldwide_organisation")

assert_equal expected_links, presented_item.links
assert_valid_against_links_schema({ links: presented_item.links }, "worldwide_organisation")
end

test "uses the title for the formatted_title when the locale is not en" do
I18n.with_locale(:it) do
worldwide_org = create(:editionable_worldwide_organisation, title: "Consolato Generale Britannico Milano")

presented_item = present(worldwide_org)

assert_equal "Consolato Generale Britannico Milano", presented_item.content.dig(:details, :logo, :formatted_title)
end
end

test "uses the title for the formatted_title when the the logo_formatted_name is absent" do
worldwide_org = create(:editionable_worldwide_organisation, logo_formatted_name: nil)

presented_item = present(worldwide_org)

assert_equal "Editionable worldwide organisation title", presented_item.content.dig(:details, :logo, :formatted_title)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def present(...)
body: "<div class=\"govspeak\"><p>Some stuff</p>\n</div>",
logo: {
crest: "single-identity",
formatted_title: "Locationia\nEmbassy",
formatted_title: "Locationia<br/>Embassy",
},
ordered_corporate_information_pages: [
{
Expand Down Expand Up @@ -159,4 +159,22 @@ def present(...)
], presented_item.content[:routes]
end
end

test "uses the title for the formatted_title when the locale is not en" do
I18n.with_locale(:it) do
worldwide_org = create(:worldwide_organisation, name: "Consolato Generale Britannico Milano")

presented_item = present(worldwide_org)

assert_equal "Consolato Generale Britannico Milano", presented_item.content.dig(:details, :logo, :formatted_title)
end
end

test "uses the title for the formatted_title when the the logo_formatted_name is absent" do
worldwide_org = create(:worldwide_organisation)

presented_item = present(worldwide_org)

assert_equal worldwide_org.name, presented_item.content.dig(:details, :logo, :formatted_title)
end
end
Loading