Moving PostVisibility to aspects, WIP, model specs pass

app/controllers/aspects_controller.rb

@@ -79,7 +79,7 @@ class AspectsController < ApplicationController
     @aspect = current_user.aspects.where(:id => params[:id]).first
 
     begin
-      current_user.drop_aspect @aspect
+      @aspect.destroy
       flash[:notice] = I18n.t 'aspects.destroy.success', :name => @aspect.name
       redirect_to aspects_path
     rescue ActiveRecord::StatementInvalid => e

app/models/aspect.rb

@@ -8,8 +8,8 @@ class Aspect < ActiveRecord::Base
   has_many :aspect_memberships
   has_many :contacts, :through => :aspect_memberships
 
-  has_many :post_visibilities
+  has_many :aspect_visibilities
-  has_many :posts, :through => :post_visibilities
+  has_many :posts, :through => :aspect_visibilities
 
   validates_presence_of :name
   validates_length_of :name, :maximum => 20

app/models/aspect_membership.rb

@@ -11,7 +11,6 @@ class AspectMembership < ActiveRecord::Base
 
   before_destroy :ensure_membership
   
-  
   def ensure_membership
     if self.contact.aspect_memberships.count == 1
       errors[:base] << I18n.t('shared.contact_list.cannot_remove')

app/models/aspect_visibility.rb

@@ -0,0 +1,13 @@
+#   Copyright (c) 2010, Diaspora Inc.  This file is
+#   licensed under the Affero General Public License version 3 or later.  See
+#   the COPYRIGHT file.
+
+class AspectVisibility < ActiveRecord::Base
+
+  belongs_to :aspect
+  validates_presence_of :aspect
+
+  belongs_to :post
+  validates_presence_of :post
+
+end

app/models/contact.rb

@@ -13,6 +13,10 @@ class Contact < ActiveRecord::Base
 
   has_many :aspect_memberships
   has_many :aspects, :through => :aspect_memberships
+
+  has_many :post_visibilities
+  has_many :posts, :through => :post_visibilities
+  
   validate :not_contact_for_self
   validates_uniqueness_of :person_id, :scope => :user_id
 

app/models/jobs/receive_local_batch.rb

@@ -15,14 +15,13 @@ module Job
       notify_mentioned_users(post)
     end
     def self.create_visibilities(post, recipient_user_ids)
-      aspects = Aspect.where(:user_id => recipient_user_ids).joins(:contacts).where(:contacts => {:person_id => post.author_id}).select('aspects.id, aspects.user_id')
+      contacts = Contact.where(:user_id => recipient_user_ids, :person_id => post.author_id)
-      Rails.logger.info(:event => :rlb_aspects, :aspect_ids => aspects.map{|a| a.id}.join(','))
+      contacts.each do |contact|
-      aspects.each do |aspect|
         begin
-          PostVisibility.create(:aspect_id => aspect.id, :post_id => post.id)
+          PostVisibility.create(:contact_id => contact.id, :post_id => post.id)
         rescue ActiveRecord::RecordNotUnique => e
-          Rails.logger.info(:event => :unexpected_pv, :aspect_id => aspect.id, :post_id => post.id)
+          Rails.logger.info(:event => :unexpected_pv, :contact_id => contact.id, :post_id => post.id)
-          #The post was already visible to that aspect
+          #The post was already visible to that user
         end
       end
     end

app/models/post.rb

@@ -16,8 +16,12 @@ class Post < ActiveRecord::Base
   has_many :comments, :order => 'created_at ASC'
   has_many :likes, :conditions => '`likes`.`positive` = 1', :dependent => :delete_all
   has_many :dislikes, :conditions => '`likes`.`positive` = 0', :class_name => 'Like', :dependent => :delete_all
+
+  has_many :aspect_visibilities
+  has_many :aspects, :through => :aspect_visibilities
+
   has_many :post_visibilities
-  has_many :aspects, :through => :post_visibilities
+  has_many :contacts, :through => :post_visibilities
   has_many :mentions, :dependent => :destroy
   belongs_to :author, :class_name => 'Person'
 
@@ -25,7 +29,11 @@ class Post < ActiveRecord::Base
   @@per_page = 10
 
   def user_refs
-    self.post_visibilities.count
+    if AspectVisibility.exists?(:post_id => self.id)
+      self.post_visibilities.count + 1
+    else
+      self.post_visibilities.count
+    end
   end
 
   def diaspora_handle= nd
@@ -68,7 +76,7 @@ class Post < ActiveRecord::Base
 
     local_post = Post.where(:guid => self.guid).first
     if local_post && local_post.author_id == self.author_id
-      known_post = user.visible_posts(:guid => self.guid).first
+      known_post = user.raw_visible_posts.where(:guid => self.guid).first
       if known_post
         if known_post.mutable?
           known_post.update_attributes(self.attributes)
@@ -76,14 +84,16 @@ class Post < ActiveRecord::Base
           Rails.logger.info("event=receive payload_type=#{self.class} update=true status=abort sender=#{self.diaspora_handle} reason=immutable existing_post=#{known_post.id}")
         end
       else
-        user.add_post_to_aspects(local_post)
+        contact = user.contact_for(person)
+        PostVisibility.create(:post_id => self.id, :contact_id => contact.id)
         user.notify_if_mentioned(local_post)
         Rails.logger.info("event=receive payload_type=#{self.class} update=true status=complete sender=#{self.diaspora_handle} existing_post=#{local_post.id}")
         return local_post
       end
     elsif !local_post
       self.save
-      user.add_post_to_aspects(self)
+      contact = user.contact_for(person)
+      PostVisibility.create(:post_id => self.id, :contact_id => contact.id)
       user.notify_if_mentioned(self)
       Rails.logger.info("event=receive payload_type=#{self.class} update=false status=complete sender=#{self.diaspora_handle}")
       return self

app/models/post_visibility.rb

@@ -4,12 +4,10 @@
 
 class PostVisibility < ActiveRecord::Base
 
-  belongs_to :aspect
+  belongs_to :contact
-  validates_presence_of :aspect
+  validates_presence_of :contact
 
   belongs_to :post
   validates_presence_of :post
-  has_one :user, :through => :aspect
-  has_one :person, :through => :post, :foreign_key => :author_id
 
 end

app/models/user.rb

@@ -91,23 +91,18 @@ class User < ActiveRecord::Base
   end
 
   ######### Aspects ######################
-  def drop_aspect(aspect)
-      aspect.destroy
-  end
-
   def move_contact(person, to_aspect, from_aspect)
     return true if to_aspect == from_aspect
     contact = contact_for(person)
-    if add_contact_to_aspect(contact, to_aspect)
+
-      membership = contact ? contact.aspect_memberships.where(:aspect_id => from_aspect.id).first : nil
+    add_contact_to_aspect(contact, to_aspect)
-      return ( membership && membership.destroy )
+    
-    else
+    membership = contact ? AspectMembership.where(:contact_id => contact.id, :aspect_id => from_aspect.id).first : nil
-      false
+    return(membership && membership.destroy)
-    end
   end
 
   def add_contact_to_aspect(contact, aspect)
-    return true if contact.aspect_memberships.where(:aspect_id => aspect.id).count > 0
+    return true if AspectMembership.exists?(:contact_id => contact.id, :aspect_id => aspect.id)
     contact.aspect_memberships.create!(:aspect => aspect)
   end
 
@@ -138,14 +133,6 @@ class User < ActiveRecord::Base
     post.notify_person(self.person) if post.mentions? self.person
   end
 
-  def add_post_to_aspects(post)
-    return unless self.contact_for(post.author)
-
-    Rails.logger.debug("event=add_post_to_aspects user_id=#{self.id} post_id=#{post.id}")
-    add_to_streams(post, self.aspects_with_person(post.author))
-    post
-  end
-
   def add_to_streams(post, aspects_to_insert)
     post.socket_to_user(self, :aspect_ids => aspects_to_insert.map{|x| x.id}) if post.respond_to? :socket_to_user
     aspects_to_insert.each do |aspect|

db/migrate/20110328202414_post_visibilities_on_contacts.rb

@@ -0,0 +1,58 @@
+class PostVisibilitiesOnContacts < ActiveRecord::Migration
+  def self.move_author_pvs_to_aspect_pvs
+    where_clause = <<SQL
+      FROM post_visibilities as pv 
+        INNER JOIN aspects ON aspects.id = pv.aspect_id
+        INNER JOIN posts ON posts.id = pv.post_id
+          INNER JOIN people ON posts.author_id = people.id
+      WHERE people.owner_id = aspects.user_id
+SQL
+
+    ids = execute("SELECT pv.id #{where_clause}").to_a
+
+    unless ids.blank?
+      execute("INSERT into aspect_visibilities SELECT pv.post_id, pv.aspect_id #{where_clause}")
+
+      execute <<SQL
+      DELETE FROM post_visibilities
+      WHERE post_visibilities.id IN (#{ids.join(',')})
+SQL
+    end
+  end
+
+  def self.set_pv_contact_ids
+    execute <<SQL
+    UPDATE post_visibilities as pv
+      INNER JOIN posts ON posts.id = pv.post_id
+        INNER JOIN people ON posts.author_id = people.id
+      INNER JOIN aspects ON aspects.id = pv.aspect_id
+        INNER JOIN users ON users.id = aspects.user_id
+          INNER JOIN contacts ON contacts.user_id = users.id
+    SET pv.contact_id = contacts.id
+    WHERE people.id = contacts.person_id
+SQL
+  end
+
+  def self.up
+    create_table :aspect_visibilities do |t|
+      t.integer :post_id, :null => false
+      t.integer :aspect_id, :null => false
+    end
+    add_index :aspect_visibilities, [:post_id, :aspect_id], :unique => true
+    add_foreign_key :aspect_visibilities, :aspects, :dependent => :delete
+    add_foreign_key :aspect_visibilities, :posts, :dependent => :delete
+
+    add_column :post_visibilities, :contact_id, :integer, :null => false
+    add_index :post_visibilities, [:contact_id, :post_id], :unique => true
+
+    move_author_pvs_to_aspect_pvs
+    set_pv_contact_ids
+
+    remove_index :post_visibilities, [:aspect_id, :post_id]
+    remove_column :post_visibilities, :aspect_id
+  end
+
+  def self.down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 20110323213655) do
+ActiveRecord::Schema.define(:version => 20110328202414) do
 
   create_table "aspect_memberships", :force => true do |t|
     t.integer  "aspect_id",  :null => false
@@ -23,6 +23,14 @@ ActiveRecord::Schema.define(:version => 20110323213655) do
   add_index "aspect_memberships", ["aspect_id"], :name => "index_aspect_memberships_on_aspect_id"
   add_index "aspect_memberships", ["contact_id"], :name => "index_aspect_memberships_on_contact_id"
 
+  create_table "aspect_visibilities", :force => true do |t|
+    t.integer "post_id",   :null => false
+    t.integer "aspect_id", :null => false
+  end
+
+  add_index "aspect_visibilities", ["aspect_id"], :name => "aspect_visibilities_aspect_id_fk"
+  add_index "aspect_visibilities", ["post_id", "aspect_id"], :name => "index_aspect_visibilities_on_post_id_and_aspect_id", :unique => true
+
   create_table "aspects", :force => true do |t|
     t.string   "name",                                :null => false
     t.integer  "user_id",                             :null => false
@@ -198,14 +206,13 @@ ActiveRecord::Schema.define(:version => 20110323213655) do
   add_index "people", ["owner_id"], :name => "index_people_on_owner_id", :unique => true
 
   create_table "post_visibilities", :force => true do |t|
-    t.integer  "aspect_id",  :null => false
     t.integer  "post_id",    :null => false
     t.datetime "created_at"
     t.datetime "updated_at"
+    t.integer  "contact_id", :null => false
   end
 
-  add_index "post_visibilities", ["aspect_id", "post_id"], :name => "index_post_visibilities_on_aspect_id_and_post_id", :unique => true
+  add_index "post_visibilities", ["contact_id", "post_id"], :name => "index_post_visibilities_on_contact_id_and_post_id", :unique => true
-  add_index "post_visibilities", ["aspect_id"], :name => "index_post_visibilities_on_aspect_id"
   add_index "post_visibilities", ["post_id"], :name => "index_post_visibilities_on_post_id"
 
   create_table "posts", :force => true do |t|
@@ -366,6 +373,9 @@ ActiveRecord::Schema.define(:version => 20110323213655) do
   add_foreign_key "aspect_memberships", "aspects", :name => "aspect_memberships_aspect_id_fk"
   add_foreign_key "aspect_memberships", "contacts", :name => "aspect_memberships_contact_id_fk", :dependent => :delete
 
+  add_foreign_key "aspect_visibilities", "aspects", :name => "aspect_visibilities_aspect_id_fk", :dependent => :delete
+  add_foreign_key "aspect_visibilities", "posts", :name => "aspect_visibilities_post_id_fk", :dependent => :delete
+
   add_foreign_key "comments", "people", :name => "comments_author_id_fk", :column => "author_id", :dependent => :delete
   add_foreign_key "comments", "posts", :name => "comments_post_id_fk", :dependent => :delete
 

features/step_definitions/user_steps.rb

@@ -131,20 +131,6 @@ Given /^a user with username "([^"]*)" is connected with "([^"]*)"$/ do |arg1, a
   connect_users(user1, user1.aspects.first, user2, user2.aspects.first)
 end
 
-Given /^a user with email "([^\"]*)" has posted a status message "([^\"]*)" in all aspects$/ do |arg1, arg2|
-  user = User.where(:email => arg1).first
-  status_message = user.build_post(:status_message, :text => arg2)
-
-  def status_message.socket_to_user(a1, a2)
-    ;
-  end
-
-  user.add_to_streams(status_message, user.aspects)
-  status_message.save!
-  bob = User.where(:email => "bob@bob.bob").first
-  raise bob.visible_posts.inspect
-end
-
 Given /^there is a user "([^\"]*)" who's tagged "([^\"]*)"$/ do |full_name, tag|
   username = full_name.gsub(/\W/, "").underscore
   Given "a user named \"#{full_name}\" with email \"#{username}@example.com\""
@@ -152,4 +138,4 @@ Given /^there is a user "([^\"]*)" who's tagged "([^\"]*)"$/ do |full_name, tag|
   user.profile.tag_string = tag
   user.profile.build_tags
   user.profile.save!
-end
+end

lib/collect_user_photos.rb

@@ -5,7 +5,7 @@ module PhotoMover
     FileUtils::mkdir_p temp_dir
     Dir.chdir 'tmp/exports'
 
-    photos = user.visible_posts(:author_id => user.person.id, :type => 'Photo')
+    photos = user.raw_visible_posts(:author_id => user.person.id, :type => 'Photo')
 
     photos_dir = "#{user.id}/photos"
     FileUtils::mkdir_p photos_dir

lib/diaspora/user/connecting.rb

@@ -84,15 +84,10 @@ module Diaspora
 
       def remove_contact(contact)
         bad_person_id = contact.person_id
-        posts = raw_visible_posts.where(:author_id => bad_person_id).all
+        posts = contact.posts.all
-        visibilities = PostVisibility.joins(:post, :aspect).where(
+        contact.post_visibilities.delete_all
-          :posts => {:author_id => bad_person_id},
-          :aspects => {:user_id => self.id}
-        )
-        visibility_ids = visibilities.map{|v| v.id}
-        PostVisibility.where(:id => visibility_ids).delete_all
         posts.each do |post|
-          if post.post_visibilities(true).count < 1
+          if post.user_refs < 1
             post.destroy
           end
         end

lib/diaspora/user/querying.rb

@@ -11,8 +11,12 @@ module Diaspora
       end
 
       def raw_visible_posts
-        Post.joins(:aspects).where(:pending => false,
+        post_ids = []
-                                   :aspects => {:user_id => self.id}).select('DISTINCT `posts`.*')
+        post_ids = Post.joins(:contacts).where(:contacts => {:user_id => self.id}).map{|p| p.id}
+
+        post_ids += Post.joins(:aspect_visibilities => :aspect).where(:aspects => {:user_id => self.id}).select('posts.id').map{|p| p.id}
+
+        Post.where(:id => post_ids, :pending => false).select('DISTINCT `posts`.*')
       end
 
       def visible_photos
@@ -21,24 +25,12 @@ module Diaspora
           ).where(:aspects => {:user_id => self.id}).select('DISTINCT `posts`.*').order("posts.updated_at DESC")
       end
 
-      def visible_posts( opts = {} )
-        order = opts.delete(:order)
-        order ||= 'created_at DESC'
-        opts[:type] ||= ["StatusMessage", "Photo"]
-
-        if (aspect = opts[:by_members_of]) && opts[:by_members_of] != :all
-          raw_visible_posts.where(:aspects => {:id => aspect.id}).order(order)
-        else
-          self.raw_visible_posts.where(opts).order(order)
-        end
-      end
-
       def contact_for(person)
         return nil unless person
         contact_for_person_id(person.id)
       end
       def aspects_with_post(post_id)
-        self.aspects.joins(:post_visibilities).where(:post_visibilities => {:post_id => post_id})
+        self.aspects.joins(:aspect_visibilities).where(:aspect_visibilities => {:post_id => post_id})
       end
 
       def contact_for_person_id(person_id)
@@ -78,11 +70,14 @@ module Diaspora
       end
 
       def posts_from(person)
-        asp = Aspect.arel_table
+        con = Contact.arel_table
         p = Post.arel_table
-        person.posts.joins(:aspects).includes(:comments).where(
+        post_ids = []
-          p[:public].eq(true).or(asp[:user_id].eq(self.id))
+        if contact = self.contact_for(person)
-        ).select('DISTINCT `posts`.*').order("posts.created_at DESC")
+          post_ids = contact.post_visibilities.select('post_visibilities.post_id').map{|p| p.post_id}
+        end
+        post_ids += person.posts.where(:public => true, :pending => false).select('posts.id').map{|p| p.id}
+        Post.where(:id => post_ids).select('DISTINCT `posts`.*').order("posts.created_at DESC")
       end
     end
   end

spec/integration/receiving_spec.rb

@@ -66,7 +66,7 @@ describe 'a user receives a post' do
       bob.dispatch_post(sm, :to => bob.aspects.first)
     end
 
-    alice.visible_posts.count.should == 1
+    alice.raw_visible_posts.count.should == 1
   end
 
   context 'mentions' do
@@ -174,9 +174,6 @@ describe 'a user receives a post' do
         @post = Factory.create(:status_message, :author => @person)
         @post.post_visibilities.should be_empty
         receive_with_zord(@user1, @person, @post.to_diaspora_xml)
-        @aspect.post_visibilities.reset
-        @aspect.posts(true).should include(@post)
-        @post.post_visibilities.reset
       end
 
       it 'deletes a post if the noone links to it' do
@@ -197,6 +194,7 @@ describe 'a user receives a post' do
 
       @user1.disconnect(@contact)
       @status_message.reload
+      @status_message.post_visibilities.reset
       @status_message.user_refs.should == 2
     end
 

spec/misc_spec.rb

@@ -46,7 +46,7 @@ describe 'making sure the spec runner works' do
 
     it 'allows posting after running' do
       message = @user1.post(:status_message, :text => "Connection!", :to => @aspect1.id)
-      @user2.reload.visible_posts.should include message
+      @user2.reload.raw_visible_posts.should include message
     end
   end
 

spec/models/aspect_spec.rb

@@ -120,32 +120,6 @@ describe Aspect do
       aspect.posts.include?(status_message).should be true
     end
 
-    it 'should add post to aspect via receive method' do
-      aspect  = user.aspects.create(:name => 'losers')
-      aspect2 = user2.aspects.create(:name => 'winners')
-      connect_users(user, aspect, user2, aspect2)
-
-      message = user2.post(:status_message, :text => "Hey Dude", :to => aspect2.id)
-
-      aspect.reload
-      aspect.posts.include?(message).should be true
-      user.visible_posts(:by_members_of => aspect).include?(message).should be true
-    end
-
-    it 'should retract the post from the aspects as well' do
-      aspect  = user.aspects.create(:name => 'losers')
-      aspect2 = user2.aspects.create(:name => 'winners')
-      connect_users(user, aspect, user2, aspect2)
-
-      message = user2.post(:status_message, :text => "Hey Dude", :to => aspect2.id)
-
-      aspect.reload.post_ids.include?(message.id).should be true
-
-      fantasy_resque do
-        retraction = user2.retract(message)
-      end
-      aspect.posts(true).include?(message).should be false
-    end
   end
 
   context "aspect management" do
@@ -176,23 +150,7 @@ describe Aspect do
         user.reload
       end
 
-      it 'should keep the contact\'s posts in previous aspect' do
+      describe 'User#move_contact' do
-        aspect.post_ids.count.should == 1
-        user.move_contact(user2.person, user.aspects.create(:name => "Another aspect"), aspect)
-
-
-        aspect.reload
-        aspect.post_ids.count.should == 1
-      end
-
-      it 'should not delete other peoples posts' do
-        connect_users(user, aspect, user3, aspect3)
-        user.move_contact(user3.person, user.aspects.create(:name => "Another aspect"), aspect)
-        aspect.reload
-        aspect.posts.should == [@message]
-      end
-
-      describe '#move_contact' do
         it 'should be able to move a contact from one of users existing aspects to another' do
           user.move_contact(user2.person, aspect1, aspect)
 

spec/models/conversation_visibility_spec.rb

@@ -1,19 +0,0 @@
-require 'spec_helper'
-
-describe ConversationVisibility do
-  before do
-    @user = alice
-    @aspect = @user.aspects.create(:name => 'Boozers')
-
-    @person = Factory(:person)
-    @post = Factory(:status_message, :author => @person)
-  end
-  it 'has an aspect' do
-    pv = PostVisibility.new(:aspect => @aspect)
-    pv.aspect.should == @aspect
-  end
-  it 'has a post' do
-    pv = PostVisibility.new(:post => @post)
-    pv.post.should == @post
-  end
-end

spec/models/jobs/receive_local_batch_spec.rb

@@ -25,12 +25,12 @@ describe Job::ReceiveLocalBatch do
   end
   describe '.create_visibilities' do
     it 'creates a visibility for each user' do
-      PostVisibility.exists?(:aspect_id => bob.aspects.first.id, :post_id => @post.id).should be_false
+      PostVisibility.exists?(:contact_id => bob.contact_for(alice.person).id, :post_id => @post.id).should be_false
       Job::ReceiveLocalBatch.create_visibilities(@post, [bob.id])
-      PostVisibility.exists?(:aspect_id => bob.aspects.first.id, :post_id => @post.id).should be_true
+      PostVisibility.exists?(:contact_id => bob.contact_for(alice.person).id, :post_id => @post.id).should be_true
     end
     it 'does not raise if a visibility already exists' do
-      PostVisibility.create!(:aspect_id => bob.aspects.first.id, :post_id => @post.id)
+      PostVisibility.create!(:contact_id => bob.contact_for(alice.person).id, :post_id => @post.id)
       lambda {
         Job::ReceiveLocalBatch.create_visibilities(@post, [bob.id])
       }.should_not raise_error

spec/models/mention_spec.rb

@@ -9,23 +9,19 @@ describe Mention do
     before do
       @user = alice
       @aspect1 = @user.aspects.create(:name => 'second_aspect')
-      @mentioned_user = bob
-      @non_friend = eve
 
-      @sm = @user.build_post(:status_message, :text => "hi @{#{@mentioned_user.name}; #{@mentioned_user.diaspora_handle}}", :to => @user.aspects.first)
     end
 
     it 'notifies the person being mentioned' do
-      Notification.should_receive(:notify).with(@mentioned_user, anything(), @sm.author)
+      sm = @user.build_post(:status_message, :text => "hi @{#{bob.name}; #{bob.diaspora_handle}}", :to => @user.aspects.first)
-      @sm.receive(@mentioned_user, @mentioned_user.person)
+      Notification.should_receive(:notify).with(bob, anything(), sm.author)
+      sm.receive(bob, alice.person)
     end
 
     it 'should not notify a user if they do not see the message' do
-      connect_users(@user, @aspect1, @non_friend, @non_friend.aspects.first)
+      Notification.should_not_receive(:notify).with(alice, anything(), bob.person)
-
+      sm2 = bob.build_post(:status_message, :text => "stuff @{#{alice.name}; #{alice.diaspora_handle}}", :to => bob.aspects.first)
-      Notification.should_not_receive(:notify).with(@mentioned_user, anything(), @user.person)
+      sm2.receive(eve, bob.person)
-      sm2 = @user.post(:status_message, :text => "stuff @{#{@non_friend.name}; #{@non_friend.diaspora_handle}}", :to => @user.aspects.first)
-      sm2.receive(@non_friend, @non_friend.person)
     end
   end
 

spec/models/post_visibility_spec.rb

@@ -1,19 +0,0 @@
-require 'spec_helper'
-
-describe PostVisibility do
-  before do
-    @user = alice
-    @aspect = @user.aspects.create(:name => 'Boozers')
-
-    @person = Factory(:person)
-    @post = Factory(:status_message, :author => @person)
-  end
-  it 'has an aspect' do
-    pv = PostVisibility.new(:aspect => @aspect)
-    pv.aspect.should == @aspect
-  end
-  it 'has a post' do
-    pv = PostVisibility.new(:post => @post)
-    pv.post.should == @post
-  end
-end

spec/models/user/attack_vectors_spec.rb

@@ -49,7 +49,7 @@ describe "attack vectors" do
     zord = Postzord::Receiver.new(user, :salmon_xml => salmon_xml)
     zord.perform
 
-    user3.reload.visible_posts.should_not include(StatusMessage.find(original_message.id))
+    user3.reload.raw_visible_posts.should_not include(StatusMessage.find(original_message.id))
   end
 
   context 'malicious contact attack vector' do

spec/models/user/connecting_spec.rb

@@ -291,13 +291,6 @@ describe Diaspora::UserModules::Connecting do
           bob.reload.raw_visible_posts.include?(@message).should be_false
         end
 
-        it "deletes the disconnected user's posts from the aspect's posts" do
-          Post.count.should == 1
-          bob.aspects.first.reload.posts.include?(@message).should be_true
-          bob.disconnect bob.contact_for(alice.person)
-          bob.aspects.first.reload.posts.include?(@message).should be_false
-          Post.count.should == 1
-        end
       end
     end
   end

spec/models/user/querying_spec.rb

@@ -7,21 +7,19 @@ require 'spec_helper'
 describe User do
 
   before do
-    @alice = alice
+    @alices_aspect = alice.aspects.first
-    @alices_aspect = @alice.aspects.first
+    @eves_aspect = eve.aspects.first
-    @eve = eve
-    @eves_aspect = @eve.aspects.first
   end
 
   describe "#raw_visible_posts" do
     it "returns all the posts the user can see" do
-      connect_users(@eve, @eves_aspect, @alice, @alices_aspect)
+      connect_users(eve, @eves_aspect, alice, @alices_aspect)
-      self_post = @alice.post(:status_message, :text => "hi", :to => @alices_aspect.id)
+      self_post = alice.post(:status_message, :text => "hi", :to => @alices_aspect.id)
-      visible_post = @eve.post(:status_message, :text => "hello", :to => @eves_aspect.id)
+      visible_post = eve.post(:status_message, :text => "hello", :to => @eves_aspect.id)
-      dogs = @eve.aspects.create(:name => "dogs")
+      dogs = eve.aspects.create(:name => "dogs")
-      invisible_post = @eve.post(:status_message, :text => "foobar", :to => dogs.id)
+      invisible_post = eve.post(:status_message, :text => "foobar", :to => dogs.id)
 
-      stream = @alice.raw_visible_posts
+      stream = alice.raw_visible_posts
       stream.should include(self_post)
       stream.should include(visible_post)
       stream.should_not include(invisible_post)
@@ -30,20 +28,20 @@ describe User do
 
   context 'with two posts' do
     before do
-      connect_users(@eve, @eves_aspect, @alice, @alices_aspect)
+      connect_users(eve, @eves_aspect, alice, @alices_aspect)
-      aspect3 = @alice.aspects.create(:name => "Snoozers")
+      aspect3 = alice.aspects.create(:name => "Snoozers")
-      @status_message1 = @eve.post :status_message, :text => "hi", :to => @eves_aspect.id
+      @status_message1 = eve.post :status_message, :text => "hi", :to => @eves_aspect.id
-      @status_message2 = @eve.post :status_message, :text => "hey", :public => true , :to => @eves_aspect.id
+      @status_message2 = eve.post :status_message, :text => "hey", :public => true , :to => @eves_aspect.id
-      @status_message3 = @alice.post :status_message, :text => "hey", :public => true , :to => @alices_aspect.id
+      @status_message3 = alice.post :status_message, :text => "hey", :public => true , :to => @alices_aspect.id
-      @status_message4 = @eve.post :status_message, :text => "blah", :public => true , :to => @eves_aspect.id
+      @status_message4 = eve.post :status_message, :text => "blah", :public => true , :to => @eves_aspect.id
-      @status_message5 = @alice.post :status_message, :text => "secrets", :to => aspect3.id
+      @status_message5 = alice.post :status_message, :text => "secrets", :to => aspect3.id
 
-      @pending_status_message = @eve.post :status_message, :text => "hey", :public => true , :to => @eves_aspect.id, :pending => true
+      @pending_status_message = eve.post :status_message, :text => "hey", :public => true , :to => @eves_aspect.id, :pending => true
     end
 
     describe "#visible_posts" do
       it "queries by person id" do
-        query = @eve.visible_posts(:author_id => @eve.person.id)
+        query = eve.raw_visible_posts.where(:author_id => eve.person.id)
         query.include?(@status_message1).should == true
         query.include?(@status_message2).should == true
         query.include?(@status_message3).should == false
@@ -52,7 +50,7 @@ describe User do
       end
 
       it "selects public posts" do
-        query = @eve.visible_posts(:public => true)
+        query = eve.raw_visible_posts.where(:public => true)
         query.include?(@status_message1).should == false
         query.include?(@status_message2).should == true
         query.include?(@status_message3).should == true
@@ -61,7 +59,7 @@ describe User do
       end
 
       it "selects non public posts" do
-        query = @eve.visible_posts(:public => false)
+        query = eve.raw_visible_posts.where(:public => false)
         query.include?(@status_message1).should == true
         query.include?(@status_message2).should == false
         query.include?(@status_message3).should == false
@@ -70,27 +68,18 @@ describe User do
       end
 
       it "selects by message contents" do
-        query = @eve.visible_posts(:text=> "hi")
+        query = eve.raw_visible_posts.where(:text=> "hi")
         query.should == [@status_message1]
       end
 
       it "does not return pending posts" do
         @pending_status_message.pending.should be_true
-        @eve.visible_posts.should_not include @pending_status_message
+        eve.raw_visible_posts.should_not include @pending_status_message
       end
 
-      it "queries by aspect" do
-        query = @alice.visible_posts(:by_members_of => @alices_aspect)
-        query.include?(@status_message1).should == true
-        query.include?(@status_message2).should == true
-        query.include?(@status_message3).should == true
-        query.include?(@status_message4).should == true
-        query.include?(@status_message5).should == false
-        @alice.visible_posts(:by_members_of => @alice.aspects.create(:name => "aaaaah")).should be_empty
-      end
       it '#find_visible_post_by_id' do
-        @eve.find_visible_post_by_id(@status_message1.id).should == @status_message1
+        eve.find_visible_post_by_id(@status_message1.id).should == @status_message1
-        @eve.find_visible_post_by_id(@status_message5.id).should == nil
+        eve.find_visible_post_by_id(@status_message5.id).should == nil
       end
     end
   end
@@ -98,7 +87,7 @@ describe User do
   context 'with two users' do
     describe '#people_in_aspects' do
       it 'returns people objects for a users contact in each aspect' do
-        @alice.people_in_aspects([@alices_aspect]).should == [bob.person]
+        alice.people_in_aspects([@alices_aspect]).should == [bob.person]
       end
 
       it 'returns local/remote people objects for a users contact in each aspect' do
@@ -110,37 +99,37 @@ describe User do
         asp2 = local_user2.aspects.create(:name => "brb")
         asp3 = remote_user.aspects.create(:name => "ttyl")
 
-        connect_users(@alice, @alices_aspect, local_user1, asp1)
+        connect_users(alice, @alices_aspect, local_user1, asp1)
-        connect_users(@alice, @alices_aspect, local_user2, asp2)
+        connect_users(alice, @alices_aspect, local_user2, asp2)
-        connect_users(@alice, @alices_aspect, remote_user, asp3)
+        connect_users(alice, @alices_aspect, remote_user, asp3)
 
         local_person = remote_user.person
         local_person.owner_id = nil
         local_person.save
         local_person.reload
 
-        @alice.people_in_aspects([@alices_aspect]).count.should == 4
+        alice.people_in_aspects([@alices_aspect]).count.should == 4
-        @alice.people_in_aspects([@alices_aspect], :type => 'remote').count.should == 1
+        alice.people_in_aspects([@alices_aspect], :type => 'remote').count.should == 1
-        @alice.people_in_aspects([@alices_aspect], :type => 'local').count.should == 3
+        alice.people_in_aspects([@alices_aspect], :type => 'local').count.should == 3
       end
 
       it 'does not return people not connected to user on same pod' do
         3.times { Factory(:user) }
-        @alice.people_in_aspects([@alices_aspect]).count.should == 1
+        alice.people_in_aspects([@alices_aspect]).count.should == 1
       end
 
       it "only returns non-pending contacts" do
-        @alice.send_contact_request_to(Factory(:user).person, @alices_aspect)
+        alice.send_contact_request_to(Factory(:user).person, @alices_aspect)
         @alices_aspect.reload
-        @alice.reload
+        alice.reload
 
-        @alice.people_in_aspects([@alices_aspect]).should == [bob.person]
+        alice.people_in_aspects([@alices_aspect]).should == [bob.person]
       end
 
       it "returns an empty array when passed an aspect the user doesn't own" do
         other_user = Factory(:user_with_aspect)
-        connect_users(@eve, @eve.aspects.first, other_user, other_user.aspects.first)
+        connect_users(eve, eve.aspects.first, other_user, other_user.aspects.first)
-        @alice.people_in_aspects([other_user.aspects.first]).should == []
+        alice.people_in_aspects([other_user.aspects.first]).should == []
       end
     end
   end
@@ -149,46 +138,46 @@ describe User do
     let(:person_one) { Factory.create :person }
     let(:person_two) { Factory.create :person }
     let(:person_three) { Factory.create :person }
-    let(:aspect) { @alice.aspects.create(:name => 'heroes') }
+    let(:aspect) { alice.aspects.create(:name => 'heroes') }
     describe '#contact_for_person_id' do
       it 'returns a contact' do
-        contact = Contact.create(:user => @alice, :person => person_one, :aspects => [aspect])
+        contact = Contact.create(:user => alice, :person => person_one, :aspects => [aspect])
-        @alice.contacts << contact
+        alice.contacts << contact
-        @alice.contact_for_person_id(person_one.id).should be_true
+        alice.contact_for_person_id(person_one.id).should be_true
       end
 
       it 'returns the correct contact' do
-        contact = Contact.create(:user => @alice, :person => person_one, :aspects => [aspect])
+        contact = Contact.create(:user => alice, :person => person_one, :aspects => [aspect])
-        @alice.contacts << contact
+        alice.contacts << contact
 
-        contact2 = Contact.create(:user => @alice, :person => person_two, :aspects => [aspect])
+        contact2 = Contact.create(:user => alice, :person => person_two, :aspects => [aspect])
-        @alice.contacts << contact2
+        alice.contacts << contact2
 
-        contact3 = Contact.create(:user => @alice, :person => person_three, :aspects => [aspect])
+        contact3 = Contact.create(:user => alice, :person => person_three, :aspects => [aspect])
-        @alice.contacts << contact3
+        alice.contacts << contact3
 
-        @alice.contact_for_person_id(person_two.id).person.should == person_two
+        alice.contact_for_person_id(person_two.id).person.should == person_two
       end
 
       it 'returns nil for a non-contact' do
-        @alice.contact_for_person_id(person_one.id).should be_nil
+        alice.contact_for_person_id(person_one.id).should be_nil
       end
 
       it 'returns nil when someone else has contact with the target' do
-        contact = Contact.create(:user => @alice, :person => person_one, :aspects => [aspect])
+        contact = Contact.create(:user => alice, :person => person_one, :aspects => [aspect])
-        @alice.contacts << contact
+        alice.contacts << contact
-        @eve.contact_for_person_id(person_one.id).should be_nil
+        eve.contact_for_person_id(person_one.id).should be_nil
       end
     end
 
     describe '#contact_for' do
       it 'takes a person_id and returns a contact' do
-        @alice.should_receive(:contact_for_person_id).with(person_one.id)
+        alice.should_receive(:contact_for_person_id).with(person_one.id)
-        @alice.contact_for(person_one)
+        alice.contact_for(person_one)
       end
 
       it 'returns nil if the input is nil' do
-        @alice.contact_for(nil).should be_nil
+        alice.contact_for(nil).should be_nil
       end
     end
   end
@@ -197,13 +186,13 @@ describe User do
     let!(:user5) {Factory(:user)}
 
     it 'should not have a pending request before connecting' do
-      request = @alice.request_from(user5.person)
+      request = alice.request_from(user5.person)
       request.should be_nil
     end
 
     it 'should have a pending request after sending a request' do
-      @alice.send_contact_request_to(user5.person, @alice.aspects.first)
+      alice.send_contact_request_to(user5.person, alice.aspects.first)
-      request = user5.request_from(@alice.person)
+      request = user5.request_from(alice.person)
       request.should_not be_nil
     end
   end
@@ -218,21 +207,21 @@ describe User do
     end
 
     it 'displays public posts for a non-contact' do
-      @alice.posts_from(@user3.person).should include @public_message
+      alice.posts_from(@user3.person).should include @public_message
     end
 
     it 'does not display private posts for a non-contact' do
-      @alice.posts_from(@user3.person).should_not include @private_message
+      alice.posts_from(@user3.person).should_not include @private_message
     end
 
     it 'displays private and public posts for a non-contact after connecting' do
-      connect_users(@alice, @alices_aspect, @user3, @aspect3)
+      connect_users(alice, @alices_aspect, @user3, @aspect3)
       new_message = @user3.post(:status_message, :text=> "hey there", :to => @aspect3.id)
 
-      @alice.reload
+      alice.reload
 
-      @alice.posts_from(@user3.person).should include @public_message
+      alice.posts_from(@user3.person).should include @public_message
-      @alice.posts_from(@user3.person).should include new_message
+      alice.posts_from(@user3.person).should include new_message
     end
 
     it 'displays recent posts first' do
@@ -243,7 +232,7 @@ describe User do
       msg4.created_at = Time.now+14
       msg4.save!
 
-      @alice.posts_from(@user3.person).map{|p| p.id}.should == [msg4, msg3, @public_message].map{|p| p.id}
+      alice.posts_from(@user3.person).map{|p| p.id}.should == [msg4, msg3, @public_message].map{|p| p.id}
     end
   end
 end

spec/models/user_spec.rb

@@ -324,18 +324,18 @@ describe User do
     end
   end
 
-  context 'aspects' do
+  describe 'foreign key between aspects and contacts' do
     it 'should delete an empty aspect' do
       empty_aspect = alice.aspects.create(:name => 'decoy')
       alice.aspects(true).include?(empty_aspect).should == true
-      alice.drop_aspect(empty_aspect)
+      empty_aspect.destroy
       alice.aspects(true).include?(empty_aspect).should == false
     end
 
     it 'should not delete an aspect with contacts' do
       aspect = alice.aspects.first
       aspect.contacts.count.should > 0
-      proc { alice.drop_aspect(aspect) }.should raise_error ActiveRecord::StatementInvalid
+      proc { aspect.destroy }.should raise_error ActiveRecord::StatementInvalid
       alice.aspects.include?(aspect).should == true
     end
   end