This patch updates assign_owner_roles call only for template/assembly/deployable/deployment/instance. TODO: pool, pool_family, provider, provider_account
From: Jan Provaznik jprovazn@redhat.com
assign_owner_roles moved to model. For now it's only for template/assembly/deployable/deployment. TODO: do same for pool/pool_families/... and others where we call assign_owner_roles --- .../image_factory/deployables_controller.rb | 2 +- .../image_factory/templates_controller.rb | 4 ++-- .../controllers/resources/instances_controller.rb | 1 - src/app/models/assembly.rb | 7 +++---- src/app/models/deployable.rb | 7 +++---- src/app/models/deployment.rb | 4 ++-- src/app/models/instance.rb | 1 + src/app/models/template.rb | 9 ++++++--- src/db/migrate/20110427071851_add_owner_id.rb | 13 +++++++++++++ src/features/step_definitions/assembly_steps.rb | 6 +++--- src/features/step_definitions/deployable_steps.rb | 2 +- src/features/step_definitions/deployment_steps.rb | 2 +- src/spec/factories/assembly.rb | 1 + src/spec/factories/deployable.rb | 1 + src/spec/factories/template.rb | 1 + 15 files changed, 39 insertions(+), 22 deletions(-) create mode 100644 src/db/migrate/20110427071851_add_owner_id.rb
diff --git a/src/app/controllers/image_factory/deployables_controller.rb b/src/app/controllers/image_factory/deployables_controller.rb index 1721d44..90242f3 100644 --- a/src/app/controllers/image_factory/deployables_controller.rb +++ b/src/app/controllers/image_factory/deployables_controller.rb @@ -41,8 +41,8 @@ class ImageFactory::DeployablesController < ApplicationController def create require_privilege(Privilege::CREATE, Template) @deployable = Deployable.new(params[:deployable]) + @deployable.owner = current_user if @deployable.save - @deployable.assign_owner_roles(current_user) flash[:notice] = "Deployable added." redirect_to image_factory_deployable_url(@deployable) else diff --git a/src/app/controllers/image_factory/templates_controller.rb b/src/app/controllers/image_factory/templates_controller.rb index c619f0c..88e075d 100644 --- a/src/app/controllers/image_factory/templates_controller.rb +++ b/src/app/controllers/image_factory/templates_controller.rb @@ -73,12 +73,12 @@ class ImageFactory::TemplatesController < ApplicationController check_create_permission @tpl = Template.new(params[:tpl]) @tpl.packages = params[:packages] + @tpl.owner = current_user if @tpl.save - @tpl.assign_owner_roles(current_user) flash[:notice] = "Template saved." @tpl.set_complete if params[:create_deployable] - Deployable.create!(:name => @tpl.name, :assemblies => @tpl.assemblies) + Deployable.create!(:name => @tpl.name, :assemblies => @tpl.assemblies, :owner => current_user) end redirect_to image_factory_templates_path else diff --git a/src/app/controllers/resources/instances_controller.rb b/src/app/controllers/resources/instances_controller.rb index efa9559..37aadeb 100644 --- a/src/app/controllers/resources/instances_controller.rb +++ b/src/app/controllers/resources/instances_controller.rb @@ -57,7 +57,6 @@ class Resources::InstancesController < ApplicationController @instance.transaction do @instance.save! # set owner permissions: - @instance.assign_owner_roles(current_user) @task = InstanceTask.create!({:user => current_user, :task_target => @instance, :action => InstanceTask::ACTION_CREATE}) diff --git a/src/app/models/assembly.rb b/src/app/models/assembly.rb index 8313f15..99c083b 100644 --- a/src/app/models/assembly.rb +++ b/src/app/models/assembly.rb @@ -52,20 +52,19 @@ class Assembly < ActiveRecord::Base has_many :permissions, :as => :permission_object, :dependent => :destroy, :include => [:role], :order => "permissions.id ASC" + belongs_to :owner, :class_name => "User", :foreign_key => "owner_id" + after_create "assign_owner_roles(owner)"
before_validation :generate_uuid before_save :update_xml
- has_many :permissions, :as => :permission_object, :dependent => :destroy, - :include => [:role], - :order => "permissions.id ASC" - validates_presence_of :uuid validates_uniqueness_of :uuid validates_presence_of :name validates_uniqueness_of :name validates_length_of :name, :maximum => 255 validates_presence_of :architecture + validates_presence_of :owner_id
def self.default_privilege_target_type Template diff --git a/src/app/models/deployable.rb b/src/app/models/deployable.rb index 25a8737..f2264f9 100644 --- a/src/app/models/deployable.rb +++ b/src/app/models/deployable.rb @@ -49,19 +49,18 @@ class Deployable < ActiveRecord::Base has_many :permissions, :as => :permission_object, :dependent => :destroy, :include => [:role], :order => "permissions.id ASC" + belongs_to :owner, :class_name => "User", :foreign_key => "owner_id" + after_create "assign_owner_roles(owner)"
before_validation :generate_uuid before_save :update_xml
- has_many :permissions, :as => :permission_object, :dependent => :destroy, - :include => [:role], - :order => "permissions.id ASC" - validates_presence_of :uuid validates_uniqueness_of :uuid validates_presence_of :name validates_uniqueness_of :name validates_length_of :name, :maximum => 255 + validates_presence_of :owner_id
before_destroy :destroyable?
diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb index 85698d6..b75b49d 100644 --- a/src/app/models/deployment.rb +++ b/src/app/models/deployment.rb @@ -49,11 +49,12 @@ class Deployment < ActiveRecord::Base
belongs_to :realm belongs_to :frontend_realm - belongs_to :owner, :class_name => "User", :foreign_key => "owner_id"
has_many :permissions, :as => :permission_object, :dependent => :destroy, :include => [:role], :order => "permissions.id ASC" + belongs_to :owner, :class_name => "User", :foreign_key => "owner_id" + after_create "assign_owner_roles(owner)"
validates_presence_of :pool_id validates_presence_of :deployable_id @@ -121,7 +122,6 @@ class Deployment < ActiveRecord::Base :owner => user, :hardware_profile => HardwareProfile.find(hw_profiles[assembly.id.to_s]) ) - instance.assign_owner_roles(user) task = InstanceTask.create!({:user => user, :task_target => instance, :action => InstanceTask::ACTION_CREATE}) diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb index 2f5776d..c9c5c7f 100644 --- a/src/app/models/instance.rb +++ b/src/app/models/instance.rb @@ -89,6 +89,7 @@ class Instance < ActiveRecord::Base :include => [:role], :order => "permissions.id ASC" has_many :events, :as => :source, :dependent => :destroy + after_create "assign_owner_roles(owner)"
validates_presence_of :pool_id validates_presence_of :hardware_profile_id diff --git a/src/app/models/template.rb b/src/app/models/template.rb index b8e8804..004a47b 100644 --- a/src/app/models/template.rb +++ b/src/app/models/template.rb @@ -45,6 +45,9 @@ class Template < ActiveRecord::Base has_many :permissions, :as => :permission_object, :dependent => :destroy, :include => [:role], :order => "permissions.id ASC" + belongs_to :owner, :class_name => "User", :foreign_key => "owner_id" + after_create "assign_owner_roles(owner)" + after_create :ensure_assembly
validates_presence_of :uuid validates_uniqueness_of :uuid @@ -54,8 +57,7 @@ class Template < ActiveRecord::Base validates_presence_of :platform validates_presence_of :platform_version validates_presence_of :architecture - - after_create :ensure_assembly + validates_presence_of :owner_id
def no_instances? unless instances.empty? @@ -177,7 +179,8 @@ class Template < ActiveRecord::Base def ensure_assembly self.assemblies.create!({ :name => self.name, - :architecture => self.architecture + :architecture => self.architecture, + :owner => self.owner }) unless self.assemblies.count > 0 end end diff --git a/src/db/migrate/20110427071851_add_owner_id.rb b/src/db/migrate/20110427071851_add_owner_id.rb new file mode 100644 index 0000000..205bea4 --- /dev/null +++ b/src/db/migrate/20110427071851_add_owner_id.rb @@ -0,0 +1,13 @@ +class AddOwnerId < ActiveRecord::Migration + def self.up + add_column :templates, :owner_id, :integer + add_column :assemblies, :owner_id, :integer + add_column :deployables, :owner_id, :integer + end + + def self.down + remove_column :templates, :owner_id + remove_column :assemblies, :owner_id + remove_column :deployables, :owner_id + end +end diff --git a/src/features/step_definitions/assembly_steps.rb b/src/features/step_definitions/assembly_steps.rb index df70370..383114d 100644 --- a/src/features/step_definitions/assembly_steps.rb +++ b/src/features/step_definitions/assembly_steps.rb @@ -1,14 +1,14 @@ Given /^there is an assembly named "([^"]*)"$/ do |name| - Assembly.create!(:name => name, :architecture => 'x86_64') + Assembly.create!(:name => name, :architecture => 'x86_64', :owner => user) end
Given /^there is an assembly named "([^"]*)" belonging to "([^"]*)"$/ do |assembly_name, deployable_name| deployable = Deployable.find_by_name(deployable_name) - deployable.assemblies.create!(:name => assembly_name, :architecture => 'x86_64') + deployable.assemblies.create!(:name => assembly_name, :architecture => 'x86_64', :owner => user) # Assembly.create!(:name => assembly, :architecture => 'x86_64', :deployable => Deployable.find_by_name(deployable)) end
When /^I check the "([^"]*)" assembly$/ do |name| assembly = Assembly.find_by_name(name) check("assembly_checkbox_#{assembly.id}") -end \ No newline at end of file +end diff --git a/src/features/step_definitions/deployable_steps.rb b/src/features/step_definitions/deployable_steps.rb index 539510c..374d43e 100644 --- a/src/features/step_definitions/deployable_steps.rb +++ b/src/features/step_definitions/deployable_steps.rb @@ -11,7 +11,7 @@ Then /^I should have a deployable named "([^"]*)"$/ do |name| end
Given /^there is a deployable named "([^"]*)"$/ do |name| - @deployable = Deployable.create!(:name => name) + @deployable = Deployable.create!(:name => name, :owner => user) end
When /^I check the "([^"]*)" deployable$/ do |name| diff --git a/src/features/step_definitions/deployment_steps.rb b/src/features/step_definitions/deployment_steps.rb index 870c60c..7af353c 100644 --- a/src/features/step_definitions/deployment_steps.rb +++ b/src/features/step_definitions/deployment_steps.rb @@ -1,5 +1,5 @@ Given /^there is a deployment named "([^"]*)" belonging to "([^"]*)" owned by "([^"]*)"$/ do |deployment_name, deployable_name, owner_name| user = Factory(:user, :login => owner_name) - deployable = Deployable.create!(:name => deployable_name) + deployable = Deployable.create!(:name => deployable_name, :owner => user) deployable.deployments.create!({:name => deployment_name, :pool => Pool.first, :owner => user}) end diff --git a/src/spec/factories/assembly.rb b/src/spec/factories/assembly.rb index af290dc..dd55e0b 100644 --- a/src/spec/factories/assembly.rb +++ b/src/spec/factories/assembly.rb @@ -1,5 +1,6 @@ Factory.define :assembly do |a| a.sequence(:name) { |n| "assembly#{n}" } a.architecture 'x86_64' + a.association :owner, :factory => :user a.templates { |t| [t.association(:template)] } end diff --git a/src/spec/factories/deployable.rb b/src/spec/factories/deployable.rb index 60b160b..62699ba 100644 --- a/src/spec/factories/deployable.rb +++ b/src/spec/factories/deployable.rb @@ -1,4 +1,5 @@ Factory.define :deployable do |a| a.sequence(:name) { |n| "deployable#{n}" } a.assemblies { |t| [t.association(:assembly)] } + a.association :owner, :factory => :user end diff --git a/src/spec/factories/template.rb b/src/spec/factories/template.rb index 9640cbc..3c71acd 100644 --- a/src/spec/factories/template.rb +++ b/src/spec/factories/template.rb @@ -1,6 +1,7 @@ Factory.define :template do |i| i.sequence(:name) { |n| "template#{n}" } i.platform 'fedora13' + i.association :owner, :factory => :user i.after_build do |tpl| if tpl.respond_to?(:stub!) tpl.stub!(:upload).and_return(true)
From: Jan Provaznik jprovazn@redhat.com
--- .../image_factory/image_imports_controller.rb | 2 +- src/app/models/image.rb | 13 +++++++------ src/lib/tasks/dc_tasks.rake | 4 ++-- src/spec/models/image_spec.rb | 2 +- 4 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/src/app/controllers/image_factory/image_imports_controller.rb b/src/app/controllers/image_factory/image_imports_controller.rb index 87128af..45ba6d6 100644 --- a/src/app/controllers/image_factory/image_imports_controller.rb +++ b/src/app/controllers/image_factory/image_imports_controller.rb @@ -7,7 +7,7 @@ class ImageFactory::ImageImportsController < ApplicationController
def create begin - Image.import(ProviderAccount.find(params[:provider_account_id]), params[:ami_id]) + Image.import(ProviderAccount.find(params[:provider_account_id]), params[:ami_id], current_user) flash[:notice]="Image successfully imported" redirect_to image_factory_templates_path kick_condor diff --git a/src/app/models/image.rb b/src/app/models/image.rb index 923dd23..00a674b 100644 --- a/src/app/models/image.rb +++ b/src/app/models/image.rb @@ -119,16 +119,16 @@ class Image < ActiveRecord::Base Delayed::Job.enqueue(BuildJob.new(img.id)) img end - def self.single_import(providername, username, password, image_id) + def self.single_import(providername, username, password, image_id, user) account = Image.get_account(providername, username, password) - Image.import(account, image_id) + Image.import(account, image_id, user) end
- def self.bulk_import(providername, username, password, images) + def self.bulk_import(providername, username, password, images, user) account = Image.get_account(providername, username, password) images.each do |image| begin - Image.import(account, image['id']) + Image.import(account, image['id'], user) $stderr.puts "imported image with id '#{image['id']}'" rescue $stderr.puts "failed to import image with id '#{image['id']}'" @@ -136,7 +136,7 @@ class Image < ActiveRecord::Base end end
- def self.import(account, image_id) + def self.import(account, image_id, user) if image_id.blank? raise "image id should not be blank" end @@ -158,7 +158,8 @@ class Image < ActiveRecord::Base :architecture => raw_image.architecture}, :complete => true, :uploaded => true, - :imported => true + :imported => true, + :owner => user ) template.save!
diff --git a/src/lib/tasks/dc_tasks.rake b/src/lib/tasks/dc_tasks.rake index 8255d32..ea775a9 100644 --- a/src/lib/tasks/dc_tasks.rake +++ b/src/lib/tasks/dc_tasks.rake @@ -92,7 +92,7 @@ namespace :dc do exit(1) end #account = get_account(args.provider, args.account) - img = Image.single_import(args.provider, args.username, args.password, args.image) + img = Image.single_import(args.provider, args.username, args.password, args.image, User.find_by_login('admin')) puts "Imported image with id '#{args.image}'" end
@@ -103,7 +103,7 @@ namespace :dc do exit(1) end list = YAML.load_file(args.yml_file) - Image.bulk_import(args.provider, args.username, args.password, list) + Image.bulk_import(args.provider, args.username, args.password, list, User.find_by_login('admin')) end
def get_account(provider_name, account_name) diff --git a/src/spec/models/image_spec.rb b/src/spec/models/image_spec.rb index 9635f20..b902fe1 100644 --- a/src/spec/models/image_spec.rb +++ b/src/spec/models/image_spec.rb @@ -61,7 +61,7 @@ describe Image do lambda do lambda do lambda do - img = Image.import(account, 'mock') + img = Image.import(account, 'mock', Factory(:user)) img.should_not be_nil img.template.uploaded.should be_true end.should change(Image, :count).by(1)
On 04/27/2011 03:05 PM, jprovazn@redhat.com wrote:
This patch updates assign_owner_roles call only for template/assembly/deployable/deployment/instance. TODO: pool, pool_family, provider, provider_account
aeolus-devel mailing list aeolus-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/aeolus-devel
ACK to the series
aeolus-devel@lists.fedorahosted.org