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

Fixes #31918 - Host Registration - Activation key field improvements #9177

Merged
merged 1 commit into from Mar 2, 2021

Conversation

stejskalleos
Copy link
Contributor

@theforeman-bot
Copy link

Issues: #31918

@stejskalleos
Copy link
Contributor Author

@ares

Copy link
Member

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

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

Thanks @stejskalleos!

Just one small comment and question below

<a rel="popover" data-content="<%= help %>" data-trigger="focus" data-container="body" data-html="true" tabindex="-1">
<span class="pficon pficon-info "></span>
</a>
</label>
<div class='col-md-4'>
<%= text_field_tag 'activation_key', params[:activation_key], class: 'form-control' %>
<%= text_field_tag 'activation_key', params[:activation_key], class: 'form-control', required: true %>
Copy link
Member

Choose a reason for hiding this comment

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

This will make the field required regardless of which OS is selected. Is that the intention?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, for Foreman with Katello we want to make it required regardless of which OS is selected.
This will be changed in Foreman 2.5, where Activation keys will be hidden as advanced field,
and users will use host group as primary source of the data.
There is a redmine PR for it: https://projects.theforeman.org/issues/31240

app/views/foreman/hosts/_registration_commands.html.erb Outdated Show resolved Hide resolved
Copy link
Member

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

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

Changes LGTM, thanks @stejskalleos!

@jeremylenz jeremylenz merged commit e574ca6 into Katello:master Mar 2, 2021
jturel pushed a commit to jturel/katello that referenced this pull request Apr 30, 2021
…nd if 'Activation Key' has been kept blank.

Fixes #31918 - Host Registration - Activation key field improvements (Katello#9177)

(cherry picked from commit e574ca6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants