From b566ea8ab3c56f79edb0ed9a1caa3c28038af961 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Tue, 27 Jun 2023 10:44:04 +0200 Subject: [PATCH] Fixes #32848 - Support linking to docs.theforeman.org An effort is under way to use docs.theforeman.org as the primary documentation source. It is already the official documentation source for Katello, but until now it wasn't easily possible to (correctly) link to it. The documentation is built in 3 flavors: * Katello (index-katello.html) * Debian/Ubuntu (index-deb.html) * Enterprise Linux (index-el.html) --- app/controllers/links_controller.rb | 17 +++++++ app/helpers/application_helper.rb | 4 +- config/settings.rb | 5 +++ test/controllers/links_controller_test.rb | 45 +++++++++++++++++++ test/helpers/application_helper_test.rb | 20 +++++++++ test/test_helper.rb | 11 +++++ .../javascripts/react_app/common/helpers.js | 13 +++++- .../RegistrationCommandsPageConstants.js | 10 ----- .../RegistrationCommandsPage/index.js | 9 ++-- 9 files changed, 117 insertions(+), 17 deletions(-) delete mode 100644 webpack/assets/javascripts/react_app/routes/RegistrationCommands/RegistrationCommandsPage/RegistrationCommandsPageConstants.js diff --git a/app/controllers/links_controller.rb b/app/controllers/links_controller.rb index adb8d025853..6dea70eb1a2 100644 --- a/app/controllers/links_controller.rb +++ b/app/controllers/links_controller.rb @@ -25,9 +25,18 @@ def external_url(type:, options: {}) 'https://projects.theforeman.org/projects/foreman/issues' when 'vmrc' 'https://www.vmware.com/go/download-vmrc' + when 'docs' + params.require(:section) + guide, chapter, flavor = params.permit(:section, :chapter, :flavor).values_at(:section, :chapter, :flavor) + flavor ||= self.class.new_docs_flavor + docs_url(guide: guide, chapter: chapter, flavor: flavor) end end + def self.new_docs_flavor + Foreman::Plugin.installed?('katello') ? 'katello' : SETTINGS[:docs_os_flavor] + end + private def validate_root_url @@ -49,6 +58,14 @@ def documentation_url(section = "", options = {}) root_url + (section || '') end + # For new documentation at docs.theforeman.org + def docs_url(guide:, flavor:, chapter: nil) + version = SETTINGS[:version].tag == 'develop' ? 'nightly' : SETTINGS[:version].short + chapter_hash = "##{chapter}" if chapter + + "https://docs.theforeman.org/#{version}/#{guide}/index-#{flavor}.html#{chapter_hash}" + end + def plugin_documentation_url name, version, section, root_url = plugin_documentation_params.values_at(:name, :version, :section, :root_url) path = root_url || foreman_org_path("plugins") diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index a90715344b8..cb6f62fb609 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -305,8 +305,8 @@ def edit_inline(object, property, options = {}) editable(object, property, {:type => type, :title => title, :value => value, :class => klass, :source => select_values, :url => update_url, :placeholder => placeholder}.compact) end - def documentation_url(section = "", options = {}) - main_app.external_link_url(options.merge(type: 'manual', params: { section: section })) + def documentation_url(section = nil, type: 'manual', **options) + main_app.external_link_url(type: type, section: section, params: options) end def spinner(text = '', options = {}) diff --git a/config/settings.rb b/config/settings.rb index a6f9632fb18..c40c9683106 100644 --- a/config/settings.rb +++ b/config/settings.rb @@ -10,6 +10,11 @@ # Load settings from env variables SETTINGS.deep_merge!(Foreman::EnvSettingsLoader.new.to_h) +# foreman-documentation builds different flavors for Debian and Enterprise +# Linux. It also builds for Katello, but we can't detect that here so the key +# is docs_os_flavor instead of docs_flavor. +SETTINGS[:docs_os_flavor] ||= File.exist?('/etc/debian_version') ? 'foreman-deb' : 'foreman-el' + # Force setting to true until all code using it is removed [:locations_enabled, :organizations_enabled, :unattended].each do |setting| SETTINGS[setting] = true diff --git a/test/controllers/links_controller_test.rb b/test/controllers/links_controller_test.rb index 33826b4ce3e..40133743fca 100644 --- a/test/controllers/links_controller_test.rb +++ b/test/controllers/links_controller_test.rb @@ -70,5 +70,50 @@ class LinksControllerTest < ActionController::TestCase assert_redirected_to /15.0/ assert_redirected_to /Usage/ end + + test 'new docs on nightly' do + get :show, params: { + type: 'docs', + section: 'TestSection', + chapter: 'TestChapter', + } + + assert_redirected_to %r{https://docs\.theforeman\.org/nightly/TestSection/index-(foreman-(deb|el)|katello)\.html#TestChapter} + end + + test 'new docs on a stable release' do + with_temporary_settings(version: Foreman::Version.new('3.9.1')) do + get :show, params: { + type: 'docs', + section: 'TestSection', + chapter: 'TestChapter', + } + + assert_redirected_to %r{https://docs\.theforeman\.org/3\.9/TestSection/index-(foreman-(deb|el)|katello)\.html#TestChapter} + end + end + + describe '#new_docs_flavor' do + test 'on Enterprise Linux' do + Foreman::Plugin.stubs(:installed?).with('katello').returns(false) + with_temporary_settings(docs_os_flavor: 'foreman-el') do + assert_equal(LinksController.new_docs_flavor, 'foreman-el') + end + end + + test 'on Debian' do + Foreman::Plugin.stubs(:installed?).with('katello').returns(false) + with_temporary_settings(docs_os_flavor: 'foreman-deb') do + assert_equal(LinksController.new_docs_flavor, 'foreman-deb') + end + end + + test 'on Enterprise Linux with Katello' do + Foreman::Plugin.stubs(:installed?).with('katello').returns(true) + with_temporary_settings(docs_os_flavor: 'foreman-el') do + assert_equal(LinksController.new_docs_flavor, 'katello') + end + end + end end end diff --git a/test/helpers/application_helper_test.rb b/test/helpers/application_helper_test.rb index 3d38d6bb233..c209c2171b3 100644 --- a/test/helpers/application_helper_test.rb +++ b/test/helpers/application_helper_test.rb @@ -32,6 +32,18 @@ def test_generate_link_for assert_match /my_plugin/, url end + test '#documentation_url and new docs page' do + url = documentation_url('TestSection', { type: 'plugin_manual', name: 'foreman_my_plugin', version: '1.2' }) + + assert_match %r{links/plugin_manual/TestSection\?name=foreman_my_plugin&version=1\.2}, url + end + + test '#documentation_url and new docs page' do + url = documentation_url('TestSection', { type: 'docs', chapter: 'test_chapter' }) + + assert_match %r{links/docs/TestSection\?chapter=test_chapter}, url + end + test '#documentation_button forwards options to #documentation_url' do expects(:icon_text).returns('http://nowhere.com') expects(:link_to).returns('test'.html_safe) @@ -39,6 +51,14 @@ def test_generate_link_for documentation_button '2.2PluginSection', :root_url => 'http://www.theforeman.org/my_plugin/v0.1/index.html#' end + + test '#documentation_button forwards plugin manual options to #documentation_url' do + expects(:icon_text).returns('http://nowhere.com') + expects(:link_to).returns('test'.html_safe) + expects(:documentation_url).with('2.2PluginSection', { type: 'plugin_manual', name: 'foreman_my_plugin', version: '1.2' }) + + documentation_button '2.2PluginSection', type: 'plugin_manual', name: 'foreman_my_plugin', version: '1.2' + end end describe 'accessible resources' do diff --git a/test/test_helper.rb b/test/test_helper.rb index 34cffb7ff56..24fbbbc57ca 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -188,6 +188,17 @@ def set_basic_auth(user, password) def disable_webpack Webpack::Rails::Manifest.stubs(:asset_paths).returns([]) end + + def with_temporary_settings(**kwargs) + old_settings = SETTINGS.slice(*kwargs.keys) + begin + SETTINGS.update(kwargs) + + yield + ensure + SETTINGS.update(old_settings) + end + end end class GraphQLQueryTestCase < ActiveSupport::TestCase diff --git a/webpack/assets/javascripts/react_app/common/helpers.js b/webpack/assets/javascripts/react_app/common/helpers.js index e1b0e8ebc16..91a7038615d 100644 --- a/webpack/assets/javascripts/react_app/common/helpers.js +++ b/webpack/assets/javascripts/react_app/common/helpers.js @@ -158,11 +158,22 @@ export const stringIsPositiveNumber = value => { /** * Get manual url based on version - * @param {String} section - section id for foreman documetation + * @param {String} section - section id for foreman documentation */ export const getManualURL = section => foremanUrl(`/links/manual/${section}`); export const getWikiURL = section => foremanUrl(`/links/wiki/${section}`); +/** + * Get the documentation URL + * @param {String} guide - The guide containing the documentation + * @param {String} chapter - Optional chapter within the guide + * @returns {String} + */ +export const getDocsURL = (guide, chapter = null) => { + const url = foremanUrl(`/links/docs/${guide}`); + return chapter ? `{url}?chapter={encodeURIComponent(chapter)}` : url; +}; + /** * Transform the Date object to date string accepted in the server * @param {Date} diff --git a/webpack/assets/javascripts/react_app/routes/RegistrationCommands/RegistrationCommandsPage/RegistrationCommandsPageConstants.js b/webpack/assets/javascripts/react_app/routes/RegistrationCommands/RegistrationCommandsPage/RegistrationCommandsPageConstants.js deleted file mode 100644 index c7c1b4ae929..00000000000 --- a/webpack/assets/javascripts/react_app/routes/RegistrationCommands/RegistrationCommandsPage/RegistrationCommandsPageConstants.js +++ /dev/null @@ -1,10 +0,0 @@ -import URI from 'urijs'; -import { foremanUrl } from '../../../../foreman_tools'; - -export const docUrl = (foremanVersion) => { - const rootUrl = `https://docs.theforeman.org/${foremanVersion}/` - const section = 'Managing_Hosts/index-foreman-el.html#registering-a-host_managing-hosts' - - const url = new URI({path: '/links/manual', query: { root_url: rootUrl, section: section }}); - return foremanUrl(url.href()); -} diff --git a/webpack/assets/javascripts/react_app/routes/RegistrationCommands/RegistrationCommandsPage/index.js b/webpack/assets/javascripts/react_app/routes/RegistrationCommands/RegistrationCommandsPage/index.js index b6eeb4d758c..9d7be931a69 100644 --- a/webpack/assets/javascripts/react_app/routes/RegistrationCommands/RegistrationCommandsPage/index.js +++ b/webpack/assets/javascripts/react_app/routes/RegistrationCommands/RegistrationCommandsPage/index.js @@ -15,10 +15,10 @@ import { } from '@patternfly/react-core'; import { translate as __ } from '../../../common/I18n'; +import { getDocsURL } from '../../../common/helpers'; import { useForemanOrganization, useForemanLocation, - useForemanVersion, } from '../../../Root/Context/ForemanContext'; import { STATUS } from '../../../constants'; import PageLayout from '../../common/PageLayout/PageLayout'; @@ -38,7 +38,6 @@ import { selectPluginData, } from './RegistrationCommandsPageSelectors'; import { dataAction, commandAction } from './RegistrationCommandsPageActions'; -import { docUrl } from './RegistrationCommandsPageConstants'; import General from './components/General'; import Advanced from './components/Advanced'; @@ -52,7 +51,6 @@ const RegistrationCommandsPage = () => { // Context const currentOrganization = useForemanOrganization(); const currentLocation = useForemanLocation(); - const foremanVersion = useForemanVersion(); // Form tabs const [activeTab, setActiveTab] = useState(0); @@ -180,7 +178,10 @@ const RegistrationCommandsPage = () => { ouiaId="register-host-documentation-button" component="a" className="btn-docs" - href={docUrl(foremanVersion)} + href={getDocsURL( + 'Managing_Hosts', + 'registering-a-host_managing-hosts' + )} rel="noreferrer" target="_blank" variant="secondary"