Fixes #32848 - Support linking to docs.theforeman.org #9756

ekohl · 2023-06-27T09:50:48Z

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)

This is currently untested, but is based on #8613.

theforeman-bot · 2023-06-27T09:50:52Z

Issues: #32848

app/controllers/links_controller.rb

ekohl · 2023-06-27T09:54:05Z

app/helpers/application_helper.rb

@@ -302,8 +302,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)


I think this should be safe, but I'm not 100% sure.

Works fine!

app/controllers/links_controller.rb

ShimShtein · 2023-07-05T10:53:45Z

app/helpers/application_helper.rb

-  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)


external_link_url should receive only two parameters: type and options. I think splitting the section out of the options will cause a different output

Are you sure? The route is includes the section:

foreman/config/routes.rb

Line 558 in df3a0b0

get 'links/:type(/:section)' => 'links#show', :as => 'external_link', :constraints => { section: %r{.*} }

Now I'll immediately admit my knowledge of Rails routing may be lacking, but I don't see a difference between type and section there, other than that section is optional.

Well, you are correct:

[9] pry(main)> external_link_path(type: 'foo', section: 'bar', zzz: 'qqq') => "/links/foo/bar?zzz=qqq"

As long as it works - I'm good :)

I didn't check if passing it without a section also works, but I'm assuming it does.

It does. When no section is passed, the final url does not contain a fragment.
So, as expected.

ekohl · 2023-10-26T11:52:28Z

app/controllers/links_controller.rb

@@ -25,6 +25,8 @@ def external_url(type:, options: {})
      'https://projects.theforeman.org/projects/foreman/issues'
    when 'vmrc'
      'https://www.vmware.com/go/download-vmrc'
+    when 'docs'
+      docs_url(guide: options['section'], **options.slice('chapter', 'flavor'))


I think generally you don't want to link to other flavors, but this is here so you can do something like "this requires Katello, see katello-flavor".

ekohl · 2023-10-30T17:23:13Z

I think this was during the NodeJS 14 migration:

[2023-10-26T15:14:52.894Z] ExecJS::RuntimeUnavailable: Could not find a JavaScript runtime. See https://github.com/rails/execjs for a list of available runtimes.

[test unit]

ekohl · 2023-10-31T15:07:24Z

Now with more tests. I think this is ready now.

Thorben-D

Looks good to me...
Thanks, @ekohl

Thorben-D · 2023-11-02T10:07:13Z

app/helpers/application_helper.rb

@@ -302,8 +302,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)


Works fine!

Thorben-D · 2023-11-02T10:17:23Z

app/helpers/application_helper.rb

-  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)


It does. When no section is passed, the final url does not contain a fragment.
So, as expected.

adamruzicka

Two nitpicks, but apart from that it works nicely.

app/services/foreman/version.rb

app/controllers/links_controller.rb

adamruzicka

LGTM, let's get this in

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)

ekohl · 2023-11-08T16:41:29Z

Updated tests to stub installed? so tests should be green now.

adamruzicka · 2023-11-09T11:07:13Z

Thank you @ekohl, @Thorben-D & @ShimShtein !

theforeman-bot added Not yet reviewed UI labels Jun 27, 2023

ekohl commented Jun 27, 2023

View reviewed changes

ekohl force-pushed the 32848-doc-url branch from fd496d6 to a792ed1 Compare June 27, 2023 10:10

ekohl mentioned this pull request Jun 27, 2023

Fixes #37041 - Change link to docs.theforeman.org theforeman/foreman_remote_execution#818

Merged

ekohl force-pushed the 32848-doc-url branch 2 times, most recently from 587c3f9 to 1e1e668 Compare June 27, 2023 13:26

ekohl mentioned this pull request Jun 27, 2023

Rely on the plugin_manual mechanism in Foreman theforeman/foreman_remote_execution#820

Open

ekohl requested a review from ShimShtein July 3, 2023 14:45

ShimShtein reviewed Jul 5, 2023

View reviewed changes

app/controllers/links_controller.rb Outdated Show resolved Hide resolved

ShimShtein reviewed Jul 5, 2023

View reviewed changes

ekohl force-pushed the 32848-doc-url branch from 1e1e668 to 1491b6f Compare July 5, 2023 11:24

ekohl mentioned this pull request Aug 7, 2023

Fixes #36644 - Validate root_url before using it #9795

Merged

ekohl force-pushed the 32848-doc-url branch from 1491b6f to c3068be Compare October 26, 2023 11:23

ekohl marked this pull request as ready for review October 26, 2023 11:23

ekohl commented Oct 26, 2023

View reviewed changes

ekohl force-pushed the 32848-doc-url branch 3 times, most recently from edcc0ce to 545d262 Compare October 26, 2023 15:09

ekohl force-pushed the 32848-doc-url branch 2 times, most recently from 8674e69 to f26bb3e Compare October 31, 2023 15:07

Thorben-D approved these changes Nov 2, 2023

View reviewed changes

pr-processor bot removed the Not yet reviewed label Nov 2, 2023

adamruzicka reviewed Nov 6, 2023

View reviewed changes

app/services/foreman/version.rb Outdated Show resolved Hide resolved

app/controllers/links_controller.rb Outdated Show resolved Hide resolved

ekohl force-pushed the 32848-doc-url branch from f26bb3e to 6e097c1 Compare November 6, 2023 15:41

adamruzicka previously approved these changes Nov 7, 2023

View reviewed changes

ekohl dismissed adamruzicka’s stale review via 71117c5 November 8, 2023 16:41

ekohl force-pushed the 32848-doc-url branch from 6e097c1 to 71117c5 Compare November 8, 2023 16:41

adamruzicka approved these changes Nov 9, 2023

View reviewed changes

adamruzicka merged commit b566ea8 into theforeman:develop Nov 9, 2023
14 checks passed

ekohl deleted the 32848-doc-url branch November 9, 2023 11:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #32848 - Support linking to docs.theforeman.org #9756

Fixes #32848 - Support linking to docs.theforeman.org #9756

ekohl commented Jun 27, 2023

theforeman-bot commented Jun 27, 2023

ekohl Jun 27, 2023

Thorben-D Nov 2, 2023

ShimShtein Jul 5, 2023

ekohl Jul 5, 2023

ShimShtein Jul 5, 2023

ekohl Jul 5, 2023

Thorben-D Nov 2, 2023

ekohl Oct 26, 2023

ekohl commented Oct 30, 2023

ekohl commented Oct 31, 2023

Thorben-D left a comment

Thorben-D Nov 2, 2023

Thorben-D Nov 2, 2023

adamruzicka left a comment

adamruzicka left a comment

ekohl commented Nov 8, 2023

adamruzicka commented Nov 9, 2023

Fixes #32848 - Support linking to docs.theforeman.org #9756

Fixes #32848 - Support linking to docs.theforeman.org #9756

Conversation

ekohl commented Jun 27, 2023

theforeman-bot commented Jun 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ekohl commented Oct 30, 2023

ekohl commented Oct 31, 2023

Thorben-D left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamruzicka left a comment

Choose a reason for hiding this comment

adamruzicka left a comment

Choose a reason for hiding this comment

ekohl commented Nov 8, 2023

adamruzicka commented Nov 9, 2023