From 6beeecefd8a7e24e0ca66755af9bf077f28433e3 Mon Sep 17 00:00:00 2001
From: Maxwell Salzberg <maxwell@joindiaspora.com>
Date: Thu, 18 Aug 2011 18:27:24 -0700
Subject: [PATCH] add better messages telling a user that they sent share
requests if they try and email exsisting users
---
app/controllers/invitations_controller.rb | 9 +++++++--
app/models/invitation.rb | 13 +++++++++++--
spec/controllers/services_controller_spec.rb | 4 +++-
spec/models/invitation_spec.rb | 8 ++++----
spec/models/user_spec.rb | 2 +-
5 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/app/controllers/invitations_controller.rb b/app/controllers/invitations_controller.rb
index 924254a836..7b070c1c85 100644
--- a/app/controllers/invitations_controller.rb
+++ b/app/controllers/invitations_controller.rb
@@ -87,14 +87,19 @@ class InvitationsController < Devise::InvitationsController
def extract_messages(invites)
success_message = "Invites Successfully Sent to: "
failure_message = "There was a problem with: "
+ following_message = " already are on Diaspora, so you are now sharing with them."
successes, failures = invites.partition{|x| x.persisted? }
- success_message += successes.map{|k| k.identifier }.join(', ')
- failure_message += failures.map{|k| k.identifier }.join(', ')
+ followings, real_failures = failures.partition{|x| x.errors[:recipient].present? }
+
+ success_message += successes.map{|k| k.identifier }.to_sentence
+ failure_message += real_failures.map{|k| k.identifier }.to_sentence
+ following_message += followings.map{|k| k.identifier}.to_sentence
messages = []
messages << success_message if successes.present?
messages << failure_message if failures.present?
+ messages << following_message if followings.present?
messages.join('\n')
end
diff --git a/app/models/invitation.rb b/app/models/invitation.rb
index 344bc7d048..c75375dab3 100644
--- a/app/models/invitation.rb
+++ b/app/models/invitation.rb
@@ -12,8 +12,10 @@ class Invitation < ActiveRecord::Base
before_validation :set_email_as_default_service
+ # before_create :share_with_exsisting_user, :if => :recipient_id?
validates_presence_of :identifier, :service
validate :valid_identifier?
+ validate :recipient_not_on_pod?
validates_presence_of :sender, :aspect, :unless => :admin?
validate :ensure_not_inviting_self, :on => :create, :unless => :admin?
validate :sender_owns_aspect?, :unless => :admin?
@@ -36,11 +38,11 @@ class Invitation < ActiveRecord::Base
users_on_pod = User.where(:email => emails, :invitation_token => nil)
#share with anyone whose email you entered who is on the pod
- emails = emails - users_on_pod.map{|u| u.email}
users_on_pod.each{|u| opts[:sender].share_with(u.person, opts[:aspect])}
emails.map! do |e|
- Invitation.create(opts.merge(:identifier => e))
+ user = users_on_pod.find{|u| u.email == e}
+ Invitation.create(opts.merge(:identifier => e, :recipient => user))
end
emails
end
@@ -140,6 +142,13 @@ class Invitation < ActiveRecord::Base
end
+ def recipient_not_on_pod?
+ return true if self.recipient.nil?
+ if self.recipient.username?
+ errors[:recipient] << "The user '#{self.identifier}' (#{self.recipient.diaspora_handle}) is already on this pod, so we sent them a share request"
+ end
+ end
+
# @note Validation
def valid_identifier?
return false unless self.identifier
diff --git a/spec/controllers/services_controller_spec.rb b/spec/controllers/services_controller_spec.rb
index 1333262f46..8012c880c7 100644
--- a/spec/controllers/services_controller_spec.rb
+++ b/spec/controllers/services_controller_spec.rb
@@ -146,7 +146,9 @@ describe ServicesController do
end
it 'does not create a duplicate invitation' do
- inv = Invitation.create!(:sender => @user, :recipient => eve, :aspect => @user.aspects.first, :identifier => eve.email)
+ invited_user = Factory.build(:user, :username =>nil)
+ invited_user.save(:validate => false)
+ inv = Invitation.create!(:sender => @user, :recipient => invited_user, :aspect => @user.aspects.first, :identifier => eve.email)
@invite_params[:invitation_id] = inv.id
lambda {
diff --git a/spec/models/invitation_spec.rb b/spec/models/invitation_spec.rb
index 329df29719..4e5b531df7 100644
--- a/spec/models/invitation_spec.rb
+++ b/spec/models/invitation_spec.rb
@@ -13,12 +13,12 @@ describe Invitation do
end
describe 'validations' do
before do
- @invitation = Factory.build(:invitation, :sender => user, :recipient => eve, :aspect => user.aspects.first)
+ @invitation = Factory.build(:invitation, :sender => user, :recipient => nil, :aspect => user.aspects.first)
end
it 'is valid' do
@invitation.sender.should == user
- @invitation.recipient.should == eve
+ @invitation.recipient.should == nil
@invitation.aspect.should == user.aspects.first
@invitation.should be_valid
end
@@ -89,13 +89,13 @@ describe Invitation do
invites.all?{|x| x.persisted?}.should be_true
end
- it 'shares with people who are already on the pod and does not create an invite for them' do
+ it 'shares with people who are already on the pod' do
Factory(:user, :email => @emails.first)
invites = nil
expect{
invites = Invitation.batch_invite(@emails, @opts)
}.to change(eve.contacts, :count).by(1)
- invites.count.should be 1
+ invites.count.should be 2
end
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index a3e898d383..100fd8087e 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -565,7 +565,7 @@ describe User do
end
it 'removes invitations to the user' do
- Invitation.create!(:sender => eve, :recipient => alice, :identifier => alice.email, :aspect => eve.aspects.first)
+ Invitation.new(:sender => eve, :recipient => alice, :identifier => alice.email, :aspect => eve.aspects.first).save(:validate => false)
lambda {
alice.destroy
}.should change {alice.invitations_to_me(true).count }.by(-1)
--
GitLab