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 #4978 BZ1082698 - environment and content view no longer required #3983

Merged
merged 1 commit into from
Apr 23, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class Api::V1::CandlepinProxiesController < Api::V1::ApiController

before_filter :proxy_request_path, :proxy_request_body
before_filter :set_organization_id
before_filter :find_organization, :only => [:rhsm_index, :consumer_activate]
before_filter :find_organization, :only => [:rhsm_index, :consumer_activate, :consumer_create]
Copy link
Member

Choose a reason for hiding this comment

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

This change seems to cause http://projects.theforeman.org/issues/5465, any reason for this change?

before_filter :find_default_organization_and_or_environment, :only => [:consumer_create, :index, :consumer_activate]
before_filter :find_optional_organization, :only => [:consumer_create, :hypervisors_update, :index, :consumer_activate]
before_filter :find_only_environment, :only => [:consumer_create]
Expand Down Expand Up @@ -243,8 +243,7 @@ def consumer_activate
User.current = User.hidden.first
activation_keys = find_activation_keys

@system = System.new(system_params.merge(:environment => activation_keys[0].environment,
:content_view => activation_keys[0].content_view))
@system = System.new(system_params)
sync_task(::Actions::Katello::System::Create, @system, activation_keys)
@system.reload

Expand Down Expand Up @@ -417,7 +416,7 @@ def get_content_view_environments(label = nil)
end

def system_params
system_params = params.slice(:name, :owner, :facts, :installedProducts)
system_params = params.slice(:name, :organization_id, :facts, :installedProducts)

if params.key?(:cp_type)
system_params[:cp_type] = params[:cp_type]
Expand Down
51 changes: 33 additions & 18 deletions app/controllers/katello/api/v2/activation_keys_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class Api::V2::ActivationKeysController < Api::V2::ApiController

before_filter :verify_presence_of_organization_or_environment, :only => [:index]
before_filter :find_environment, :only => [:index, :create, :update]
before_filter :find_optional_organization, :only => [:index]
before_filter :find_optional_organization, :only => [:index, :create]
before_filter :find_activation_key, :only => [:show, :update, :destroy,
:available_system_groups, :add_system_groups, :remove_system_groups]
before_filter :authorize
Expand Down Expand Up @@ -73,13 +73,13 @@ def index
param :label, String, :desc => "unique label"
param :description, String, :desc => "description"
param :environment, Hash, :desc => "environment"
param :environment_id, :identifier, :desc => "environment id", :required => true
param :content_view_id, :identifier, :desc => "content view id", :required => true
param :usage_limit, :number, :desc => "maximum number of uses"
param :environment_id, :identifier, :desc => "environment id"
param :content_view_id, :identifier, :desc => "content view id"
param :usage_limit, :number, :desc => "maximum number of registered content hosts, or 'unlimited'"
def create
@activation_key = ActivationKey.create!(activation_key_params) do |activation_key|
activation_key.environment = @environment
activation_key.organization = @environment.organization
activation_key.environment = @environment if @environment
activation_key.organization = @organization
activation_key.user = current_user
end
respond
Expand All @@ -92,7 +92,7 @@ def create
param :description, String, :desc => "description"
param :environment_id, :identifier, :desc => "environment id", :required => true
param :content_view_id, :identifier, :desc => "content view id", :required => true
param :usage_limit, :number, :desc => "maximum number of uses"
param :usage_limit, :number, :desc => "maximum number of registered content hosts, or 'unlimited'"
def update
@activation_key.update_attributes(activation_key_params)
respond
Expand Down Expand Up @@ -148,7 +148,7 @@ def remove_system_groups

def find_environment
environment_id = params[:environment_id]
environment_id = params[:environment][:id] if !environment_id && params.key?(:environment)
environment_id = params[:environment][:id] if !environment_id && params[:environment]
return if !environment_id

@environment = KTEnvironment.find(environment_id)
Expand Down Expand Up @@ -185,17 +185,32 @@ def verify_presence_of_organization_or_environment
end

def activation_key_params
if params[:environment] && params[:environment][:id]
params[:environment_id] = params[:environment][:id]
params.delete(:environment)
end
if params[:content_view] && params[:content_view][:id]
params[:content_view_id] = params[:content_view][:id]
params.delete(:content_view)
key_params = params.require(:activation_key).permit(:name,
:description,
:environment_id,
:organization_id,
:content_view_id,
:system_group_ids => [])

key_params[:environment_id] = params[:environment][:id] if params[:environment].try(:[], :id)
key_params[:content_view_id] = params[:content_view][:id] if params[:content_view].try(:[], :id)
key_params[:usage_limit] = int_limit(params)

key_params
end

def int_limit(key_params)
limit = key_params[:activation_key].try(:[], :usage_limit)
if limit.nil?
limit = -1
elsif limit == 'unlimited' #_('Unlimited') || limit == 'Unlimited' || limit == _('unlimited') || limit == 'unlimited'
limit = -1
else
limit = Integer(limit) rescue nil
fail(HttpErrors::BadRequest, _("Invalid usage limit value of '%{value}'") %
{:value => key_params[:activation_key][:usage_limit]}) if limit.nil?
end
attrs = [:name, :description, :environment_id, :usage_limit, :organization_id, :content_view_id,
:system_group_ids => []]
params.require(:activation_key).permit(*attrs)
limit
end
end
end
3 changes: 2 additions & 1 deletion app/lib/actions/candlepin/consumer/create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class Create < Candlepin::Abstract
param :autoheal
param :release_ver
param :service_level
param :uuid
param :capabilities
param :activation_keys
end
Expand All @@ -39,7 +40,7 @@ def run
input[:autoheal],
input[:release_ver],
input[:service_level],
"",
input[:uuid],
input[:capabilities],
input[:activation_keys])
end
Expand Down
36 changes: 35 additions & 1 deletion app/lib/actions/katello/system/activation_keys.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,41 @@ module System
class ActivationKeys < Actions::Base
def plan(system, activation_keys)
activation_keys ||= []
system.system_group_ids = activation_keys.map(&:system_group_ids).flatten.compact.uniq

set_environment_and_content_view(system, activation_keys)
set_system_groups(system, activation_keys)
set_association(system, activation_keys)
end

def set_association(system, activation_keys)
system.activation_keys = activation_keys
end

def set_environment_and_content_view(system, activation_keys)
return if system.content_view

activation_key = activation_keys.detect do |act_key|
act_key.environment && act_key.content_view
end
if activation_key
system.environment = activation_key.environment
system.content_view = activation_key.content_view
else
fail _('At least one activation key must have a lifecycle environment and content view assigned to it')
end
end

def set_system_groups(system, activation_keys)
system_group_ids = activation_keys.flat_map(&:system_group_ids).compact.uniq

system_group_ids.each do |system_group_id|
system_group = ::Katello::SystemGroup.find(system_group_id)
if system_group.max_systems >= 0 && system_group.systems.length >= system_group.max_systems
fail _("System group '%{name}' exceeds maximum usage limit of '%{limit}'") %
{:limit => system_group.max_systems, :name => system_group.name}
end
end
system.system_group_ids = system_group_ids
end
end
end
Expand Down
4 changes: 3 additions & 1 deletion app/lib/actions/katello/system/create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ class Create < Actions::EntryAction
def plan(system, activation_keys = [])
system.disable_auto_reindex!

plan_action(Katello::System::ActivationKeys, system, activation_keys)
activation_key_plan = plan_action(Katello::System::ActivationKeys, system, activation_keys)
return if activation_key_plan.error

cp_create = plan_action(Candlepin::Consumer::Create,
cp_environment_id: system.cp_environment_id,
Expand All @@ -32,6 +33,7 @@ def plan(system, activation_keys = [])
autoheal: system.autoheal,
release_ver: system.release,
service_level: system.serviceLevel,
uuid: system.uuid,
capabilities: system.capabilities,
activation_keys: activation_keys)
system.save!
Expand Down
2 changes: 1 addition & 1 deletion app/lib/katello/api/v2/error_handling.rb
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def respond_for_exception(exception, options = {})

respond_to do |format|
#json has to be displayMessage for older RHEL 5.7 subscription managers
format.json { render :json => { :displayMessage => options[:display_message], :errors => options[:errors] }, :status => options[:status] }
format.json { render :json => { :displayMessage => options[:display_message], :errors => options[:errors]}, :status => options[:status] }
format.all { render :text => options[:text], :status => options[:status] }
end
end
Expand Down
18 changes: 10 additions & 8 deletions app/models/katello/activation_key.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,23 @@ class ActivationKey < Katello::Model
validates :name, :presence => true
validates :name, :uniqueness => {:scope => :organization_id}
validates_with Validators::KatelloDescriptionFormatValidator, :attributes => :description
validates :environment, :presence => true
validate :environment_exists
validates :content_view, :presence => true, :allow_blank => false
validates_each :usage_limit do |record, attr, value|
if !value.nil? && (value < -1 || value == 0 || (value != -1 && value < record.usage_count))
# we don't let users to set usage limit lower than current usage
record.errors[attr] << _("must be higher than current usage (%s) or unlimited" % record.usage_count)
if value.nil?
record.errors[attr] << _("cannot be nil")
elsif value > -1 && value < record.systems.length
# we don't let users to set usage limit lower than current in-use
record.errors[attr] << _("cannot be lower than current usage count (%s)" % record.systems.length)
end
end
validates_with Validators::ContentViewEnvironmentValidator

scope :in_environment, lambda { |env| where(:environment_id => env) }

def environment_exists
if environment.nil?
if environment_id && environment.nil?
errors.add(:environment, _("ID: %s doesn't exist ") % environment_id)
elsif environment.organization != self.organization
elsif !environment.nil? && environment.organization != self.organization
errors.add(:environment, _("name: %s doesn't exist ") % environment.name)
end
end
Expand Down Expand Up @@ -147,7 +147,9 @@ def as_json(*args)
private

def set_default_content_view
self.content_view = self.environment.try(:default_content_view) unless self.content_view
if self.environment && self.content_view.nil?
self.content_view = self.environment.try(:default_content_view)
end
end

end
Expand Down
7 changes: 4 additions & 3 deletions app/models/katello/glue/elastic_search/activation_key.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ def self.included(base)
end

def extended_json
to_ret = {:environment => self.environment.name,
:name_sort => name.downcase,
:content_view => self.content_view.try(:name)
to_ret = {
:environment => self.environment.try(:name),
:name_sort => name.downcase,
:content_view => self.content_view.try(:name)
}
to_ret
end
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class AllowNullContentViewToActivationKey < ActiveRecord::Migration
def up
change_column :katello_activation_keys, :environment_id, :integer, :null => true
end

def down
change_column :katello_activation_keys, :environment_id, :integer, :null => false
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -65,19 +65,6 @@
</span>
</div>

<div class="detail">
<span class="info-label" translate>Content View</span>
<span class="info-value"
alch-edit-select="activationKey.content_view.name"
readonly="activationKey.readonly"
selector="activationKey.content_view_id"
options="contentViews()"
on-cancel="cancelContentViewUpdate()"
on-save="saveContentView(activationKey)"
edit-trigger="editContentView">
</span>
</div>

<div class="detail">
<div alert type="'info'" ng-show="editContentView">
<p translate>You must select a new content view before your change of environment can be saved. Use the cancel button on content view selection to revert your environment selection.</p>
Expand All @@ -90,6 +77,18 @@
</span>
</div>

<div class="detail">
<span class="info-label" translate>Content View</span>
<span class="info-value"
alch-edit-select="activationKey.content_view.name"
readonly="activationKey.readonly"
selector="activationKey.content_view_id"
options="contentViews()"
on-cancel="cancelContentViewUpdate()"
on-save="saveContentView(activationKey)"
edit-trigger="editContentView">
</span>
</div>
</section>
</div>
</section>
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ <h2 class="pull-left" translate>New Activation Key</h2>
</textarea>
</div>

<div class="form-group required">
<div class="form-group">
<label class="control-label col-sm-2" translate>Environment</label>
<div class="col-sm-5 input">
<span path-selector="environments"
Expand All @@ -82,8 +82,7 @@ <h2 class="pull-left" translate>New Activation Key</h2>
ng-model="activationKey.content_view_id"
ng-options="contentView.id as contentView.name for contentView in contentViews"
tabindex="4"
autofocus
required>
autofocus>
</select>
<span class="help-block" ng-show="activationKey.environment !== undefined && contentViews.length === 0" translate>
The selected environment contains no Content Views, please select a different environment.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
<table alch-table="table" class="table table-striped table-full" ng-class="{'table-mask': table.working}">
<thead>
<tr alch-table-head row-select>
<tr alch-table-head>
<th alch-table-column sortable>{{ "Name" | translate }}</th>
</tr>
</thead>

<tbody>
<tr alch-table-row
ng-repeat="activationKey in table.rows"
row-select="product"
active-row="stateIncludes('activation-keys.details', {activationKeyId: activationKey.id})">
<td alch-table-cell>
<a ui-sref="activation-keys.details.info({activationKeyId: activationKey.id})">
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,24 @@
<table class="table table-striped" ng-class="{'table-mask': table.working}">
<thead>
<tr alch-table-head row-select>
<tr alch-table-head>
<th alch-table-column="name" sortable>{{ "Name" | translate }}</th>
<th alch-table-column="limit" sortable>{{ "Limit" | translate }}</th>
<th alch-table-column="environment" sortable>{{ "Environment" | translate }}</th>
<th alch-table-column="contentView" sortable>{{ "Content View" | translate }}</th>
</tr>
</thead>

<tbody>
<tr alch-table-row ng-repeat="activationKey in table.rows" row-select="activationKey">
<tr alch-table-row ng-repeat="activationKey in table.rows">
<td alch-table-cell>
<a ui-sref="activation-keys.details.info({activationKeyId: activationKey.id})">
{{ activationKey.name }}
</a>
<i class="icon-chevron-right selected-icon" ng-show="activationKey.selected"></i>
</td>
<td alch-table-cell>{{ activationKey.usage_limit | unlimitedFilter }}</td>
<td alch-table-cell>{{ activationKey.environment.name || "" }}</td>
<td alch-table-cell>{{ activationKey.content_view.name || "" }}</td>
</tr>
</tbody>
</table>
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,13 @@ <h4 translate>Subscriptions</h4>
None
</span>
<ul ng-show="contentHost.activation_keys.length > 0">
<li ng-repeat="ak in contentHost.activation_keys">
<li ng-repeat="activation_key in contentHost.activation_keys">
<span>
<a ng-href="{{ getActivationKeyLink(ak) }}">{{ ak.name }}</a>
<a ui-sref="activation-keys.details.info({activationKeyId: activation_key.id})">
{{ activation_key.name }}
</a>
</span>
<span ng-show="ak.description"> - {{ ak.description }}</span>
<span ng-show="activation_key.description"> - {{ activation_key.description }}</span>
</li>
</ul>
</span>
Expand Down