From ce06a524b1322ead97b7bfa1315d09628161ff13 Mon Sep 17 00:00:00 2001 From: Deborah Chua Date: Wed, 26 Feb 2025 14:47:36 +0000 Subject: [PATCH 1/9] Add organisation logo model and tests - Added image,crest and formatted title to the logo model - This model will be used in the organisation model --- app/models/logo.rb | 18 +++++++++++++++ spec/models/logo_spec.rb | 50 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 app/models/logo.rb create mode 100644 spec/models/logo_spec.rb diff --git a/app/models/logo.rb b/app/models/logo.rb new file mode 100644 index 0000000000..136c4b851d --- /dev/null +++ b/app/models/logo.rb @@ -0,0 +1,18 @@ +class Logo + attr_reader :crest, :formatted_title, :image + + def initialize(logo) + @image = get_image(logo["image"]) + @crest = logo["crest"] + @formatted_title = logo["formatted_title"] + end + + def get_image(logo_image) + return unless logo_image + + OpenStruct.new( + alt_text: logo_image["alt_text"], + url: logo_image["url"], + ) + end +end diff --git a/spec/models/logo_spec.rb b/spec/models/logo_spec.rb new file mode 100644 index 0000000000..9bc4dc1c15 --- /dev/null +++ b/spec/models/logo_spec.rb @@ -0,0 +1,50 @@ +RSpec.describe Logo do + let(:logo) do + { + "crest" => "dbt", + "formatted_title" => "Department for
Business & Trade", + } + end + + describe "#crest" do + it "gets the crest" do + expect(described_class.new(logo).crest) + .to eq(logo["crest"]) + end + end + + describe "#formatted_title" do + it "gets the formatted title" do + expect(described_class.new(logo).formatted_title) + .to eq(logo["formatted_title"]) + end + end + + describe "#image" do + context "when there is an image" do + let(:logo) do + { + "formatted_title" => "Forensic Science
Regulator", + "image" => { + "alt_text" => "Forensic Science Regulator", + "url" => "https://example.com/sample.png", + }, + } + end + + it "gets the image alt_text" do + expect(described_class.new(logo).image.alt_text).to eq(logo["image"]["alt_text"]) + end + + it "gets the image url" do + expect(described_class.new(logo).image.url).to eq(logo["image"]["url"]) + end + end + + context "when there is no image" do + it "returns nil" do + expect(described_class.new(logo).image).to be_nil + end + end + end +end From 199c5fa49147f6300df12663024bcd58747be944 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Wed, 19 Feb 2025 12:05:36 +0000 Subject: [PATCH 2/9] Create Organisation model The Organisation model will be used to model the organisation links in a ContentItem as well as eventually be used to render the organisation page itself when it is moved from collections. Added logo and brand to organisation model in preparation for moving corporate information pages from government-frontend Co-authored-by: Deborah Chua Co-authored-by: Leena Gupte Co-authored-by: Keith Lawrence Co-authored-by: Ramya Vidapanakal --- app/models/organisation.rb | 17 +++++++++++++++++ spec/models/organisation_spec.rb | 18 ++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 app/models/organisation.rb create mode 100644 spec/models/organisation_spec.rb diff --git a/app/models/organisation.rb b/app/models/organisation.rb new file mode 100644 index 0000000000..841795d10e --- /dev/null +++ b/app/models/organisation.rb @@ -0,0 +1,17 @@ +class Organisation < ContentItem + attr_reader :logo + + def initialize(organisation_data) + super(organisation_data) + @logo = Logo.new(organisation_data.dig("details", "logo")) + @organisation_data = organisation_data + end + + def brand + organisation_data.dig("details", "brand") + end + +private + + attr_reader :organisation_data +end diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb new file mode 100644 index 0000000000..7200c7ff8f --- /dev/null +++ b/spec/models/organisation_spec.rb @@ -0,0 +1,18 @@ +RSpec.describe Organisation do + let(:content_store_response) do + GovukSchemas::Example.find("organisation", example_name: "organisation") + end + + describe "#brand" do + it "gets the brand" do + expect(described_class.new(content_store_response).brand).not_to be_nil + expect(described_class.new(content_store_response).brand).to eq(content_store_response.dig("details", "brand")) + end + end + + describe "#logo" do + it "gets the logo" do + expect(described_class.new(content_store_response).logo).to be_instance_of(Logo) + end + end +end From 06fe04b1d939a2c37af14da010111b2d3e76b145 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Wed, 19 Feb 2025 12:38:15 +0000 Subject: [PATCH 3/9] Add organisations method to content item Organisations method relies on a private method that gets and models links. Almost all content items have organisations, so we are safe to add that utility method. Co-authored-by: Deborah Chua Co-authored-by: Leena Gupte Co-authored-by: Keith Lawrence Co-authored-by: Ramya Vidapanakal --- app/models/content_item.rb | 10 ++++++++++ spec/models/content_item_spec.rb | 14 ++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/app/models/content_item.rb b/app/models/content_item.rb index 20514b4454..8483533375 100644 --- a/app/models/content_item.rb +++ b/app/models/content_item.rb @@ -39,6 +39,10 @@ def initialize(content_store_response, override_content_store_hash: nil) REGEX_IS_A = /is_an?_(.*)\?/ + def organisations + linked("organisations") + end + def respond_to_missing?(method_name, _include_private = false) method_name.to_s =~ REGEX_IS_A ? true : super end @@ -73,6 +77,12 @@ def contributors private + def linked(type) + return [] if content_store_hash.dig("links", type).blank? + + content_store_hash.dig("links", type).map { |hash| ContentItemFactory.build(hash) } + end + def get_attachments(attachment_hash) return [] unless attachment_hash diff --git a/spec/models/content_item_spec.rb b/spec/models/content_item_spec.rb index 13cd734a8a..e361647e20 100644 --- a/spec/models/content_item_spec.rb +++ b/spec/models/content_item_spec.rb @@ -212,4 +212,18 @@ expect(content_item.meta_section).to eq("title of the parent's parent link") end end + + describe "#organisations" do + subject(:content_item) { described_class.new(content_store_response) } + + let(:content_store_response) do + GovukSchemas::Example.find("answer", example_name: "answer") + end + + it "gets all organisations linked to the content item" do + expect(content_item.organisations.count).to eq(content_store_response.dig("links", "organisations").count) + expect(content_item.organisations.first.title) + .to eq(content_store_response.dig("links", "organisations", 0, "title")) + end + end end From df2922e4e32db192a6bd2da36847743f2ee9e92f Mon Sep 17 00:00:00 2001 From: ramya vidapanakal Date: Thu, 27 Feb 2025 16:19:12 +0000 Subject: [PATCH 4/9] Pass the contributors method a list of objects This allows us to simplify how the contributors list is built. We should only need to create the contributors once by passing it a list of link models rather than duplicating the mapping code in all of the content item models. The idea is that each contributors method in the child class will pass the contributors method in the parent ContentItem class as list of objects. For example for CaseStudy, that would be a list of modelled Organisation and Worldwide Organisation objects. This is the list that is passed in. It still defaults to organisations as that is the bare minimum that appears in the From field on documents that this contributors method is used to populate. Even though the list of content items can be different, as they all inherit from ContentItem, we know that they will have a title and base_path, so can be processed in the same way. Co-authored-by: Deborah Chua Co-authored-by: Leena Gupte Co-authored-by: Keith Lawrence Co-authored-by: Ramya Vidapanakal --- app/models/content_item.rb | 6 ++++-- spec/models/content_item_spec.rb | 30 ++++++------------------------ 2 files changed, 10 insertions(+), 26 deletions(-) diff --git a/app/models/content_item.rb b/app/models/content_item.rb index 8483533375..18f77c68ca 100644 --- a/app/models/content_item.rb +++ b/app/models/content_item.rb @@ -71,8 +71,10 @@ def meta_section )&.downcase end - def contributors - organisations + def contributors(content_items = organisations) + content_items.map do |content_item| + { "title" => content_item.title, "base_path" => content_item.base_path } + end end private diff --git a/spec/models/content_item_spec.rb b/spec/models/content_item_spec.rb index e361647e20..9a0b0eca53 100644 --- a/spec/models/content_item_spec.rb +++ b/spec/models/content_item_spec.rb @@ -154,33 +154,15 @@ end describe "#contributors" do - subject(:content_item) do - described_class.new( - { - "links" => { - "organisations" => [ - { - "analytics_identifier" => "8888", - "content_id" => "11234500", - "api_path" => "/api/content/government/organisations/uk-health-security-agency", - "api_url" => "https://www.gov.uk/api/content/government/organisations/uk-health-security-agency", - "base_path" => "/government/organisations/uk-health-security-agency", - "document_type" => "organisation", - "title" => "UK Health Security Agency", - "web_url" => "https://www.gov.uk/government/organisations/uk-health-security-agency", - }, - ], - }, - }, - ) - end + subject(:content_item) { described_class.new(content_store_response) } + + let(:content_store_response) { GovukSchemas::Example.find("answer", example_name: "answer") } - it "returns the organisations content_id, base_path and title" do + it "returns the organisations base_path and title" do expect(content_item.contributors).to eq([ { - "content_id" => "11234500", - "base_path" => "/government/organisations/uk-health-security-agency", - "title" => "UK Health Security Agency", + "base_path" => content_store_response.dig("links", "organisations", 0, "base_path"), + "title" => content_store_response.dig("links", "organisations", 0, "title"), }, ]) end From 4049fe4f5782e5b91209173c30c6112708832236 Mon Sep 17 00:00:00 2001 From: Leena Gupte Date: Thu, 20 Feb 2025 16:32:40 +0000 Subject: [PATCH 5/9] Update contributors method in case study model The contributors method now passes a list of emphasised organisations objects and worldwide organisations object to the parent class contributors method. Since worldwide organisations are not always present, add a test for them, which also adds coverage to the private linked method in content_item. Co-authored-by: Deborah Chua Co-authored-by: Leena Gupte Co-authored-by: Keith Lawrence Co-authored-by: Ramya Vidapanakal --- app/models/case_study.rb | 3 ++- spec/models/case_study_spec.rb | 31 +++++++++++++++++++++++++------ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/app/models/case_study.rb b/app/models/case_study.rb index 9532abf222..c5c22a6ead 100644 --- a/app/models/case_study.rb +++ b/app/models/case_study.rb @@ -4,6 +4,7 @@ class CaseStudy < ContentItem include Organisations def contributors - (organisations_ordered_by_emphasis + worldwide_organisations).uniq + contributors_list = (organisations_ordered_by_emphasis + worldwide_organisations).uniq + super(contributors_list) end end diff --git a/spec/models/case_study_spec.rb b/spec/models/case_study_spec.rb index bbaaf8b54d..5abd41a573 100644 --- a/spec/models/case_study_spec.rb +++ b/spec/models/case_study_spec.rb @@ -1,4 +1,6 @@ RSpec.describe CaseStudy do + subject(:case_study) { described_class.new(content_store_response) } + let(:content_store_response) do GovukSchemas::Example.find("case_study", example_name: "doing-business-in-spain") end @@ -10,18 +12,35 @@ describe "#contributors" do it "returns the organisations ordered by emphasis followed by worldwide organisations" do - content_item = described_class.new(content_store_response) - worldwide_organisations = content_store_response.dig("links", "worldwide_organisations") organisations = content_store_response.dig("links", "organisations") expected_contributors = [ - { "title" => organisations[1]["title"], "base_path" => organisations[1]["base_path"], "content_id" => organisations[1]["content_id"] }, - { "title" => organisations[0]["title"], "base_path" => organisations[0]["base_path"], "content_id" => organisations[0]["content_id"] }, - { "title" => worldwide_organisations[0]["title"], "base_path" => worldwide_organisations[0]["base_path"], "content_id" => worldwide_organisations[0]["content_id"] }, + { "title" => organisations[1]["title"], "base_path" => organisations[1]["base_path"] }, + { "title" => organisations[0]["title"], "base_path" => organisations[0]["base_path"] }, + { "title" => worldwide_organisations[0]["title"], "base_path" => worldwide_organisations[0]["base_path"] }, ] - expect(content_item.contributors).to eq(expected_contributors) + expect(case_study.contributors).to eq(expected_contributors) + end + + context "with no worldwide organisations" do + let(:content_store_response) do + example = GovukSchemas::Example.find("case_study", example_name: "doing-business-in-spain") + example["links"].delete("worldwide_organisations") + example + end + + it "returns just the organisations ordered by emphasis" do + organisations = content_store_response.dig("links", "organisations") + + expected_contributors = [ + { "title" => organisations[1]["title"], "base_path" => organisations[1]["base_path"] }, + { "title" => organisations[0]["title"], "base_path" => organisations[0]["base_path"] }, + ] + + expect(case_study.contributors).to eq(expected_contributors) + end end end end From 984dff90b0d8bcc9e71a4a4b873fe3d496da1806 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Wed, 19 Feb 2025 15:20:48 +0000 Subject: [PATCH 6/9] Add content_id to content item Co-authored-by: Deborah Chua Co-authored-by: Leena Gupte Co-authored-by: Keith Lawrence Co-authored-by: Ramya Vidapanakal --- app/models/content_item.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/models/content_item.rb b/app/models/content_item.rb index 18f77c68ca..d2001ea0ad 100644 --- a/app/models/content_item.rb +++ b/app/models/content_item.rb @@ -2,9 +2,7 @@ class ContentItem include Withdrawable - include Organisations - - attr_reader :attachments, :base_path, :body, :content_store_hash, + attr_reader :attachments, :base_path, :body, :content_id, :content_store_hash, :content_store_response, :description, :document_type, :first_public_at, :first_published_at, :image, :links, :locale, :phase, :public_updated_at, :schema_name, :title @@ -16,6 +14,7 @@ def initialize(content_store_response, override_content_store_hash: nil) @content_store_hash = override_content_store_hash || content_store_response.to_hash @body = content_store_hash.dig("details", "body") + @content_id = content_store_hash["content_id"] @image = content_store_hash.dig("details", "image") @description = content_store_hash["description"] @document_type = content_store_hash["document_type"] From 95c0852a6c1047e7bc827c2a8c0ff416bc3c15fe Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Wed, 19 Feb 2025 14:59:11 +0000 Subject: [PATCH 7/9] Rename Organisations concern -> EmphasisedOrganisations We now model organisations separately, so this concern can be updated to deal with emphasised organisations. Updated the emphasised organisations test to check that the first organisation is an emphasised organisation in a list of organisation models. Co-authored-by: Deborah Chua Co-authored-by: Leena Gupte Co-authored-by: Keith Lawrence Co-authored-by: Ramya Vidapanakal --- .../{organisations.rb => emphasised_organisations.rb} | 10 ++-------- .../{organisations.rb => emphasised_organisations.rb} | 6 +++++- 2 files changed, 7 insertions(+), 9 deletions(-) rename app/models/concerns/{organisations.rb => emphasised_organisations.rb} (50%) rename spec/support/concerns/{organisations.rb => emphasised_organisations.rb} (68%) diff --git a/app/models/concerns/organisations.rb b/app/models/concerns/emphasised_organisations.rb similarity index 50% rename from app/models/concerns/organisations.rb rename to app/models/concerns/emphasised_organisations.rb index 866d677b71..5018413b7b 100644 --- a/app/models/concerns/organisations.rb +++ b/app/models/concerns/emphasised_organisations.rb @@ -1,22 +1,16 @@ -module Organisations +module EmphasisedOrganisations extend ActiveSupport::Concern included do def organisations_ordered_by_emphasis organisations.sort_by { |organisation| emphasised?(organisation) ? -1 : 1 } end - - def organisations - (content_store_hash.dig("links", "organisations") || []).map do |organisation| - { "title" => organisation["title"], "base_path" => organisation["base_path"], "content_id" => organisation["content_id"] } - end - end end private def emphasised?(organisation) - organisation["content_id"].in?(emphasised_organisations) + organisation.content_id.in?(emphasised_organisations) end def emphasised_organisations diff --git a/spec/support/concerns/organisations.rb b/spec/support/concerns/emphasised_organisations.rb similarity index 68% rename from spec/support/concerns/organisations.rb rename to spec/support/concerns/emphasised_organisations.rb index 1a70a0be20..8e029404e1 100644 --- a/spec/support/concerns/organisations.rb +++ b/spec/support/concerns/emphasised_organisations.rb @@ -2,7 +2,11 @@ let(:content_store_response) { GovukSchemas::Example.find(schema, example_name: schema) } it "knows it has emphasised organisations" do - expect(described_class.new(content_store_response).contributors.first["content_id"]).to eq(content_store_response["details"]["emphasised_organisations"].first) + first_organisation = content_store_response["links"]["organisations"].find do |link| + link["content_id"] == content_store_response["details"]["emphasised_organisations"].first + end + + expect(described_class.new(content_store_response).organisations_ordered_by_emphasis.first.title).to eq(first_organisation["title"]) end end From d60bd8ea6367a2edf4a7e90f37fea6441d96586f Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Wed, 19 Feb 2025 14:59:33 +0000 Subject: [PATCH 8/9] Remove EmphasisedOrganisations concern from root Content Item and include in CaseStudy explicitly Co-authored-by: Deborah Chua Co-authored-by: Leena Gupte Co-authored-by: Keith Lawrence Co-authored-by: Ramya Vidapanakal --- app/models/case_study.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/case_study.rb b/app/models/case_study.rb index c5c22a6ead..577c41a6eb 100644 --- a/app/models/case_study.rb +++ b/app/models/case_study.rb @@ -1,7 +1,7 @@ class CaseStudy < ContentItem + include EmphasisedOrganisations include Updatable include WorldwideOrganisations - include Organisations def contributors contributors_list = (organisations_ordered_by_emphasis + worldwide_organisations).uniq From 4bc7fa88d6d212a3e505f35e2893f56237a41ccb Mon Sep 17 00:00:00 2001 From: Leena Gupte Date: Thu, 20 Feb 2025 16:50:42 +0000 Subject: [PATCH 9/9] Update worldwide organisations to return a list of models We call the linked method in the content_item model which returns the title and base path Co-authored-by: Deborah Chua Co-authored-by: Leena Gupte Co-authored-by: Keith Lawrence Co-authored-by: Ramya Vidapanakal --- app/models/concerns/worldwide_organisations.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/models/concerns/worldwide_organisations.rb b/app/models/concerns/worldwide_organisations.rb index 2a744c6f9b..920f18fc7d 100644 --- a/app/models/concerns/worldwide_organisations.rb +++ b/app/models/concerns/worldwide_organisations.rb @@ -3,9 +3,7 @@ module WorldwideOrganisations included do def worldwide_organisations - (content_store_hash.dig("links", "worldwide_organisations") || []).map do |organisation| - { "title" => organisation["title"], "base_path" => organisation["base_path"], "content_id" => organisation["content_id"] } - end + linked("worldwide_organisations") end end end