From b1c0facfe6cbfae91e6b9a7c6204485534859128 Mon Sep 17 00:00:00 2001 From: danielvincent <danielgrippi@gmail.com> Date: Tue, 14 Dec 2010 17:08:53 -0800 Subject: [PATCH] initiating a request now just creates a pending contact instead of persisting a request --- app/controllers/application_controller.rb | 2 +- app/controllers/people_controller.rb | 1 + app/controllers/requests_controller.rb | 15 +++-- app/mailers/notifier.rb | 3 +- app/models/aspect.rb | 4 +- app/models/contact.rb | 25 +++++++- app/models/jobs/mail_request_acceptance.rb | 4 +- app/models/request.rb | 10 +-- app/views/notifier/request_accepted.html.haml | 2 - app/views/notifier/request_accepted.text.haml | 2 - config/locales/diaspora/en.yml | 7 ++- features/support/env.rb | 11 ---- lib/diaspora/user/connecting.rb | 48 +++++--------- .../invitations_controller_spec.rb | 15 +---- spec/controllers/people_controller_spec.rb | 6 +- spec/controllers/publics_controller_spec.rb | 4 -- spec/controllers/requests_controller_spec.rb | 2 +- spec/helper_methods.rb | 14 ----- spec/mailers/notifier_spec.rb | 10 +-- spec/models/contact_spec.rb | 62 ++++++++++++++++++- spec/models/invitation_spec.rb | 8 ++- spec/models/request_spec.rb | 21 ------- spec/models/user/connecting_spec.rb | 40 +++++++----- spec/models/user/invite_spec.rb | 3 +- spec/spec_helper.rb | 15 +++-- 25 files changed, 173 insertions(+), 161 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3e6a05a257..0bf2bef0db 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -31,7 +31,7 @@ class ApplicationController < ActionController::Base end def count_requests - @request_count = current_user.requests_for_me.count if current_user + @request_count = current_user.pending_requests.count if current_user end def set_invites diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index fdea0ad0bf..628640ef97 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -22,6 +22,7 @@ class PeopleController < ApplicationController end end end + def hashes_for_people people, aspects ids = people.map{|p| p.id} requests = {} diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 696b37ff1c..db0d8aba14 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -36,15 +36,18 @@ class RequestsController < ApplicationController current_user.accept_and_respond(existing_request.id, aspect.id) redirect_to :back else - @request = Request.instantiate(:to => person, - :from => current_user.person, - :into => aspect) - if @request.save - current_user.dispatch_request(@request) + + @contact = Contact.new(:user => current_user, + :person => person, + :aspect_ids => [aspect.id], + :pending => true) + + if @contact.save + @contact.dispatch_request flash.now[:notice] = I18n.t('requests.create.sent') redirect_to :back else - flash.now[:error] = @request.errors.full_messages.join(', ') + flash.now[:error] = @contact.errors.full_messages.join(', ') redirect_to :back end end diff --git a/app/mailers/notifier.rb b/app/mailers/notifier.rb index 6cb3b2a4ae..73f42a446a 100644 --- a/app/mailers/notifier.rb +++ b/app/mailers/notifier.rb @@ -33,10 +33,9 @@ class Notifier < ActionMailer::Base :subject => I18n.t('notifier.new_request.subject', :from => @sender.name), :host => APP_CONFIG[:pod_uri].host) end - def request_accepted(recipient_id, sender_id, aspect_id) + def request_accepted(recipient_id, sender_id) @receiver = User.find_by_id(recipient_id) @sender = Person.find_by_id(sender_id) - @aspect = Aspect.find_by_id(aspect_id) log_mail(recipient_id, sender_id, 'request_accepted') diff --git a/app/models/aspect.rb b/app/models/aspect.rb index e9671a442e..f59c77e3da 100644 --- a/app/models/aspect.rb +++ b/app/models/aspect.rb @@ -6,12 +6,10 @@ class Aspect include MongoMapper::Document key :name, String - key :request_ids, Array key :post_ids, Array many :contacts, :foreign_key => 'aspect_ids', :class_name => 'Contact' - many :requests, :in => :request_ids, :class_name => 'Request' - many :posts, :in => :post_ids, :class_name => 'Post' + many :posts, :in => :post_ids, :class_name => 'Post' belongs_to :user, :class_name => 'User' diff --git a/app/models/contact.rb b/app/models/contact.rb index 8e7ddfc9be..57262a4326 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -5,13 +5,36 @@ class Contact include MongoMapper::Document + key :pending, Boolean + + key :user_id, ObjectId belongs_to :user validates_presence_of :user + key :person_id, ObjectId belongs_to :person validates_presence_of :person + validates_uniqueness_of :person_id, :scope => :user_id + + validate :not_contact_for_self key :aspect_ids, Array, :typecast => 'ObjectId' many :aspects, :in => :aspect_ids, :class_name => 'Aspect' - + + def dispatch_request + request = self.generate_request + self.user.push_to_people(request, [self.person]) + request + end + + def generate_request + Request.new(:from => self.user, :to => self.person) + end + + private + def not_contact_for_self + if person.owner_id == user.id + errors[:base] << 'Cannot create self-contact' + end + end end diff --git a/app/models/jobs/mail_request_acceptance.rb b/app/models/jobs/mail_request_acceptance.rb index 6e00b43739..7cdeb6ae0a 100644 --- a/app/models/jobs/mail_request_acceptance.rb +++ b/app/models/jobs/mail_request_acceptance.rb @@ -1,8 +1,8 @@ module Jobs class MailRequestAcceptance @queue = :mail - def self.perform(recipient_id, sender_id, aspect_id) - Notifier.request_accepted(recipient_id, sender_id, aspect_id).deliver + def self.perform(recipient_id, sender_id) + Notifier.request_accepted(recipient_id, sender_id).deliver end end end diff --git a/app/models/request.rb b/app/models/request.rb index 019d8d1d3d..15a5371135 100644 --- a/app/models/request.rb +++ b/app/models/request.rb @@ -2,9 +2,9 @@ # licensed under the Affero General Public License version 3 or later. See # the COPYRIGHT file. -class Request +class Request require File.join(Rails.root, 'lib/diaspora/webhooks') - + include MongoMapper::Document include Diaspora::Webhooks include ROXML @@ -22,12 +22,12 @@ class Request validate :not_already_connected_if_not_sent validate :no_pending_request, :if => :sent validate :not_friending_yourself - + scope :from, lambda { |person| target = (person.is_a?(User) ? person.person : person) where(:from_id => target.id) } - + scope :to, lambda { |person| target = (person.is_a?(User) ? person.person : person) where(:to_id => target.id) @@ -59,7 +59,7 @@ class Request def recipient_handle to.diaspora_handle end - + def recipient_handle= recipient_handle self.to = Person.first(:diaspora_handle => recipient_handle) end diff --git a/app/views/notifier/request_accepted.html.haml b/app/views/notifier/request_accepted.html.haml index c8b225911e..2f77616f66 100644 --- a/app/views/notifier/request_accepted.html.haml +++ b/app/views/notifier/request_accepted.html.haml @@ -3,8 +3,6 @@ %p = "#{@sender.name} (#{@sender.diaspora_handle})" = t('.accepted') - = link_to @aspect.name, aspect_url(@aspect) - = t('.aspect') %br = t('notifier.love') diff --git a/app/views/notifier/request_accepted.text.haml b/app/views/notifier/request_accepted.text.haml index eade8373d7..a815609b96 100644 --- a/app/views/notifier/request_accepted.text.haml +++ b/app/views/notifier/request_accepted.text.haml @@ -1,8 +1,6 @@ = t('notifier.hello', :name => @receiver.profile.first_name) = "#{@sender.name} (#{@sender.diaspora_handle})" = t('notifier.request_accepted.accepted') -= "#{@aspect.name} #{t('notifier.request_accepted.aspect')}\n" -= "#{aspect_url(@aspect)}" = "#{t('notifier.love')} \n" diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index a975340fa4..bb0489b9fb 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -53,6 +53,10 @@ en: attributes: diaspora_handle: taken: "is already taken" + contact: + attributes: + person_id: + taken: "must be unique among this user's contacts" application: helper: unknown_person: "unknown person" @@ -402,8 +406,7 @@ en: sign_in: "sign in here" request_accepted: subject: "%{name} has accepted your contact request on Diaspora*" - accepted: "has accepted your contact request. They are now in your" - aspect: "aspect." + accepted: "has accepted your contact request!" home: show: share_what_you_want: "Share what you want, with whom you want." diff --git a/features/support/env.rb b/features/support/env.rb index 93119226c1..0e70c173dc 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -66,17 +66,6 @@ class FakeRedis true end end -class User - def send_contact_request_to(desired_contact, aspect) - request = Request.instantiate(:to => desired_contact, - :from => self.person, - :into => aspect) - if request.save! - dispatch_request request - end - request - end -end Before do UserFixer.regenerate_user_fixtures diff --git a/lib/diaspora/user/connecting.rb b/lib/diaspora/user/connecting.rb index 62fc304db9..8a6e53458b 100644 --- a/lib/diaspora/user/connecting.rb +++ b/lib/diaspora/user/connecting.rb @@ -6,21 +6,17 @@ module Diaspora module UserModules module Connecting def send_contact_request_to(desired_contact, aspect) - request = Request.instantiate(:to => desired_contact, - :from => self.person, - :into => aspect) - if request.save! - dispatch_request request + contact = Contact.new(:person => desired_contact, + :user => self, + :pending => true) + contact.aspects << aspect + + if contact.save! + request = contact.dispatch_request + request + else + nil end - request - end - def dispatch_request(request) - self.pending_requests << request - self.save - - request.into.requests << request - request.into.save - push_to_people request, [request.to] end def accept_contact_request(request, aspect) @@ -56,11 +52,11 @@ module Diaspora def receive_contact_request(contact_request) #response from a contact request you sent - if original_request = original_request(contact_request) - receive_request_acceptance(contact_request, original_request) + if original_contact = self.contact_for(contact_request.from) + receive_request_acceptance(contact_request, original_contact) #this is a new contact request - elsif !request_from_me?(contact_request) + elsif contact_request.from != self.person if contact_request.save! self.pending_requests << contact_request self.save! @@ -74,16 +70,14 @@ module Diaspora contact_request end - def receive_request_acceptance(received_request, sent_request) - destination_aspect = self.aspect_by_id(sent_request.into_id) - activate_contact(received_request.from, destination_aspect) + def receive_request_acceptance(received_request, contact) + contact.pending = false + contact.save Rails.logger.info("event=contact_request status=received_acceptance from=#{received_request.from.diaspora_handle} to=#{self.diaspora_handle}") received_request.destroy - pending_requests.delete(sent_request) - sent_request.destroy self.save - self.mail(Jobs::MailRequestAcceptance, self.id, received_request.from.id, destination_aspect.id) + self.mail(Jobs::MailRequestAcceptance, self.id, received_request.from.id) end def disconnect(bad_contact) @@ -129,14 +123,6 @@ module Diaspora aspect.save! end - def request_from_me?(request) - request.from == self.person - end - - def original_request(response) - pending_requests.first(:from_id => self.person.id, :to_id => response.from.id) - end - def requests_for_me pending_requests.select { |req| req.to == self.person } end diff --git a/spec/controllers/invitations_controller_spec.rb b/spec/controllers/invitations_controller_spec.rb index 5a5e7c8df4..83901b998a 100644 --- a/spec/controllers/invitations_controller_spec.rb +++ b/spec/controllers/invitations_controller_spec.rb @@ -11,22 +11,9 @@ describe InvitationsController do let!(:user) {make_user} let!(:aspect){user.aspects.create(:name => "WIN!!")} - + before do request.env["devise.mapping"] = Devise.mappings[:user] - module Resque - def enqueue(mod, *args) - mod.send(:perform, *args) - end - end - end - - after do - module Resque - def enqueue(mod, *args) - true - end - end end describe "#create" do diff --git a/spec/controllers/people_controller_spec.rb b/spec/controllers/people_controller_spec.rb index c281f99e0d..b3649e198a 100644 --- a/spec/controllers/people_controller_spec.rb +++ b/spec/controllers/people_controller_spec.rb @@ -44,14 +44,14 @@ describe PeopleController do hash = @hashes[4] hash[:person].should == @people[4] hash[:contact].should be_true - hash[:request].should be_false + hash[:contact].should_not be_pending hash[:aspects].should == user.aspects end it 'has the correct result for a requested person' do hash = @hashes[2] hash[:person].should == @people[2] - hash[:contact].should be_false - hash[:request].should be_true + hash[:contact].should be_true + hash[:contact].should be_pending hash[:aspects].should == user.aspects end end diff --git a/spec/controllers/publics_controller_spec.rb b/spec/controllers/publics_controller_spec.rb index 3c617cd157..1689ab613d 100644 --- a/spec/controllers/publics_controller_spec.rb +++ b/spec/controllers/publics_controller_spec.rb @@ -13,10 +13,6 @@ describe PublicsController do describe '#receive' do let(:xml) { "<walruses></walruses>" } - before do - Jobs::ReceiveSalmon.stub!(:perform) - end - it 'succeeds' do post :receive, "id" => user.person.id.to_s, "xml" => xml response.should be_success diff --git a/spec/controllers/requests_controller_spec.rb b/spec/controllers/requests_controller_spec.rb index 9544e65ab6..146843649b 100644 --- a/spec/controllers/requests_controller_spec.rb +++ b/spec/controllers/requests_controller_spec.rb @@ -56,7 +56,7 @@ describe RequestsController do lambda { post :create, @params }.should change(Contact,:count).by(1) - new_contact = @user.reload.contact_for(@other_user) + new_contact = @user.reload.contact_for(@other_user.person) new_contact.should_not be_nil new_contact.should be_pending end diff --git a/spec/helper_methods.rb b/spec/helper_methods.rb index 722a199e6e..377f278af6 100644 --- a/spec/helper_methods.rb +++ b/spec/helper_methods.rb @@ -8,20 +8,6 @@ module HelperMethods Mocha::Mockery.instance.stubba.unstub_all end - def get_models - models = [] - Dir.glob( File.dirname(__FILE__) + '/../app/models/*' ).each do |f| - models << File.basename( f ).gsub( /^(.+).rb/, '\1') - end - models - end - - def stub_user_message_handle_methods(user) - user.stub!(:push_to_people) - user.stub!(:push_to_hub) - user.stub!(:push_to_person) - end - def fantasy_resque former_value = $process_queue $process_queue = true diff --git a/spec/mailers/notifier_spec.rb b/spec/mailers/notifier_spec.rb index 4c7e0779c0..7d335d9ff1 100644 --- a/spec/mailers/notifier_spec.rb +++ b/spec/mailers/notifier_spec.rb @@ -4,7 +4,6 @@ require 'spec_helper' describe Notifier do let!(:user) {make_user} - let!(:aspect) {user.aspects.create(:name => "science")} let!(:person) {Factory.create :person} before do @@ -64,8 +63,8 @@ describe Notifier do end end - describe "#request_accpeted" do - let!(:request_accepted_mail) {Notifier.request_accepted(user.id, person.id, aspect.id)} + describe "#request_accepted" do + let!(:request_accepted_mail) {Notifier.request_accepted(user.id, person.id)} it 'goes to the right person' do request_accepted_mail.to.should == [user.email] end @@ -74,13 +73,8 @@ describe Notifier do request_accepted_mail.body.encoded.include?(user.person.profile.first_name).should be true end - it 'has the name of person sending the request' do request_accepted_mail.body.encoded.include?(person.name).should be true end - - it 'has the name of the aspect in the body' do - request_accepted_mail.body.encoded.include?(aspect.name).should be true - end end end diff --git a/spec/models/contact_spec.rb b/spec/models/contact_spec.rb index d645d5a2e3..fa12b9fc56 100644 --- a/spec/models/contact_spec.rb +++ b/spec/models/contact_spec.rb @@ -7,19 +7,77 @@ require 'spec_helper' describe Contact do describe 'validations' do let(:contact){Contact.new} - + it 'requires a user' do contact.valid? contact.errors.full_messages.should include "User can't be blank" end - + it 'requires a person' do contact.valid? contact.errors.full_messages.should include "Person can't be blank" end + it 'ensures user is not making a contact for himself' do + user = make_user + + contact.person = user.person + contact.user = user + + contact.valid? + contact.errors.full_messages.should include "Cannot create self-contact" + end + it 'has many aspects' do contact.associations[:aspects].type.should == :many end + + it 'validates uniqueness' do + user = make_user + person = Factory(:person) + + contact2 = Contact.create(:user => user, + :person => person) + + contact2.should be_valid + + contact.user = user + contact.person = person + contact.should_not be_valid + end + end + + + context 'requesting' do + before do + @contact = Contact.new + @user = make_user + @person = Factory(:person) + + @contact.user = @user + @contact.person = @person + end + + describe '#generate_request' do + it 'makes a request' do + @contact.stub(:user).and_return(@user) + request = @contact.generate_request + + request.from.should == @user + request.to.should == @person + end + end + + describe '#dispatch_request' do + it 'pushes to people' do + @contact.stub(:user).and_return(@user) + @user.should_receive(:push_to_people).with(anything, [@contact.person]) + @contact.dispatch_request + end + it 'persists no request' do + @contact.dispatch_request + Request.from(@user).to(@person).first.should be_nil + end + end end end diff --git a/spec/models/invitation_spec.rb b/spec/models/invitation_spec.rb index 68d129c0c9..d6bec03111 100644 --- a/spec/models/invitation_spec.rb +++ b/spec/models/invitation_spec.rb @@ -250,7 +250,13 @@ describe Invitation do it 'creates a request, and sends it to the new user' do lambda { @invitation.to_request! - }.should change(Request, :count).by(2) + }.should change(Request, :count).by(1) + end + it 'creates a pending contact for the inviter' do + lambda { + @invitation.to_request! + }.should change(Contact, :count).by(1) + @invitation.from.contact_for(@new_user.person).should be_pending end describe 'return values' do before do diff --git a/spec/models/request_spec.rb b/spec/models/request_spec.rb index 0f6bc89ca4..6a7026cd1d 100644 --- a/spec/models/request_spec.rb +++ b/spec/models/request_spec.rb @@ -79,16 +79,6 @@ describe Request do end end - describe '#request_from_me' do - it 'recognizes requests from me' do - user.request_from_me?(request).should be_true - end - - it 'recognized when a request is not from me' do - user2.request_from_me?(request).should be_false - end - end - context 'quering request through user' do it 'finds requests for that user' do request @@ -97,17 +87,6 @@ describe Request do end end - describe '#original_request' do - it 'returns nil on a request from me' do - request - user.original_request(request).should be_nil - end - it 'returns the original request on a response to a request from me' do - new_request = request.reverse_for(user2) - user.original_request(new_request).should == request - end - end - describe '.hashes_for_person' do before do @user = make_user diff --git a/spec/models/user/connecting_spec.rb b/spec/models/user/connecting_spec.rb index d9a6b2e077..7735898b6b 100644 --- a/spec/models/user/connecting_spec.rb +++ b/spec/models/user/connecting_spec.rb @@ -18,13 +18,30 @@ describe Diaspora::UserModules::Connecting do let(:aspect2) { user2.aspects.create(:name => "aspect two") } describe '#send_contact_request_to' do - it "should assign a request to a aspect for the user that sent it out" do - aspect.requests.size.should == 0 + it 'should not be able to contact request an existing contact' do + user.activate_contact(user2.person, aspect1) - user.send_contact_request_to(person, aspect) + proc { + user.send_contact_request_to(user2.person, aspect1) + }.should raise_error(MongoMapper::DocumentNotValid) + end - aspect.reload - aspect.requests.size.should == 1 + it 'should not be able to contact request no-one' do + proc { + user.send_contact_request_to(nil, aspect) + }.should raise_error(MongoMapper::DocumentNotValid) + end + it 'creates a pending contact' do + proc { + user.send_contact_request_to(user2.person, aspect1) + }.should change(Contact, :count).by(1) + user.contact_for(user2.person).pending.should == true + user.contact_for(user2.person).should be_pending + end + it 'persists no request for requester' do + proc { + user.send_contact_request_to(user2.person, aspect1) + }.should_not change{user.reload.pending_requests.count} end end @@ -65,7 +82,7 @@ describe Diaspora::UserModules::Connecting do Request.find(@acceptance.id).should be_nil end it 'enqueues a mail job' do - Resque.should_receive(:enqueue).with(Jobs::MailRequestAcceptance, user.id, user2.person.id, aspect.id).once + Resque.should_receive(:enqueue).with(Jobs::MailRequestAcceptance, user.id, user2.person.id).once user.receive_request(@acceptance, user2.person) end end @@ -107,17 +124,6 @@ describe Diaspora::UserModules::Connecting do end end - it 'should not be able to contact request an existing contact' do - connect_users(user, aspect, user2, aspect2) - proc { user.send_contact_request_to(user2.person, aspect1) - }.should raise_error(MongoMapper::DocumentNotValid, /already connected/) - end - - it 'should not be able to contact request no-one' do - proc { user.send_contact_request_to(nil, aspect) - }.should raise_error(MongoMapper::DocumentNotValid) - end - describe 'multiple users accepting/rejecting the same person' do before do diff --git a/spec/models/user/invite_spec.rb b/spec/models/user/invite_spec.rb index e4d401801d..32de1ad5d2 100644 --- a/spec/models/user/invite_spec.rb +++ b/spec/models/user/invite_spec.rb @@ -34,7 +34,7 @@ describe User do it 'throws if you try to add someone you"re connected to' do connect_users(inviter, aspect, another_user, wrong_aspect) inviter.reload - proc{inviter.invite_user(another_user.email, aspect.id)}.should raise_error /already connected/ + proc{inviter.invite_user(another_user.email, aspect.id)}.should raise_error MongoMapper::DocumentNotValid end end @@ -75,7 +75,6 @@ describe User do it 'resolves incoming invitations into contact requests' do invited_user.reload.pending_requests.count.should == 1 - inviter.reload.pending_requests.count.should == 1 end context 'after request acceptance' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f5efe92031..2d9901daee 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -68,13 +68,16 @@ ImageUploader.enable_processing = false class User def send_contact_request_to(desired_contact, aspect) fantasy_resque do - request = Request.instantiate(:to => desired_contact, - :from => self.person, - :into => aspect) - if request.save! - dispatch_request request + contact = Contact.new(:person => desired_contact, + :user => self, + :pending => true) + contact.aspects << aspect + + if contact.save! + contact.dispatch_request + else + nil end - request end end -- GitLab