Welcome! <%= @user.name %>.
+To confirm your registration, please go to the signup confirmation page..
+ + \ No newline at end of file diff --git a/lib/jam_ruby/app/views/jam_ruby/user_mailer/welcome_message.text.erb b/lib/jam_ruby/app/views/jam_ruby/user_mailer/welcome_message.text.erb new file mode 100644 index 000000000..3667d0935 --- /dev/null +++ b/lib/jam_ruby/app/views/jam_ruby/user_mailer/welcome_message.text.erb @@ -0,0 +1,2 @@ +Welcome! <%= @user.name %> +To confirm your registration, please go to the signup confirmation page at : <%= @signup_confirm_url %>. \ No newline at end of file diff --git a/lib/jam_ruby/base_manager.rb b/lib/jam_ruby/base_manager.rb new file mode 100644 index 000000000..d2970c24b --- /dev/null +++ b/lib/jam_ruby/base_manager.rb @@ -0,0 +1,31 @@ +module JamRuby + class BaseManager + + attr_accessor :pg_conn + + def initialize(options={}) + @log = Logging.logger[self] + @pg_conn = options[:conn] + + unless PG.threadsafe? + raise Exception, "a non-threadsafe build of libpq is present." + end + end + + # Creates a connection manager, and associates the connection created by active_record with ourselves + def self.active_record_transaction(&block) + + manager = self.new + ActiveRecord::Base.connection_pool.with_connection do |connection| + # create a transaction, and pass the current connection to ConnectionManager. + # this lets the entire operation work with the same transaction, + # across Rails ActiveRecord and the pg-gem based code in ConnectionManager. + manager.pg_conn = connection.instance_variable_get("@connection") + + connection.transaction do + block.call(manager) + end + end + end + end +end \ No newline at end of file diff --git a/lib/jam_ruby/connection_manager.rb b/lib/jam_ruby/connection_manager.rb index d3adb12a6..0ba6111e3 100644 --- a/lib/jam_ruby/connection_manager.rb +++ b/lib/jam_ruby/connection_manager.rb @@ -1,5 +1,5 @@ module JamRuby -# All writes should occur through the ConnectionManager +# All writes should occur through the ConnectionManager for the connection table # Reads can occur freely elsewhere, though # Because connections are tied to the websocket-connection and we bookkeep them in the database purely # for 'SQL convenience', this is a obvious place we can go away from a database @@ -11,19 +11,15 @@ module JamRuby # This may make sense in the short term if we are still managing connections in the database, but # we move to the node-js in the websocket gateway (because the websocket gateway needs to call some of these methods). # Or of course we could just port the relevant methods to node-js - class ConnectionManager + class ConnectionManager < BaseManager - attr_accessor :mq_router, :pg_conn + attr_accessor :mq_router def initialize(options={}) + super(options) @log = Logging.logger[self] @mq_router = MQRouter.new - @pg_conn = options[:conn] @message_factory = MessageFactory.new - - unless PG.threadsafe? - raise Exception, "a non-threadsafe build of libpq is present." - end end def update_staleness() @@ -248,20 +244,6 @@ module JamRuby return friend_ids end - # Creates a connection manager, and associates the connection created by active_record with ourselves - def self.active_record_transaction(&block) - connection_manager = ConnectionManager.new - ActiveRecord::Base.connection_pool.with_connection do |connection| - # create a transaction, and pass the current connection to ConnectionManager. - # this lets the entire operation work with the same transaction, - # across Rails ActiveRecord and the pg-gem based code in ConnectionManager. - connection_manager.pg_conn = connection.instance_variable_get("@connection") - - connection.transaction do - block.call(connection_manager) - end - end - end end end \ No newline at end of file diff --git a/lib/jam_ruby/init.rb b/lib/jam_ruby/init.rb new file mode 100644 index 000000000..adf32cf04 --- /dev/null +++ b/lib/jam_ruby/init.rb @@ -0,0 +1,3 @@ +# initialize actionmailer +ActionMailer::Base.raise_delivery_errors = true +ActionMailer::Base.view_paths = File.expand_path('../../jam_ruby/app/views/', __FILE__) \ No newline at end of file diff --git a/lib/jam_ruby/models/user.rb b/lib/jam_ruby/models/user.rb index 495abc3e1..3c5798f4c 100644 --- a/lib/jam_ruby/models/user.rb +++ b/lib/jam_ruby/models/user.rb @@ -1,16 +1,18 @@ module JamRuby class User < ActiveRecord::Base include Tire::Model::Search - include Tire::Model::Callbacks - attr_accessible :first_name, :last_name, :name, :email, :password, :password_confirmation + attr_accessible :first_name, :last_name, :name, :email, :password, :password_confirmation, :city, :state, :country attr_accessor :updating_password self.primary_key = 'id' # authorizations (for facebook, etc -- omniauth) - has_many :user_authorizations + has_many :user_authorizations, :class_name => "JamRuby::UserAuthorization" + # account + belongs_to :account, :class_name => "JamRuby::Account" + # connections (websocket-gateway) has_many :connections, :class_name => "JamRuby::Connection" @@ -62,9 +64,9 @@ module JamRuby after_save :limit_to_five_instruments + validates :name, uniqueness: {case_sensitive: false}, presence: true, length: {maximum: 50} validates :first_name, presence: true, length: {maximum: 50} validates :last_name, presence: true, length: {maximum: 50} - validates :name, presence: true, length: {maximum: 50} VALID_EMAIL_REGEX = /\A[\w+\-.]+@[a-z\d\-.]+\.[a-z]+\z/i validates :email, presence: true, format: {with: VALID_EMAIL_REGEX}, uniqueness: {case_sensitive: false} @@ -93,10 +95,6 @@ module JamRuby updating_password || new_record? end - #def name - # return self.first_name + " " + self.last_name - #end - def friends?(user) return self.friends.exists?(user) end @@ -110,7 +108,7 @@ module JamRuby end def following_count - return self.followings.size + return self.followings.size + self.band_followings.size end def to_s @@ -120,13 +118,18 @@ module JamRuby end # helper method for creating / updating a User - def self.save(params) + def self.save(params) if params[:id].nil? user = User.new() else user = User.find(params[:id]) end + # account id + unless params[:account_id].nil? + user.account_id = params[:account_id] + end + # first name unless params[:first_name].nil? user.first_name = params[:first_name] @@ -162,6 +165,21 @@ module JamRuby user.musician = params[:musician] end + # gender + unless params[:gender].nil? + account.gender = params[:gender] + end + + # birthdate + unless params[:birth_date].nil? + account.birth_date = params[:birth_date] + end + + # ISP + unless params[:internet_service_provider].nil? + account.internet_service_provider = params[:internet_service_provider] + end + # city unless params[:city].nil? user.city = params[:city] @@ -227,6 +245,15 @@ module JamRuby }.to_json end + # only put users into the search index if their eail is confirmed + after_save do + update_index if email_confirmed + end + + after_destroy do + update_index + end + class << self def create_search_index Tire.index(User.index_name) do @@ -258,7 +285,6 @@ module JamRuby ### Elasticsearch/Tire integration - private def create_remember_token self.remember_token = SecureRandom.urlsafe_base64 diff --git a/lib/jam_ruby/user_manager.rb b/lib/jam_ruby/user_manager.rb new file mode 100644 index 000000000..cbcf746f0 --- /dev/null +++ b/lib/jam_ruby/user_manager.rb @@ -0,0 +1,70 @@ +module JamRuby + class UserManager < BaseManager + + def initialize(options={}) + super(options) + @log = Logging.logger[self] + end + + # throws ActiveRecord::RecordNotFound if instrument is invalid + # throws an email delivery error if unable to connect out to SMTP + def signup(name, email, password, password_confirmation, + city, state, country, instruments, signup_confirm_url) + user = User.new + + UserManager.active_record_transaction do |user_manager| + user.name = name + user.email = email + user.password = password + user.password_confirmation = password_confirmation + user.admin = false + user.email_confirmed = false + user.city = city + user.state = state + user.country = country + unless instruments.nil? + instruments.each do |musician_instrument_param| + instrument = Instrument.find(musician_instrument_param[:instrument_id]) + musician_instrument = MusicianInstrument.new + musician_instrument.user = user + musician_instrument.instrument = instrument + musician_instrument.proficiency_level = musician_instrument_param[:proficiency_level] + musician_instrument.priority = musician_instrument_param[:priority] + musician_instrument.save + user.musician_instruments << musician_instrument + end + end + user.signup_token = SecureRandom.urlsafe_base64 + + user.save + + if user.errors.any? + raise ActiveRecord::Rollback + else + # any errors here should also rollback the transaction; that's OK. If emails aren't going to be delivered, + # it's already a really bad situation; make user signup again + UserMailer.welcome_message(user, signup_confirm_url + "/" + user.signup_token).deliver + end + end + + return user + end + + # throws RecordNotFound if signup token is invalid; i.e., if it's nil, empty string, or not belonging to a user + def signup_confirm(signup_token) + if signup_token.nil? || signup_token.empty? + # there are plenty of confirmed users with nil signup_tokens, so we can't look on it + raise ActiveRecord::RecordNotFound + else + UserManager.active_record_transaction do |user_manager| + # throws ActiveRecord::RecordNotFound if invalid + user = User.find_by_signup_token!(signup_token) + user.signup_token = nil + user.email_confirmed = true + user.save + return user + end + end + end + end +end \ No newline at end of file diff --git a/spec/factories.rb b/spec/factories.rb index 6ccd2c674..bc82f9e43 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -4,6 +4,7 @@ FactoryGirl.define do sequence(:email) { |n| "person_#{n}@example.com"} password "foobar" password_confirmation "foobar" + email_confirmed true factory :admin do admin true @@ -26,4 +27,8 @@ FactoryGirl.define do factory :friendship, :class => JamRuby::Friendship do end + + factory :band, :class => JamRuby::Band do + + end end \ No newline at end of file diff --git a/spec/jam_ruby/models/search_spec.rb b/spec/jam_ruby/models/search_spec.rb index 90a1545e7..dfcc79692 100644 --- a/spec/jam_ruby/models/search_spec.rb +++ b/spec/jam_ruby/models/search_spec.rb @@ -11,11 +11,9 @@ describe Search do def create_peachy_data - @user = User.save(first_name: "Peach", last_name: "Pit", name: "peachy", email: "user@example.com", - password: "foobar", password_confirmation: "foobar", musician: true) - @fan = User.save(first_name: "Peach Peach", last_name: "Pit", name: "peachy", email: "fan@example.com", - password: "foobar", password_confirmation: "foobar", musician: false) - @band = Band.save(name: "Peach pit", website: "www.bands.com", biography: "zomg we rock") + @user = FactoryGirl.create(:user, first_name: "Peach", last_name: "Pit", name: "peachy", email: "user@example.com", musician: true) + @fan = FactoryGirl.create(:user, first_name: "Peach Peach", last_name: "Pit", name: "peachy2", email: "fan@example.com", musician: false) + @band = FactoryGirl.create(:band, name: "Peach pit", website: "www.bands.com", biography: "zomg we rock") end def assert_peachy_data diff --git a/spec/jam_ruby/models/tire_search_spec.rb b/spec/jam_ruby/models/tire_search_spec.rb index 099bb17d6..2987a40e9 100644 --- a/spec/jam_ruby/models/tire_search_spec.rb +++ b/spec/jam_ruby/models/tire_search_spec.rb @@ -20,8 +20,7 @@ describe "tire search" do end it "full search for single user" do - @user = User.save(first_name: "User", last_name: "One", name: "test", email: "user@example.com", - password: "foobar", password_confirmation: "foobar", musician: true) + @user = FactoryGirl.create(:user, first_name: "User", last_name: "One", name: "test", email: "user@example.com", musician: true) User.search_index.refresh s = Tire.search ['test-jamruby-users', 'test-jamruby-bands'], :load => true do @@ -35,7 +34,7 @@ describe "tire search" do end it "full search for single band" do - @band = Band.save(name: "Example Band", website: "www.bands.com", biography: "zomg we rock") + @band = FactoryGirl.create(:band, name: "Example Band", website: "www.bands.com", biography: "zomg we rock") Band.search_index.refresh s = Tire.search ['test-jamruby-users', 'test-jamruby-bands'], :load => true do @@ -49,9 +48,9 @@ describe "tire search" do end it "full search for a band & user" do - @user = User.save(first_name: "Peach", last_name: "Foo", name: "test", email: "user@example.com", - password: "foobar", password_confirmation: "foobar", musician: true) - @band = Band.save(name: "Peach pit", website: "www.bands.com", biography: "zomg we rock") + @user = FactoryGirl.create(:user, first_name: "Peach", last_name: "Foo", name: "test", email: "user@example.com", musician: true) + @band = FactoryGirl.create(:band, name: "Peach pit", website: "www.bands.com", biography: "zomg we rock") + User.search_index.refresh Band.search_index.refresh @@ -71,9 +70,9 @@ describe "tire search" do it "pagination" do - @user = User.save(first_name: "Peach", last_name: "foo", name: "test", email: "user@example.com", - password: "foobar", password_confirmation: "foobar", musician: true) - @band = Band.save(name: "Peach pit", website: "www.bands.com", biography: "zomg we rock") + @user = FactoryGirl.create(:user, first_name: "Peach", last_name: "foo", name: "test", email: "user@example.com", musician: true) + @band = FactoryGirl.create(:band, name: "Peach pit", website: "www.bands.com", biography: "zomg we rock") + User.search_index.refresh Band.search_index.refresh @@ -112,9 +111,7 @@ describe "tire search" do s.results.total.should == 0 - - @user = User.save(first_name: "Peach", last_name: "Pit", name: "peachy", email: "user@example.com", - password: "foobar", password_confirmation: "foobar", musician: true) + @user = FactoryGirl.create(:user, first_name: "Peach", last_name: "Pit", name: "peachy", email: "user@example.com", musician: true) User.search_index.refresh sleep 1 # https://jamkazam.atlassian.net/browse/VRFS-69 diff --git a/spec/jam_ruby/models/user_search_spec.rb b/spec/jam_ruby/models/user_search_spec.rb index e7ec9ca8a..20525778e 100644 --- a/spec/jam_ruby/models/user_search_spec.rb +++ b/spec/jam_ruby/models/user_search_spec.rb @@ -6,9 +6,8 @@ describe User do User.delete_search_index User.create_search_index - @user = User.save(first_name: "Example", last_name: "User", name: "peachy", email: "user@example.com", - password: "foobar", password_confirmation: "foobar", musician: true) - + @user = FactoryGirl.create(:user, first_name: "Example", last_name: "User", name: "peachy", email: "user@example.com", + password: "foobar", password_confirmation: "foobar", musician: true, email_confirmed: true) # you have to poke elasticsearch because it will batch requests internally for a second User.search_index.refresh end @@ -61,12 +60,31 @@ describe User do end it "should tokenize correctly" do - @user2 = User.save(first_name: "peaches", last_name: "test", name: "peachy", email: "peach@example.com", - password: "foobar", password_confirmation: "foobar", musician: true) + @user2 = FactoryGirl.create(:user, first_name: "peaches", last_name: "test", name: "peachy2", email: "peach@example.com", + password: "foobar", password_confirmation: "foobar", musician: true, email_confirmed: true) User.search_index.refresh ws = User.search("pea") ws.results.length.should == 1 user_result = ws.results[0] user_result.id.should == @user2.id + end + + + it "users who have signed up, but not confirmed should not show up in search index" do + @user3 = FactoryGirl.create(:user, first_name: "unconfirmed", last_name: "unconfirmed", name: "unconfirmed", email: "unconfirmed@example.com", + password: "foobar", password_confirmation: "foobar", musician: true, email_confirmed: false) + User.search_index.refresh + ws = User.search("unconfirmed") + ws.results.length.should == 0 + + # Ok, confirm the user, and see them show up + @user3.email_confirmed = true + @user3.save + User.search_index.refresh + + ws = User.search("unconfirmed") + ws.results.length.should == 1 + user_result = ws.results[0] + user_result.id.should == @user3.id end end \ No newline at end of file diff --git a/spec/jam_ruby/user_manager_spec.rb b/spec/jam_ruby/user_manager_spec.rb new file mode 100644 index 000000000..74f8ef3ff --- /dev/null +++ b/spec/jam_ruby/user_manager_spec.rb @@ -0,0 +1,92 @@ +require 'spec_helper' + +# these tests avoid the use of ActiveRecord and FactoryGirl to do blackbox, non test-instrumented tests +describe UserManager do + + + before(:each) do + @user_manager = UserManager.new(:conn => @conn) + UserMailer.deliveries.clear + end + + describe "signup" do + it "signup successfully" do + @user = @user_manager.signup("bob", "bob@jamkazam.com", "foobar", "foobar", "Austin", "TX", "USA", nil, "http://localhost:3000/confirm" ) + + @user.errors.any?.should be_false + @user.name.should == "bob" + @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.instruments.length.should == 0 + @user.signup_token.should_not be_nil + + UserMailer.deliveries.length.should == 1 + end + + it "signup successfully with instruments" do + @user = @user_manager.signup("bob", "bob@jamkazam.com", "foobar", "foobar", "Austin", "TX", "USA", + [{ :instrument_id => "electric guitar", :proficiency_level => 3, :priority => 0}], "http://localhost:3000/confirm" ) + + @user.errors.any?.should be_false + @user.instruments.length.should == 1 + musician_instrument = @user.musician_instruments[0] + musician_instrument.instrument.should == Instrument.find("electric guitar") + musician_instrument.proficiency_level.should == 3 + end + + it "duplicate signup failure" do + @user = @user_manager.signup("bob", "bob@jamkazam.com", "foobar", "foobar", "Austin", "TX", "USA", 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", "bob@jamkazam.com", "foobar", "foobar", "Austin", "TX", "USA", 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" + + # change email so that name appears dupped + @user = @user_manager.signup("bob", "bobbie@jamkazam.com", "foobar", "foobar", "Austin", "TX", "USA", nil, "http://localhost:3000/confirm" ) + UserMailer.deliveries.length.should == 1 + @user.errors.any?.should be_true + @user.errors[:name][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, "http://localhost:3000/confirm" ) + UserMailer.deliveries.length.should == 0 + @user.errors.any?.should be_true + @user.errors[:name][0].should == "can't be blank" + end + + it "fail on no username" do + @user = @user_manager.signup("murp", "", "foobar", "foobar", "Austin", "TX", "USA", 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", "bob@jamkazam.com", "foobar", "foobar", "Austin", "TX", "USA", nil, "http://localhost:3000/confirm" ) + @user = @user_manager.signup_confirm(@user.signup_token) + @user.email_confirmed.should be_true + end + + it "fail to confirm bogus signup token" do + expect { @user_manager.signup_confirm("murp") }.to raise_error ActiveRecord::RecordNotFound + end + + it "fail to confirm empty signup token" do + expect { @user_manager.signup_confirm("") }.to raise_error ActiveRecord::RecordNotFound + end + + it "fail to confirm nil signup token" do + expect { @user_manager.signup_confirm(nil) }.to raise_error ActiveRecord::RecordNotFound + end + end +end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb new file mode 100644 index 000000000..5364de7e5 --- /dev/null +++ b/spec/mailers/user_mailer_spec.rb @@ -0,0 +1,44 @@ +require "spec_helper" + +describe UserMailer do + + let(:user) { FactoryGirl.create(:user) } + + before(:each) do + UserMailer.deliveries.clear + end + + it "should send welcome email" do + signup_confirmation_url = "http://example.com/confirm" + signup_confirmation_url_with_token = "#{signup_confirmation_url}/#{user.signup_token}" + UserMailer.welcome_message(user, signup_confirmation_url_with_token).deliver + + UserMailer.deliveries.length.should == 1 + mail = UserMailer.deliveries[0] + + mail['from'].to_s.should == UserMailer::DEFAULT_SENDER + mail['to'].to_s.should == user.email + mail.multipart?.should == true # because we send plain + html + + # verify that the messages are correctly configured + mail.html_part.body.include?("Welcome").should be_true + mail.html_part.body.include?(signup_confirmation_url_with_token).should be_true + mail.text_part.body.include?("Welcome").should be_true + mail.text_part.body.include?(signup_confirmation_url_with_token).should be_true + end + + it "should send reset password" do + UserMailer.reset_password(user).deliver + + UserMailer.deliveries.length.should == 1 + mail = UserMailer.deliveries[0] + + mail['from'].to_s.should == UserMailer::DEFAULT_SENDER + mail['to'].to_s.should == user.email + mail.multipart?.should == true # because we send plain + html + + # verify that the messages are correctly configured + mail.html_part.body.include?("Reset").should be_true + mail.text_part.body.include?("Reset").should be_true + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index bef938bf1..214236359 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -19,6 +19,9 @@ require 'factories' include JamRuby +# put ActionMailer into test mode +ActionMailer::Base.delivery_method = :test + #uncomment the following line to use spork with the debugger #require 'spork/ext/ruby-debug' @@ -44,7 +47,7 @@ Spork.prefork do config.before(:suite) do DatabaseCleaner.strategy = :transaction - DatabaseCleaner.clean_with(:truncation) + DatabaseCleaner.clean_with(:truncation, {:except => %w[instruments genres] }) end config.before(:each) do