[email protected] wrote:
> From: martyntaylor <[email protected]>
>
> rebase
> ---
> src/app/controllers/pool_controller.rb | 7 +-----
> src/app/models/pool.rb | 3 --
> src/app/models/user.rb | 1 -
> src/app/services/registration_service.rb | 3 +-
> src/db/migrate/20090803141507_create_pools.rb | 1 -
> src/features/pool.feature | 10 ++++----
> src/features/step_definitions/pool_steps.rb | 6 +++-
> src/spec/controllers/users_controller_spec.rb | 2 +-
> src/spec/factories/permission.rb | 6 +++++
> src/spec/factories/pool.rb | 1 -
> .../services/data_service_active_record_spec.rb | 22 +++++++------------
> 11 files changed, 26 insertions(+), 36 deletions(-)
>
> diff --git a/src/app/controllers/pool_controller.rb
> b/src/app/controllers/pool_controller.rb
> index 187da09..baa9431 100644
> --- a/src/app/controllers/pool_controller.rb
> +++ b/src/app/controllers/pool_controller.rb
> @@ -79,11 +79,6 @@ class PoolController < ApplicationController
> def create
> require_privilege(Privilege::POOL_MODIFY)
>
> - #FIXME: owner is set to current user for self-service account creation,
> - # but in the more general case we need a way for the admin to pick
> - # a user
> - params[:pool][:owner_id] = @current_user.id
> -
> #FIXME: This should probably be in a transaction
> @pool = Pool.new(params[:pool])
> # FIXME: do we need any more handling around save failures? What if perm
> @@ -96,7 +91,7 @@ class PoolController < ApplicationController
>
> @pool.zone = Zone.default
> @pool.save!
> - perm = Permission.new(:user => @pool.owner,
> + perm = Permission.new(:user => @current_user,
> :role => Role.find_by_name("Instance Creator and
> User"),
> :permission_object => @pool)
>
I'm not sure this part is really desired here -- an admin will be
creating the pool, but we're giving basic user level permissions here.
Although we need to set self-service permissions on the pool when new
users are added, I don't really think we need to add this at pool
creation time for the admin.
I think we should leave this part out entirely, unless leaving it out
breaks something here.
Scott
> perm.save!
> diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
> index d186b20..582b440 100644
> --- a/src/app/models/pool.rb
> +++ b/src/app/models/pool.rb
> @@ -22,7 +22,6 @@
> class Pool < ActiveRecord::Base
> include PermissionedObject
> has_many :instances, :dependent => :destroy
> - belongs_to :owner, :class_name => "User", :foreign_key => "owner_id"
> belongs_to :quota
> belongs_to :zone
>
> @@ -31,10 +30,8 @@ class Pool < ActiveRecord::Base
>
>
>
> - validates_presence_of :owner_id
> validates_presence_of :name
> validates_presence_of :zone
> - validates_uniqueness_of :name, :scope => :owner_id
> validates_uniqueness_of :exported_as, :if => :exported_as
>
> has_many :permissions, :as => :permission_object, :dependent => :destroy,
> diff --git a/src/app/models/user.rb b/src/app/models/user.rb
> index fc0cbdc..0281a45 100644
> --- a/src/app/models/user.rb
> +++ b/src/app/models/user.rb
> @@ -23,7 +23,6 @@ class User < ActiveRecord::Base
> acts_as_authentic
>
> has_many :permissions
> - has_many :owned_pools, :class_name => "Pool", :foreign_key => "owner_id"
> has_many :owned_instances, :class_name => "Instance", :foreign_key =>
> "owner_id"
>
> belongs_to :quota
> diff --git a/src/app/services/registration_service.rb
> b/src/app/services/registration_service.rb
> index a9fbfca..140b92e 100644
> --- a/src/app/services/registration_service.rb
> +++ b/src/app/services/registration_service.rb
> @@ -14,8 +14,7 @@ class RegistrationService
> begin
> User.transaction do
> @user.save!
> - @pool = Pool.create!({ :name => @user.login, :owner => @user,
> - :zone => Zone.default})
> + @pool = Pool.create!({ :name => @user.login, :zone => Zone.default})
>
> @quota = Quota.new
> @quota.save!
> diff --git a/src/db/migrate/20090803141507_create_pools.rb
> b/src/db/migrate/20090803141507_create_pools.rb
> index d60687d..e0c1dc7 100644
> --- a/src/db/migrate/20090803141507_create_pools.rb
> +++ b/src/db/migrate/20090803141507_create_pools.rb
> @@ -24,7 +24,6 @@ class CreatePools < ActiveRecord::Migration
> create_table :pools do |t|
> t.string :name, :null => false
> t.string :exported_as
> - t.integer :owner_id, :null => false
> t.integer :quota_id
> t.integer :zone_id, :null => false
> t.integer :lock_version, :default => 0
> diff --git a/src/features/pool.feature b/src/features/pool.feature
> index ee1af53..ae0f10a 100644
> --- a/src/features/pool.feature
> +++ b/src/features/pool.feature
> @@ -21,7 +21,7 @@ Feature: Manage Pools
> And I should have a pool named "mockpool"
>
> Scenario: View Pool's Hardware Profiles
> - Given I own a pool named "mockpool"
> + Given I have Pool Creator permissions on a pool named "mockpool"
> And the Pool has the following Hardware Profiles:
> | name | memory | cpu |storage | architecture |
> | m1-small | 1740 | 2 | 160.0 | i386 |
> @@ -37,7 +37,7 @@ Feature: Manage Pools
> | m1-xlarge | 8192 | 8 | 1690.0 | x86_64 |
>
> Scenario: View Pool's Realms
> - Given I own a pool named "mockpool"
> + Given I have Pool Creator permissions on a pool named "mockpool"
> And the Pool has the following Realms named "Europe, United States"
> And I am on the instances page
> When I follow "mockpool"
> @@ -48,7 +48,7 @@ Feature: Manage Pools
>
> @tag
> Scenario: View Pool's Quota Usage
> - Given I own a pool named "mockpool"
> + Given I have Pool Creator permissions on a pool named "mockpool"
> And the Pool has a quota with following capacities:
> | resource | capacity |
> | maximum_running_instances | 10 |
> @@ -60,5 +60,5 @@ Feature: Manage Pools
> Then I should be on the show pool page
> When I follow "Quota"
> Then I should see the following:
> - | Running Instances | 10 | 8 | 2 |
> - | Total Instances | 15 | 15 | 0 |
> \ No newline at end of file
> + | Running Instances | 10 | 8 |
> + | Total Instances | 15 | 15 |
> \ No newline at end of file
> diff --git a/src/features/step_definitions/pool_steps.rb
> b/src/features/step_definitions/pool_steps.rb
> index 75de2e2..5643f48 100644
> --- a/src/features/step_definitions/pool_steps.rb
> +++ b/src/features/step_definitions/pool_steps.rb
> @@ -3,9 +3,9 @@ Given /^I am an authorised user$/ do
> @user = @admin_permission.user
> end
>
> -Given /^I own a pool named "([^\"]*)"$/ do |name|
> +Given /^I have Pool Creator permissions on a pool named "([^\"]*)"$/ do
> |name|
> @pool = Factory(:pool, :name => name)
> - @user.owned_pools << @pool
> + Factory(:pool_creator_permission, :user => @user, :permission_object =>
> @pool)
> end
>
> Given /^the Pool has the following Hardware Profiles:$/ do |table|
> @@ -65,6 +65,8 @@ Given /^the Pool has a quota with following capacities:$/
> do |table|
> end
>
> @quota = Factory(:quota, quota_hash)
> + @quota.save!
> +
> @pool.quota_id = @quota.id
> @pool.save
> end
> diff --git a/src/spec/controllers/users_controller_spec.rb
> b/src/spec/controllers/users_controller_spec.rb
> index 29c2f41..5add3c7 100644
> --- a/src/spec/controllers/users_controller_spec.rb
> +++ b/src/spec/controllers/users_controller_spec.rb
> @@ -31,7 +31,7 @@ describe UsersController do
> }.should change{ User.count }
> p = Pool.find_by_name("tuser2")
> p.should_not be_nil
> - assigns[:user].login.should == p.owner.login
> +
> p.name.should == "tuser2"
> p.permissions.size.should == 1
> p.permissions.any? {
> diff --git a/src/spec/factories/permission.rb
> b/src/spec/factories/permission.rb
> index 0aad3a2..8b9fd69 100644
> --- a/src/spec/factories/permission.rb
> +++ b/src/spec/factories/permission.rb
> @@ -19,3 +19,9 @@ Factory.define :pool_creator_permission, :parent =>
> :permission do |p|
> p.permission_object { |r| r.association(:mock_provider) }
> p.user { |u| u.association(:pool_creator) }
> end
> +
> +Factory.define :instance_creator_and_user_permission, :parent => :permission
> do |p|
> + p.role { |r| Role.find(:first, :conditions => ['name = ?', 'Instance
> Creator and User']) }
> + p.permission_object { |r| r.association(:pool) }
> + p.user { |u| u.association(:user) }
> +end
> \ No newline at end of file
> diff --git a/src/spec/factories/pool.rb b/src/spec/factories/pool.rb
> index 12a4376..da5525e 100644
> --- a/src/spec/factories/pool.rb
> +++ b/src/spec/factories/pool.rb
> @@ -1,6 +1,5 @@
> Factory.define :pool do |p|
> p.name 'mypool'
> - p.association :owner, :factory => :pool_user
> p.association :zone, :factory => :zone
> end
>
> diff --git a/src/spec/services/data_service_active_record_spec.rb
> b/src/spec/services/data_service_active_record_spec.rb
> index 9842146..b443fa9 100644
> --- a/src/spec/services/data_service_active_record_spec.rb
> +++ b/src/spec/services/data_service_active_record_spec.rb
> @@ -58,8 +58,7 @@ describe DataServiceActiveRecord do
> end
>
> it "should calculate the average, max and min task submission times" do
> - user = Factory :user
> - pool = Factory(:pool, :owner => user)
> + pool = Factory(:pool)
> instance = Factory(:instance, :pool_id => pool.id)
>
> start_time = Time.utc(2010,"jan",1,20,15,1)
> @@ -86,8 +85,7 @@ describe DataServiceActiveRecord do
> end
>
> it "should create data points for the average, max and min task submission
> times between two times at given intervals" do
> - user = Factory :user
> - pool = Factory(:pool, :owner => user)
> + pool = Factory(:pool)
> instance = Factory(:instance, :pool_id => pool.id)
>
> expected_averages = [ 20, 40, 60, 80, 100]
> @@ -164,8 +162,7 @@ describe DataServiceActiveRecord do
> runtime3 = [100, 200, 300, 400, 500]
> runtimes = [runtime1, runtime2, runtime3]
>
> - user = Factory :pool_user
> - pool = Factory(:pool, :owner => user)
> + pool = Factory(:pool)
> cloud_account = Factory :mock_cloud_account
>
> for i in 0..2 do
> @@ -188,8 +185,7 @@ describe DataServiceActiveRecord do
> end
>
> it "should generate the mean max and min instance runtimes of instances
> for a given cloud account or pool" do
> - user = Factory :pool_user
> - pool = Factory(:pool, :owner => user)
> + pool = Factory(:pool)
>
> cloud_account = Factory :mock_cloud_account
>
> @@ -208,8 +204,7 @@ describe DataServiceActiveRecord do
> end
>
> it "should calculate the average time it takes a provider to complete a
> task between two times" do
> - user = Factory :pool_user
> - pool = Factory(:pool, :owner => user)
> + pool = Factory(:pool)
> cloud_account = Factory(:mock_cloud_account)
> instance = Factory(:instance, :pool => pool, :cloud_account =>
> cloud_account)
>
> @@ -246,8 +241,8 @@ describe DataServiceActiveRecord do
> start_time = Time.utc(2010,"jan",1,20,15,1)
> create_time = start_time + 1
> end_time = create_time + 1
> - user = Factory :pool_user
> - pool = Factory(:pool, :owner => user)
> +
> + pool = Factory(:pool)
> cloud_account = Factory :mock_cloud_account
> instance = Factory(:instance, :pool => pool, :cloud_account =>
> cloud_account)
>
> @@ -293,8 +288,7 @@ describe DataServiceActiveRecord do
> failures = [5, 10, 15]
> number_of_instances = 20
>
> - user = Factory :pool_user
> - pool = Factory(:pool, :owner => user)
> + pool = Factory(:pool)
> cloud_account = Factory :mock_cloud_account
> instance = Factory(:instance, :pool => pool, :cloud_account =>
> cloud_account)
>
>
_______________________________________________
deltacloud-devel mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/deltacloud-devel
