Merge pull request #6739 from svbergerem/convert-cukes-to-rspec

[WIP] Convert some cukes to rspec

Changelog.md

@@ -92,6 +92,7 @@ Contributions are very welcome, the hard work is done!
 * Attached ShareVisibilities to the User, not the Contact [#6723](https://github.com/diaspora/diaspora/pull/6723)
 * Refactor mentions input, now based on typeahead.js [#6728](https://github.com/diaspora/diaspora/pull/6728)
 * Optimized the pod up checks [#6727](https://github.com/diaspora/diaspora/pull/6727)
+* Prune and do not create aspect visibilities for public posts [#6732](https://github.com/diaspora/diaspora/pull/6732)
 
 ## Bug fixes
 * Destroy Participation when removing interactions with a post [#5852](https://github.com/diaspora/diaspora/pull/5852)

app/controllers/photos_controller.rb

@@ -150,10 +150,12 @@ class PhotosController < ApplicationController
     @photo = current_user.build_post(:photo, params[:photo])
 
     if @photo.save
-      aspects = current_user.aspects_from_ids(params[:photo][:aspect_ids])
 
       unless @photo.pending
-        current_user.add_to_streams(@photo, aspects)
+        unless @photo.public?
+          aspects = current_user.aspects_from_ids(params[:photo][:aspect_ids])
+          current_user.add_to_streams(@photo, aspects)
+        end
         current_user.dispatch_post(@photo, :to => params[:photo][:aspect_ids])
       end
 

app/controllers/reshares_controller.rb

@@ -11,7 +11,6 @@ class ResharesController < ApplicationController
     end
 
     if @reshare.save
-      current_user.add_to_streams(@reshare, current_user.aspects)
       current_user.dispatch_post(@reshare, :url => post_url(@reshare), :additional_subscribers => @reshare.root_author)
       render :json => ExtremePostPresenter.new(@reshare, current_user), :status => 201
     else

app/models/user/querying.rb

@@ -134,7 +134,10 @@ module User::Querying
     query = opts[:klass].where(conditions)
 
     unless opts[:all_aspects?]
-      query = query.joins(:aspect_visibilities).where(aspect_visibilities: {aspect_id: opts[:by_members_of]})
+      query = query.with_aspects.where(
+        AspectVisibility.arel_table[:aspect_id].in(opts[:by_members_of])
+          .or(opts[:klass].arel_table[:public].eq(true))
+      )
     end
 
     ugly_select_clause(query, opts)

app/services/status_message_creation_service.rb

@@ -34,9 +34,9 @@ class StatusMessageCreationService
   end
 
   def destination_aspect_ids(params, user)
-    if params[:status_message][:public] || params[:status_message][:aspect_ids].first == "all_aspects"
+    if params[:status_message][:aspect_ids].first == "all_aspects"
       user.aspect_ids
-    else
+    elsif !params[:status_message][:public]
       params[:aspect_ids]
     end
   end

db/migrate/20160302025129_cleanup_aspect_visibility.rb

+class CleanupAspectVisibility < ActiveRecord::Migration
+  class AspectVisibility < ActiveRecord::Base
+  end
+
+  def up
+    AspectVisibility.joins("LEFT OUTER JOIN posts ON posts.id = aspect_visibilities.shareable_id")
+      .where(shareable_type: "Post").delete_all("posts.id is NULL")
+    AspectVisibility.joins("LEFT OUTER JOIN photos ON photos.id = aspect_visibilities.shareable_id")
+      .where(shareable_type: "Photo").delete_all("photos.id is NULL")
+    AspectVisibility.joins("INNER JOIN posts ON posts.id = aspect_visibilities.shareable_id")
+      .where(shareable_type: "Post").delete_all(posts: {public: true})
+    AspectVisibility.joins("INNER JOIN photos ON photos.id = aspect_visibilities.shareable_id")
+      .where(shareable_type: "Photo").delete_all(photos: {public: true})
+
+    remove_columns :aspect_visibilities, :created_at, :updated_at
+  end
+
+  def down
+    add_column :aspect_visibilities, :created_at, :datetime
+    add_column :aspect_visibilities, :updated_at, :datetime
+
+    User.all.each do |user|
+      user.posts.where(public: true).each {|post| user.add_to_streams(post, user.aspects) }
+      user.photos.where(public: true).each {|photo| user.add_to_streams(photo, user.aspects) }
+    end
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20160225232049) do
+ActiveRecord::Schema.define(version: 20160302025129) do
 
   create_table "account_deletions", force: :cascade do |t|
     t.string   "diaspora_handle", limit: 255
@@ -31,11 +31,9 @@ ActiveRecord::Schema.define(version: 20160225232049) do
   add_index "aspect_memberships", ["contact_id"], name: "index_aspect_memberships_on_contact_id", using: :btree
 
   create_table "aspect_visibilities", force: :cascade do |t|
-    t.integer  "shareable_id",   limit: 4,                    null: false
-    t.integer  "aspect_id",      limit: 4,                    null: false
-    t.datetime "created_at",                                  null: false
-    t.datetime "updated_at",                                  null: false
-    t.string   "shareable_type", limit: 255, default: "Post", null: false
+    t.integer "shareable_id",   limit: 4,                    null: false
+    t.integer "aspect_id",      limit: 4,                    null: false
+    t.string  "shareable_type", limit: 255, default: "Post", null: false
   end
 
   add_index "aspect_visibilities", ["aspect_id"], name: "index_aspect_visibilities_on_aspect_id", using: :btree

features/desktop/activity_stream.feature

@@ -8,35 +8,6 @@ Feature: The activity stream
     And a user with email "bob@bob.bob" is connected with "alice@alice.alice"
     When "alice@alice.alice" has posted a status message with a photo
 
-  Scenario: Sorting
-    When I sign in as "bob@bob.bob"
-
-    Given I expand the publisher
-    When I write the status message "A- I like turtles"
-    And I submit the publisher
-    
-    Given I expand the publisher
-    When I write the status message "B- barack obama is your new bicycle"
-    And I submit the publisher
-
-    Given I expand the publisher
-    When I write the status message "C- barack obama is a square"
-    And I submit the publisher
-
-    When I go to the activity stream page
-    Then "C- barack obama is a square" should be post 1
-    And "B- barack obama is your new bicycle" should be post 2
-    And "A- I like turtles" should be post 3
-
-    When I like the post "A- I like turtles" in the stream
-    And I comment "Sassy sawfish" on "C- barack obama is a square"
-    And I like the post "B- barack obama is your new bicycle" in the stream
-    
-    When I go to the activity stream page
-    Then "B- barack obama is your new bicycle" should be post 1
-    And "C- barack obama is a square" should be post 2
-    And "A- I like turtles" should be post 3
-
   Scenario: delete a comment
     When I sign in as "bob@bob.bob"
     And I am on "alice@alice.alice"'s page
@@ -70,35 +41,3 @@ Feature: The activity stream
     And I unlike the post "Look at this dog" in the stream
     And I go to the activity stream page
     Then I should not see "Look at this dog"
-
-  Scenario: multiple participations
-    When I sign in as "bob@bob.bob"
-    And I am on "alice@alice.alice"'s page
-    Then I should see "Look at this dog"
-
-    When I like the post "Look at this dog" in the stream
-    And I go to the activity stream page
-    Then I should see "Look at this dog"
-
-    When I am on "alice@alice.alice"'s page
-    Then I should see "Look at this dog"
-
-    When I focus the comment field
-    And I fill in the following:
-        | text            | is that a poodle?    |
-    And I press "Comment"
-
-    And I go to the activity stream page
-    Then I should see "Look at this dog"
-
-    When I am on "alice@alice.alice"'s page
-    And I unlike the post "Look at this dog" in the stream
-    And I go to the activity stream page
-    Then I should see "Look at this dog"
-
-    When I am on "alice@alice.alice"'s page
-    And I click to delete the first comment
-    And I confirm the alert
-
-    And I go to the activity stream page
-    Then I should not see "Look at this dog"

features/desktop/connects_users.feature

@@ -12,25 +12,6 @@ Feature: following and being followed
     And I add the person to my "Besties" aspect
     And I sign out
 
-  Scenario: seeing a follower's posts on their profile page, but not in your stream
-    Given "bob@bob.bob" has a non public post with text "I am following you"
-    When I sign in as "alice@alice.alice"
-    And I am on "bob@bob.bob"'s page
-    Then I should see "I am following you"
-
-    When I go to the home page
-    Then I should not see "I am following you"
-
-  Scenario: seeing public posts of someone you follow
-    Given "alice@alice.alice" has a public post with text "I am ALICE"
-
-    When I sign in as "bob@bob.bob"
-    And I am on "alice@alice.alice"'s page
-    Then I should see "I am ALICE"
-
-    When I go to the home page
-    Then I should see "I am ALICE"
-
   Scenario: I follow a malicious user
     When I sign in as "bob@bob.bob"
     And I go to the edit profile page
@@ -44,21 +25,6 @@ Feature: following and being followed
     And I add the person to my "Besties" aspect
     Then I should see a flash message containing "You have started sharing with <script>alert(0)//!"
 
-  Scenario: seeing non-public posts of someone you follow who also follows you
-    When I sign in as "alice@alice.alice"
-    And I am on "bob@bob.bob"'s page
-    And I add the person to my "Besties" aspect
-    And I add the person to my "Unicorns" aspect
-    And I go to the home page
-    Then I should have 1 contact in "Unicorns"
-    And I should have 1 contact in "Besties"
-
-    When I click the publisher and post "I am following you back"
-    And I sign out
-    And I sign in as "bob@bob.bob"
-    Then I should have 1 contacts in "Besties"
-    And I should see "I am following you back"
-
   Scenario: adding someone who follows you while creating a new aspect
     When I sign in as "alice@alice.alice"
     And I am on "bob@bob.bob"'s page

lib/diaspora/shareable.rb

@@ -8,7 +8,7 @@ module Diaspora
   module Shareable
     def self.included(model)
       model.instance_eval do
-        has_many :aspect_visibilities, as: :shareable, validate: false
+        has_many :aspect_visibilities, as: :shareable, validate: false, dependent: :delete_all
         has_many :aspects, through: :aspect_visibilities
 
         has_many :share_visibilities, as: :shareable, dependent: :delete_all
@@ -24,6 +24,10 @@ module Diaspora
           joins("LEFT OUTER JOIN share_visibilities ON share_visibilities.shareable_id = #{table_name}.id")
         }
 
+        scope :with_aspects, -> {
+          joins("LEFT OUTER JOIN aspect_visibilities ON aspect_visibilities.shareable_id = #{table_name}.id")
+        }
+
         def self.owned_or_visible_by_user(user)
           with_visibility.where(
             visible_by_user(user).or(arel_table[:public].eq(true)

spec/controllers/reshares_controller_spec.rb

@@ -33,11 +33,6 @@ describe ResharesController, :type => :controller do
         }.to change(Reshare, :count).by(1)
       end
 
-      it 'after save, calls add to streams' do
-        expect(bob).to receive(:add_to_streams)
-        post_request!
-      end
-
       it 'calls dispatch' do
         expect(bob).to receive(:dispatch_post).with(anything, hash_including(:additional_subscribers))
         post_request!

spec/lib/evil_query_spec.rb

 require 'spec_helper'
 
 describe EvilQuery::MultiStream do
-  let(:evil_query) { EvilQuery::MultiStream.new(alice, 'created_at', Time.now-1.week, true) }
+  let(:evil_query) { EvilQuery::MultiStream.new(alice, "created_at", Time.zone.now, true) }
+
   describe 'community_spotlight_posts!' do
     it 'does not raise an error' do
       expect { evil_query.community_spotlight_posts! }.to_not raise_error
     end
   end
+
+  describe "make_relation!" do
+    it "includes public posts of someone you follow" do
+      alice.share_with(eve.person, alice.aspects.first)
+      public_post = eve.post(:status_message, text: "public post", to: "all", public: true)
+      expect(evil_query.make_relation!.map(&:id)).to include(public_post.id)
+    end
+
+    it "includes private posts of contacts with a mutual relationship" do
+      alice.share_with(eve.person, alice.aspects.first)
+      eve.share_with(alice.person, eve.aspects.first)
+      private_post = eve.post(:status_message, text: "private post", to: eve.aspects.first.id, public: false)
+      expect(evil_query.make_relation!.map(&:id)).to include(private_post.id)
+    end
+
+    it "doesn't include posts of followers that you don't follow back" do
+      eve.share_with(alice.person, eve.aspects.first)
+      public_post = eve.post(:status_message, text: "public post", to: "all", public: true)
+      private_post = eve.post(:status_message, text: "private post", to: eve.aspects.first.id, public: false)
+      expect(evil_query.make_relation!.map(&:id)).not_to include(public_post.id)
+      expect(evil_query.make_relation!.map(&:id)).not_to include(private_post.id)
+    end
+  end
 end
 
 describe EvilQuery::Participation do
@@ -65,4 +89,29 @@ describe EvilQuery::Participation do
       expect(posts.map(&:id)).to eq([@status_messageE.id, @status_messageA.id, @status_messageB.id])
     end
   end
+
+  describe "multiple participations" do
+    before do
+      @status_message = FactoryGirl.create(:status_message, author: bob.person)
+      @like = alice.like!(@status_message)
+      @comment = alice.comment!(@status_message, "party")
+    end
+
+    let(:posts) { EvilQuery::Participation.new(alice).posts }
+
+    it "includes Posts with multiple participations" do
+      expect(posts.map(&:id)).to eq([@status_message.id])
+    end
+
+    it "includes Posts with multiple participation after removing one participation" do
+      @like.destroy
+      expect(posts.map(&:id)).to eq([@status_message.id])
+    end
+
+    it "doesn't includes Posts after removing all of their participations" do
+      @like.destroy
+      @comment.destroy
+      expect(posts.map(&:id)).not_to include(@status_message.id)
+    end
+  end
 end

spec/lib/stream/person_spec.rb

@@ -2,12 +2,20 @@ require 'spec_helper'
 require Rails.root.join('spec', 'shared_behaviors', 'stream')
 
 describe Stream::Person do
-  before do
-    @stream = Stream::Person.new(alice, bob.person, :max_time => Time.now, :order => 'updated_at')
+  describe "shared behaviors" do
+    before do
+      @stream = Stream::Person.new(alice, bob.person, max_time: Time.zone.now, order: "updated_at")
+    end
+
+    it_should_behave_like "it is a stream"
   end
 
-  describe 'shared behaviors' do
-    it_should_behave_like 'it is a stream'
+  describe "#posts" do
+    it "calls user#posts_from if the user is present" do
+      stream = Stream::Person.new(alice, bob.person, max_time: Time.zone.now, order: "updated_at")
+      expect(alice).to receive(:posts_from).with(bob.person)
+      stream.posts
+    end
   end
 
   it "returns the most recent posts" do
@@ -29,5 +37,4 @@ describe Stream::Person do
 
     expect(fetched_posts).to eq(posts)
   end
-
 end