From 434b33986861d00fad7cf31e3c2cda0d42d0f9f6 Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Mon, 23 Sep 2024 14:10:07 +0100 Subject: [PATCH] Add special_route_publisher class and rake tasks - Replace calls to publishing-api with direct calls to the Command classes responsible for serving the relevant actions. - Add test suite for rake tests that tests the larger points (does the file load the correct number of routes and publish them, does it return error messages for missing paths, etc). - Add test suite for special_route_publisher to test finer points (default locales, does it handle links/additional routes, etc) - Replace puts with Rails.logger.info, which should show when run from the command line but also logs properly and doesn't affect tests. - Remove catch for KeyError - it's not really critical that we allow the rake tasks to run with broken data, the data should be kept valid. --- lib/tasks/special_route_publisher.rb | 107 ++++++++++++++ lib/tasks/special_routes.rake | 24 ++++ spec/fixtures/homepage.yaml | 10 ++ spec/fixtures/special_routes.yaml | 16 +++ .../lib/tasks/special_route_publisher_spec.rb | 66 +++++++++ spec/lib/tasks/special_routes_spec.rb | 135 ++++++++++++++++++ 6 files changed, 358 insertions(+) create mode 100644 lib/tasks/special_route_publisher.rb create mode 100644 lib/tasks/special_routes.rake create mode 100644 spec/fixtures/homepage.yaml create mode 100644 spec/fixtures/special_routes.yaml create mode 100644 spec/lib/tasks/special_route_publisher_spec.rb create mode 100644 spec/lib/tasks/special_routes_spec.rb diff --git a/lib/tasks/special_route_publisher.rb b/lib/tasks/special_route_publisher.rb new file mode 100644 index 0000000000..6343182107 --- /dev/null +++ b/lib/tasks/special_route_publisher.rb @@ -0,0 +1,107 @@ +module Tasks + class SpecialRoutePublisher + def self.publish_special_routes + new.publish_routes(load_special_routes) + end + + def self.publish_special_routes_for_app(app_name) + routes = load_special_routes.filter { |r| r[:rendering_app] == app_name } + + if routes.any? + new.publish_routes(routes) + else + Rails.logger.info("No routes for #{app_name} in lib/data/special_routes.yaml") + end + end + + def self.publish_one_route(base_path) + route = load_special_routes.find { |r| r[:base_path] == base_path } + + if route + new.publish_routes([route]) + else + Rails.logger.info("Route needs to be added to /lib/data/special_routes.yaml") + end + end + + def self.unpublish_one_route(base_path, alternative_path = nil) + route = load_special_routes.find { |r| r[:base_path] == base_path } + + if route && alternative_path + Commands::V2::Unpublish.call({ content_id: route.fetch(:content_id), type: "redirect", alternative_path: }) + elsif route + Commands::V2::Unpublish.call({ content_id: route.fetch(:content_id), type: "gone" }) + else + Rails.logger.info("Route needs to be added to /lib/data/special_routes.yaml") + end + end + + def self.publish_homepage + new.publish_routes(load_homepage) + end + + def publish_route(route) + routes = get_routes(route) + + routes.each { |r| Rails.logger.info("Publishing #{r[:type]} route #{r[:path]}, routing to #{route.fetch(:rendering_app)}...") } + + content_id = route.fetch(:content_id) + + # Always request a path reservation before publishing the special route, + # with the flag to override any existing publishing app. + # This allows for routes that were previously published by other apps to + # be added to `special_routes.yaml` and "just work". + # Commands::ReservePath.call(path_item) + Commands::ReservePath.call({ + base_path: route.fetch(:base_path), + publishing_app: "publishing-api", + override_existing: true, + }) + + Commands::V2::PutContent.call({ + content_id:, + base_path: route.fetch(:base_path), + document_type: route.fetch(:document_type, "special_route"), + schema_name: route.fetch(:document_type, "special_route"), + title: route.fetch(:title), + description: route.fetch(:description, ""), + locale: route.fetch(:locale, "en"), + details: {}, + routes:, + publishing_app: "publishing-api", + rendering_app: route.fetch(:rendering_app), + public_updated_at: Time.zone.now.iso8601, + update_type: route.fetch(:update_type, "major"), + }) + + if route[:links] + Commands::V2::PatchLinkSet.call({ content_id:, links: route[:links] }) + end + + Commands::V2::Publish.call({ content_id:, update_type: nil, locale: route.fetch(:locale, "en") }) + end + + def get_routes(route) + routes = [ + { + path: route.fetch(:base_path), + type: route.fetch(:type, "exact"), + }, + ] + + routes + route.fetch(:additional_routes, []).map { |ar| { path: ar[:base_path], type: ar.fetch(:type, "exact") } } + end + + def publish_routes(routes) + routes.each { |r| publish_route(r) } + end + + def self.load_special_routes + YAML.load_file(Rails.root.join("lib/data/special_routes.yaml")) + end + + def self.load_homepage + YAML.load_file(Rails.root.join("lib/data/homepage.yaml")) + end + end +end diff --git a/lib/tasks/special_routes.rake b/lib/tasks/special_routes.rake new file mode 100644 index 0000000000..89eb5ec013 --- /dev/null +++ b/lib/tasks/special_routes.rake @@ -0,0 +1,24 @@ +desc "Publish special routes (except the homepage)" +task publish_special_routes: :environment do + Tasks::SpecialRoutePublisher.publish_special_routes +end + +desc "Publish all special routes for a single application" +task :publish_special_routes_for_app, [:app_name] => :environment do |_, args| + Tasks::SpecialRoutePublisher.publish_special_routes_for_app(args.app_name) +end + +desc "Publish a single special route" +task :publish_one_special_route, [:base_path] => :environment do |_, args| + Tasks::SpecialRoutePublisher.publish_one_route(args.base_path) +end + +desc "Unpublish a single special route, with a type of 'gone' or 'redirect'" +task :unpublish_one_special_route, %i[base_path alternative_path] => :environment do |_, args| + Tasks::SpecialRoutePublisher.unpublish_one_route(args.base_path, args.alternative_path) +end + +desc "Publish the homepage" +task publish_homepage: :environment do + Tasks::SpecialRoutePublisher.publish_homepage +end diff --git a/spec/fixtures/homepage.yaml b/spec/fixtures/homepage.yaml new file mode 100644 index 0000000000..0270fe0e50 --- /dev/null +++ b/spec/fixtures/homepage.yaml @@ -0,0 +1,10 @@ +- :content_id: "f3bbdec2-0e62-4520-a7fd-6ffd5d36e03a" + :base_path: "/" + :document_type: "homepage" + :title: "GOV.UK homepage" + :rendering_app: "frontend" + :links: + :organisations: + - "af07d5a5-df63-4ddc-9383-6a666845ebe9" + :primary_publishing_organisation: + - "af07d5a5-df63-4ddc-9383-6a666845ebe9" \ No newline at end of file diff --git a/spec/fixtures/special_routes.yaml b/spec/fixtures/special_routes.yaml new file mode 100644 index 0000000000..8faf7cc9a9 --- /dev/null +++ b/spec/fixtures/special_routes.yaml @@ -0,0 +1,16 @@ +- :content_id: "c1f08359-21f7-49c1-8811-54bf6690b6a3" + :base_path: "/account/home" + :title: "Account home page" + :rendering_app: "frontend" + +- :content_id: "e5098fc2-ad79-4c6f-882b-410831c12f60" + :base_path: "/account/saved-pages/add" + :title: "Save a page" + :rendering_app: "frontend" + +- :content_id: "cdcad470-21c6-4e19-b644-9e499de1ad12" + :base_path: "/media" + :title: "Government Uploads" + :description: "Handles redirects for /media/:id/:filename path for asset manager" + :type: "prefix" + :rendering_app: "government-frontend" diff --git a/spec/lib/tasks/special_route_publisher_spec.rb b/spec/lib/tasks/special_route_publisher_spec.rb new file mode 100644 index 0000000000..8bb5ca1841 --- /dev/null +++ b/spec/lib/tasks/special_route_publisher_spec.rb @@ -0,0 +1,66 @@ +RSpec.describe Tasks::SpecialRoutePublisher do + let(:content_id) { "f3bbdec2-0e62-4520-a7fd-6ffd5d36e03a" } + + before do + stub_request(:put, %r{.*content-store.*/content/.*}) + end + + describe "#publish_route" do + it "sets the rendering app" do + described_class.new.publish_route(content_id:, base_path: "/", title: "Hello", rendering_app: "frontend") + + expect(Edition.first.rendering_app).to eq("frontend") + end + + context "without a specific locale" do + it "defaults to english" do + described_class.new.publish_route(content_id:, base_path: "/", title: "Hello", rendering_app: "frontend") + + expect(Edition.first.locale).to eq("en") + end + end + + context "with a specific locale" do + it "uses the specified locale" do + described_class.new.publish_route(content_id:, base_path: "/", title: "Hello", rendering_app: "frontend", locale: "cy") + + expect(Edition.first.locale).to eq("cy") + end + end + + context "with additional routes" do + let(:additional_routes) { [base_path: "/homepage.json", type: "exact"] } + + it "calls the Publishing API with all routes specified for the item" do + described_class.new.publish_route(content_id:, base_path: "/", title: "Hello", rendering_app: "frontend", additional_routes:) + + expect(Edition.first.routes).to eq( + [ + { path: "/", type: "exact" }, + { path: "/homepage.json", type: "exact" }, + ], + ) + end + end + + context "with links" do + let(:links) do + { + organisations: %w[af07d5a5-df63-4ddc-9383-6a666845ebea], + primary_publishing_organisation: %w[af07d5a5-df63-4ddc-9383-6a666845ebe9], + } + end + + it "adds the links" do + described_class.new.publish_route(content_id:, base_path: "/", title: "Hello", rendering_app: "frontend", links:) + + link_set = Edition.first.document.link_set + expect(link_set).not_to be_nil + expect(link_set.links.first.link_type).to eq("organisations") + expect(link_set.links.first.target_content_id).to eq("af07d5a5-df63-4ddc-9383-6a666845ebea") + expect(link_set.links.second.link_type).to eq("primary_publishing_organisation") + expect(link_set.links.second.target_content_id).to eq("af07d5a5-df63-4ddc-9383-6a666845ebe9") + end + end + end +end diff --git a/spec/lib/tasks/special_routes_spec.rb b/spec/lib/tasks/special_routes_spec.rb new file mode 100644 index 0000000000..6313376ec2 --- /dev/null +++ b/spec/lib/tasks/special_routes_spec.rb @@ -0,0 +1,135 @@ +RSpec.describe "Rake tasks for publishing special routes" do + context "with special routes data" do + let(:stdout) { double(:stdout, puts: nil) } + + before do + stub_request(:put, %r{.*content-store.*/content/.*}) + end + + let(:replacement_special_routes_file) do + YAML.load_file(Rails.root.join("spec/fixtures/special_routes.yaml")) + end + + before do + original_special_routes_path = Rails.root.join("lib/data/special_routes.yaml") + allow(YAML).to receive(:load_file).with(original_special_routes_path).and_return(replacement_special_routes_file) + end + + describe "publish_special_routes" do + before do + Rake::Task["publish_special_routes"].reenable + end + + it "publishes the special routes" do + Rake::Task["publish_special_routes"].invoke + + expect(Document.count).to eq(3) + expect(Edition.count).to eq(3) + end + end + + describe "publish_special_routes_for_app" do + before do + Rake::Task["publish_special_routes_for_app"].reenable + end + + it "publishes the special routes for one particular app without publishing others" do + Rake::Task["publish_special_routes_for_app"].invoke("government-frontend") + + expect(Document.count).to eq(1) + expect(Edition.count).to eq(1) + edition = Edition.where(title: "Government Uploads").first + expect(edition).not_to be_nil + end + + it "returns a message if there are no routes for that app" do + expect(Rails.logger).to receive(:info).with(/No routes for finder-frontend in lib\/data\/special_routes.yaml/) + + Rake::Task["publish_special_routes_for_app"].invoke("finder-frontend") + + expect(Document.count).to eq(0) + end + end + + describe "publish_one_special_route" do + before do + Rake::Task["publish_one_special_route"].reenable + end + + it "publishes one special route" do + Rake::Task["publish_one_special_route"].invoke("/account/saved-pages/add") + + expect(Document.count).to eq(1) + expect(Edition.count).to eq(1) + edition = Edition.where(title: "Save a page").first + expect(edition).not_to be_nil + end + + it "returns a message if there are no records for that path" do + expect(Rails.logger).to receive(:info).with(/Route needs to be added to \/lib\/data\/special_routes.yaml/) + + Rake::Task["publish_one_special_route"].invoke("/account/saved-pages/remove") + + expect(Document.count).to eq(0) + end + end + + describe "unpublish_one_special_route" do + before do + Rake::Task["publish_one_special_route"].reenable + Rake::Task["publish_one_special_route"].invoke("/media") + Rake::Task["unpublish_one_special_route"].reenable + end + + it "unpublishes one special route" do + expect(Edition.first.state).to eq("published") + + Rake::Task["unpublish_one_special_route"].invoke("/media") + + expect(Edition.first.state).to eq("unpublished") + end + + it "unpublishes one special route with a redirect" do + expect(Edition.first.state).to eq("published") + + Rake::Task["unpublish_one_special_route"].invoke("/media", "/media2") + + expect(Edition.first.state).to eq("unpublished") + end + + it "returns a message if there are no records for that path" do + expect(Edition.first.state).to eq("published") + expect(Rails.logger).to receive(:info).with(/Route needs to be added to \/lib\/data\/special_routes.yaml/) + + Rake::Task["unpublish_one_special_route"].invoke("/account/saved-pages/remove") + + expect(Edition.first.state).to eq("published") + end + end + end + + context "with homepage data" do + before do + stub_request(:put, %r{.*content-store.*/content/.*}) + Rake::Task["publish_homepage"].reenable + end + + let(:replacement_homepage_file) do + YAML.load_file(Rails.root.join("spec/fixtures/homepage.yaml")) + end + + before do + original_homepage_path = Rails.root.join("lib/data/homepage.yaml") + allow(YAML).to receive(:load_file).with(original_homepage_path).and_return(replacement_homepage_file) + end + + it "publishes the special routes" do + Rake::Task["publish_homepage"].invoke + + expect(Document.count).to eq(1) + expect(Edition.count).to eq(1) + edition = Edition.where(title: "GOV.UK homepage").first + expect(edition).not_to be_nil + end + end +end