diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index e15d563df..4607e508a 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -39,14 +39,14 @@ class SessionsController < ApplicationController # This should probably be in a transaction somehow, meaning the user # create and the authorization create. Concern is UserManager.new.signup sends # an email and whatnot. - user = UserManager.new.signup(auth_hash[:info][:first_name], + # + # Also, should we grab their photo from facebook? + user = UserManager.new.signup(request.remote_ip, + auth_hash[:info][:first_name], auth_hash[:info][:last_name], auth_hash[:info][:email], nil, nil, - nil, # city - nil, # state - nil, # country nil, # instruments nil, # photo_url nil) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 0447ba6fe..f4bb2a1da 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -25,19 +25,16 @@ class UsersController < ApplicationController render 'new' return end - - - @user = UserManager.new.signup(params[:jam_ruby_user][:first_name], + + @user = UserManager.new.signup(request.remote_ip, + params[:jam_ruby_user][:first_name], params[:jam_ruby_user][:last_name], params[:jam_ruby_user][:email], params[:jam_ruby_user][:password], params[:jam_ruby_user][:password_confirmation], - params[:jam_ruby_user][:city], - params[:jam_ruby_user][:state], - params[:jam_ruby_user][:country], params[:jam_ruby_user][:instruments], params[:jam_ruby_user][:photo_url], - ApplicationHelper.base_uri(request) + "/confirm") + ApplicationHelper.base_uri(request) + "/confirm") # check for errors if @user.errors.any? diff --git a/lib/managers/max_mind_manager.rb b/lib/managers/max_mind_manager.rb index 7cd4e8c71..f5546f830 100644 --- a/lib/managers/max_mind_manager.rb +++ b/lib/managers/max_mind_manager.rb @@ -6,18 +6,21 @@ class MaxMindManager < BaseManager # Returns a hash with location information. Fields are nil if they can't be figured. + # This is a class method because it doesn't need to be in a transaction. def self.lookup(ip_address) city = state = country = nil - ActiveRecord::Base.connection_pool.with_connection do |connection| - pg_conn = connection.instance_variable_get("@connection") - ip_as_int = ip_address_to_int(ip_address) - pg_conn.exec("SELECT country, region, city FROM max_mind WHERE ip_bottom <= $1 AND ip_top >= $2", [ip_as_int, ip_as_int]) do |result| - if !result.nil? && result.ntuples > 0 - country = result.getvalue(0, 0) - state = result[0]['region'] - city = result[0]['city'] + unless ip_address.nil? || ip_address !~ /^\d+\.\d+\.\d+\.\d+$/ + ActiveRecord::Base.connection_pool.with_connection do |connection| + pg_conn = connection.instance_variable_get("@connection") + ip_as_int = ip_address_to_int(ip_address) + pg_conn.exec("SELECT country, region, city FROM max_mind WHERE ip_bottom <= $1 AND ip_top >= $2", [ip_as_int, ip_as_int]) do |result| + if !result.nil? && result.ntuples > 0 + country = result.getvalue(0, 0) + state = result[0]['region'] + city = result[0]['city'] + end end end end diff --git a/lib/managers/user_manager.rb b/lib/managers/user_manager.rb index 8262fc3fe..4c7f37d1f 100644 --- a/lib/managers/user_manager.rb +++ b/lib/managers/user_manager.rb @@ -10,11 +10,13 @@ class UserManager < BaseManager # Note that almost everything can be nil here. This is because when users sign up via social media, # we don't know much about them. - def signup(first_name, last_name, email, password = nil, password_confirmation = nil, - city = nil, state = nil, country = nil, instruments = nil, photo_url = nil, signup_confirm_url = nil) + def signup(remote_ip, first_name, last_name, email, password = nil, password_confirmation = nil, + instruments = nil, photo_url = nil, signup_confirm_url = nil) @user = User.new + location_hash = MaxMindManager.lookup(remote_ip); + # TODO: figure out why can't user verify_recaptcha here # ALSO: make sure we dont do the recaptcha stuff if used facebook. @@ -22,9 +24,10 @@ class UserManager < BaseManager #unless verify_recaptcha(:model => @user, :message => "recaptcha") # return @user # @user.errors.any? is true now #else - # sends email to email account for confirmation - @user = User.signup(first_name, last_name, email, password, password_confirmation, - city, state, country, instruments, photo_url, signup_confirm_url) + # sends email to email account for confirmation + @user = User.signup(first_name, last_name, email, password, password_confirmation, + location_hash[:city], location_hash[:state], location_hash[:country], + instruments, photo_url, signup_confirm_url) return @user #end diff --git a/spec/managers/user_manager_spec.rb b/spec/managers/user_manager_spec.rb index d6da1f846..894df7c8a 100644 --- a/spec/managers/user_manager_spec.rb +++ b/spec/managers/user_manager_spec.rb @@ -11,16 +11,16 @@ describe UserManager do describe "signup" do it "signup successfully" do - @user = @user_manager.signup("bob", "smith", "bob@jamkazam.com", "foobar", "foobar", "Austin", "TX", "USA", nil, nil, "http://localhost:3000/confirm" ) + @user = @user_manager.signup("127.0.0.1", "bob", "smith", "bob@jamkazam.com", "foobar", "foobar", nil, nil, "http://localhost:3000/confirm" ) @user.errors.any?.should be_false @user.first_name.should == "bob" @user.last_name.should == "smith" @user.email.should == "bob@jamkazam.com" @user.email_confirmed.should be_false - @user.city.should == "Austin" - @user.state.should == "TX" - @user.country.should == "USA" + @user.city.should be_nil + @user.state.should be_nil + @user.country.should be_nil @user.instruments.length.should == 0 @user.signup_token.should_not be_nil @@ -28,7 +28,7 @@ describe UserManager do end it "signup successfully with instruments" do - @user = @user_manager.signup("bob", "smith", "bob@jamkazam.com", "foobar", "foobar", "Austin", "TX", "USA", + @user = @user_manager.signup("127.0.0.1", "bob", "smith", "bob@jamkazam.com", "foobar", "foobar", [{ :instrument_id => "electric guitar", :proficiency_level => 3, :priority => 0}], nil, "http://localhost:3000/confirm") @user.errors.any?.should be_false @@ -38,36 +38,59 @@ describe UserManager do musician_instrument.proficiency_level.should == 3 end + it "doesnt fail if ip address is nil" do + @user = @user_manager.signup(nil, "bob", "smith", "bob@jamkazam.com", "foobar", "foobar", nil, nil, "http://localhost:3000/confirm" ) + + @user.errors.any?.should be_false + @user.city.should be_nil + @user.state.should be_nil + @user.country.should be_nil + end + + it "sets the location properly from maxmind" do + MaxMindManager.active_record_transaction do |manager| + manager.create_phony_database() + end + @user = @user_manager.signup("127.0.0.1", "bob", "smith", "bob@jamkazam.com", "foobar", "foobar", nil, nil, "http://localhost:3000/confirm" ) + + @user.errors.any?.should be_false + @user.city.should == 'City 127' + @user.state.should == 'Region 127' + @user.country.should == 'Country 127' + end + + it "duplicate signup failure" do - @user = @user_manager.signup("bob", "smith", "bob@jamkazam.com", "foobar", "foobar", "Austin", "TX", "USA", nil, nil, "http://localhost:3000/confirm") + @user = @user_manager.signup("127.0.0.1", "bob", "smith", "bob@jamkazam.com", "foobar", "foobar", nil, nil, "http://localhost:3000/confirm") UserMailer.deliveries.length.should == 1 @user.errors.any?.should be_false # exactly the same parameters; should dup on email, and send no email - @user = @user_manager.signup("bob", "smith", "bob@jamkazam.com", "foobar", "foobar", "Austin", "TX", "USA", nil, nil, "http://localhost:3000/confirm") + @user = @user_manager.signup("127.0.0.1", "bob", "smith", "bob@jamkazam.com", "foobar", "foobar", nil, nil, "http://localhost:3000/confirm") UserMailer.deliveries.length.should == 1 @user.errors.any?.should be_true @user.errors[:email][0].should == "has already been taken" end it "fail on no username" do - @user = @user_manager.signup("", "", "bob@jamkazam.com", "foobar", "foobar", "Austin", "TX", "USA", nil, nil, "http://localhost:3000/confirm") + @user = @user_manager.signup("127.0.0.1", "", "", "bob@jamkazam.com", "foobar", "foobar", nil, nil, "http://localhost:3000/confirm") UserMailer.deliveries.length.should == 0 @user.errors.any?.should be_true @user.errors[:first_name][0].should == "can't be blank" end it "fail on no email" do - @user = @user_manager.signup("murp", "blurp", "", "foobar", "foobar", "Austin", "TX", "USA", nil, nil, "http://localhost:3000/confirm" ) + @user = @user_manager.signup("127.0.0.1", "murp", "blurp", "", "foobar", "foobar", nil, nil, "http://localhost:3000/confirm" ) UserMailer.deliveries.length.should == 0 @user.errors.any?.should be_true @user.errors[:email][0].should == "can't be blank" end + end describe "signup_confirm" do it "fail on no username" do - @user = @user_manager.signup("bob", "smith", "bob@jamkazam.com", "foobar", "foobar", "Austin", "TX", "USA", nil, nil, "http://localhost:3000/confirm" ) + @user = @user_manager.signup("127.0.0.1", "bob", "smith", "bob@jamkazam.com", "foobar", "foobar", nil, nil, "http://localhost:3000/confirm" ) @user = @user_manager.signup_confirm(@user.signup_token) @user.email_confirmed.should be_true end