On Mon, Mar 15, 2010 at 7:32 PM, Mohammed Morsi <[email protected]> wrote:
> 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.
>
> Ah, good catch, forgot about that, same for below. I'll fix those in am
before I push this.
>
> <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
>
_______________________________________________
deltacloud-devel mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/deltacloud-devel
