Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite documentation URL mapping to use the TOC #58

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
File renamed without changes.
65 changes: 46 additions & 19 deletions lib/foreman_theme_satellite/documentation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
ekohl marked this conversation as resolved.
Show resolved Hide resolved
'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 = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's questionable if we really want this, but I kept it in since we had this.

'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
Expand Down
5 changes: 2 additions & 3 deletions lib/tasks/foreman_theme_satellite_tasks.rake
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will require a change to the downstream CI

puts "Using default TOC: #{toc_file}, to override please specify TOC=<toc_file>"
end

Expand All @@ -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

Expand Down
46 changes: 46 additions & 0 deletions test/integration/documentation_controller_branding_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
assert_not_includes(url, 'html-single')
end
end
end

def test_many
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we want to keep this, but it was a simple way for me to test if the old DOCS_GUIDES_LINKS were properly converted

{
'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
Loading