Looks good functionally and test wise, though the couple small items
raised before still apply, eg
On 03/15/2010 03:24 PM, Jason Guiditta wrote:
> <snip>
> diff --git a/src/app/services/registration_service.rb
> b/src/app/services/registration_service.rb
> new file mode 100644
> index 0000000..e8d2e65
> --- /dev/null
> +++ b/src/app/services/registration_service.rb
> @@ -0,0 +1,29 @@
> +class RegistrationService
> +
> + def initialize(user)
> + @user = user
> + end
> +
> + def save
> + return false unless valid?
> + begin
> + User.transaction do
> + @user.save!
> + PortalPool.transaction do
> + @portal_pool = PortalPool.create!({ :name => @user.login, :owner =>
> @user})
> + Permission.transaction do
> + Permission.create!({:user => @user,
> + :role => Role.find_by_name("Self-service Pool
> User"),
> + :permission_object => @portal_pool})
> +
>
Just think we need one transaction here.
<snip>
> diff --git a/src/spec/models/registration_service_spec.rb
> b/src/spec/models/registration_service_spec.rb
> new file mode 100644
> index 0000000..aa25a1e
> --- /dev/null
> +++ b/src/spec/models/registration_service_spec.rb
> @@ -0,0 +1,51 @@
> +require 'spec_helper'
> +
> +describe RegistrationService do
> + fixtures :all
> + before(:each) do
> + @tuser = Factory :tuser
> + end
> +
> + it "should initialize a new instance given valid attributes" do
> + RegistrationService.new(@tuser)
> + end
> +
> + describe "#save" do
> +
> + context "adding valid user with no errors" do
> + it "should create user, portal_pool and self-service permission" do
> + user = User.new({:login => 'gooduser',
> + :email => '[email protected]',
> + :password => 'password',
> + :password_confirmation => 'password'})
> + r = RegistrationService.new(user)
> + roles = Role.find(:all)
> + Rails::logger.info("Current Roles: ")
> + Rails::logger.info(roles.each {|role| role.name})
> + Rails::logger.info("ERRORS - Printing errors on user object:")
> + Rails::logger.info(
> + user.errors.each_full { |msg| "ERROR: #{msg}" }
> + )
>
This logging code should be removed, should it not?
-Mo
_______________________________________________
deltacloud-devel mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/deltacloud-devel
