views:

293

answers:

1

I'm doing maintenance work on a Rails site that's deployed using Phusion Passenger. The workflow is a little different than the standard three-tiered Railsian test-dev-production arrangement; instead, there are two separate installations of the same codebase running against parallel Oracle databases; the dev site lives at qa.domain.com, and the live site at www.domain.com

I'm experiencing different behavior in the following code snippet (from 'vendors_controller.rb', which uses AuthenticatedSystem) between the two environments:

def create
  @user = current_user || User.new(params[:user])
  @registration = Registration.new(params[:registration])

  unless current_user

    @user.user_type = 'vendor'
    @user.active = 1

    if @user.save
      @user.activate!
      @user.has_role 'owner', @user
      @user.has_role 'vendor'
      self.current_user = user = @user

      @registration.active = 1
      @registration.email = @user.email
      @registration.user_id = @user.id
      if @registration.save
        send_confirmation(@user)
        send_solicitations_notifications(@registration) if @registration.notification_desired == true
        redirect_to thank_you_vendors_path
      else
        # THIS BEHAVIOR DIFFERS ACROSS PRODUCTION AND DEVELOPMENT
        @user.destroy
        self.current_user = user = nil
        # END DIFFERENCE
        respond_to do |format|
          format.html { render :action => 'new' }
          format.xml  { render :xml => @registration.errors, :status => :unprocessable_entity }
        end
      end

    else
      respond_to do |format|
        format.html  { render :action => 'new' }
        format.xml  { render :xml => @user.errors, :status => :unprocessable_entity }
      end
    end
...

The code between the comments destroys the just-created user object if the system was unable to create a corresponding registration. It works fine on the development server but not on the production server, where the User object stubbornly hangs around the database even if saving the registration fails. Pushing the changes to production is a simple matter of uploading the controllers file and doing touch tmp/restart.txt via the shell. The two codebases are otherwise identical; what might be causing this difference?

Thanks for your consideration!

Justin

EDIT: There are a few differences in production.rb across the two installations that might help diagnose the problem. On production,

config.cache_classes = true

# Full error reports are disabled and caching is turned on
config.action_controller.consider_all_requests_local = false
config.action_controller.perform_caching             = true

while on development, those three flags are set to their inverse values. Thanks!

A: 

There are a few things you should consider changing about your code:

  1. You're not using transactions
  2. You're doing way too much in the controller

Having said that the reason why you're problem is happening is probably due to the environment differences between production and development, most likely this:

config.cache_classes = false

However, I don't think you should change this in production as it'll slow down all your actions. Instead I recommend having a staging environment which closely matches your production environment.

To fix your problem, I'd most likely rewrite the action like this:

# using before filters will keep your actions tight
before_filter :cannot_create_user, :if => :signed_in?

def create
  # setup all the objects
  @user = User.new(params[:user])

  @user.user_type = 'vendor'
  @user.active = 1

  @user.has_role 'owner', @user
  @user.has_role 'vendor'

  @registration = @user.registrations.build(params[:registration])
  @registration.active = 1
  @registration.email = @user.email

  # make sure everything is valid before trying to save and activate
  if @user.valid?
    @user.save!  # might not need this if activate calls save!
    @user.activate!

    # this should probably be a sign_in() method... 
    self.current_user = @user

    send_confirmation(@user)
    send_solicitations_notifications(@registration) if @registration.notification_desired?

    redirect_to thank_you_vendors_path
  else
    respond_to do |format|
      format.html { render :action => 'new' }
      format.xml  { render :xml => @registration.errors, :status => :unprocessable_entity }
    end
  end

...
end


protected 

def signed_in?
  !current_user.nil?
end

def cannot_create_user
  respond_to do |format|
    format.html  { render :action => 'new' }
    format.xml  { render :xml => @user.errors, :status => :unprocessable_entity }
  end
end

n.b. I didn't test this, it might not work, but you should get the idea... if you have unit tests (which I hope you do...) you should be able to drop it in and see if it works!

Your next step would be to use use accepts_nested_attribute_for for your registration object, so that it can be submitted as part of the user params.

I'd also refactor this so that all the role settings etc... are done in callbacks.

At this point your create action will most likely be really simple and you can switch your controller to use inherited resources.

I hope this helps!

jonnii
Thanks for the response, jonnii...much appreciated! I didn't write the code myself--I'm just debugging it--but your solution is much more elegant. Also, it turns out that the problem was with Phusion Passenger; I'd renamed the old version of the file to 'vendors_controller_031610.rb' in case I needed to revert quickly and it turns out that Phusion was using the outdated controller, even after being restarted. Deleting the old file solved the problem. Strange, eh?
justinbach