Skip to content

Commit

Permalink
Fixes #4811 - Allow users to delete a content view
Browse files Browse the repository at this point in the history
  • Loading branch information
David Davis committed Apr 14, 2014
1 parent 1004e1f commit 7e0b503
Show file tree
Hide file tree
Showing 14 changed files with 148 additions and 42 deletions.
14 changes: 13 additions & 1 deletion app/controllers/katello/api/v2/content_views_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ class Api::V2::ContentViewsController < Api::V2::ApiController

wrap_parameters :include => (ContentView.attribute_names + %w(repository_ids component_ids))

resource_description do
api_version "v2"
end

def_param_group :content_view do
param :description, String, :desc => "Description for the content view"
param :repository_ids, Array, :desc => "List of repository ids"
Expand All @@ -45,7 +49,8 @@ def rules
:history => view_rule,
:available_puppet_module_names => view_rule,
:remove_from_environment => promote_rule,
:remove => edit_rule
:remove => edit_rule,
:destroy => edit_rule
}
end

Expand Down Expand Up @@ -201,6 +206,13 @@ def remove
respond_for_async :resource => task
end

api :DELETE, "/content_views/:id", "Delete a content view"
param :id, :number, :desc => "content view numeric identifier", :required => true
def destroy
task = async_task(::Actions::Katello::ContentView::Destroy, @view)
respond_for_async :resource => task
end

private

def find_content_view
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/katello/auto_complete_search.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ def auto_complete_search

# search provides the ability to pass a filter parameter in the request... on pages that have the
# environment selector, we use this filter to communicate which environment the results should be provided for...
if !params[:filter].nil? && controller_name.singularize.camelize.constantize.respond_to?('by_env')
@items = controller_name.singularize.camelize.constantize.readable(@readable_by).by_env(params[:filter]).complete_for(params[:search], @filter)
if !params[:filter].nil? && controller_name.singularize.camelize.constantize.respond_to?('in_environment')
@items = controller_name.singularize.camelize.constantize.readable(@readable_by).in_environment(params[:filter]).complete_for(params[:search], @filter)
else
@items = controller_name.singularize.camelize.constantize.readable(@readable_by).complete_for(params[:search], @filter)
end
Expand Down
39 changes: 39 additions & 0 deletions app/lib/actions/katello/content_view/destroy.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#
# Copyright 2014 Red Hat, Inc.
#
# This software is licensed to you under the GNU General Public
# License as published by the Free Software Foundation; either version
# 2 of the License (GPLv2) or (at your option) any later version.
# There is NO WARRANTY for this software, express or implied,
# including the implied warranties of MERCHANTABILITY,
# NON-INFRINGEMENT, or FITNESS FOR A PARTICULAR PURPOSE. You should
# have received a copy of GPLv2 along with this software; if not, see
# http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.

module Actions
module Katello
module ContentView
class Destroy < Actions::EntryAction

def plan(content_view)
action_subject(content_view)
content_view.check_ready_to_destroy!

sequence do
concurrence do
content_view.content_view_versions.each do |version|
plan_action(ContentViewVersion::Destroy, version)
end
end

content_view.destroy
end
end

def humanized_name
_("Delete")
end
end
end
end
end
2 changes: 1 addition & 1 deletion app/lib/actions/katello/content_view/remove_version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class RemoveVersion < Actions::EntryAction

def plan(version)
action_subject(version.content_view)
version.check_ready_to_delete!
version.check_ready_to_destroy!

history = ::Katello::ContentViewHistory.create!(:content_view_version => version,
:user => ::User.current.login,
Expand Down
2 changes: 1 addition & 1 deletion app/lib/actions/katello/content_view_version/destroy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ module ContentViewVersion
class Destroy < Actions::Base

def plan(version)
version.check_ready_to_delete!
version.check_ready_to_destroy!

sequence do
concurrence do
Expand Down
54 changes: 27 additions & 27 deletions app/models/katello/content_view.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ class ContentView < Katello::Model

CONTENT_DIR = "content_views"

before_destroy :confirm_not_promoted # RAILS3458: this needs to come before associations

belongs_to :organization, :inverse_of => :content_views, :class_name => "::Organization"

has_many :content_view_environments, :class_name => "Katello::ContentViewEnvironment", :dependent => :destroy
Expand Down Expand Up @@ -76,21 +74,6 @@ def self.in_environment(env)
where("#{Katello::ContentViewEnvironment.table_name}.environment_id = ?", env.id)
end

def self.promoted(safe = false)
# retrieve the view, if it has been promoted (i.e. exists in more than 1 environment)
relation = select("distinct #{Katello::ContentView.table_name}.*").
joins(:content_view_versions => :environments).
where("#{Katello::KTEnvironment.table_name}.library" => false).
where("#{Katello::ContentView.table_name}.default" => false)

if safe
# do not include group and having in returned relation
self.where :id => relation.all.map(&:id)
else
relation
end
end

def to_s
name
end
Expand Down Expand Up @@ -574,19 +557,36 @@ def check_ready_to_publish!
def check_remove_from_environment!(env)
errors = []

if (env_systems = systems.by_env(env)).any?
errors << _("Cannot remove '%{view}' from environment '%{env}' due to dependent systems: %{list}.") %
{view: self.name, env: env.name, list: env_systems.map(&:name).join(", ")}
end
dependencies = {systems: _("systems"),
distributors: _("distributors"),
activation_keys: _("activation keys")
}

if (env_distrs = distributors.by_env(env)).any?
errors << _("Cannot remove '%{view}' from environment '%{env}' due to dependent distributors: %{list}.") %
{view: self.name, env: env.name, list: env_distrs.map(&:name).join(", ")}
dependencies.each do |key, name|
if (models = self.association(key).scoped.in_environment(env)).any?
errors << _("Cannot delete '%{view}' from environment '%{env}' due to associated %{dependent}: %{names}.") %
{view: self.name, env: env.name, names: models.map(&:name).join(", ")}
end
end

if (keys = activation_keys.in_environment(env)).any?
errors << _("Cannot remove '%{view}' from environment '%{env}' due to dependent activation keys: %{list}.") %
{env: env.name, list: keys.map(&:name).join(", ")}
fail errors.join(" ") if errors.any?
return true
end

def check_ready_to_destroy!
errors = []

dependencies = {environments: _("environments"),
systems: _("systems"),
distributors: _("distributors"),
activation_keys: _("activation keys")
}

dependencies.each do |key, name|
if (models = self.association(key).scoped).any?
errors << _("Cannot delete '%{view}' due to associated %{dependent}: %{names}.") %
{view: self.name, dependent: name, names: models.map(&:name).join(", ")}
end
end

fail errors.join(" ") if errors.any?
Expand Down
4 changes: 2 additions & 2 deletions app/models/katello/content_view_environment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ def owner
end

def systems
content_view.systems.by_env(environment)
content_view.systems.in_environment(environment)
end

def distributors
content_view.distributors.by_env(environment)
content_view.distributors.in_environment(environment)
end

def activation_keys
Expand Down
4 changes: 2 additions & 2 deletions app/models/katello/content_view_version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class ContentViewVersion < Katello::Model
include AsyncOrchestration
include Authorization::ContentViewVersion

before_destroy :check_ready_to_delete!
before_destroy :check_ready_to_destroy!

belongs_to :content_view, :class_name => "Katello::ContentView", :inverse_of => :content_view_versions
has_many :content_view_environments, :class_name => "Katello::ContentViewEnvironment",
Expand Down Expand Up @@ -295,7 +295,7 @@ def check_ready_to_promote!
fail _("Default content view versions cannot be promoted") if default?
end

def check_ready_to_delete!
def check_ready_to_destroy!
if environments.any? && !organization.being_deleted?
fail _("Cannot delete version while it is in environments: %s") % environments.map(&:name).join(",")
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/katello/distributor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class Distributor < Katello::Model

after_create :init_default_custom_info

scope :by_env, lambda { |env| where('environment_id = ?', env) unless env.nil?}
scope :in_environment, lambda { |env| where('environment_id = ?', env) unless env.nil?}
scope :completer_scope, lambda { |options| readable(options[:organization_id])}

def organization
Expand Down
2 changes: 1 addition & 1 deletion app/models/katello/system.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class System < Katello::Model
before_create :fill_defaults
after_create :init_default_custom_info

scope :by_env, lambda { |env| where('environment_id = ?', env) unless env.nil?}
scope :in_environment, lambda { |env| where('environment_id = ?', env) unless env.nil?}
scope :completer_scope, lambda { |options| readable(options[:organization_id])}

def add_system_group(system_group)
Expand Down
22 changes: 22 additions & 0 deletions test/actions/katello/content_view_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -204,4 +204,26 @@ class RemoveTest < TestBase
end
end

class DestroyTest < TestBase
let(:action_class) { ::Actions::Katello::ContentView::Destroy }

let(:content_view) do
katello_content_views(:library_dev_view)
end

it 'plans' do
view = Katello::ContentView.create!(:name => "New view",
:organization => content_view.organization
)
version = Katello::ContentViewVersion.create!(:content_view => view,
:version => 1
)

action.expects(:action_subject).with(view.reload)
view.expects(:destroy).returns(true)
plan_action(action, view)
assert_action_planed_with(action, ::Actions::Katello::ContentViewVersion::Destroy, version)
end
end

end
17 changes: 17 additions & 0 deletions test/controllers/api/v2/content_views_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -228,5 +228,22 @@ def test_publish_default_view
assert_response 400
assert_equal version_count, view.versions.reload.count
end

def test_destroy
view = ContentView.create!(:name => "Cat",
:organization => @organization
)
delete :destroy, :id => view.id
assert_response :success
end

def test_destroy_protected
allowed_perms = [@create_permission, @update_permission]
denied_perms = [@no_permission, @read_permission]

assert_protected_action(:destroy, allowed_perms, denied_perms) do
delete :destroy, :id => @content_view.id
end
end
end
end
13 changes: 9 additions & 4 deletions test/models/activation_key_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,19 @@
module Katello
class ActivationKeyTest < ActiveSupport::TestCase

def self.before_suite
models = ["ActivationKey", "KTEnvironment", "ContentViewEnvironment", "ContentView"]
disable_glue_layers([], models, true)
end

def setup
@dev_key = katello_activation_keys(:dev_key)
@dev_view = katello_content_views(:library_dev_view)
@lib_view = katello_content_views(:library_view)
@dev_key = ActivationKey.find(katello_activation_keys(:dev_key))
@dev_view = ContentView.find(katello_content_views(:library_dev_view))
@lib_view = ContentView.find(katello_content_views(:library_view))
end

test "can have content view" do
@dev_key = katello_activation_keys(:dev_key)
@dev_key = ActivationKey.find(katello_activation_keys(:dev_key))
@dev_key.content_view = @dev_view
assert @dev_key.save!
assert_not_nil @dev_key.content_view
Expand Down
11 changes: 11 additions & 0 deletions test/models/content_view_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -271,5 +271,16 @@ def test_check_remove_from_environment!
@library_dev_view.check_remove_from_environment!(@dev)
end
end

def test_check_ready_to_destroy!
assert_raises(RuntimeError) do
@library_dev_view.check_ready_to_destroy!
end

view = ContentView.create!(:name => "Cat",
:organization => @organization
)
assert view.check_ready_to_destroy!
end
end
end

0 comments on commit 7e0b503

Please sign in to comment.