From eb8c540ac116d96880cff29d6b54d64413db441a Mon Sep 17 00:00:00 2001 From: danielgrippi Date: Tue, 16 Aug 2011 19:26:39 -0700 Subject: [PATCH] MS DG IZ major invite refactor. all tests are green minus weird stuff on dans computer, need to checkout this out on pivots --- app/controllers/admins_controller.rb | 12 +- app/controllers/invitations_controller.rb | 61 ++-- app/controllers/services_controller.rb | 3 +- app/models/invitation.rb | 102 ++++-- app/models/job/mail/invite_user_by_email.rb | 6 +- app/models/job/resend_invitation.rb | 2 +- app/models/service_user.rb | 6 +- app/models/user.rb | 71 ++--- .../mailer/invitation_instructions.haml | 2 +- ...0110816061820_add_fields_to_invitations.rb | 6 + db/schema.rb | 5 +- features/step_definitions/user_steps.rb | 6 +- lib/rake_helpers.rb | 12 +- spec/controllers/admins_controller_spec.rb | 10 +- .../invitations_controller_spec.rb | 71 +++-- spec/controllers/services_controller_spec.rb | 2 +- spec/factories.rb | 9 + spec/lib/rake_helper_spec.rb | 5 +- spec/models/invitation_spec.rb | 300 ++++-------------- .../jobs/mail/invite_user_by_email_spec.rb | 16 +- spec/models/jobs/resend_invitation_spec.rb | 14 +- spec/models/user/invite_spec.rb | 90 ------ spec/models/user_spec.rb | 99 +++++- 23 files changed, 391 insertions(+), 519 deletions(-) delete mode 100644 spec/models/user/invite_spec.rb diff --git a/app/controllers/admins_controller.rb b/app/controllers/admins_controller.rb index 5a6256221..fbd1ed0a5 100644 --- a/app/controllers/admins_controller.rb +++ b/app/controllers/admins_controller.rb @@ -8,12 +8,12 @@ class AdminsController < ApplicationController @users = params[:user].empty? ? [] : User.where(params[:user]) end - def admin_inviter - opts = {:service => 'email', :identifier => params[:identifier]} - existing_user = Invitation.find_existing_user('email', params[:identifier]) - opts.merge!(:existing_user => existing_user) if existing_user - Invitation.create_invitee(opts) - flash[:notice] = "invitation sent to #{params[:identifier]}" + def admin_inviter + user = User.find_by_email params[:idenitifer] + unless user + Invitation.create(:service => 'email', :identifer => params[:identifier], :admin => true) + flash[:notice] = "invitation sent to #{params[:identifier]}" + end redirect_to user_search_path end diff --git a/app/controllers/invitations_controller.rb b/app/controllers/invitations_controller.rb index be2993777..0bfab60c5 100644 --- a/app/controllers/invitations_controller.rb +++ b/app/controllers/invitations_controller.rb @@ -5,6 +5,7 @@ class InvitationsController < Devise::InvitationsController before_filter :check_token, :only => [:edit] + before_filter :check_if_invites_open, :only =>[:create] def new @sent_invitations = current_user.invitations_from_me.includes(:recipient) @@ -16,35 +17,14 @@ class InvitationsController < Devise::InvitationsController end def create - unless AppConfig[:open_invitations] - flash[:error] = I18n.t 'invitations.create.no_more' - redirect_to :back - return - end - aspect = params[:user].delete(:aspects) + aspect_id = params[:user].delete(:aspect_id) message = params[:user].delete(:invite_messages) emails = params[:user][:email].to_s.gsub(/\s/, '').split(/, */) + #NOTE should we try and find users by email here? probs + aspect = Aspect.find(aspect_id) + invites = Invitation.batch_build(:sender => current_user, :aspect => aspect, :emails => emails, :service => 'email') - good_emails, bad_emails = emails.partition{|e| e.try(:match, Devise.email_regexp)} - - if good_emails.include?(current_user.email) - if good_emails.length == 1 - flash[:error] = I18n.t 'invitations.create.own_address' - redirect_to :back - return - else - bad_emails.push(current_user.email) - good_emails.delete(current_user.email) - end - end - - good_emails.each{|e| Resque.enqueue(Job::Mail::InviteUserByEmail, current_user.id, e, aspect, message)} - - if bad_emails.any? - flash[:error] = I18n.t('invitations.create.sent') + good_emails.join(', ') + " "+ I18n.t('invitations.create.rejected') + bad_emails.join(', ') - else - flash[:notice] = I18n.t('invitations.create.sent') + good_emails.join(', ') - end + flash[:notice] = extract_messages(invites) redirect_to :back end @@ -59,7 +39,7 @@ class InvitationsController < Devise::InvitationsController user.accept_invitation!(params[:user]) user.seed_aspects rescue Exception => e #What exception is this trying to rescue? If it is ActiveRecord::NotFound, we should say so. - raise e unless e.respond_to?(:record) + raise e user = nil record = e.record record.errors.delete(:person) @@ -90,12 +70,37 @@ class InvitationsController < Devise::InvitationsController @resource = User.find_by_invitation_token(params[:invitation_token]) render 'devise/mailer/invitation_instructions', :layout => false end - protected + protected def check_token if User.find_by_invitation_token(params[:invitation_token]).nil? flash[:error] = I18n.t 'invitations.check_token.not_found' redirect_to root_url end end + + def check_if_invites_open + unless AppConfig[:open_invitations] + flash[:error] = I18n.t 'invitations.create.no_more' + redirect_to :back + return + end + end + + # @param invites [Array] Invitations to be sent. + # @return [String] A full list of success and error messages. + def extract_messages(invites) + success_message = "Invites Successfully Sent to: " + failure_message = "There was a problem with: " + successes, failures = invites.partition{|x| x.persisted? } + + success_message += successes.map{|k| k.identifier }.join(', ') + failure_message += failures.map{|k| k.identifier }.join(', ') + + messages = [] + messages << success_message if successes.present? + messages << failure_message if failures.present? + + messages.join('\n') + end end diff --git a/app/controllers/services_controller.rb b/app/controllers/services_controller.rb index c9a332399..9c3386944 100644 --- a/app/controllers/services_controller.rb +++ b/app/controllers/services_controller.rb @@ -59,7 +59,8 @@ class ServicesController < ApplicationController if i_id = params[:invitation_id] invited_user = Invitation.find(i_id).recipient else - invited_user = current_user.invite_user(params[:aspect_id], params[:provider], @uid) + invite = Invitation.create(:service => params[:provider], :identifier => @uid, :sender => current_user, :aspect => current_user.aspects.find(params[:aspect_id])) + invited_user = invite.attach_recipient! end @subject = t('services.inviter.join_me_on_diaspora') diff --git a/app/models/invitation.rb b/app/models/invitation.rb index ca8fcc0db..553a4d54b 100644 --- a/app/models/invitation.rb +++ b/app/models/invitation.rb @@ -8,54 +8,73 @@ class Invitation < ActiveRecord::Base belongs_to :recipient, :class_name => 'User' belongs_to :aspect - validates_presence_of :sender, - :recipient, - :aspect, - :identifier, - :service + validates_presence_of :identifier, :service - attr_accessible :sender, :recipient, :aspect, :service, :identifier + validates_presence_of :sender, :aspect, :unless => :admin? + attr_accessible :sender, :recipient, :aspect, :service, :identifier, :admin before_validation :set_email_as_default_service - before_validation :attach_recipient, :on => :create - before_create :ensure_not_inviting_self + validate :ensure_not_inviting_self, :on => :create validate :valid_identifier? - validates_uniqueness_of :sender, :scope => :recipient + validates_uniqueness_of :sender_id, :scope => [:identifier, :service], :unless => :admin? - def set_email_as_default_service - self.service ||='email' + after_create :queue_send! #TODO make this after_commit :queue_saved!, :on => :create + + + # @note options hash is passed through to [Invitation.new] + # @see [Invitation.new] + # + # @option opts [Array] :emails + # @return [Array] An array of initialized [Invitation] models. + def self.batch_build(opts) + emails = opts.delete(:emails) + emails.map! do |e| + Invitation.create(opts.merge(:identifier => e)) + end + emails end + # Downcases the incoming service identifier and assigns it + # + # @param ident [String] Service identifier + # @see super def identifier=(ident) ident.downcase! if ident super end - def not_inviting_yourself - if self.identifier == self.sender.email - errors[:base] << 'You can not invite yourself' - end - end - - def attach_recipient - self.recipient = User.find_or_create_by_invitation(self) - end - - def skip_invitation? + # Determine if we want to skip emailing the recipient. + # + # @return [Boolean] + # @return [void] + def skip_email? self.service != 'email' end - # @return Contact - def share_with! - if contact = sender.share_with(recipient.person, aspect) - self.destroy + # Attach a recipient [User] to the [Invitation] unless + # there is one already present. + # + # @return [User] The recipient. + def attach_recipient! + unless self.recipient.present? + self.recipient = User.find_or_create_by_invitation(self) + self.save end - contact + self.recipient end - def invite! - recipient.skip_invitation = self.skip_invitation? + # Find or create user, and send that resultant User an + # invitation. + # + # @return [Invitation] self + def send! + self.attach_recipient! + + # Sets an instance variable in User (set by devise invitable) + # This determines whether an email should be sent to the recipient. + recipient.skip_invitation = self.skip_email? + recipient.invite! # Logging the invitation action @@ -63,11 +82,12 @@ class Invitation < ActiveRecord::Base log_hash.merge({:inviter => self.sender.diaspora_handle, :invitier_uid => self.sender.id, :inviter_created_at_unix => self.sender.created_at.to_i}) if self.sender Rails.logger.info(log_hash) - recipient + self end + # @return [Invitation] self def resend - self.invite! + self.send! end # @return [String] @@ -83,7 +103,27 @@ class Invitation < ActiveRecord::Base end end + def queue_send! + unless self.recipient.present? + Resque.enqueue(Job::Mail::InviteUserByEmail, self.id) + end + end + + # @note before_save + def set_email_as_default_service + self.service ||= 'email' + end + + # @note Validation + def ensure_not_inviting_self + if !self.admin? && self.identifier == self.sender.email + errors[:base] << 'You can not invite yourself' + end + end + + # @note Validation def valid_identifier? + return false unless self.identifier if self.service == 'email' unless self.identifier.match(Devise.email_regexp) errors[:base] << 'invalid email' diff --git a/app/models/job/mail/invite_user_by_email.rb b/app/models/job/mail/invite_user_by_email.rb index 6e4275f60..2864798e4 100644 --- a/app/models/job/mail/invite_user_by_email.rb +++ b/app/models/job/mail/invite_user_by_email.rb @@ -7,9 +7,9 @@ module Job module Mail class InviteUserByEmail < Base @queue = :mail - def self.perform(sender_id, email, aspect_id, invite_message) - user = User.find(sender_id) - user.invite_user(aspect_id, 'email', email, invite_message) + def self.perform(invite_id) + invite = Invitation.find(invite_id) + invite.send! end end end diff --git a/app/models/job/resend_invitation.rb b/app/models/job/resend_invitation.rb index 12cf073b4..c4d2d79a8 100644 --- a/app/models/job/resend_invitation.rb +++ b/app/models/job/resend_invitation.rb @@ -7,7 +7,7 @@ module Job class ResendInvitation < Base @queue = :mail def self.perform(invitation_id) - inv = Invitation.where(:id => invitation_id).first + inv = Invitation.find(invitation_id) inv.resend end end diff --git a/app/models/service_user.rb b/app/models/service_user.rb index a721ae505..7157d26c1 100644 --- a/app/models/service_user.rb +++ b/app/models/service_user.rb @@ -30,8 +30,8 @@ class ServiceUser < ActiveRecord::Base self.contact = self.service.user.contact_for(self.person) end - self.invitation = Invitation.joins(:recipient).where(:sender_id => self.service.user_id, - :users => {:invitation_service => self.service.provider, - :invitation_identifier => self.uid}).first + self.invitation = Invitation.where(:sender_id => self.service.user_id, + :service => self.service.provider, + :identifier => self.uid).first end end diff --git a/app/models/user.rb b/app/models/user.rb index 76470547b..b8e8ed6d7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -60,13 +60,11 @@ class User < ActiveRecord::Base :invitation_identifier - - def self.find_or_create_by_invitation(invitation) + # @return [User] + def self.find_by_invitation(invitation) service = invitation.service identifier = invitation.identifier - #find users that may already exsist - #1. if service == 'email' existing_user = User.where(:email => identifier).first else @@ -78,8 +76,13 @@ class User < ActiveRecord::Base existing_user = i.recipient if i end - if existing_user - return existing_user + existing_user + end + + # @return [User] + def self.find_or_create_by_invitation(invitation) + if existing_user = self.find_by_invitation(invitation) + existing_user else self.create_from_invitation!(invitation) end @@ -89,7 +92,7 @@ class User < ActiveRecord::Base user = User.new user.generate_keys user.send(:generate_invitation_token) - #user.invitations << invitation + #user.invitations_to_me << invitation # we need to make a custom validator here to make this safer user.save(:validate => false) user @@ -310,21 +313,6 @@ class User < ActiveRecord::Base end end - - - ###Invitations############ - def invite_user(aspect_id, service, identifier, invite_message = "") - if aspect = aspects.find(aspect_id) - Invitation.invite(:service => service, - :identifier => identifier, - :from => self, - :into => aspect, - :message => invite_message) - else - false - end - end - # This method is called when an invited user accepts his invitation # # @param [Hash] opts the options to accept the invitation with @@ -333,25 +321,29 @@ class User < ActiveRecord::Base # @option opts [String] :password_confirmation def accept_invitation!(opts = {}) log_hash = {:event => :invitation_accepted, :username => opts[:username], :uid => self.id} - log_hash[:inviter] = invitations_to_me.first.sender.diaspora_handle if invitations_to_me.first - begin - if self.invited? - self.setup(opts) - self.invitation_token = nil - self.password = opts[:password] - self.password_confirmation = opts[:password_confirmation] - self.save! - invitations_to_me.each{|invitation| invitation.share_with!} - log_hash[:status] = "success" - Rails.logger.info log_hash + log_hash[:inviter] = invitations_to_me.first.sender.diaspora_handle if invitations_to_me.first && invitations_to_me.first.sender - self.reload # Because to_request adds a request and saves elsewhere - self + if self.invited? + self.setup(opts) + self.invitation_token = nil + self.password = opts[:password] + self.password_confirmation = opts[:password_confirmation] + self.save! + + # moved old Invitation#share_with! logic into here, + # but i don't think we want to destroy the invitation + # anymore. we may want to just call self.share_with + invitations_to_me.each do |invitation| + if !invitation.admin? && invitation.sender.share_with(self.person, invitation.aspect) + invitation.destroy + end end - rescue Exception => e - log_hash[:status] = "failure" - Rails.logger.info log_hash - raise e + + log_hash[:status] = "success" + Rails.logger.info(log_hash) + + self.reload # Because to_request adds a request and saves elsewhere + self end end @@ -365,6 +357,7 @@ class User < ActiveRecord::Base def setup(opts) self.username = opts[:username] self.email = opts[:email] + self.language ||= 'en' self.valid? errors = self.errors errors.delete :person diff --git a/app/views/devise/mailer/invitation_instructions.haml b/app/views/devise/mailer/invitation_instructions.haml index fc24f2d92..b945d898f 100644 --- a/app/views/devise/mailer/invitation_instructions.haml +++ b/app/views/devise/mailer/invitation_instructions.haml @@ -1,5 +1,5 @@ - @invs = @resource.invitations_to_me --if @invs.count > 0 +-unless @invs.first.admin? !!! %html %head diff --git a/db/migrate/20110816061820_add_fields_to_invitations.rb b/db/migrate/20110816061820_add_fields_to_invitations.rb index 45b36952d..f8005ef0c 100644 --- a/db/migrate/20110816061820_add_fields_to_invitations.rb +++ b/db/migrate/20110816061820_add_fields_to_invitations.rb @@ -2,10 +2,16 @@ class AddFieldsToInvitations < ActiveRecord::Migration def self.up add_column :invitations, :service, :string add_column :invitations, :identifier, :string + add_column :invitations, :admin, :boolean, :default => false + change_column :invitations, :recipient_id, :integer, :null => true + change_column :invitations, :sender_id, :integer, :null => true end def self.down remove_column :invitations, :service remove_column :invitations, :identifier + remove_column :invitations, :admin + change_column :invitations, :recipient_id, :integer, :null => false + change_column :invitations, :sender_id, :integer, :null => false end end diff --git a/db/schema.rb b/db/schema.rb index 1bc0eae57..b94110f5f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -99,13 +99,14 @@ ActiveRecord::Schema.define(:version => 20110816061820) do create_table "invitations", :force => true do |t| t.text "message" - t.integer "sender_id", :null => false - t.integer "recipient_id", :null => false + t.integer "sender_id" + t.integer "recipient_id" t.integer "aspect_id" t.datetime "created_at" t.datetime "updated_at" t.string "service" t.string "identifier" + t.boolean "admin", :default => false end add_index "invitations", ["aspect_id"], :name => "index_invitations_on_aspect_id" diff --git a/features/step_definitions/user_steps.rb b/features/step_definitions/user_steps.rb index 951b255c1..25f0aa54f 100644 --- a/features/step_definitions/user_steps.rb +++ b/features/step_definitions/user_steps.rb @@ -35,13 +35,15 @@ Given /^a user named "([^\"]*)" with email "([^\"]*)"$/ do |name, email| end Given /^I have been invited by an admin$/ do - @me = Invitation.create_invitee(:service => 'email', :identifier => "new_invitee@example.com") + i = Invitation.create!(:admin => true, :service => 'email', :identifier => "new_invitee@example.com") + @me = i.attach_recipient! end Given /^I have been invited by a user$/ do @inviter = Factory(:user) aspect = @inviter.aspects.create(:name => "Rocket Scientists") - @me = @inviter.invite_user(aspect.id, 'email', "new_invitee@example.com", "Hey, tell me about your rockets!") + i = Invitation.create!(:aspect => aspect, :sender => @inviter, :service => 'email', :identifier => "new_invitee@example.com", :message =>"Hey, tell me about your rockets!") + @me = i.attach_recipient! end When /^I click on my name$/ do diff --git a/lib/rake_helpers.rb b/lib/rake_helpers.rb index 2e842c750..b796ae5f2 100644 --- a/lib/rake_helpers.rb +++ b/lib/rake_helpers.rb @@ -21,9 +21,17 @@ module RakeHelpers churn_through = n backer_name = backers[n+offset][1].to_s.strip backer_email = backers[n+offset][0].to_s.strip - unless User.find_by_email(backer_email) || User.find_by_invitation_identifier(backer_email) + + possible_user = User.find_by_email(backer_email) + possible_invite = Invitation.find_by_identifier(backer_email) + possible_user ||= possible_invite.recipient if possible_invite.present? + + unless possible_user puts "#{n}: sending email to: #{backer_name} #{backer_email}" unless Rails.env == 'test' - Invitation.create_invitee(:service => 'email', :identifier => backer_email, :name => backer_name ) unless test + unless test + i = Invitation.new(:service => 'email', :identifier => backer_email, :admin => true) + i.send! + end else puts "user with the email exists: #{backer_email} , #{backer_name} " unless Rails.env == 'test' end diff --git a/spec/controllers/admins_controller_spec.rb b/spec/controllers/admins_controller_spec.rb index 5dfd068ae..d9cca8040 100644 --- a/spec/controllers/admins_controller_spec.rb +++ b/spec/controllers/admins_controller_spec.rb @@ -85,19 +85,11 @@ describe AdminsController do end it 'invites a new user' do - Invitation.should_receive(:create_invitee).with(:service => 'email', :identifier => 'bob@moms.com') + Invitation.should_receive(:create) get :admin_inviter, :identifier => 'bob@moms.com' response.should redirect_to user_search_path flash.notice.should include("invitation sent") end - - it 'passes an existing user to create_invitee' do - Factory.create(:user, :email => 'bob@moms.com') - bob = User.where(:email => 'bob@moms.com').first - Invitation.should_receive(:find_existing_user).with('email', 'bob@moms.com').and_return(bob) - Invitation.should_receive(:create_invitee).with(:service => 'email', :identifier => 'bob@moms.com', :existing_user => bob) - get :admin_inviter, :identifier => 'bob@moms.com' - end end end diff --git a/spec/controllers/invitations_controller_spec.rb b/spec/controllers/invitations_controller_spec.rb index 460917783..a2256366e 100644 --- a/spec/controllers/invitations_controller_spec.rb +++ b/spec/controllers/invitations_controller_spec.rb @@ -11,6 +11,7 @@ describe InvitationsController do AppConfig[:open_invitations] = true @user = alice @aspect = @user.aspects.first + @invite = {:invite_message=>"test", :aspect_id=> @aspect.id.to_s, :email=>"abc@example.com"} request.env["devise.mapping"] = Devise.mappings[:user] Webfinger.stub_chain(:new, :fetch).and_return(Factory(:person)) @@ -19,43 +20,37 @@ describe InvitationsController do describe "#create" do before do sign_in :user, @user - @invite = {:invite_message=>"test", :aspect_id=> @aspect.id.to_s, :email=>"abc@example.com"} @controller.stub!(:current_user).and_return(@user) request.env["HTTP_REFERER"]= 'http://test.host/cats/foo' end - it 'calls the resque job Job::InviteUser' do - Resque.should_receive(:enqueue) - post :create, :user => @invite + it 'saves and invitation' do + expect { + post :create, :user => @invite + }.should change(Invitation, :count).by(1) end it 'handles a comma seperated list of emails' do - Resque.should_receive(:enqueue).twice() - post :create, :user => @invite.merge( + expect{ + post :create, :user => @invite.merge( :email => "foofoofoofoo@example.com, mbs@gmail.com") + }.should change(Invitation, :count).by(2) end it 'handles a comma seperated list of emails with whitespace' do - Resque.should_receive(:enqueue).twice() - post :create, :user => @invite.merge( - :email => "foofoofoofoo@example.com , mbs@gmail.com") - end - - it 'displays a message that tells the user how many invites were sent, and which REJECTED' do - post :create, :user => @invite.merge( - :email => "mbs@gmail.com, foo@bar.com, foo.com, lala@foo, cool@bar.com") - flash[:error].should_not be_empty - flash[:error].should =~ /foo\.com/ - flash[:error].should =~ /lala@foo/ + expect { + post :create, :user => @invite.merge( + :email => "foofoofoofoo@example.com , mbs@gmail.com") + }.should change(Invitation, :count).by(2) end it "allows invitations without if invitations are open" do open_bit = AppConfig[:open_invitations] AppConfig[:open_invitations] = true - Resque.should_receive(:enqueue).once - post :create, :user => @invite - + expect{ + post :create, :user => @invite + }.to change(Invitation, :count).by(1) AppConfig[:open_invitations] = open_bit end @@ -67,16 +62,19 @@ describe InvitationsController do it 'strips out your own email' do lambda { post :create, :user => @invite.merge(:email => @user.email) - }.should_not change(User, :count) + }.should_not change(Invitation, :count) - Resque.should_receive(:enqueue).once - post :create, :user => @invite.merge(:email => "hello@example.org, #{@user.email}") + expect{ + post :create, :user => @invite.merge(:email => "hello@example.org, #{@user.email}") + }.should change(Invitation, :count).by(1) end end describe "#update" do before do - @invited_user = @user.invite_user(@aspect.id, 'email', "a@a.com") + invite = Factory(:invitation, :sender => @user, :service => 'email', :identifier => "a@a.com") + @invited_user = invite.attach_recipient! + @accept_params = {:user=> {:password_confirmation =>"password", :email => "a@a.com", @@ -137,7 +135,8 @@ describe InvitationsController do @controller.stub!(:current_user).and_return(@user) request.env["HTTP_REFERER"]= 'http://test.host/cats/foo' - @invited_user = @user.invite_user(@aspect.id, 'email', "a@a.com", "") + invite = Factory(:invitation, :sender => @user, :service => 'email', :identifier => "a@a.com") + @invited_user = invite.attach_recipient! end it 'calls resend invitation if one exists' do @@ -148,12 +147,24 @@ describe InvitationsController do end it 'does not send an invitation for a different user' do - @user2 = bob - @aspect2 = @user2.aspects.create(:name => "cats") - @user2.invite_user(@aspect2.id, 'email', "b@b.com", "") - invitation2 = @user2.reload.invitations_from_me.first + invitation2 = Factory(:invitation, :sender => bob, :service => 'email', :identifier => "a@a.com") + Resque.should_not_receive(:enqueue) - put :resend, :id => invitation2.id + put :resend, :id => invitation2.id + end + end + + + describe '#extract_messages' do + before do + sign_in alice + end + it 'displays a message that tells the user how many invites were sent, and which REJECTED' do + post :create, :user => @invite.merge( + :email => "mbs@gmail.com, foo@bar.com, foo.com, lala@foo, cool@bar.com") + flash[:notice].should_not be_empty + flash[:notice].should =~ /foo\.com/ + flash[:notice].should =~ /lala@foo/ end end end diff --git a/spec/controllers/services_controller_spec.rb b/spec/controllers/services_controller_spec.rb index e7674e675..1333262f4 100644 --- a/spec/controllers/services_controller_spec.rb +++ b/spec/controllers/services_controller_spec.rb @@ -146,7 +146,7 @@ describe ServicesController do end it 'does not create a duplicate invitation' do - inv = Invitation.create!(:sender_id => @user.id, :recipient_id => eve.id, :aspect_id => @user.aspects.first.id) + inv = Invitation.create!(:sender => @user, :recipient => eve, :aspect => @user.aspects.first, :identifier => eve.email) @invite_params[:invitation_id] = inv.id lambda { diff --git a/spec/factories.rb b/spec/factories.rb index e5bca7665..e90ab36d3 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -96,6 +96,15 @@ Factory.define :reshare do |r| r.association(:author, :factory => :person) end +Factory.define :invitation do |i| + i.service "email" + i.identifier "bob.smith@smith.com" + i.association :sender, :factory => :user_with_aspect + i.after_build do |i| + i.aspect = i.sender.aspects.first + end +end + Factory.define :service do |service| service.nickname "sirrobertking" service.type "Services::Twitter" diff --git a/spec/lib/rake_helper_spec.rb b/spec/lib/rake_helper_spec.rb index 6f2b6d40b..f511065d6 100644 --- a/spec/lib/rake_helper_spec.rb +++ b/spec/lib/rake_helper_spec.rb @@ -14,8 +14,9 @@ describe RakeHelpers do Devise.mailer.deliveries = [] end it 'should send emails to each backer' do - Invitation.should_receive(:create_invitee).exactly(3).times - process_emails(@csv, 100, 1, false) + expect{ + process_emails(@csv, 100, 1, false) + }.to change(User, :count).by(3) end it 'should not send the email to the same email twice' do diff --git a/spec/models/invitation_spec.rb b/spec/models/invitation_spec.rb index 71c27b7a8..30b081737 100644 --- a/spec/models/invitation_spec.rb +++ b/spec/models/invitation_spec.rb @@ -6,7 +6,6 @@ require 'spec_helper' describe Invitation do let(:user) { alice } - let(:aspect) { user.aspects.first } before do @email = 'maggie@example.com' @@ -14,292 +13,101 @@ describe Invitation do end describe 'validations' do before do - aspect - @invitation = Invitation.new(:sender => user, :recipient => eve, :aspect => aspect) + @invitation = Factory.build(:invitation, :sender => user, :recipient => eve, :aspect => user.aspects.first) end + it 'is valid' do @invitation.sender.should == user @invitation.recipient.should == eve - @invitation.aspect.should == aspect + @invitation.aspect.should == user.aspects.first @invitation.should be_valid end - it 'is from a user' do - @invitation.sender = nil - @invitation.should_not be_valid - end - it 'is to a user' do - @invitation.recipient = nil - @invitation.should_not be_valid - end - it 'is into an aspect' do - @invitation.aspect = nil - @invitation.should_not be_valid + + it 'ensures the sender is placing the recipient into one of his aspects' do + pending end end it 'has a message' do - @invitation = Invitation.new(:sender => user, :recipient => eve, :aspect => aspect) + @invitation = Factory.build(:invitation, :sender => user, :recipient => eve, :aspect => user.aspects.first) @invitation.message = "!" @invitation.message.should == "!" end - describe '.find_existing_user' do - let(:inv) { Invitation.find_existing_user(@type, @identifier) } - context 'send a request to an existing' do - context 'active user' do - it 'by email' do - @identifier = alice.email - @type = 'email' - inv.should == alice - end - - it 'by service' do - uid = '123324234' - alice.services << Services::Facebook.new(:uid => uid) - alice.save - - @type = 'facebook' - @identifier = uid - - inv.should == alice - end - end - - context 'invited user' do - it 'by email' do - @identifier = alice.email - @type = 'email' - - alice.invitation_identifier = @identifier - alice.invitation_service = @type - alice.save - inv.should == alice - end - - it 'by service' do - fb_id = 'abc123' - alice.invitation_service = 'facebook' - alice.invitation_identifier = fb_id - alice.save - - @identifier = fb_id - @type = 'facebook' - inv.should == alice - end - end + describe 'the invite process' do + before do end - end - describe '.invite' do - it 'creates an invitation' do + it 'works for a new user' do + invite = Invitation.new(:sender => alice, :aspect => alice.aspects.first, :service => 'email', :identifier => 'foo@bar.com') lambda { - Invitation.invite(:service => 'email', :identifier => @email, :from => user, :into => aspect) - }.should change(Invitation, :count).by(1) - end - - it 'associates the invitation with the inviter' do - lambda { - Invitation.invite(:service => 'email', :identifier => @email, :from => user, :into => aspect) - }.should change { user.reload.invitations_from_me.count }.by(1) - end - - it 'associates the invitation with the invitee' do - new_user = Invitation.invite(:service => 'email', :identifier => @email, :from => user, :into => aspect) - new_user.invitations_to_me.count.should == 1 - end - - it 'creates a user' do - lambda { - Invitation.invite(:from => user, :service => 'email', :identifier => @email, :into => aspect) + invite.save + invite.send! }.should change(User, :count).by(1) end - it 'returns the new user' do - new_user = Invitation.invite(:from => user, :service => 'email', :identifier => @email, :into => aspect) - new_user.is_a?(User).should be_true - new_user.email.should == @email + it 'works for a current user(with the right email)' do + invite = Invitation.create(:sender => alice, :aspect => alice.aspects.first, :service => 'email', :identifier => bob.email) + lambda { + invite.send! + }.should_not change(User, :count) end - it 'adds the inviter to the invited_user' do - new_user = Invitation.invite(:from => user, :service => 'email', :identifier => @email, :into => aspect) - new_user.invitations_to_me.first.sender.should == user + it 'works for a current user(with the same fb id)' do + bob.services << Factory.build(:service, :type => "Services::Facebook") + invite = Invitation.create(:sender => alice, :aspect => alice.aspects.first, :service => 'facebook', :identifier => bob.services.first.uid) + lambda { + invite.send! + }.should_not change(User, :count) + end + it 'is able to resend an invite' do end - it 'adds an optional message' do - message = "How've you been?" - new_user = Invitation.invite(:from => user, :service => 'email', :identifier => @email, :into => aspect, :message => message) - new_user.invitations_to_me.first.message.should == message + it 'handles the case when that user has an invite but not a user' do end - it 'sends a contact request to a user with that email into the aspect' do - user.should_receive(:share_with).with(eve.person, aspect) - Invitation.invite(:from => user, :service => 'email', :identifier => eve.email, :into => aspect) - end - - context 'invalid email' do - it 'return a user with errors' do - new_user = Invitation.invite(:service => 'email', :identifier => "fkjlsdf", :from => user, :into => aspect) - new_user.should have(1).errors_on(:email) - new_user.should_not be_persisted - end + it 'handles the case where that user has an invite but has not yet accepted' do end end - - describe '.create_invitee' do - context "when we're resending an invitation" do - before do - @valid_params = {:from => user, - :service => 'email', - :identifier => @email, - :into => aspect, - :message => @message} - @invitee = Invitation.create_invitee(:service => 'email', :identifier => @email) - @valid_params[:existing_user] = @invitee - end - - it "does not create a user" do - expect { Invitation.create_invitee(@valid_params) }.should_not change(User, :count) - end - - it "sends mail" do - expect { - Invitation.create_invitee(@valid_params) - }.should change { Devise.mailer.deliveries.size }.by(1) - end - - it "does not set the key" do - expect { - Invitation.create_invitee(@valid_params) - }.should_not change { @invitee.reload.serialized_private_key } - end - - it "does not change the invitation token" do - old_token = @invitee.invitation_token - Invitation.create_invitee(@valid_params) - @invitee.reload.invitation_token.should == old_token - end - end - context 'with an inviter' do - before do - @message = "whatever" - @valid_params = {:from => user, :service => 'email', :identifier => @email, :into => aspect, :message => @message} - end - - it "sends mail" do - expect { - Invitation.create_invitee(@valid_params) - }.should change { Devise.mailer.deliveries.size }.by(1) - end - - it "includes the message in the email" do - Invitation.create_invitee(@valid_params) - Devise.mailer.deliveries.last.to_s.should include(@message) - end - - it "has no translation missing" do - Invitation.create_invitee(@valid_params) - Devise.mailer.deliveries.last.body.raw_source.should_not match(/(translation_missing.+)/) - end - - it "doesn't create a user if the email is invalid" do - new_user = Invitation.create_invitee(@valid_params.merge(:identifier => 'fdfdfdfdf')) - new_user.should_not be_persisted - new_user.should have(1).error_on(:email) - end - - it "does not save a user with an empty string email" do - @valid_params[:service] = 'facebook' - @valid_params[:identifier] = '3423423' - Invitation.create_invitee(@valid_params) - @valid_params[:identifier] = 'dfadsfdas' - expect { Invitation.create_invitee(@valid_params) }.should_not raise_error - end - end - - context 'with no inviter' do - it 'sends an email that includes the right things' do - Invitation.create_invitee(:service => 'email', :identifier => @email) - Devise.mailer.deliveries.first.to_s.should include("Email not displaying correctly?") - end - it 'creates a user' do - expect { - Invitation.create_invitee(:service => 'email', :identifier => @email) - }.should change(User, :count).by(1) - end - it 'sends email to the invited user' do - expect { - Invitation.create_invitee(:service => 'email', :identifier => @email) - }.should change { Devise.mailer.deliveries.size }.by(1) - end - it 'does not create an invitation' do - expect { - Invitation.create_invitee(:service => 'email', :identifier => @email) - }.should_not change(Invitation, :count) - end - end - end - + describe '.resend' do before do - aspect - user.invite_user(aspect.id, 'email', "a@a.com", "") - @invitation = user.reload.invitations_from_me.first + @invitation = Factory(:invitation, :sender => alice, :aspect => alice.aspects.first, :service => 'email', :identifier => 'a@a.com') end it 'sends another email' do - lambda { @invitation.resend }.should change(Devise.mailer.deliveries, :count).by(1) - end - end - - describe '#share_with!' do - before do - @new_user = Invitation.invite(:from => user, :service => 'email', :identifier => @email, :into => aspect) - acceptance_params = {:invitation_token => "abc", - :username => "user", - :email => @email, - :password => "secret", - :password_confirmation => "secret", - :person => {:profile => {:first_name => "Bob", :last_name => "Smith"}}} - @new_user.setup(acceptance_params) - @new_user.person.save - @new_user.save - @invitation = @new_user.invitations_to_me.first - end - - it 'destroys the invitation' do lambda { - @invitation.share_with! - }.should change(Invitation, :count).by(-1) - end - - it 'creates a contact for the inviter and invitee' do - lambda { - @invitation.share_with! - }.should change(Contact, :count).by(2) + @invitation.resend + }.should change(Devise.mailer.deliveries, :count).by(1) end end describe '#recipient_identifier' do it 'calls email if the invitation_service is email' do - alice.invite_user(aspect.id, 'email', "a@a.com", "") - invitation = alice.reload.invitations_from_me.first - invitation.recipient_identifier.should == 'a@a.com' + email = 'abc@abc.com' + invitation = Factory(:invitation, :sender => alice, :service => 'email', :identifier => email, :aspect => alice.aspects.first) + invitation.recipient_identifier.should == email end - it 'gets the name if the invitation_service is facebook' do - alice.services << Services::Facebook.new(:uid => "13234895") - alice.reload.services(true).first.service_users.create(:uid => "23526464", :photo_url => 'url', :name => "Remote User") - alice.invite_user(aspect.id, 'facebook', "23526464", '') - invitation = alice.reload.invitations_from_me.first - invitation.recipient_identifier.should == "Remote User" - end - it 'does not error if the facebook user is not recorded' do - alice.services << Services::Facebook.new(:uid => "13234895") - alice.reload.services(true).first.service_users.create(:uid => "23526464", :photo_url => 'url', :name => "Remote User") - alice.invite_user(aspect.id, 'facebook', "23526464", '') - alice.services.first.service_users.delete_all - invitation = alice.reload.invitations_from_me.first - invitation.recipient_identifier.should == "A Facebook user" + + context 'facebook' do + before do + @uid = '23526464' + @service = "facebook" + alice.services << Services::Facebook.new(:uid => "13234895") + alice.reload.services(true).first.service_users.create(:uid => @uid, :photo_url => 'url', :name => "Remote User") + end + + it 'gets the name if the invitation_service is facebook' do + invitation = Factory(:invitation, :sender => alice, :identifier => @uid, :service => @service, :aspect => alice.aspects.first) + invitation.recipient_identifier.should == "Remote User" + end + + it 'does not error if the facebook user is not recorded' do + invitation = Factory(:invitation, :sender => alice, :identifier => @uid, :service => @service, :aspect => alice.aspects.first) + alice.services.first.service_users.delete_all + invitation.recipient_identifier.should == "A Facebook user" + end end end end diff --git a/spec/models/jobs/mail/invite_user_by_email_spec.rb b/spec/models/jobs/mail/invite_user_by_email_spec.rb index 8f3181737..0bc8beb0e 100644 --- a/spec/models/jobs/mail/invite_user_by_email_spec.rb +++ b/spec/models/jobs/mail/invite_user_by_email_spec.rb @@ -4,20 +4,14 @@ describe Job::Mail::InviteUserByEmail do before do @sender = alice @email = 'bob@bob.com' - @aspect_id = alice.aspects.first.id + @aspect = alice.aspects.first @message = 'invite message' - - User.stub(:find){ |id| - if id == @sender.id - @sender - else - nil - end - } end it 'calls invite_user with email param' do - @sender.should_receive(:invite_user).with(@aspect_id, 'email', @email, @message) - Job::Mail::InviteUserByEmail.perform(@sender.id, @email, @aspect_id, @message) + invitation = Invitation.create(:sender => @sender, :identifier => @email, :service => "email", :aspect => @aspect, :message => @message) + invitation.should_receive(:send!) + Invitation.stub(:find).and_return(invitation) + Job::Mail::InviteUserByEmail.perform(invitation.id) end end diff --git a/spec/models/jobs/resend_invitation_spec.rb b/spec/models/jobs/resend_invitation_spec.rb index de7f54074..25969047e 100644 --- a/spec/models/jobs/resend_invitation_spec.rb +++ b/spec/models/jobs/resend_invitation_spec.rb @@ -5,17 +5,13 @@ require 'spec_helper' describe Job::ResendInvitation do - describe '#perfom_delegate' do + describe '#perfom' do it 'should call .resend on the object' do - user = alice - aspect = user.aspects.create(:name => "cats") - user.invite_user(aspect.id, 'email', "a@a.com", "") - invitation = user.reload.invitations_from_me.first + invite = Factory(:invitation, :service => 'email', :identifier => 'foo@bar.com') - #Notification.should_receive(:notify).with(instance_of(User), instance_of(StatusMessage), instance_of(Person)) - Invitation.stub(:where).with(:id => invitation.id ).and_return([invitation]) - invitation.should_receive(:resend) - Job::ResendInvitation.perform(invitation.id) + Invitation.stub(:find).and_return(invite) + invite.should_receive(:resend) + Job::ResendInvitation.perform(invite.id) end end end diff --git a/spec/models/user/invite_spec.rb b/spec/models/user/invite_spec.rb deleted file mode 100644 index 41ce0e6c7..000000000 --- a/spec/models/user/invite_spec.rb +++ /dev/null @@ -1,90 +0,0 @@ -# Copyright (c) 2010, Diaspora Inc. This file is -# licensed under the Affero General Public License version 3 or later. See -# the COPYRIGHT file. - -require 'spec_helper' - -describe User do - context "creating invites" do - before do - @aspect = eve.aspects.first - @email = "bob@bob.com" - end - - it 'requires your aspect' do - lambda { - eve.invite_user(alice.aspects.first.id, "email", "maggie@example.com") - }.should raise_error ActiveRecord::RecordNotFound - end - - it 'takes a service parameter' do - @invite_params = {:service => 'email'} - Invitation.should_receive(:invite).with(hash_including(@invite_params)) - eve.invite_user(@aspect.id, 'email', @email) - end - - it 'takes an indentifier parameter' do - @invite_params = {:identifier => @email} - Invitation.should_receive(:invite).with(hash_including(@invite_params)) - eve.invite_user(@aspect.id, 'email', @email) - end - - it 'calls Invitation.invite' do - Invitation.should_receive(:invite) - eve.invite_user(@aspect.id, 'email', @email) - end - - it 'has an invitation' do - eve.invite_user(@aspect.id, 'email', @email).invitations_to_me.count.should == 1 - end - - it 'creates it with an email' do - eve.invite_user(@aspect.id, 'email', @email).email.should == @email - end - - it "throws if you try to add someone you're connected to" do - connect_users(eve, @aspect, alice, alice.aspects.first) - lambda { - eve.invite_user(@aspect.id, 'email', alice.email) - }.should raise_error ActiveRecord::RecordNotUnique - end - - it 'does not invite people I already invited' do - eve.invite_user(@aspect.id, 'email', "email1@example.com") - lambda { - eve.invite_user(@aspect.id, 'email', "email1@example.com") - }.should raise_error /You already invited this person/ - end - end - - describe "#accept_invitation!" do - before do - invite_pre = Invitation.invite(:from => eve, :service => 'email', :identifier => 'invitee@example.org', :into => eve.aspects.first).reload - @person_count = Person.count - @invited_user = invite_pre.accept_invitation!(:invitation_token => "abc", - :email => "a@a.com", - :username => "user", - :password => "secret", - :password_confirmation => "secret", - :person => {:profile => {:first_name => "Bob", - :last_name => "Smith"}} ) - - end - - context 'after invitation acceptance' do - it 'destroys the invitations' do - @invited_user.invitations_to_me.count.should == 0 - end - - it "should create the person with the passed in params" do - Person.count.should == @person_count + 1 - @invited_user.person.profile.first_name.should == "Bob" - end - - it 'resolves incoming invitations into contact requests' do - eve.contacts.where(:person_id => @invited_user.person.id).count.should == 1 - end - end - end -end - diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f34193fd5..7c472fd6c 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -318,6 +318,55 @@ describe User do end end + describe '.find_by_invitation' do + let(:invited_user) { + inv = Factory.build(:invitation, :recipient => @recipient, :service => @type, :identifier => @identifier) + User.find_by_invitation(inv) + } + + context 'send a request to an existing' do + before do + @recipient = alice + end + + context 'active user' do + it 'by service' do + @type = 'facebook' + @identifier = '123456' + + @recipient.services << Services::Facebook.new(:uid => @identifier) + @recipient.save + + invited_user.should == @recipient + end + + it 'by email' do + @type = 'email' + @identifier = alice.email + + invited_user.should == @recipient + end + end + + context 'invited user' do + it 'by service and identifier' do + @identifier = alice.email + @type = 'email' + invited_user.should == alice + end + end + + context 'not on server (not yet invited)' do + it 'returns nil' do + @recipient = nil + @identifier = 'foo@bar.com' + @type = 'email' + invited_user.should be_nil + end + end + end + end + describe 'update_user_preferences' do before do @pref_count = UserPreference::VALID_EMAIL_TYPES.count @@ -484,14 +533,14 @@ describe User do describe '#destroy' do it 'removes invitations from the user' do - alice.invite_user alice.aspects.first.id, 'email', 'blah@blah.blah' + Factory(:invitation, :sender => alice) lambda { alice.destroy }.should change {alice.invitations_from_me(true).count }.by(-1) end it 'removes invitations to the user' do - Invitation.create(:sender_id => eve.id, :recipient_id => alice.id, :aspect_id => eve.aspects.first.id) + Invitation.create!(:sender => eve, :recipient => alice, :identifier => alice.email, :aspect => eve.aspects.first) lambda { alice.destroy }.should change {alice.invitations_to_me(true).count }.by(-1) @@ -810,6 +859,52 @@ describe User do end end + describe "#accept_invitation!" do + before do + fantasy_resque do + @invitation = Factory.create(:invitation, :sender => eve, :identifier => 'invitee@example.org', :aspect => eve.aspects.first) + end + @invitation.reload + @form_params = {:invitation_token => "abc", + :email => "a@a.com", + :username => "user", + :password => "secret", + :password_confirmation => "secret", + :person => {:profile => {:first_name => "Bob", + :last_name => "Smith"}}} + + end + + context 'after invitation acceptance' do + it 'destroys the invitations' do + user = @invitation.recipient.accept_invitation!(@form_params) + user.invitations_to_me.count.should == 0 + end + + it "should create the person with the passed in params" do + lambda { + @invitation.recipient.accept_invitation!(@form_params) + }.should change(Person, :count).by(1) + end + + it 'resolves incoming invitations into contact requests' do + user = @invitation.recipient.accept_invitation!(@form_params) + eve.contacts.where(:person_id => user.person.id).count.should == 1 + end + end + + context 'from an admin' do + it 'should work' do + i = nil + fantasy_resque do + i = Invitation.create!(:admin => true, :service => 'email', :identifier => "new_invitee@example.com") + end + i.reload + i.recipient.accept_invitation!(@form_params) + end + end + end + describe '#retract' do before do @retraction = mock