From 447cf9761765631aff44e7f8d90f61b0f1b6e8d0 Mon Sep 17 00:00:00 2001 From: Laura Ghiorghisor Date: Tue, 11 Feb 2025 20:49:34 +0000 Subject: [PATCH] Extract correct metadata --- app/models/document.rb | 27 ++++- app/presenters/document_presenter.rb | 5 +- .../schemas/trademark_decisions.json | 51 ++++++--- spec/controllers/documents_controller_spec.rb | 101 ++++++++++++++++++ spec/models/attachment_collection_spec.rb | 16 +++ spec/models/attachment_spec.rb | 17 +++ spec/models/document_spec.rb | 15 ++- 7 files changed, 211 insertions(+), 21 deletions(-) diff --git a/app/models/document.rb b/app/models/document.rb index c1614c89a..bf99db0ef 100644 --- a/app/models/document.rb +++ b/app/models/document.rb @@ -83,6 +83,8 @@ def set_attributes(attrs, keys = nil) keys.each do |key| public_send(:"#{clean_key(key.to_s)}=", param_value(attrs, key)) end + + set_nested_facet_attributes(attrs) end def to_h @@ -371,7 +373,14 @@ def custom_error_message_fields end def self.format_specific_fields - finder_schema.facets.map { |facet| facet["key"].to_sym }.freeze + fields = [] + + finder_schema.facets.each do |facet| + fields << facet["key"].to_sym + fields << facet["sub_facet_key"].to_sym if facet["sub_facet_key"] + end + + fields end def self.apply_validations @@ -386,6 +395,22 @@ def self.apply_validations private + def set_nested_facet_attributes(attrs) + nested_facets = finder_schema.facets.select { |f| f.dig("specialist_publisher_properties", "nested_facet") == true } + nested_facets.each do |nested_facet| + nested_facet_values = param_value(attrs, nested_facet["key"]) + main_facet_value = [] + sub_facet_value = [] + nested_facet_values&.each do |main_and_sub_tuple| + parsed_tuple = JSON.parse(main_and_sub_tuple) + main_facet_value << parsed_tuple["main_facet_value"] + sub_facet_value << parsed_tuple["sub_facet_value"] + end + public_send(:"#{clean_key(nested_facet['key'].to_s)}=", main_facet_value.uniq) if main_facet_value.any? + public_send(:"#{clean_key(nested_facet['sub_facet_key']&.to_s)}=", sub_facet_value.uniq) if sub_facet_value.any? + end + end + def param_value(params, key) date_param_value(params, key) || params.fetch(key, nil) end diff --git a/app/presenters/document_presenter.rb b/app/presenters/document_presenter.rb index ac6cdae3b..1d7587220 100644 --- a/app/presenters/document_presenter.rb +++ b/app/presenters/document_presenter.rb @@ -63,10 +63,7 @@ def attachments end def metadata - fields = document.format_specific_fields - metadata = fields.index_with do |field| - document.public_send(field) - end + metadata = document.format_specific_metadata metadata[:bulk_published] = document.bulk_published diff --git a/lib/documents/schemas/trademark_decisions.json b/lib/documents/schemas/trademark_decisions.json index 1ab0b257a..cee5845e3 100644 --- a/lib/documents/schemas/trademark_decisions.json +++ b/lib/documents/schemas/trademark_decisions.json @@ -920,24 +920,30 @@ "type": "text" }, { + "sub_facet_key": "ipo_grounds_sub_section", + "sub_facet_name": "Grounds Sub Section", "allowed_values": [ { "label": "Section 3(1) Graphical Representation", "value": "section-3-1-graphical-representation", - "sub_facets": [{ - "label": "Is it a sign?", - "value": "section-3-1-graphical-representation-is-it-a-sign" - }, - { - "label": "Is it graphically represented?", - "value": "section-3-1-graphical-representation-is-it-graphically-represented" - },{ - "label": "Is it capable of distinguishing?", - "value": "section-3-1-graphical-representation-is-it-capable-of-distinguishing" - },{ - "label": "Not Applicable", - "value": "section-3-1-graphical-representation-not-applicable" - }] + "sub_facets": [ + { + "label": "Is it a sign?", + "value": "section-3-1-graphical-representation-is-it-a-sign" + }, + { + "label": "Is it graphically represented?", + "value": "section-3-1-graphical-representation-is-it-graphically-represented" + }, + { + "label": "Is it capable of distinguishing?", + "value": "section-3-1-graphical-representation-is-it-capable-of-distinguishing" + }, + { + "label": "Not Applicable", + "value": "section-3-1-graphical-representation-not-applicable" + } + ] }, { "label": "Section 3(1) Descriptiveness/Distinctiveness", @@ -949,7 +955,21 @@ }, { "label": "Section 3(3) Immoral and Deceptive Marks", - "value": "section-3-3-immoral-and-deceptive-marks" + "value": "section-3-3-immoral-and-deceptive-marks", + "sub_facets": [ + { + "label": "Contrary to public policy / accepted principles of morality", + "value": "section-3-3-immoral-and-deceptive-marks-contrary-to-public-policy-accepted-principles-of-morality" + }, + { + "label": "Deceptive as to nature, quality etc.", + "value": "section-3-3-immoral-and-deceptive-marks-deceptive-as-to-nature-quality-etc" + }, + { + "label": "Not Applicable", + "value": "section-3-3-immoral-and-deceptive-marks-not-applicable" + } + ] }, { "label": "Section 3(4) and Section 3(5) Other Issues", @@ -1007,6 +1027,7 @@ "preposition": "with", "specialist_publisher_properties": { "select": "multiple", + "nested_facet": true, "validations": { "required": {} } diff --git a/spec/controllers/documents_controller_spec.rb b/spec/controllers/documents_controller_spec.rb index 042d86c5d..081e58ac2 100644 --- a/spec/controllers/documents_controller_spec.rb +++ b/spec/controllers/documents_controller_spec.rb @@ -41,4 +41,105 @@ expect(subject).to redirect_to(documents_path(document_type_slug: "cma-cases")) end end + + describe "POST create" do + let(:stub_content_id) { "test-content-id" } + + before do + stub_any_publishing_api_put_content + stub_any_publishing_api_patch_links + expect(SecureRandom).to receive(:uuid).and_return(stub_content_id) + end + + it "should create a document and send draft to Publishing API" do + params = { + "authenticity_token" => "[FILTERED]", + "data_ethics_guidance_document" => { + "title" => "test", + "summary" => "test", + "body" => "test", + "locale" => "en", + "data_ethics_guidance_document_ethical_theme" => [ + "", + "fairness", + "societal-wellbeing", + ], + "data_ethics_guidance_document_organisation_alias" => [""], + "data_ethics_guidance_document_project_phase" => [""], + "data_ethics_guidance_document_technology_area" => [""], + }, + "save" => "", + "document_type_slug" => "data-ethics-guidance-documents", + } + + expected_sent_payload = request_json_includes( + "details" => { + "body" => [{ "content_type" => "text/govspeak", "content" => "test" }], + "metadata" => { + "data_ethics_guidance_document_ethical_theme" => %w[fairness societal-wellbeing], + }, + "max_cache_time" => 10, + "temporary_update_type" => false, + }, + ) + + post :create, params: params + + assert_publishing_api_put_content(stub_content_id, expected_sent_payload) + expect(subject).to redirect_to(document_path(DataEthicsGuidanceDocument.admin_slug, "#{stub_content_id}:en")) + end + + it "should handle nested facets when sending draft to Publishing API" do + params = { + "authenticity_token" => "[FILTERED]", + "trademark_decision" => { + "title" => "Test", + "summary" => "test", + "body" => "asdfasdf", + "locale" => "en", + "ipo_class" => "1", + "ipo_decision_date(1i)" => "2020", + "ipo_decision_date(2i)" => "1", + "ipo_decision_date(3i)" => "1", + "ipo_appointed_person_hearing_officer" => "mr-n-abraham", + "ipo_person_or_company_involved" => "", + "ipo_grounds_section" => [ + "", + "{\"main_facet_value\":\"section-3-1-graphical-representation\",\"sub_facet_value\":\"section-3-1-graphical-representation-is-it-graphically-represented\"}", + "{\"main_facet_value\":\"section-3-1-graphical-representation\",\"sub_facet_value\":\"section-3-1-graphical-representation-is-it-capable-of-distinguishing\"}", + "{\"main_facet_value\":\"section-3-3-immoral-and-deceptive-marks\",\"sub_facet_value\":\"section-3-3-immoral-and-deceptive-marks-deceptive-as-to-nature-quality-etc\"}", + ], + }, + "save" => "", + "document_type_slug" => "trademark-decisions", + } + + expected_sent_payload = request_json_includes( + "details" => { + "body" => [{ "content_type" => "text/govspeak", "content" => "asdfasdf" }], + "metadata" => { + "ipo_class" => "1", + "ipo_decision_date" => "2020-01-01", + "ipo_appointed_person_hearing_officer" => "mr-n-abraham", + "ipo_grounds_section" => [ + "section-3-1-graphical-representation", + "section-3-3-immoral-and-deceptive-marks", + ], + "ipo_grounds_sub_section" => [ + "section-3-1-graphical-representation-is-it-graphically-represented", + "section-3-1-graphical-representation-is-it-capable-of-distinguishing", + "section-3-3-immoral-and-deceptive-marks-deceptive-as-to-nature-quality-etc", + ], + }, + "max_cache_time" => 10, + "temporary_update_type" => false, + }, + ) + + post :create, params: params + + assert_publishing_api_put_content(stub_content_id, expected_sent_payload) + expect(subject).to redirect_to(document_path(TrademarkDecision.admin_slug, "#{stub_content_id}:en")) + end + end end diff --git a/spec/models/attachment_collection_spec.rb b/spec/models/attachment_collection_spec.rb index 838cdde52..e1f5ea758 100644 --- a/spec/models/attachment_collection_spec.rb +++ b/spec/models/attachment_collection_spec.rb @@ -5,6 +5,22 @@ let(:attachment_gif) { Attachment.new(title: "great gif") } let(:missing_attachment) { Attachment.new(title: "cool csv") } let(:attachments) { described_class.new([attachment_jpeg, attachment_gif]) } + let(:finder_schema) do + schema = FinderSchema.new + schema.assign_attributes({ + base_path: "/my-document-types", + target_stack: "live", + filter: { + "format" => "my_format", + }, + content_id: @finder_content_id, + }) + schema + end + + before do + allow(FinderSchema).to receive(:load_from_schema).and_return(finder_schema) + end describe "#find" do it "returns the correct attachment" do diff --git a/spec/models/attachment_spec.rb b/spec/models/attachment_spec.rb index c6b95364b..99ec6721a 100644 --- a/spec/models/attachment_spec.rb +++ b/spec/models/attachment_spec.rb @@ -3,6 +3,23 @@ RSpec.describe Attachment do include GdsApi::TestHelpers::AssetManager + let(:finder_schema) do + schema = FinderSchema.new + schema.assign_attributes({ + base_path: "/my-document-types", + target_stack: "live", + filter: { + "format" => "my_format", + }, + content_id: @finder_content_id, + }) + schema + end + + before do + allow(FinderSchema).to receive(:load_from_schema).and_return(finder_schema) + end + describe ".all_from_publishing_api" do context "when the payload has attachments in details" do let(:attachment_payload) { FactoryBot.create(:attachment_payload) } diff --git a/spec/models/document_spec.rb b/spec/models/document_spec.rb index 7aba7d7f9..c366948ff 100644 --- a/spec/models/document_spec.rb +++ b/spec/models/document_spec.rb @@ -526,6 +526,10 @@ def initialize(params = {}) describe "validations" do subject { Document.from_publishing_api(payload) } + before do + allow(FinderSchema).to receive(:load_from_schema).and_return(finder_schema) + end + context "when the document is a draft" do let(:payload) { FactoryBot.create(:document) } @@ -626,6 +630,8 @@ def initialize(params = {}) describe "when called on a class that mismatches the document_type" do it "raises a helpful error" do + allow(FinderSchema).to receive(:load_from_schema).and_return(finder_schema) + expect { CmaCase.find(document.content_id, document.locale) }.to raise_error(/wrong type/) @@ -636,6 +642,10 @@ def initialize(params = {}) describe "attachment methods" do let(:attachment) { Attachment.new } + before do + allow(FinderSchema).to receive(:load_from_schema).and_return(finder_schema) + end + describe "#attachments=" do it "creates an AttachmentCollection with the given attachments" do subject.attachments = [attachment] @@ -691,7 +701,10 @@ def initialize(params = {}) end describe "#set_temporary_update_type!" do - before { subject.publication_state = "published" } + before do + allow(FinderSchema).to receive(:load_from_schema).and_return(finder_schema) + subject.publication_state = "published" + end context "when the document has an update_type" do before do