From c46d68bfb20710b8dab730811cdbe69e75be60c2 Mon Sep 17 00:00:00 2001 From: Harriet H-W Date: Tue, 19 Nov 2024 12:05:51 +0000 Subject: [PATCH 1/9] add service to get html for host document preview calls the gov.uk frontend to get the raw HTML of the page in the future we could find the embedded content block inside that HTML and transform it. --- .../get_preview_content.rb | 40 +++++++++++++++++++ .../app/services/get_preview_content_test.rb | 32 +++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb create mode 100644 lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb diff --git a/lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb b/lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb new file mode 100644 index 00000000000..a513ad58cd4 --- /dev/null +++ b/lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb @@ -0,0 +1,40 @@ +require "net/http" +require "json" +require "uri" + +module ContentBlockManager + class GetPreviewContent + def initialize(content_id:) + @content_id = content_id + end + + def preview_content + { + title: content_item["title"], + html:, + } + end + + private + + def content_item + @content_item ||= begin + response = Services.publishing_api.get_content(@content_id) + response.parsed_content + end + end + + def frontend_base_path + Rails.env.development? ? Plek.external_url_for("government-frontend") : Plek.website_root + end + + def frontend_path + frontend_base_path + content_item["base_path"] + end + + def html + uri = URI(frontend_path) + @html ||= Nokogiri::HTML.parse(Net::HTTP.get(uri)) + end + end +end diff --git a/lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb b/lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb new file mode 100644 index 00000000000..5188f6a72e9 --- /dev/null +++ b/lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb @@ -0,0 +1,32 @@ +require "test_helper" + +class ContentBlockManager::GetPreviewContentTest < ActiveSupport::TestCase + extend Minitest::Spec::DSL + + let(:described_class) { ContentBlockManager::GetPreviewContent } + let(:host_content_id) { "64570503-7a7f-4fca-80c1-e6dce7278419" } + let(:host_title) { "Test" } + let(:host_base_path) { "/test" } + let(:uri_mock) { mock } + let(:fake_frontend_response) do + "

test

" + end + + describe "#preview_content" do + setup do + stub_publishing_api_has_item(content_id: host_content_id, title: host_title, base_path: host_base_path) + end + + it "returns the title and raw frontend HTML for a document" do + Net::HTTP.expects(:get).with(URI(Plek.website_root + host_base_path)).returns(fake_frontend_response) + Nokogiri::HTML.expects(:parse).with(fake_frontend_response).returns(fake_frontend_response) + + expected_content = { + title: host_title, + html: fake_frontend_response, + } + + assert_equal expected_content, ContentBlockManager::GetPreviewContent.new(content_id: host_content_id).preview_content + end + end +end From 2d5ccecd8eca1b8eda00ea01b45c6fc8b48c71b6 Mon Sep 17 00:00:00 2001 From: Harriet H-W Date: Tue, 19 Nov 2024 12:06:34 +0000 Subject: [PATCH 2/9] return content id for host sp that we can use this id to get the preview HTML --- .../app/models/content_block_manager/host_content_item.rb | 1 + .../services/content_block_manager/get_host_content_items.rb | 1 + .../content_block_manager/test/factories/host_content_item.rb | 2 ++ .../test/unit/app/services/get_host_content_items_test.rb | 4 ++++ 4 files changed, 8 insertions(+) diff --git a/lib/engines/content_block_manager/app/models/content_block_manager/host_content_item.rb b/lib/engines/content_block_manager/app/models/content_block_manager/host_content_item.rb index c9b71b08430..41b11c94bf1 100644 --- a/lib/engines/content_block_manager/app/models/content_block_manager/host_content_item.rb +++ b/lib/engines/content_block_manager/app/models/content_block_manager/host_content_item.rb @@ -9,6 +9,7 @@ class HostContentItem < Data.define( :last_edited_at, :unique_pageviews, :instances, + :host_content_id, ) def last_edited_at diff --git a/lib/engines/content_block_manager/app/services/content_block_manager/get_host_content_items.rb b/lib/engines/content_block_manager/app/services/content_block_manager/get_host_content_items.rb index d06af4456b3..554f2bc9547 100644 --- a/lib/engines/content_block_manager/app/services/content_block_manager/get_host_content_items.rb +++ b/lib/engines/content_block_manager/app/services/content_block_manager/get_host_content_items.rb @@ -30,6 +30,7 @@ def items last_edited_at: item["last_edited_at"], unique_pageviews: item["unique_pageviews"], instances: item["instances"], + host_content_id: item["host_content_id"], ) end diff --git a/lib/engines/content_block_manager/test/factories/host_content_item.rb b/lib/engines/content_block_manager/test/factories/host_content_item.rb index 03fc23659fe..267bd5e8482 100644 --- a/lib/engines/content_block_manager/test/factories/host_content_item.rb +++ b/lib/engines/content_block_manager/test/factories/host_content_item.rb @@ -9,6 +9,7 @@ last_edited_at { 2.days.ago.to_s } unique_pageviews { 123 } instances { 1 } + host_content_id { SecureRandom.uuid } initialize_with do new(title:, @@ -19,6 +20,7 @@ last_edited_by_editor_id:, last_edited_at:, unique_pageviews:, + host_content_id:, instances:) end end diff --git a/lib/engines/content_block_manager/test/unit/app/services/get_host_content_items_test.rb b/lib/engines/content_block_manager/test/unit/app/services/get_host_content_items_test.rb index 5946d0a2f1c..5d00aa1f4a8 100644 --- a/lib/engines/content_block_manager/test/unit/app/services/get_host_content_items_test.rb +++ b/lib/engines/content_block_manager/test/unit/app/services/get_host_content_items_test.rb @@ -7,6 +7,8 @@ class ContentBlockManager::GetHostContentItemsTest < ActiveSupport::TestCase let(:target_content_id) { SecureRandom.uuid } + let(:host_content_id) { SecureRandom.uuid } + let(:response_body) do { "content_id" => SecureRandom.uuid, @@ -22,6 +24,7 @@ class ContentBlockManager::GetHostContentItemsTest < ActiveSupport::TestCase "last_edited_at" => "2023-01-01T08:00:00.000Z", "unique_pageviews" => 123, "instances" => 1, + "host_content_id" => host_content_id, "primary_publishing_organisation" => { "content_id" => SecureRandom.uuid, "title" => "bar", @@ -110,6 +113,7 @@ class ContentBlockManager::GetHostContentItemsTest < ActiveSupport::TestCase assert_equal result[0].last_edited_at, Time.zone.parse(response_body["results"][0]["last_edited_at"]) assert_equal result[0].unique_pageviews, response_body["results"][0]["unique_pageviews"] assert_equal result[0].instances, response_body["results"][0]["instances"] + assert_equal result[0].host_content_id, response_body["results"][0]["host_content_id"] assert_equal result[0].publishing_organisation, expected_publishing_organisation end From e00c4e843e3dce4248ba1249c6c77509b9ed6ad2 Mon Sep 17 00:00:00 2001 From: Harriet H-W Date: Tue, 19 Nov 2024 12:06:51 +0000 Subject: [PATCH 3/9] add preview route for host documents --- lib/engines/content_block_manager/config/routes.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/engines/content_block_manager/config/routes.rb b/lib/engines/content_block_manager/config/routes.rb index 89bd93958ba..3fcfbc080f8 100644 --- a/lib/engines/content_block_manager/config/routes.rb +++ b/lib/engines/content_block_manager/config/routes.rb @@ -12,6 +12,9 @@ resources :editions, only: %i[new create destroy], path_names: { new: ":block_type/new" } do member do resources :workflow, only: %i[show update], controller: "editions/workflow", param: :step + resources :host_content, only: %i[preview], controller: "editions/host_content", param: :id do + get :preview, to: "editions/host_content#preview" + end end end end From 6e1014572a005bda9917c1278eb1a2bfcbfb1615 Mon Sep 17 00:00:00 2001 From: Harriet H-W Date: Tue, 19 Nov 2024 12:07:12 +0000 Subject: [PATCH 4/9] and preview view --- .../editions/host_content/preview.html.erb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 lib/engines/content_block_manager/app/views/content_block_manager/content_block/editions/host_content/preview.html.erb diff --git a/lib/engines/content_block_manager/app/views/content_block_manager/content_block/editions/host_content/preview.html.erb b/lib/engines/content_block_manager/app/views/content_block_manager/content_block/editions/host_content/preview.html.erb new file mode 100644 index 00000000000..57639866e5b --- /dev/null +++ b/lib/engines/content_block_manager/app/views/content_block_manager/content_block/editions/host_content/preview.html.erb @@ -0,0 +1,16 @@ +<% content_for :page_title, "Preview content block in host document" %> +<% content_for :context, "Preview content block" %> +<% content_for :title, @preview_content[:title] %> +<% content_for :title_margin_bottom, 0 %> + +
+
+

Document title: <%= @preview_content[:title] %>

+
+
+
+
+
+ +
+
From 4f504e4ef631e35417917c427cc67448dcdf252a Mon Sep 17 00:00:00 2001 From: Harriet H-W Date: Tue, 19 Nov 2024 12:08:10 +0000 Subject: [PATCH 5/9] show preview of host document during edit journey --- .../show/host_editions_table_component.rb | 7 ++-- .../editions/host_content_controller.rb | 6 +++ .../content_block/documents/show.html.erb | 1 + .../editions/workflow/review_links.html.erb | 1 + .../features/edit_object.feature | 4 +- .../content_block_manager_steps.rb | 37 ++++++++++++++++++- .../host_editions_table_component_test.rb | 28 ++++++++++++-- 7 files changed, 74 insertions(+), 10 deletions(-) create mode 100644 lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/host_content_controller.rb diff --git a/lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/host_editions_table_component.rb b/lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/host_editions_table_component.rb index ed3e9a51174..5f56dbe7dc8 100644 --- a/lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/host_editions_table_component.rb +++ b/lib/engines/content_block_manager/app/components/content_block_manager/content_block/document/show/host_editions_table_component.rb @@ -3,12 +3,13 @@ class ContentBlockManager::ContentBlock::Document::Show::HostEditionsTableComponent < ViewComponent::Base TABLE_ID = "host_editions" - def initialize(caption:, host_content_items:, is_preview: false, current_page: nil, order: nil) + def initialize(caption:, host_content_items:, content_block_edition:, is_preview: false, current_page: nil, order: nil) @caption = caption @host_content_items = host_content_items @is_preview = is_preview @current_page = current_page.presence || 1 @order = order.presence || ContentBlockManager::GetHostContentItems::DEFAULT_ORDER + @content_block_edition = content_block_edition end def current_page @@ -25,7 +26,7 @@ def base_pagination_path private - attr_reader :caption, :host_content_items, :order + attr_reader :caption, :host_content_items, :order, :content_block_edition def rows return [] unless host_content_items @@ -81,7 +82,7 @@ def users def frontend_path(content_item) if @is_preview - Plek.external_url_for("draft-origin") + content_item.base_path + helpers.content_block_manager.content_block_manager_content_block_host_content_preview_path(id: content_block_edition.id, host_content_id: content_item.host_content_id) else Plek.website_root + content_item.base_path end diff --git a/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/host_content_controller.rb b/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/host_content_controller.rb new file mode 100644 index 00000000000..d1f0cd85ad8 --- /dev/null +++ b/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/host_content_controller.rb @@ -0,0 +1,6 @@ +class ContentBlockManager::ContentBlock::Editions::HostContentController < ContentBlockManager::BaseController + def preview + host_content_id = params[:host_content_id] + @preview_content = ContentBlockManager::GetPreviewContent.new(content_id: host_content_id).preview_content + end +end diff --git a/lib/engines/content_block_manager/app/views/content_block_manager/content_block/documents/show.html.erb b/lib/engines/content_block_manager/app/views/content_block_manager/content_block/documents/show.html.erb index eb3879fadcc..30b83d3ab34 100644 --- a/lib/engines/content_block_manager/app/views/content_block_manager/content_block/documents/show.html.erb +++ b/lib/engines/content_block_manager/app/views/content_block_manager/content_block/documents/show.html.erb @@ -25,6 +25,7 @@ host_content_items: @host_content_items, current_page: @page, order: @order, + content_block_edition: @content_block_document.latest_edition, ), ) %> diff --git a/lib/engines/content_block_manager/app/views/content_block_manager/content_block/editions/workflow/review_links.html.erb b/lib/engines/content_block_manager/app/views/content_block_manager/content_block/editions/workflow/review_links.html.erb index 86cb877ffa9..10abdeb21f4 100644 --- a/lib/engines/content_block_manager/app/views/content_block_manager/content_block/editions/workflow/review_links.html.erb +++ b/lib/engines/content_block_manager/app/views/content_block_manager/content_block/editions/workflow/review_links.html.erb @@ -18,6 +18,7 @@ host_content_items: @host_content_items, current_page: @page, order: @order, + content_block_edition: @content_block_edition, ), ) %> diff --git a/lib/engines/content_block_manager/features/edit_object.feature b/lib/engines/content_block_manager/features/edit_object.feature index 1e1e0e7a051..2efbe7b215c 100644 --- a/lib/engines/content_block_manager/features/edit_object.feature +++ b/lib/engines/content_block_manager/features/edit_object.feature @@ -96,8 +96,10 @@ Feature: Edit a content object And I visit the Content Block Manager home page Then I should still see the live edition on the homepage + @javascript Scenario: GDS editor can preview a host document When I revisit the edit page And I fill out the form Then I am shown where the changes will take place - And the host documents link to the draft content store + When I click on the first host document + Then The preview page opens in a new tab diff --git a/lib/engines/content_block_manager/features/step_definitions/content_block_manager_steps.rb b/lib/engines/content_block_manager/features/step_definitions/content_block_manager_steps.rb index e8062f95129..ead616abd3b 100644 --- a/lib/engines/content_block_manager/features/step_definitions/content_block_manager_steps.rb +++ b/lib/engines/content_block_manager/features/step_definitions/content_block_manager_steps.rb @@ -437,7 +437,7 @@ def should_show_edit_form_for_email_address_content_block(document_title, email_ { "title" => "Content #{i}", "document_type" => "document", - "base_path" => "/", + "base_path" => "/host-content-path-#{i}", "content_id" => SecureRandom.uuid, "last_edited_by_editor_id" => SecureRandom.uuid, "last_edited_at" => 2.days.ago.to_s, @@ -486,7 +486,40 @@ def should_show_edit_form_for_email_address_content_block(document_title, email_ end When("I click on the first host document") do - click_on @dependent_content.first["title"] + @current_host_document = @dependent_content.first + stub_request( + :get, + "#{Plek.find('publishing-api')}/v2/content/#{@current_host_document['host_content_id']}", + ).to_return( + status: 200, + body: { + details: { + body: "

title

", + }, + title: @current_host_document["title"], + document_type: "news_story", + base_path: @current_host_document["base_path"], + publishing_app: "test", + }.to_json, + ) + + stub_request( + :get, + Plek.website_root + @current_host_document["base_path"], + ).to_return( + status: 200, + body: "

#{@current_host_document['title']}

iframe preview

", + ) + + click_on @current_host_document["title"] +end + +Then("The preview page opens in a new tab") do + page.switch_to_window(page.windows.last) + assert_text "Preview content block" + within_frame "preview" do + assert_text @current_host_document["title"] + end end When(/^I save and continue$/) do diff --git a/lib/engines/content_block_manager/test/components/content_block/document/show/host_editions_table_component_test.rb b/lib/engines/content_block_manager/test/components/content_block/document/show/host_editions_table_component_test.rb index 8aee3d11199..82884197dd8 100644 --- a/lib/engines/content_block_manager/test/components/content_block/document/show/host_editions_table_component_test.rb +++ b/lib/engines/content_block_manager/test/components/content_block/document/show/host_editions_table_component_test.rb @@ -2,7 +2,7 @@ class ContentBlockManager::ContentBlock::Document::Show::HostEditionsTableComponentTest < ViewComponent::TestCase extend Minitest::Spec::DSL - include Rails.application.routes.url_helpers + include ContentBlockManager::Engine.routes.url_helpers include ActionView::Helpers::DateHelper let(:described_class) { ContentBlockManager::ContentBlock::Document::Show::HostEditionsTableComponent } @@ -28,6 +28,7 @@ class ContentBlockManager::ContentBlock::Document::Show::HostEditionsTableCompon "last_edited_at" => Time.zone.now.to_s, "publishing_organisation" => publishing_organisation, "unique_pageviews" => unique_pageviews, + "host_content_id" => SecureRandom.uuid, "instances" => 1, ) end @@ -40,12 +41,17 @@ class ContentBlockManager::ContentBlock::Document::Show::HostEditionsTableCompon ) end + let(:content_block_edition) do + build(:content_block_edition, :email_address, id: SecureRandom.uuid) + end + def self.it_returns_unknown_user it "returns Unknown user" do render_inline( described_class.new( caption:, host_content_items:, + content_block_edition:, ), ) @@ -54,7 +60,7 @@ def self.it_returns_unknown_user end around do |test| - with_request_url content_block_manager_path do + with_request_url content_block_manager_root_path do test.call end end @@ -65,6 +71,7 @@ def self.it_returns_unknown_user described_class.new( caption:, host_content_items:, + content_block_edition:, ), ) @@ -91,6 +98,7 @@ def self.it_returns_unknown_user described_class.new( caption:, host_content_items:, + content_block_edition:, ), ) assert_no_selector "tbody .govuk-table__cell a", text: host_content_item.publishing_organisation["title"] @@ -101,12 +109,13 @@ def self.it_returns_unknown_user it "links to the organisation instead of printing the name" do organisation = create(:organisation, content_id: host_content_item.publishing_organisation["content_id"], name: host_content_item.publishing_organisation["title"]) - expected_href = admin_organisation_path(organisation) + expected_href = Rails.application.routes.url_helpers.admin_organisation_path(organisation) render_inline( described_class.new( caption:, host_content_items:, + content_block_edition:, ), ) assert_selector "tbody .govuk-table__cell a", @@ -130,6 +139,7 @@ def self.it_returns_unknown_user described_class.new( caption:, host_content_items:, + content_block_edition:, ), ) @@ -165,6 +175,7 @@ def self.it_returns_unknown_user described_class.new( caption:, host_content_items:, + content_block_edition:, ), ) @@ -179,10 +190,11 @@ def self.it_returns_unknown_user is_preview: true, caption:, host_content_items:, + content_block_edition:, ), ) - assert_selector "a[href='#{Plek.external_url_for('draft-origin') + host_content_item.base_path}']", text: host_content_item.title + assert_selector "a[href='#{content_block_manager_content_block_host_content_preview_path(id: content_block_edition.id, host_content_id: host_content_item.host_content_id)}']", text: host_content_item.title end end @@ -192,6 +204,7 @@ def self.it_returns_unknown_user described_class.new( caption:, host_content_items:, + content_block_edition:, ), ) @@ -203,6 +216,7 @@ def self.it_returns_unknown_user described_class.new( caption:, host_content_items:, + content_block_edition:, ), ) @@ -223,6 +237,7 @@ def self.it_returns_unknown_user caption:, host_content_items:, order:, + content_block_edition:, ), ) @@ -235,6 +250,7 @@ def self.it_returns_unknown_user caption:, host_content_items:, order: "-#{order}", + content_block_edition:, ), ) @@ -259,6 +275,7 @@ def self.it_returns_unknown_user described_class.new( caption:, host_content_items:, + content_block_edition:, ), ) @@ -281,6 +298,7 @@ def self.it_returns_unknown_user described_class.new( caption:, host_content_items:, + content_block_edition:, ), ) @@ -293,6 +311,7 @@ def self.it_returns_unknown_user described_class.new( caption:, host_content_items:, + content_block_edition:, ), ) @@ -306,6 +325,7 @@ def self.it_returns_unknown_user caption:, host_content_items:, current_page: 2, + content_block_edition:, ), ) From ec94304a2a42032b3d8417159a97c17a5f32a91c Mon Sep 17 00:00:00 2001 From: Harriet H-W Date: Fri, 22 Nov 2024 15:50:52 +0000 Subject: [PATCH 6/9] replace existing content block with one under preview here we iterate through the snapshot of HTML from the frontend, and find the `` containing the existing edition of the Content Block we are previewing. We then replace it with the `render` output of the content block we are currently previewing. --- .../editions/host_content_controller.rb | 3 +- .../get_preview_content.rb | 23 ++++++++++++-- .../app/services/get_preview_content_test.rb | 31 +++++++++++++++---- 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/host_content_controller.rb b/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/host_content_controller.rb index d1f0cd85ad8..bf628a8b20d 100644 --- a/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/host_content_controller.rb +++ b/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/host_content_controller.rb @@ -1,6 +1,7 @@ class ContentBlockManager::ContentBlock::Editions::HostContentController < ContentBlockManager::BaseController def preview host_content_id = params[:host_content_id] - @preview_content = ContentBlockManager::GetPreviewContent.new(content_id: host_content_id).preview_content + content_block_edition = ContentBlockManager::ContentBlock::Edition.find(params[:id]) + @preview_content = ContentBlockManager::GetPreviewContent.new(content_id: host_content_id, content_block_edition:).preview_content end end diff --git a/lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb b/lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb index a513ad58cd4..572b70486ca 100644 --- a/lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb +++ b/lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb @@ -4,8 +4,9 @@ module ContentBlockManager class GetPreviewContent - def initialize(content_id:) + def initialize(content_id:, content_block_edition:) @content_id = content_id + @content_block_edition = content_block_edition end def preview_content @@ -17,6 +18,10 @@ def preview_content private + def html + @html ||= preview_html + end + def content_item @content_item ||= begin response = Services.publishing_api.get_content(@content_id) @@ -32,9 +37,21 @@ def frontend_path frontend_base_path + content_item["base_path"] end - def html + def preview_html uri = URI(frontend_path) - @html ||= Nokogiri::HTML.parse(Net::HTTP.get(uri)) + nokogiri_html = Nokogiri::HTML.parse(Net::HTTP.get(uri)) + replace_existing_content_blocks(nokogiri_html) + end + + def replace_existing_content_blocks(nokogiri_html) + existing_content_block_spans(nokogiri_html).each do |span| + span.replace @content_block_edition.render + end + nokogiri_html + end + + def existing_content_block_spans(nokogiri_html) + nokogiri_html.css("span[data-content-id=\"#{@content_block_edition.document.content_id}\"]") end end end diff --git a/lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb b/lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb index 5188f6a72e9..9dc661b0c30 100644 --- a/lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb +++ b/lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb @@ -4,12 +4,25 @@ class ContentBlockManager::GetPreviewContentTest < ActiveSupport::TestCase extend Minitest::Spec::DSL let(:described_class) { ContentBlockManager::GetPreviewContent } - let(:host_content_id) { "64570503-7a7f-4fca-80c1-e6dce7278419" } + let(:host_content_id) { SecureRandom.uuid } + let(:preview_content_id) { SecureRandom.uuid } let(:host_title) { "Test" } let(:host_base_path) { "/test" } let(:uri_mock) { mock } let(:fake_frontend_response) do - "

test

" + "

test

example@example.com" + end + let(:block_render) do + "new@new.com" + end + let(:expected_html) do + "

test

#{block_render}" + end + let(:document) do + build(:content_block_document, :email_address, content_id: preview_content_id) + end + let(:block_to_preview) do + build(:content_block_edition, :email_address, document:, details: { "email_address" => "new@new.com" }) end describe "#preview_content" do @@ -17,16 +30,22 @@ class ContentBlockManager::GetPreviewContentTest < ActiveSupport::TestCase stub_publishing_api_has_item(content_id: host_content_id, title: host_title, base_path: host_base_path) end - it "returns the title and raw frontend HTML for a document" do + it "returns the title and preview HTML for a document" do Net::HTTP.expects(:get).with(URI(Plek.website_root + host_base_path)).returns(fake_frontend_response) - Nokogiri::HTML.expects(:parse).with(fake_frontend_response).returns(fake_frontend_response) + block_to_preview.expects(:render).returns(block_render) expected_content = { title: host_title, - html: fake_frontend_response, + html: Nokogiri::HTML.parse(expected_html), } - assert_equal expected_content, ContentBlockManager::GetPreviewContent.new(content_id: host_content_id).preview_content + actual_content = ContentBlockManager::GetPreviewContent.new( + content_id: host_content_id, + content_block_edition: block_to_preview, + ).preview_content + + assert_equal expected_content[:title], actual_content[:title] + assert_equal expected_content[:html].to_s, actual_content[:html].to_s end end end From 5696a4fa66bbf777621d35c67ca944c4e71e7e25 Mon Sep 17 00:00:00 2001 From: Harriet H-W Date: Fri, 22 Nov 2024 17:23:00 +0000 Subject: [PATCH 7/9] provisionally style preview of content blocks a minimal styling to help us see the changed blocks in the build - TBC on actual UI design. --- .../get_preview_content.rb | 19 ++++++++++++++++--- .../app/services/get_preview_content_test.rb | 5 ++++- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb b/lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb index 572b70486ca..f43dcd83982 100644 --- a/lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb +++ b/lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb @@ -44,13 +44,26 @@ def preview_html end def replace_existing_content_blocks(nokogiri_html) - existing_content_block_spans(nokogiri_html).each do |span| + replace_blocks(nokogiri_html) + style_blocks(nokogiri_html) + nokogiri_html + end + + def replace_blocks(nokogiri_html) + content_block_spans(nokogiri_html).each do |span| span.replace @content_block_edition.render end - nokogiri_html end - def existing_content_block_spans(nokogiri_html) + BLOCK_STYLE = "background-color: yellow;".freeze + + def style_blocks(nokogiri_html) + content_block_spans(nokogiri_html).each do |span| + span["style"] = "background-color: yellow;" + end + end + + def content_block_spans(nokogiri_html) nokogiri_html.css("span[data-content-id=\"#{@content_block_edition.document.content_id}\"]") end end diff --git a/lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb b/lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb index 9dc661b0c30..56cde5d78bd 100644 --- a/lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb +++ b/lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb @@ -15,8 +15,11 @@ class ContentBlockManager::GetPreviewContentTest < ActiveSupport::TestCase let(:block_render) do "new@new.com" end + let(:block_render_with_style) do + "new@new.com" + end let(:expected_html) do - "

test

#{block_render}" + "

test

#{block_render_with_style}" end let(:document) do build(:content_block_document, :email_address, content_id: preview_content_id) From 19a095306b9911de02c3d2743b6945575465b1da Mon Sep 17 00:00:00 2001 From: Harriet H-W Date: Mon, 25 Nov 2024 14:37:53 +0000 Subject: [PATCH 8/9] refactor PreviewContent to have a model class this keeps the API querying and transformation in a service, and creates a data class for the model. --- .../editions/host_content_controller.rb | 2 +- .../content_block_manager/preview_content.rb | 4 ++++ .../get_preview_content.rb | 22 ++++++++++--------- .../editions/host_content/preview.html.erb | 6 ++--- .../test/factories/preview_content.rb | 10 +++++++++ .../unit/app/models/preview_content_test.rb | 14 ++++++++++++ .../app/services/get_preview_content_test.rb | 8 +++---- 7 files changed, 48 insertions(+), 18 deletions(-) create mode 100644 lib/engines/content_block_manager/app/models/content_block_manager/preview_content.rb create mode 100644 lib/engines/content_block_manager/test/factories/preview_content.rb create mode 100644 lib/engines/content_block_manager/test/unit/app/models/preview_content_test.rb diff --git a/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/host_content_controller.rb b/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/host_content_controller.rb index bf628a8b20d..9d8ca3f5dff 100644 --- a/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/host_content_controller.rb +++ b/lib/engines/content_block_manager/app/controllers/content_block_manager/content_block/editions/host_content_controller.rb @@ -2,6 +2,6 @@ class ContentBlockManager::ContentBlock::Editions::HostContentController < Conte def preview host_content_id = params[:host_content_id] content_block_edition = ContentBlockManager::ContentBlock::Edition.find(params[:id]) - @preview_content = ContentBlockManager::GetPreviewContent.new(content_id: host_content_id, content_block_edition:).preview_content + @preview_content = ContentBlockManager::GetPreviewContent.for_content_id(content_id: host_content_id, content_block_edition:) end end diff --git a/lib/engines/content_block_manager/app/models/content_block_manager/preview_content.rb b/lib/engines/content_block_manager/app/models/content_block_manager/preview_content.rb new file mode 100644 index 00000000000..822e61fe9ad --- /dev/null +++ b/lib/engines/content_block_manager/app/models/content_block_manager/preview_content.rb @@ -0,0 +1,4 @@ +module ContentBlockManager + class PreviewContent < Data.define(:title, :html) + end +end diff --git a/lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb b/lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb index f43dcd83982..6877e0ba322 100644 --- a/lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb +++ b/lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb @@ -4,20 +4,21 @@ module ContentBlockManager class GetPreviewContent - def initialize(content_id:, content_block_edition:) - @content_id = content_id - @content_block_edition = content_block_edition + def self.for_content_id(content_id:, content_block_edition:) + new(content_id:, content_block_edition:).for_content_id end - def preview_content - { - title: content_item["title"], - html:, - } + def for_content_id + ContentBlockManager::PreviewContent.new(title: content_item["title"], html:) end private + def initialize(content_id:, content_block_edition:) + @content_id = content_id + @content_block_edition = content_block_edition + end + def html @html ||= preview_html end @@ -50,8 +51,9 @@ def replace_existing_content_blocks(nokogiri_html) end def replace_blocks(nokogiri_html) + @preview_content_block_render ||= @content_block_edition.render content_block_spans(nokogiri_html).each do |span| - span.replace @content_block_edition.render + span.replace @preview_content_block_render end end @@ -59,7 +61,7 @@ def replace_blocks(nokogiri_html) def style_blocks(nokogiri_html) content_block_spans(nokogiri_html).each do |span| - span["style"] = "background-color: yellow;" + span["style"] = BLOCK_STYLE end end diff --git a/lib/engines/content_block_manager/app/views/content_block_manager/content_block/editions/host_content/preview.html.erb b/lib/engines/content_block_manager/app/views/content_block_manager/content_block/editions/host_content/preview.html.erb index 57639866e5b..5126c7bb2ae 100644 --- a/lib/engines/content_block_manager/app/views/content_block_manager/content_block/editions/host_content/preview.html.erb +++ b/lib/engines/content_block_manager/app/views/content_block_manager/content_block/editions/host_content/preview.html.erb @@ -1,16 +1,16 @@ <% content_for :page_title, "Preview content block in host document" %> <% content_for :context, "Preview content block" %> -<% content_for :title, @preview_content[:title] %> +<% content_for :title, @preview_content.title %> <% content_for :title_margin_bottom, 0 %>
-

Document title: <%= @preview_content[:title] %>

+

Document title: <%= @preview_content.title %>

- +
diff --git a/lib/engines/content_block_manager/test/factories/preview_content.rb b/lib/engines/content_block_manager/test/factories/preview_content.rb new file mode 100644 index 00000000000..4288e1b535f --- /dev/null +++ b/lib/engines/content_block_manager/test/factories/preview_content.rb @@ -0,0 +1,10 @@ +FactoryBot.define do + factory :preview_content, class: "ContentBlockManager::PreviewContent" do + title { "Example Title" } + html { "

Example HTML

" } + + initialize_with do + new(title:, html:) + end + end +end diff --git a/lib/engines/content_block_manager/test/unit/app/models/preview_content_test.rb b/lib/engines/content_block_manager/test/unit/app/models/preview_content_test.rb new file mode 100644 index 00000000000..93bfb75fe59 --- /dev/null +++ b/lib/engines/content_block_manager/test/unit/app/models/preview_content_test.rb @@ -0,0 +1,14 @@ +require "test_helper" + +class ContentBlockManager::PreviewContentTest < ActiveSupport::TestCase + extend Minitest::Spec::DSL + + let(:title) { "Ministry of Example" } + let(:html) { "

Ministry of Example

" } + let(:preview_content) { build(:preview_content, title:, html:) } + + it "returns title and html" do + assert_equal preview_content.title, title + assert_equal preview_content.html, html + end +end diff --git a/lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb b/lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb index 56cde5d78bd..f8a075e85f7 100644 --- a/lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb +++ b/lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb @@ -42,13 +42,13 @@ class ContentBlockManager::GetPreviewContentTest < ActiveSupport::TestCase html: Nokogiri::HTML.parse(expected_html), } - actual_content = ContentBlockManager::GetPreviewContent.new( + actual_content = ContentBlockManager::GetPreviewContent.for_content_id( content_id: host_content_id, content_block_edition: block_to_preview, - ).preview_content + ) - assert_equal expected_content[:title], actual_content[:title] - assert_equal expected_content[:html].to_s, actual_content[:html].to_s + assert_equal expected_content[:title], actual_content.title + assert_equal expected_content[:html].to_s, actual_content.html.to_s end end end From 3f460cc81764fbd9a151bdab035e83a954d5cc4d Mon Sep 17 00:00:00 2001 From: Harriet H-W Date: Tue, 26 Nov 2024 09:35:39 +0000 Subject: [PATCH 9/9] add error handling for `Net::HTTP.get` to frontend the `Net::HTTP.get` doesn't seem to throw an error when the frontend end is down/unavailable. According to [docs it looks like it should throw](https://ruby-doc.org/stdlib-2.7.0/libdoc/net/http/rdoc/Net/HTTP.html#class-Net::HTTP-label-Response+Data ) HTTPServiceUnavailable but it doesn't seem to in practice, instead it returns the raw HTML of the "Service Temporarily Unavailable" page. In the case where the call does error, it looks like we either have a [long list of all possible errors, or have a catchall `StandardError` rescue ](https://stackoverflow.com/a/11802674) - this seemed the most practical to add. We can revisit this in future if it becomes more of a priority, perhaps looking at the raw HTML that's returned to determine if it's what we expect. --- .../get_preview_content.rb | 13 +++++++++- .../app/services/get_preview_content_test.rb | 25 +++++++++++++++++-- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb b/lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb index 6877e0ba322..cd270302725 100644 --- a/lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb +++ b/lib/engines/content_block_manager/app/services/content_block_manager/get_preview_content.rb @@ -40,7 +40,7 @@ def frontend_path def preview_html uri = URI(frontend_path) - nokogiri_html = Nokogiri::HTML.parse(Net::HTTP.get(uri)) + nokogiri_html = html_snapshot_from_frontend(uri) replace_existing_content_blocks(nokogiri_html) end @@ -68,5 +68,16 @@ def style_blocks(nokogiri_html) def content_block_spans(nokogiri_html) nokogiri_html.css("span[data-content-id=\"#{@content_block_edition.document.content_id}\"]") end + + ERROR_HTML = "

Preview not found

".freeze + + def html_snapshot_from_frontend(uri) + begin + raw_html = Net::HTTP.get(uri) + rescue StandardError + raw_html = ERROR_HTML + end + Nokogiri::HTML.parse(raw_html) + end end end diff --git a/lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb b/lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb index f8a075e85f7..c36075e7d4a 100644 --- a/lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb +++ b/lib/engines/content_block_manager/test/unit/app/services/get_preview_content_test.rb @@ -21,6 +21,9 @@ class ContentBlockManager::GetPreviewContentTest < ActiveSupport::TestCase let(:expected_html) do "

test

#{block_render_with_style}" end + let(:error_html) do + "

Preview not found

" + end let(:document) do build(:content_block_document, :email_address, content_id: preview_content_id) end @@ -28,14 +31,14 @@ class ContentBlockManager::GetPreviewContentTest < ActiveSupport::TestCase build(:content_block_edition, :email_address, document:, details: { "email_address" => "new@new.com" }) end - describe "#preview_content" do + describe "#for_content_id" do setup do stub_publishing_api_has_item(content_id: host_content_id, title: host_title, base_path: host_base_path) + block_to_preview.expects(:render).returns(block_render) end it "returns the title and preview HTML for a document" do Net::HTTP.expects(:get).with(URI(Plek.website_root + host_base_path)).returns(fake_frontend_response) - block_to_preview.expects(:render).returns(block_render) expected_content = { title: host_title, @@ -50,5 +53,23 @@ class ContentBlockManager::GetPreviewContentTest < ActiveSupport::TestCase assert_equal expected_content[:title], actual_content.title assert_equal expected_content[:html].to_s, actual_content.html.to_s end + + it "shows an error template when the request to the frontend throws an error" do + exception = StandardError.new("Something went wrong") + Net::HTTP.expects(:get).with(URI(Plek.website_root + host_base_path)).raises(exception) + + expected_content = { + title: host_title, + html: Nokogiri::HTML.parse(error_html), + } + + actual_content = ContentBlockManager::GetPreviewContent.for_content_id( + content_id: host_content_id, + content_block_edition: block_to_preview, + ) + + assert_equal expected_content[:title], actual_content.title + assert_equal expected_content[:html].to_s, actual_content.html.to_s + end end end