Fixes #37342 - Update foreman docs and fix specify matcher documentation #711

girijaasoni · 2024-04-12T11:13:03Z

Foreman documentation was outdated and using theforeman.org. This PR updates the doc links according to 'https://docs.theforeman.org/nightly/Managing_Configurations_Ansible/index-foreman-el.html'

It also fixes the misdirected redirect for SpecifyMatchers as reported in https://issues.redhat.com/browse/SAT-24111

Note: This Pr is blocked by RedHatSatellite/foreman_theme_satellite#45

ekohl

Please use the proper APIs. Since theforeman/foreman@b566ea8 it's no longer needed to override the root URL isn't because it's creating broken links.

You should be able to use type: 'docs' and get the right base URL (including version) and filename.

ekohl · 2024-04-12T11:27:03Z

app/helpers/foreman_ansible/ansible_plugin_helper.rb

-      major_version = ::ForemanAnsible::VERSION.split('.')[0]
-      'https://theforeman.org/plugins/foreman_ansible/'\
-        "#{major_version}.x/index.html"
+      'https://docs.theforeman.org/nightly/Managing_Configurations_Ansible/index-foreman-el.html'


Never hardcode versions.

ShimShtein

You can use documentation_button and documentation_url helpers directly, or wrap them with ansible-specific wrapper to make the code a bit more DRY.

The documentation_url helper calls the links_controller under the hood. In our case we want the docs type of link with its relevant parameters. This will also enable proper branding down the line.

ShimShtein · 2024-06-26T07:37:14Z

app/views/ansible_roles/index.html.erb

@@ -1,7 +1,8 @@
 <% title _("Ansible Roles") %>

 <% title_actions ansible_proxy_import(hash_for_import_ansible_roles_path),
-  documentation_button('#4.1ImportingRoles', :root_url => ansible_doc_url) %>
+ documentation_button('#Importing_Ansible_Roles_and_Variables_ansible', :root_url => ansible_doc_url) %>


Suggested change

documentation_button('#Importing_Ansible_Roles_and_Variables_ansible', :root_url => ansible_doc_url) %>

documentation_button(type: 'docs', 'Managing_Configurations_Ansible', chapter: 'Importing_Ansible_Roles_and_Variables_ansible') %>

I'm not sure you can pass a position argument in the middle. It's probably

Suggested change

documentation_button('#Importing_Ansible_Roles_and_Variables_ansible', :root_url => ansible_doc_url) %>

documentation_button('Managing_Configurations_Ansible', type: 'docs', chapter: 'Importing_Ansible_Roles_and_Variables_ansible') %>

Yup, makes sense. Hope it wasn't too confusing

ShimShtein · 2024-06-26T07:38:05Z

app/views/ansible_roles/welcome.html.erb

@@ -7,7 +7,7 @@
  <p><%= _('No Ansible Roles were found in Foreman. If you want to assign roles to your hosts,
             you have to import them first.').html_safe %>
  </p>
-  <p><%= link_to(_('Learn more about this in the documentation.'), documentation_url('#4.1ImportingRoles', :root_url => ansible_doc_url), target: '_blank') %></p>
+  <p><%= link_to(_('Learn more about this in the documentation.'), documentation_url('#Importing_Ansible_Roles_and_Variables_ansible', :root_url => ansible_doc_url), target: '_blank') %></p>


Suggested change

<p><%= link_to(_('Learn more about this in the documentation.'), documentation_url('#Importing_Ansible_Roles_and_Variables_ansible', :root_url => ansible_doc_url), target: '_blank') %></p>

<p><%= link_to(_('Learn more about this in the documentation.'), documentation_url(type: 'docs', 'Managing_Configurations_Ansible', chapter: 'Importing_Ansible_Roles_and_Variables_ansible'), target: '_blank') %></p>

ShimShtein · 2024-06-26T07:38:56Z

app/views/ansible_variables/_fields.erb

@@ -65,7 +65,7 @@
    </fieldset>
    </br>
    <fieldset>
-      <h2><%= _("Specify Matchers") %>  <%= documentation_button('4.2.6SmartMatchers') %></h2>
+      <h2><%= _("Specify Matchers") %>  <%= documentation_button('#Overriding_Ansible_Variables_in_foreman_ansible', :root_url => ansible_doc_url) %></h2>


Suggested change

<h2><%= _("Specify Matchers") %> <%= documentation_button('#Overriding_Ansible_Variables_in_foreman_ansible', :root_url => ansible_doc_url) %></h2>

<h2><%= _("Specify Matchers") %> <%= documentation_button(type: 'docs', 'Managing_Configurations_Ansible', chapter: 'Overriding_Ansible_Variables_in_foreman_ansible') %></h2>

ShimShtein · 2024-06-26T07:39:40Z

app/views/ansible_variables/index.html.erb

@@ -2,7 +2,7 @@
 <%= stylesheet 'foreman_ansible/foreman-ansible' %>

 <%= title_actions display_link_if_authorized(_('New Ansible Variable'), hash_for_new_ansible_variable_path, :class => "btn btn-default no-float"),
-      documentation_button('#4.3Variables', :root_url => ansible_doc_url)
+      documentation_button('#Importing_Ansible_Roles_and_Variables_ansible', :root_url => ansible_doc_url)


Suggested change

documentation_button('#Importing_Ansible_Roles_and_Variables_ansible', :root_url => ansible_doc_url)

documentation_button(type: 'docs', 'Managing_Configurations_Ansible', chapter: 'Importing_Ansible_Roles_and_Variables_ansible')

ekohl · 2024-07-22T13:53:50Z

@girijaasoni any update on this?

girijaasoni · 2024-07-26T10:28:34Z

@girijaasoni any update on this?

almost done, confused about this comment , should I update it in accordance with 6.15? It will fail for the other versions

ekohl · 2024-07-26T11:33:53Z

app/views/ansible_variables/_fields.erb

@@ -65,7 +65,7 @@
    </fieldset>
    </br>
    <fieldset>
-      <h2><%= _("Specify Matchers") %>  <%= documentation_button('4.2.6SmartMatchers') %></h2>
+      <h2><%= _("Specify Matchers") %>  <%= documentation_button('Managing_Configurations_Ansible', type: 'docs', chapter: 'Overriding_Ansible_Variables_in_foreman_ansible') %></h2>      


Suggested change

<h2><%= _("Specify Matchers") %> <%= documentation_button('Managing_Configurations_Ansible', type: 'docs', chapter: 'Overriding_Ansible_Variables_in_foreman_ansible') %></h2>

<h2><%= _("Specify Matchers") %> <%= documentation_button('Managing_Configurations_Ansible', type: 'docs', chapter: 'Overriding_Ansible_Variables_in_foreman_ansible') %></h2>

ekohl · 2024-07-26T11:33:57Z

app/views/ansible_roles/welcome.html.erb

@@ -7,7 +7,7 @@
  <p><%= _('No Ansible Roles were found in Foreman. If you want to assign roles to your hosts,
             you have to import them first.').html_safe %>
  </p>
-  <p><%= link_to(_('Learn more about this in the documentation.'), documentation_url('#4.1ImportingRoles', :root_url => ansible_doc_url), target: '_blank') %></p>
+  <p><%= link_to(_('Learn more about this in the documentation.'), documentation_url('Managing_Configurations_Ansible', type: 'docs',  chapter: 'Importing_Ansible_Roles_and_Variables_ansible'), target: '_blank') %></p> 


Suggested change

<p><%= link_to(_('Learn more about this in the documentation.'), documentation_url('Managing_Configurations_Ansible', type: 'docs', chapter: 'Importing_Ansible_Roles_and_Variables_ansible'), target: '_blank') %></p>

<p><%= link_to(_('Learn more about this in the documentation.'), documentation_url('Managing_Configurations_Ansible', type: 'docs', chapter: 'Importing_Ansible_Roles_and_Variables_ansible'), target: '_blank') %></p>

requested changes have been applied

stejskalleos · 2024-07-31T13:41:47Z

LGTM Works as expected.

stejskalleos · 2024-07-31T13:42:00Z

Thanks @girijaasoni @ekohl

Odilhao · 2024-07-31T19:02:51Z

@stejskalleos can we get this cherrypick 13.0.z?

pr-processor bot added the Not yet reviewed label Apr 12, 2024

girijaasoni mentioned this pull request Apr 12, 2024

Update ansible docs according to docs.theforeman.org/nightly urls RedHatSatellite/foreman_theme_satellite#45

Merged

ekohl requested changes Apr 12, 2024

View reviewed changes

pr-processor bot added Waiting on contributor and removed Not yet reviewed labels Apr 12, 2024

ShimShtein suggested changes Jun 26, 2024

View reviewed changes

girijaasoni force-pushed the 37342 branch from ee341e9 to 74a307e Compare July 26, 2024 10:50

pr-processor bot added Needs re-review and removed Waiting on contributor labels Jul 26, 2024

girijaasoni force-pushed the 37342 branch from 74a307e to e4a0b22 Compare July 26, 2024 11:04

ekohl previously requested changes Jul 26, 2024

View reviewed changes

pr-processor bot added Waiting on contributor and removed Needs re-review labels Jul 26, 2024

Fixes #37342 - Update foreman docs and fix specify matcher documentation

ac8ae91

girijaasoni force-pushed the 37342 branch from e4a0b22 to ac8ae91 Compare July 26, 2024 12:17

pr-processor bot added Needs re-review and removed Waiting on contributor labels Jul 26, 2024

stejskalleos self-assigned this Jul 31, 2024

stejskalleos approved these changes Jul 31, 2024

View reviewed changes

pr-processor bot removed the Needs re-review label Jul 31, 2024

stejskalleos merged commit 05f76d2 into theforeman:master Jul 31, 2024
25 of 28 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #37342 - Update foreman docs and fix specify matcher documentation #711

Fixes #37342 - Update foreman docs and fix specify matcher documentation #711

girijaasoni commented Apr 12, 2024

ekohl left a comment

ekohl Apr 12, 2024

girijaasoni Apr 16, 2024

ShimShtein left a comment

ShimShtein Jun 26, 2024

ekohl Jun 26, 2024

ShimShtein Jun 27, 2024

ShimShtein Jun 26, 2024

ShimShtein Jun 26, 2024

ShimShtein Jun 26, 2024

ekohl commented Jul 22, 2024

girijaasoni commented Jul 26, 2024 •

edited

Loading

ekohl Jul 26, 2024

ekohl Jul 26, 2024

stejskalleos commented Jul 31, 2024

stejskalleos commented Jul 31, 2024

Odilhao commented Jul 31, 2024

	documentation_button('#Importing_Ansible_Roles_and_Variables_ansible', :root_url => ansible_doc_url) %>
	documentation_button(type: 'docs', 'Managing_Configurations_Ansible', chapter: 'Importing_Ansible_Roles_and_Variables_ansible') %>

	documentation_button('#Importing_Ansible_Roles_and_Variables_ansible', :root_url => ansible_doc_url) %>
	documentation_button('Managing_Configurations_Ansible', type: 'docs', chapter: 'Importing_Ansible_Roles_and_Variables_ansible') %>

	<p><%= link_to(_('Learn more about this in the documentation.'), documentation_url('#Importing_Ansible_Roles_and_Variables_ansible', :root_url => ansible_doc_url), target: '_blank') %></p>
	<p><%= link_to(_('Learn more about this in the documentation.'), documentation_url(type: 'docs', 'Managing_Configurations_Ansible', chapter: 'Importing_Ansible_Roles_and_Variables_ansible'), target: '_blank') %></p>

	<h2><%= _("Specify Matchers") %> <%= documentation_button('#Overriding_Ansible_Variables_in_foreman_ansible', :root_url => ansible_doc_url) %></h2>
	<h2><%= _("Specify Matchers") %> <%= documentation_button(type: 'docs', 'Managing_Configurations_Ansible', chapter: 'Overriding_Ansible_Variables_in_foreman_ansible') %></h2>

Fixes #37342 - Update foreman docs and fix specify matcher documentation #711

Fixes #37342 - Update foreman docs and fix specify matcher documentation #711

Conversation

girijaasoni commented Apr 12, 2024

ekohl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShimShtein left a 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 Jul 22, 2024

girijaasoni commented Jul 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stejskalleos commented Jul 31, 2024

stejskalleos commented Jul 31, 2024

Odilhao commented Jul 31, 2024

girijaasoni commented Jul 26, 2024 •

edited

Loading