From 1b2972a772589c6c12fc424612c7af5f08a1d4b4 Mon Sep 17 00:00:00 2001 From: Minno Dang Date: Mon, 25 Nov 2024 12:50:49 +0000 Subject: [PATCH 1/2] Strip spacing from organisation name and parent organisation name Remove trailing spaces --- app/helpers/organisation_helper.rb | 6 ++-- .../app/helpers/organisation_helper_test.rb | 31 +++++++++---------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/app/helpers/organisation_helper.rb b/app/helpers/organisation_helper.rb index 2737f99693f..51c1314de74 100644 --- a/app/helpers/organisation_helper.rb +++ b/app/helpers/organisation_helper.rb @@ -22,7 +22,7 @@ def organisation_type_name(organisation) end def organisation_display_name_and_parental_relationship(organisation) - name = ERB::Util.h(organisation_display_name(organisation)) + name = ERB::Util.h(organisation_display_name(organisation)).strip type_name = organisation_type_name(organisation) relationship = ERB::Util.h(add_indefinite_article(type_name)) parents = organisation.parent_organisations.map { |parent| organisation_relationship_html(parent) } @@ -67,8 +67,8 @@ def organisation_display_name_including_parental_and_child_relationships(organis end def organisation_relationship_html(organisation) - prefix = needs_definite_article?(organisation.name) ? "the " : "" - (prefix + link_to(organisation.name, organisation.public_path, class: "brand__color")) + prefix = needs_definite_article?(organisation.name.strip) ? "the " : "" + (prefix + link_to(organisation.name.strip, organisation.public_path, class: "brand__color")) end def needs_definite_article?(phrase) diff --git a/test/unit/app/helpers/organisation_helper_test.rb b/test/unit/app/helpers/organisation_helper_test.rb index e792673eb6d..ee56b5d2f59 100644 --- a/test/unit/app/helpers/organisation_helper_test.rb +++ b/test/unit/app/helpers/organisation_helper_test.rb @@ -32,12 +32,11 @@ def strip_html_tags(html) html.gsub(/<[^>]*?>/, "") end - def assert_relationship_type_is_described_as(type_key, expected_description) - parent = create(:organisation) - child = create(:organisation, parent_organisations: [parent], organisation_type: OrganisationType.get(type_key)) - expected_text = expected_description.sub("{this_org_name}", child.name).sub("{parent_org_name}", parent.name) + def assert_relationship_type_is_described_as(type_key, expected_description, org_name, parent_org_name) + parent = create(:organisation, name: parent_org_name) + child = create(:organisation, parent_organisations: [parent], organisation_type: OrganisationType.get(type_key), name: org_name) actual_html = organisation_display_name_and_parental_relationship(child) - assert_equal expected_text, strip_html_tags(actual_html) + assert_equal expected_description, strip_html_tags(actual_html) end def assert_definite_article_skipped(parent_organisation_name) @@ -91,17 +90,17 @@ def assert_display_name_text(organisation, expected_text) assert_match %r{the #{parent.name}}, organisation_display_name_and_parental_relationship(child) end - test "relationship types are described correctly" do - assert_relationship_type_is_described_as(:ministerial_department, "{this_org_name} is a ministerial department of the {parent_org_name}.") - assert_relationship_type_is_described_as(:non_ministerial_department, "{this_org_name} is a non-ministerial department.") - assert_relationship_type_is_described_as(:executive_agency, "{this_org_name} is an executive agency, sponsored by the {parent_org_name}.") - assert_relationship_type_is_described_as(:executive_ndpb, "{this_org_name} is an executive non-departmental public body, sponsored by the {parent_org_name}.") - assert_relationship_type_is_described_as(:advisory_ndpb, "{this_org_name} is an advisory non-departmental public body, sponsored by the {parent_org_name}.") - assert_relationship_type_is_described_as(:special_health_authority, "{this_org_name} is a special health authority, sponsored by the {parent_org_name}.") - assert_relationship_type_is_described_as(:tribunal, "{this_org_name} is a tribunal of the {parent_org_name}.") - assert_relationship_type_is_described_as(:public_corporation, "{this_org_name} is a public corporation of the {parent_org_name}.") - assert_relationship_type_is_described_as(:independent_monitoring_body, "{this_org_name} is an independent monitoring body of the {parent_org_name}.") - assert_relationship_type_is_described_as(:other, "{this_org_name} works with the {parent_org_name}.") + test "relationship types are described correctly and trailing spaces are removed" do + assert_relationship_type_is_described_as(:ministerial_department, "org1 is a ministerial department of the parent org1.", "org1 ", "parent org1 ") + assert_relationship_type_is_described_as(:non_ministerial_department, "org2 is a non-ministerial department.", "org2 ", "parent org2 ") + assert_relationship_type_is_described_as(:executive_agency, "org3 is an executive agency, sponsored by the parent org3.", "org3 ", "parent org3 ") + assert_relationship_type_is_described_as(:executive_ndpb, "org4 is an executive non-departmental public body, sponsored by the parent org4.", "org4 ", "parent org4 ") + assert_relationship_type_is_described_as(:advisory_ndpb, "org5 is an advisory non-departmental public body, sponsored by the parent org5.", "org5 ", "parent org5 ") + assert_relationship_type_is_described_as(:special_health_authority, "org6 is a special health authority, sponsored by the parent org6.", "org6 ", "parent org6 ") + assert_relationship_type_is_described_as(:tribunal, "org7 is a tribunal of the parent org7.", "org7 ", "parent org7 ") + assert_relationship_type_is_described_as(:public_corporation, "org8 is a public corporation of the parent org8.", "org8 ", "parent org8 ") + assert_relationship_type_is_described_as(:independent_monitoring_body, "org9 is an independent monitoring body of the parent org9.", "org9 ", "parent org9 ") + assert_relationship_type_is_described_as(:other, "org10 works with the parent org10.", "org10 ", "parent org10 ") end test "definite article skipped for certain parent organisations" do From c2cf7d4a3168307d684ed994778349d389e75556 Mon Sep 17 00:00:00 2001 From: Minno Dang Date: Mon, 25 Nov 2024 13:35:47 +0000 Subject: [PATCH 2/2] Fix the way we display description for Organisation of type other When there are organisations with a parent and also multiple children, we need to make the description grammatically correct. At the moment it says "org 1 works with org 2 is supported by 3....". Appending "and" to make the sentence read better. --- app/helpers/organisation_helper.rb | 9 ++++++++- .../app/helpers/organisation_helper_test.rb | 18 ++++++++++++++---- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/app/helpers/organisation_helper.rb b/app/helpers/organisation_helper.rb index 51c1314de74..3a1fce07ed0 100644 --- a/app/helpers/organisation_helper.rb +++ b/app/helpers/organisation_helper.rb @@ -53,7 +53,7 @@ def organisation_display_name_including_parental_and_child_relationships(organis if child_organisations.any? organisation_name.chomp!(".") - organisation_name += organisation_type_name(organisation) != "other" ? ", supported by " : " is supported by " + organisation_name += supporting_organisation_text(organisation) child_relationships_link_text = child_organisations.size.to_s child_relationships_link_text += child_organisations.size == 1 ? " public body" : " agencies and public bodies" @@ -66,6 +66,13 @@ def organisation_display_name_including_parental_and_child_relationships(organis organisation_name.html_safe end + def supporting_organisation_text(organisation) + return ", supported by " if organisation_type_name(organisation) != "other" + return " and is supported by " if organisation.parent_organisations.any? + + " is supported by " + end + def organisation_relationship_html(organisation) prefix = needs_definite_article?(organisation.name.strip) ? "the " : "" (prefix + link_to(organisation.name.strip, organisation.public_path, class: "brand__color")) diff --git a/test/unit/app/helpers/organisation_helper_test.rb b/test/unit/app/helpers/organisation_helper_test.rb index ee56b5d2f59..d574a4f9058 100644 --- a/test/unit/app/helpers/organisation_helper_test.rb +++ b/test/unit/app/helpers/organisation_helper_test.rb @@ -118,6 +118,7 @@ def assert_display_name_text(organisation, expected_text) parent2 = create(:organisation) child = create(:organisation, parent_organisations: [parent1, parent2]) result = organisation_display_name_and_parental_relationship(child) + assert_match parent1.name, result assert_match parent2.name, result end @@ -142,7 +143,7 @@ def assert_display_name_text(organisation, expected_text) parent = create(:ministerial_department, acronym: "PAN", name: "Parent Organisation Name", child_organisations: [child]) description = organisation_display_name_including_parental_and_child_relationships(parent) - assert description.include? ", supported by" + assert_equal "PAN is a ministerial department, supported by 1 public body.", strip_html_tags(description) end test "organisations of type other with children are described correctly" do @@ -150,8 +151,7 @@ def assert_display_name_text(organisation, expected_text) parent = create(:organisation, organisation_type_key: "other", acronym: "OON", name: "Other Organisation Name", child_organisations: [child]) description = organisation_display_name_including_parental_and_child_relationships(parent) - assert description.include? "is supported by" - assert_not description.include? "is an other" + assert_equal "OON is supported by 1 public body.", strip_html_tags(description) end test "organisations of type other with no relationships are described correctly" do @@ -160,6 +160,16 @@ def assert_display_name_text(organisation, expected_text) description = organisation_display_name_including_parental_and_child_relationships(organisation) assert description.include? "Other Organisation Name" - assert_not description.include? "is an other" + assert_equal "OON", strip_html_tags(description) + end + + test "organisations of type other with parent and multiple children are described correctly" do + child1 = create(:organisation, acronym: "CO", name: "Child Organisation 1") + child2 = create(:organisation, acronym: "CO", name: "Child Organisation 2") + parent = create(:organisation, acronym: "PO", name: "Parent Organisation Name") + org = create(:organisation, organisation_type_key: "other", acronym: "TO", name: "This Organisation", child_organisations: [child1, child2], parent_organisations: [parent]) + + description = organisation_display_name_including_parental_and_child_relationships(org) + assert_equal "TO works with the Parent Organisation Name and is supported by 2 agencies and public bodies.", strip_html_tags(description) end end