Fixes #19332 - Add welcome page when missing #4475

amirfefer · 2017-04-20T15:01:25Z

Some pages are missing a welcome page.
I don't quite sure about the chosen icons.

domcleal

Other index pages that are missing welcome pages:

/models
/templates/provisioning_templates

domcleal · 2017-04-21T09:21:15Z

app/views/common_parameters/welcome.html.erb

+    <%= icon_text("globe", "", :kind => "fa") %>
+  </div>
+  <h1><%= _('Global Parameters') %></h1>
+  <p><%= _("Foreman can pass two types of parameters to Puppet via the ENC (External Node Classifier) interface - global parameters (accessible from any manifest),


This should not be so Puppet-specific, global parameters have a wide range of other uses, e.g. in templates. I'd suggest removing all references to Puppet.

It would be better to indicate that adding global parameters here at the top level may allow them to be overridden elsewhere.

The references to class parameters are also irrelevant here.

domcleal · 2017-04-21T09:26:32Z

app/views/common_parameters/welcome.html.erb

+<% content_for(:title, _("Global Parameters")) %>
+<div class="blank-slate-pf">
+  <div class="blank-slate-pf-icon">
+    <%= icon_text("globe", "", :kind => "fa") %>


list-ul or similar may also be appropriate, to represent key/value pairs.

domcleal · 2017-04-21T09:28:05Z

app/views/compute_profiles/welcome.html.erb

+  <h1><%= _('Compute Profiles') %></h1>
+  <p><%= _("A compute profile is a way of expressing a set of defaults for VMs created on a specific compute resource
+           that can be mapped to an operator-defined label. This means an administrator can express,
+           for example, what “Small”, Medium” or “Large” means on all of the individual compute resources present for a given installation.") %></p>


Missing quotation mark around "Medium".

I'd suggest replacing the Unicode quotation marks with regular " ones, or using the correct open/close Unicode quotation marks instead of only closing ones.

domcleal · 2017-04-21T09:31:00Z

app/views/ptables/welcome.html.erb

+  <h1><%= _('Partition Tables') %></h1>
+  <p><%= _("Partition templates are a subset of normal Provisioning Templates. They are handled separately because it is
+           frequently the case that an admin wants to deploy the same host template (packages, services, etc) with just
+           a different harddisk layout to account for different servers capabilities.") %></p>


Replace "harddisk" with either "disk" or "hard disk".

"servers" should be "servers'" or "server"

domcleal · 2017-04-21T09:32:34Z

app/views/ptables/welcome.html.erb

+  </div>
+  <h1><%= _('Partition Tables') %></h1>
+  <p><%= _("Partition templates are a subset of normal Provisioning Templates. They are handled separately because it is
+           frequently the case that an admin wants to deploy the same host template (packages, services, etc) with just


It'd be useful to state what they actually are (i.e. parts of provisioning templates that describe the partition layout), and not just the reasoning for their existence.

domcleal · 2017-04-21T09:35:25Z

app/views/smart_proxies/welcome.html.erb

+  </div>
+  <h1><%= _('Smart Proxies') %></h1>
+  <p><%= _("The Smart Proxy is a project which provides a restful API to various sub-systems.
+           Its goal is to provide an API for a higher level orchestration tools (such as Foreman).


Not useful here, better to state why they should be added in Foreman.

domcleal · 2017-04-21T09:36:05Z

app/views/smart_proxies/welcome.html.erb

+  <h1><%= _('Smart Proxies') %></h1>
+  <p><%= _("The Smart Proxy is a project which provides a restful API to various sub-systems.
+           Its goal is to provide an API for a higher level orchestration tools (such as Foreman).
+           The Smart proxy provides an easy way to add or extended existing subsystems and APIs using plugins.") %></p>


"Proxy", inconsistent with line above.

Not too useful to describe the plugin capability here, it should focus on why it's managed in Foreman here.

domcleal · 2017-04-21T09:37:00Z

app/views/trends/welcome.html.erb

+  </div>
+  <h1><%= _('Trends') %></h1>
+  <p><%= _("Trends in Foreman allow you to track changes in your infrastructure over time. It allows you to track both
+           Foreman related information and any puppet facts. The Trend pages give a graph of how the number of hosts with


Capitalise "Puppet"

s/give/provide/ or "show"

domcleal · 2017-04-21T09:37:33Z

app/views/trends/welcome.html.erb

+  <h1><%= _('Trends') %></h1>
+  <p><%= _("Trends in Foreman allow you to track changes in your infrastructure over time. It allows you to track both
+           Foreman related information and any puppet facts. The Trend pages give a graph of how the number of hosts with
+           that value have changed over time, and the current hosts list.") %></p>


"and list the current hosts"

domcleal · 2017-04-21T09:38:05Z

app/views/variable_lookup_keys/welcome.html.erb

+  <h1><%= _('Smart Variables') %></h1>
+  <p><%= _("Smart variables are a tool to provide global parameters (key/value data), normally to your Puppet ENC,
+           depending on a set of rules. They are intended to be a stepping stone to full parameterized classes,
+           when the class hasn’t been parameterized or in special cases when a global parameter is desired") %></p>


Missing full stop.

ohadlevy · 2017-05-06T18:38:38Z

@amirfefer ping?

amirfefer · 2017-05-10T17:37:06Z

@domcleal Thanks for your review, I've added missing pages and fix inline comments, could you re-review please?

domcleal · 2017-05-11T09:39:58Z

app/views/common_parameters/welcome.html.erb

+    <%= icon_text("list-ul", "", :kind => "fa") %>
+  </div>
+  <h1><%= _('Global Parameters') %></h1>
+  <p><%= _("Foreman’s concept of parameters maps onto Puppet’s idea of default-scope parameters. Foreman allows us to define a hierarchy of parameter inheritance, where global parameters global parameters accessible from any manifest") %></p>


Fixes #19332 - Add welcome page when missing #4475 (comment) suggested removing the Puppet references, or at least emphasising their non-Puppet use in templates etc. I don't think listing Puppet first and "manifests" (Puppet manifests?) is useful to users.

"global parameters global parameters" twice.

Missing full stop.

Reintroduced unicode apostrophe marks, use regular single quotes.

domcleal · 2017-05-11T09:42:05Z

app/views/puppetclass_lookup_keys/welcome.html.erb

+  </div>
+  <h1><%= _('Smart Class Parameters') %></h1>
+  <p><%= _("Parameterized class support permits detecting, importing, and supplying parameters direct to classes which support it,
+           via the ENC and depending on a set of rules (Smart Matchers)") %></p>


Missing full stop.

domcleal · 2017-05-11T09:42:36Z

app/views/puppetclass_lookup_keys/welcome.html.erb

+    <%= icon_text("info-circle", "", :kind => "fa") %>
+  </div>
+  <h1><%= _('Smart Class Parameters') %></h1>
+  <p><%= _("Parameterized class support permits detecting, importing, and supplying parameters direct to classes which support it,


"direct" should be "directly" or simply be left out.

domcleal · 2017-05-11T09:44:05Z

app/views/smart_proxies/welcome.html.erb

+  </div>
+  <h1><%= _('Smart Proxies') %></h1>
+  <p><%= _("The Smart Proxy is a project which provides a RESTful API to various subsystems.
+           The Smart Proxy provides an easy way to add or extended existing subsystems, via


#4475 (comment) remains unaddressed, unsure why the focus on plugins is here and not Foreman. This should explain what Foreman uses it for and why smart proxies are listed here, not about the capabilities of another project.

domcleal · 2017-05-11T09:44:29Z

app/views/smart_proxies/welcome.html.erb

+  <h1><%= _('Smart Proxies') %></h1>
+  <p><%= _("The Smart Proxy is a project which provides a RESTful API to various subsystems.
+           The Smart Proxy provides an easy way to add or extended existing subsystems, via
+           configurable DHCP (ISC DHCP and MS DHCP Servers), DNS (Bind and MS DNS Servers), Puppet, etc.") %></p>


Remove listing of supported providers, this will become out of date.

Unsure what "configurable DHCP, DNS" means?

domcleal · 2017-05-11T09:45:59Z

app/views/variable_lookup_keys/welcome.html.erb

+  <h1><%= _('Smart Variables') %></h1>
+  <p><%= _("Smart variables are a tool to provide global parameters (key/value data), normally to your Puppet ENC,
+           depending on a set of rules. They are intended to be a stepping stone to full parameterized classes,
+           when the class hasn’t been parameterized, or in special cases when a global parameter is desired") %></p>


#4475 (comment) is unfixed, missing full stop.

domcleal · 2017-05-11T09:47:36Z

app/views/models/welcome.html.erb

+    <%= icon_text("info-circle", "", :kind => "fa") %>
+  </div>
+  <h1><%= _('Hardware Models') %></h1>
+  <p><%= _("Hardware Model describes the hardware model of your hosts, including CPU class, Vendor class and general information") %></p>


The first fragment doesn't quite make sense, should this be: "Hardware models describe the hardware types of your hosts"?

Don't capitalise "Vendor".

"General information" is unclear in this context, perhaps call them "other notes".

Missing full stop.

domcleal · 2017-05-11T09:49:35Z

app/views/provisioning_templates/welcome.html.erb

+    <%= icon_text("info-circle", "", :kind => "fa") %>
+  </div>
+  <h1><%= _('Provisioning Templates') %></h1>
+  <p><%= _("The Provision Templates is the core of Foreman’s flexibility to deploy the right OS with the right options.


This doesn't make much sense, I'm not even sure what the point is. Can you rewrite it, perhaps to say that provisioning templates are used for provisioning and installation of operating systems?

domcleal · 2017-05-11T09:50:25Z

app/views/provisioning_templates/welcome.html.erb

+  </div>
+  <h1><%= _('Provisioning Templates') %></h1>
+  <p><%= _("The Provision Templates is the core of Foreman’s flexibility to deploy the right OS with the right options.
+            There are several types of template, along with a flexible matching system to deliver different templates to different Hosts.") %></p>


Remove capitalisation of "hosts", say "hosts or host groups" too.

domcleal · 2017-05-12T11:46:50Z

app/views/models/welcome.html.erb

+<% content_for(:title, _("Hardware Models")) %>
+<div class="blank-slate-pf">
+  <div class="blank-slate-pf-icon">
+    <%= icon_text("info-circle", "", :kind => "fa") %>


This should perhaps be laptop or server, something recognisable as a type of computer.

domcleal · 2017-05-12T11:47:36Z

app/views/provisioning_templates/welcome.html.erb

+<% content_for(:title, _("Provisioning Templates")) %>
+<div class="blank-slate-pf">
+  <div class="blank-slate-pf-icon">
+    <%= icon_text("info-circle", "", :kind => "fa") %>


This should be something representing a template, e.g. file-text.

ohadlevy · 2017-06-11T12:58:42Z

@amirfefer ping?

ohadlevy · 2017-10-23T14:10:15Z

@GregSutcliffe can you please have a look at this PR? thanks

amirfefer · 2017-10-25T09:01:05Z

[test]

amirfefer · 2017-10-29T08:59:21Z

[test upgrade]

ares

Thanks for the PR @amirfefer. I'm afraid text broken to multilines could cause problems during translating. I think we usually keep long lines in this case. So please remove \n like this

<% _("some
             second") %>

I left few other comments that should be addressed.

@waldenraines could you please do the grammar check when we have the final version of texts?

ares · 2017-11-22T14:34:09Z

app/views/models/welcome.html.erb

+    <%= icon_text("server", "", :kind => "fa") %>
+  </div>
+  <h1><%= _('Hardware Models') %></h1>
+  <p><%= _("HHardware models describe the hardware types of your hosts, including CPU class, vendor class and other notes.") %></p>


s/HHard/Hard/ typo

ares · 2017-11-22T14:36:00Z

app/views/ptables/welcome.html.erb

+    <%= icon_text("table", "", :kind => "fa") %>
+  </div>
+  <h1><%= _('Partition Tables') %></h1>
+  <p><%= _("Partition templates are a subset of normal provisioning templates which describe the partition layout, with just


it's not necessarily a subset, it's a different kind of templates with slightly different capabilities

ares · 2017-11-22T14:38:18Z

app/views/smart_proxies/welcome.html.erb

+    <%= icon_text("sitemap", "", :kind => "fa") %>
+  </div>
+  <h1><%= _('Smart Proxies') %></h1>
+  <p><%= _("The Smart Proxy provides an easy way to add or extended existing subsystems, via


I think this should be rephrased, something like "Smart proy provides restful API to subsystems such as DNS, DHCP, TFTP that Foreman talks to"

ares · 2017-11-22T14:40:11Z

app/views/variable_lookup_keys/welcome.html.erb

+    <%= icon_text("info-circle", "", :kind => "fa") %>
+  </div>
+  <h1><%= _('Smart Variables') %></h1>
+  <p><%= _("Smart variables are a tool to provide global parameters (key/value data), normally to your Puppet ENC,


smart variables are usually not global, they use the same rules as smart class parameters, the main difference is that they end up in different place in ENC so they end up in the same namespace as Parameters (global params, host params etc)

They are global from a Puppet perspective - that's what the Parameters block is. For Foreman they are both global and assigned to a class - If you try to have the same name in two smart vars, it'll clash even if they're assigned to different classes (I've no idea if that actually creates an error, but it won't work in the ENC for sure).

Getting all that nuance into a short text line is hard though....

Correct. How about this then?

Smart variables are a tool to provide parameters (key/value data), to a global namespace of a puppet run through your Puppet ENC,

Not bad. Maybe change to a global namespace of a puppet run through to to the top-level (global) scope of your Puppet ENC?

I don't think Dominic plans to continue with the review, please readd review if I misunderstood

amirfefer · 2017-12-04T11:53:51Z

Thanks @ares, @GregSutcliffe
Could you have a second look, please?

ares · 2017-12-18T15:10:14Z

@amirfefer tests seems to be failing, most likely rebase would help, would you mind rebasing the PR?

dLobatog

Back to WoC because of tests

waldenraines · 2018-01-05T17:06:43Z

@amirfefer ping

amirfefer · 2018-01-17T09:37:23Z

Rebased .

Edit: Katello test failure seems unrelated.

@ares would you mind to look again?

waldenraines · 2018-01-29T14:23:28Z

[test katello]

waldenraines · 2018-01-29T16:38:10Z

@dLobatog @ares tests are now passing, any other requests?

No multi-line issues in sight

dLobatog · 2018-01-31T15:00:22Z

Thanks @amirfefer @waldenraines

theforeman-bot added Needs testing Not yet reviewed labels Apr 20, 2017

domcleal suggested changes Apr 21, 2017

View reviewed changes

theforeman-bot added Waiting on contributor and removed Not yet reviewed labels Apr 21, 2017

amirfefer force-pushed the 19332 branch from 1f679b6 to 3abf7ab Compare May 10, 2017 17:35

theforeman-bot added Needs re-review Needs testing and removed Waiting on contributor labels May 10, 2017

domcleal previously requested changes May 11, 2017

View reviewed changes

theforeman-bot added Waiting on contributor and removed Needs re-review labels May 11, 2017

domcleal reviewed May 12, 2017

View reviewed changes

amirfefer mentioned this pull request Oct 18, 2017

Fixes #20896 - Remove help button from ptable #4825

Merged

amirfefer force-pushed the 19332 branch from 3abf7ab to 1e1bc90 Compare October 24, 2017 09:20

theforeman-bot added Needs re-review Needs testing and removed Waiting on contributor labels Oct 24, 2017

amirfefer force-pushed the 19332 branch from 1e1bc90 to d501455 Compare October 26, 2017 14:45

ares previously requested changes Nov 22, 2017

View reviewed changes

theforeman-bot added Waiting on contributor and removed Needs re-review labels Nov 22, 2017

amirfefer force-pushed the 19332 branch from d501455 to ab03488 Compare December 4, 2017 11:52

theforeman-bot added Needs re-review Needs testing and removed Waiting on contributor labels Dec 4, 2017

amirfefer force-pushed the 19332 branch from ab03488 to 2405472 Compare December 6, 2017 11:08

dLobatog suggested changes Dec 20, 2017

View reviewed changes

theforeman-bot added Waiting on contributor and removed Needs re-review labels Dec 20, 2017

amirfefer force-pushed the 19332 branch from 2405472 to d47da39 Compare December 26, 2017 16:20

Fixes #19332 - Add welcome page when missing

43c2733

amirfefer force-pushed the 19332 branch from d47da39 to 43c2733 Compare January 17, 2018 09:36

theforeman-bot added Needs re-review Needs testing and removed Waiting on contributor labels Jan 17, 2018

dLobatog approved these changes Jan 31, 2018

View reviewed changes

theforeman-bot removed the Needs re-review label Jan 31, 2018

dLobatog merged commit a00419f into theforeman:develop Jan 31, 2018

Fixes #19332 - Add welcome page when missing #4475

Fixes #19332 - Add welcome page when missing #4475

Conversation

amirfefer commented Apr 20, 2017 • edited

domcleal 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

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

ohadlevy commented May 6, 2017

amirfefer commented May 10, 2017

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ohadlevy commented Jun 11, 2017

ohadlevy commented Oct 23, 2017

amirfefer commented Oct 25, 2017

amirfefer commented Oct 29, 2017

ares 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amirfefer commented Dec 4, 2017

ares commented Dec 18, 2017

dLobatog left a comment

Choose a reason for hiding this comment

waldenraines commented Jan 5, 2018

amirfefer commented Jan 17, 2018 • edited

waldenraines commented Jan 29, 2018

waldenraines commented Jan 29, 2018

dLobatog commented Jan 31, 2018

amirfefer commented Apr 20, 2017 •

edited

amirfefer commented Jan 17, 2018 •

edited