From 0406fef6929748cb9e2e38e2a89fa561a6b3f9d2 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 26 Jul 2024 14:15:09 +0200 Subject: [PATCH] Rewrite documentation URL mapping to use the TOC Upstream builds various guides and each guide is built as a single large page. That page has many chapters. Technically every guide is also built into 3 flavors (foreman-deb, foreman-el, katello). Downstream builds various guides, which mostly come from upstream. Most guides are a simple translation where they're downcased, but some are rewritten to something else. A bigger difference is that each guide is built into multiple pages. This means we need to find the right page containing the chapter. The Table of Content contains a complete mapping with this information, so it can be looked up dynamically. If this still can't be found, we use the html-single where everything is built into a single page. We then just hope it exists. Otherwise the browser will just show the whole page. This is an unexpected case, so a warning is logged. The result is that a lot less maintenance is needed, because in most cases it will "just work". A downside is that it becomes hard to verify links are correct. It becomes needed to crawl through all of the source code for any documentation links, similar to how we crawl through it for translations. --- .../documentation_controller_branding.rb | 3 +- .../documentation-toc.json | 0 lib/foreman_theme_satellite/documentation.rb | 65 +++++++++++++------ lib/tasks/foreman_theme_satellite_tasks.rake | 5 +- .../documentation_controller_branding_test.rb | 46 +++++++++++++ 5 files changed, 95 insertions(+), 24 deletions(-) rename test/fixtures/toc.json => lib/foreman_theme_satellite/documentation-toc.json (100%) diff --git a/app/controllers/concerns/documentation_controller_branding.rb b/app/controllers/concerns/documentation_controller_branding.rb index fbfa580..d6fcf0a 100644 --- a/app/controllers/concerns/documentation_controller_branding.rb +++ b/app/controllers/concerns/documentation_controller_branding.rb @@ -33,8 +33,7 @@ def wiki_url(section: '') # We do not use flavor downstream, but keeping it here for the same method signature # rubocop:disable Lint/UnusedMethodArgument def docs_url(guide:, flavor:, chapter: nil) - url = ForemanThemeSatellite::Documentation::DOCS_GUIDES_LINKS.dig(guide, chapter) - url || "#{ForemanThemeSatellite.documentation_root}/#{guide.downcase}/#{chapter}" + ForemanThemeSatellite::Documentation.docs_url(guide, chapter, logger: Foreman::Logging.logger('app')) end # rubocop:enable Lint/UnusedMethodArgument end diff --git a/test/fixtures/toc.json b/lib/foreman_theme_satellite/documentation-toc.json similarity index 100% rename from test/fixtures/toc.json rename to lib/foreman_theme_satellite/documentation-toc.json diff --git a/lib/foreman_theme_satellite/documentation.rb b/lib/foreman_theme_satellite/documentation.rb index dfb8171..d1f133b 100644 --- a/lib/foreman_theme_satellite/documentation.rb +++ b/lib/foreman_theme_satellite/documentation.rb @@ -73,32 +73,59 @@ module Documentation 'foreman_discovery' => "#{ForemanThemeSatellite.documentation_root}/provisioning_hosts/discovering-hosts-on-a-network_provisioning", }.freeze - DOCS_GUIDES_LINKS = { - 'Managing_Content' => { - 'Products_and_Repositories_content-management' => "#{ForemanThemeSatellite.documentation_root}/managing_content/importing_content_content-management#Products_and_Repositories_content-management", - }, + # Guide mapping. If no entry is present, it should be assumed that it can + # be downcased + # Copied from foreman-documentation's upstream_filename_to_satellite_link.json + DOCS_GUIDE_MAPPING = { + 'Administering_Project' => 'administering_red_hat_satellite', + 'Configuring_Load_Balancer' => 'configuring_capsules_with_a_load_balancer', + 'Deploying_Project_on_AWS' => 'deploying_red_hat_satellite_on_amazon_web_services', + 'Installing_Proxy' => 'installing_capsule_server', + 'Installing_Server' => 'installing_satellite_server_in_a_connected_network_environment', + 'Installing_Server_Disconnected' => 'installing_satellite_server_in_a_disconnected_network_environment', + 'Managing_Configurations_Ansible' => 'managing_configurations_using_ansible_integration', + 'Managing_Configurations_Puppet' => 'managing_configurations_using_puppet_integration', + 'Managing_Content' => 'managing_content', + 'Managing_Hosts' => 'managing_hosts', + 'Managing_Security_Compliance' => 'managing_security_compliance', + 'Monitoring_Project' => 'monitoring_satellite_performance', + 'Planning_for_Project' => 'overview_concepts_and_deployment_considerations', + 'Provisioning_Hosts' => 'provisioning_hosts', + 'Tuning_Performance' => 'tuning_performance_of_red_hat_satellite', + 'Updating_Project' => 'updating_red_hat_satellite', + 'Upgrading_Project' => "upgrading_connected_red_hat_satellite_to_#{SATELLITE_SHORT_VERSION}", + 'Upgrading_Project_Disconnected' => "upgrading_disconnected_red_hat_satellite_to_#{SATELLITE_SHORT_VERSION}", + }.freeze + + # An upstream chapter mapping, in case downstream another chapter should be + # used. The top level key is the upstream guide name where the value is a + # mapping of upstream chapter to downstream mapping + DOCS_GUIDE_CHAPTER_MAPPING = { 'Managing_Hosts' => { - 'registering-a-host_managing-hosts' => "#{ForemanThemeSatellite.documentation_root}/managing_hosts/registering_hosts_to_server_managing-hosts#Registering_Hosts_by_Using_Global_Registration_managing-hosts", + 'registering-a-host_managing-hosts' => 'Registering_Hosts_by_Using_Global_Registration_managing-hosts', }, - 'Managing_Configurations_Ansible' => { - 'Importing_Ansible_Roles_and_Variables_ansible' => "#{ForemanThemeSatellite.documentation_root}/managing_configurations_using_ansible_integration/getting_started_with_ansible_in_satellite_ansible#Importing_Ansible_Roles_and_Variables_ansible", - 'Overriding_Ansible_Variables_in_foreman_ansible' => "#{ForemanThemeSatellite.documentation_root}/managing_configurations_using_ansible_integration/getting_started_with_ansible_in_satellite_ansible#Overriding_Ansible_Variables_in_satellite_ansible", - } }.freeze - def self.flat_docs_guides_links - nested_to_flat_k_v(nil, DOCS_GUIDES_LINKS).to_h - end + DOCS_GUIDE_PAGES = JSON.load_file(File.join(__dir__, 'documentation-toc.json')) + + def self.docs_url(guide, chapter, logger:) + root = ForemanThemeSatellite.documentation_root + downstream_guide = DOCS_GUIDE_MAPPING.fetch(guide) do + logger.debug("Could not find downstream guide for '#{guide}'") + guide.downcase + end + + if chapter + downstream_chapter = DOCS_GUIDE_CHAPTER_MAPPING.fetch(guide, chapter) || chapter - private_class_method def self.nested_to_flat_k_v(prefix, source) - key_values = [] - source.map do |k, v| - key = "#{prefix}/#{k}" - if v.is_a?(Hash) - key_values.concat(nested_to_flat_k_v(key, v)) + if (page = DOCS_GUIDE_PAGES[downstream_guide]&.find { |_page, chapters| chapters.include?(downstream_chapter) }&.first) + "#{root}/#{downstream_guide}/#{page}##{downstream_chapter}" else - key_values.concat([[key, v]]) + logger.warn("Could not find page for chapter '#{downstream_chapter}' in guide '#{downstream_guide}'") + "#{root}-single/#{downstream_guide}/index##{downstream_chapter}" end + else + "#{root}/#{downstream_guide}" end key_values end diff --git a/lib/tasks/foreman_theme_satellite_tasks.rake b/lib/tasks/foreman_theme_satellite_tasks.rake index eed2acf..a0ea41e 100644 --- a/lib/tasks/foreman_theme_satellite_tasks.rake +++ b/lib/tasks/foreman_theme_satellite_tasks.rake @@ -18,7 +18,7 @@ namespace :foreman_theme_satellite do task :validate_docs do toc_file = ENV['TOC'] unless toc_file - toc_file = File.expand_path('../../test/fixtures/toc.json', __dir__) + toc_file = File.expand_path('../foreman_theme_satellite/documentation-toc.json', __dir__) puts "Using default TOC: #{toc_file}, to override please specify TOC=" end @@ -29,13 +29,12 @@ namespace :foreman_theme_satellite do all_links = ForemanThemeSatellite::Documentation::USER_GUIDE_DICTIONARY .merge(ForemanThemeSatellite::Documentation::PLUGINS_DOCUMENTATION) - .merge(ForemanThemeSatellite::Documentation.flat_docs_guides_links) failed = all_links.filter { |_key, doc_address| doc_address.include?('/html/') && !checker.test_link(doc_address) } abort((failed.map { |key, doc_address| "FAILED: Cannot find #{doc_address} in TOC for entry: #{key}" } + ["Total failed: #{failed.count} entries"]).join("\n")) unless failed.empty? - puts "Successfully tested #{all_links.count} links" + puts "Successfully tested #{all_links.count + docs_guide_mapping.each_value.sum(&:length)} links" end end diff --git a/test/integration/documentation_controller_branding_test.rb b/test/integration/documentation_controller_branding_test.rb index 42df429..ebcc266 100644 --- a/test/integration/documentation_controller_branding_test.rb +++ b/test/integration/documentation_controller_branding_test.rb @@ -7,9 +7,55 @@ def test_docs_redirect_branded assert_redirected_to "https://docs.redhat.com/documentation/en-us/red_hat_satellite/#{ForemanThemeSatellite.documentation_version}/html/managing_hosts/registering_hosts_to_server_managing-hosts#Registering_Hosts_by_Using_Global_Registration_managing-hosts" end + def test_docs_dynamically_generated + get "/links/docs/Managing_Content?chapter=Products_and_Repositories_content-management" + + assert_redirected_to "https://docs.redhat.com/documentation/en-us/red_hat_satellite/#{ForemanThemeSatellite.documentation_version}/html/managing_content/importing_content_content-management#Products_and_Repositories_content-management" + end + + def test_docs_with_mapped_guide + get "/links/docs/Managing_Configurations_Ansible?chapter=Importing_Ansible_Roles_and_Variables_ansible" + + assert_redirected_to "https://docs.redhat.com/documentation/en-us/red_hat_satellite/#{ForemanThemeSatellite.documentation_version}/html/managing_configurations_using_ansible/getting_started_with_ansible_in_satellite_ansible_integration#Importing_Ansible_Roles_and_Variables_ansible" + end + + def test_docs_chapter_does_not_exist + get "/links/docs/Managing_Content?chapter=does-not-exist" + + assert_redirected_to "https://docs.redhat.com/documentation/en-us/red_hat_satellite/#{ForemanThemeSatellite.documentation_version}/html-single/managing_content/index#does-not-exist" + end + def test_docs_redirect_unknown_chapter get "/links/docs/Managing_Hosts" assert_redirected_to "https://docs.redhat.com/documentation/en-us/red_hat_satellite/#{ForemanThemeSatellite.documentation_version}/html/managing_hosts/" end + + def test_docs_guide_chapter_mapping + logger = mock() + + ForemanThemeSatellite::Documentation::DOCS_GUIDE_CHAPTER_MAPPING.each do |guide, chapters| + chapters.each_key do |chapter| + url = ForemanThemeSatellite::Documentation.docs_url(guide, chapter, logger: logger) + refute_includes(url, 'html-single') + end + end + end + + def test_many + { + 'Managing_Content' => { + 'Products_and_Repositories_content-management' => "#{ForemanThemeSatellite.documentation_root}/managing_content/importing_content_content-management#Products_and_Repositories_content-management", + }, + 'Managing_Configurations_Ansible' => { + 'Importing_Ansible_Roles_and_Variables_ansible' => "#{ForemanThemeSatellite.documentation_root}/managing_configurations_using_ansible_integration/getting_started_with_ansible_in_satellite_ansible#Importing_Ansible_Roles_and_Variables_ansible", + 'Overriding_Ansible_Variables_in_foreman_ansible' => "#{ForemanThemeSatellite.documentation_root}/managing_configurations_using_ansible_integration/getting_started_with_ansible_in_satellite_ansible#Overriding_Ansible_Variables_in_satellite_ansible", + }, + }.each do |guide, chapters| + chapters.each do |chapter, expected| + get "/links/docs/#{guide}?chapter=#{chapter}" + assert_redirected_to expected + end + end + end end