From 54ff7ee278d9cd5401daf1ce498c26065834e267 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Thu, 3 Mar 2016 02:04:35 +0100 Subject: [PATCH 1/6] migration to cleanup aspect_visibilities * remove all for public posts * remove all for deleted posts --- ...0160302025129_cleanup_aspect_visibility.rb | 27 +++++++++++++++++++ db/schema.rb | 10 +++---- 2 files changed, 31 insertions(+), 6 deletions(-) create mode 100644 db/migrate/20160302025129_cleanup_aspect_visibility.rb diff --git a/db/migrate/20160302025129_cleanup_aspect_visibility.rb b/db/migrate/20160302025129_cleanup_aspect_visibility.rb new file mode 100644 index 000000000..c937ac4df --- /dev/null +++ b/db/migrate/20160302025129_cleanup_aspect_visibility.rb @@ -0,0 +1,27 @@ +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 diff --git a/db/schema.rb b/db/schema.rb index 04230193d..ee9d27afa 100644 --- a/db/schema.rb +++ b/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 From f91312896789a45dfbaa0811ae57ee2d8a21d0b5 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Wed, 2 Mar 2016 02:58:27 +0100 Subject: [PATCH 2/6] don't add public shareables to aspect_visibilities --- app/controllers/photos_controller.rb | 6 ++++-- app/controllers/reshares_controller.rb | 1 - app/models/user/querying.rb | 5 ++++- app/services/status_message_creation_service.rb | 4 ++-- lib/diaspora/shareable.rb | 4 ++++ spec/controllers/reshares_controller_spec.rb | 5 ----- 6 files changed, 14 insertions(+), 11 deletions(-) diff --git a/app/controllers/photos_controller.rb b/app/controllers/photos_controller.rb index 52a9ef543..dafe53307 100644 --- a/app/controllers/photos_controller.rb +++ b/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 diff --git a/app/controllers/reshares_controller.rb b/app/controllers/reshares_controller.rb index 1e49a5aa0..4be025598 100644 --- a/app/controllers/reshares_controller.rb +++ b/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 diff --git a/app/models/user/querying.rb b/app/models/user/querying.rb index 531c8f5fa..a3104930e 100644 --- a/app/models/user/querying.rb +++ b/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) diff --git a/app/services/status_message_creation_service.rb b/app/services/status_message_creation_service.rb index 3f0093bca..d1de5c555 100644 --- a/app/services/status_message_creation_service.rb +++ b/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 diff --git a/lib/diaspora/shareable.rb b/lib/diaspora/shareable.rb index f0a5b3121..2c6b1b6ce 100644 --- a/lib/diaspora/shareable.rb +++ b/lib/diaspora/shareable.rb @@ -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) diff --git a/spec/controllers/reshares_controller_spec.rb b/spec/controllers/reshares_controller_spec.rb index 8f9090784..90b2720d9 100644 --- a/spec/controllers/reshares_controller_spec.rb +++ b/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! From 0c8d9e1907cd671650f0af9e2782c81473b8950b Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Thu, 3 Mar 2016 02:22:04 +0100 Subject: [PATCH 3/6] delete aspect_visibilities when deleting the shareable --- lib/diaspora/shareable.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/diaspora/shareable.rb b/lib/diaspora/shareable.rb index 2c6b1b6ce..a4c58ed86 100644 --- a/lib/diaspora/shareable.rb +++ b/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 From 6ef4345dae0741daf2123ad96a8e675f60883f98 Mon Sep 17 00:00:00 2001 From: Steffen van Bergerem Date: Sat, 5 Mar 2016 09:19:58 +0100 Subject: [PATCH 4/6] Remove activity stream sorting cuke already tested in spec/lib/evil_query_spec.rb --- features/desktop/activity_stream.feature | 29 ------------------------ 1 file changed, 29 deletions(-) diff --git a/features/desktop/activity_stream.feature b/features/desktop/activity_stream.feature index 81d0d72f8..783a5e12c 100644 --- a/features/desktop/activity_stream.feature +++ b/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 From 6e6a5c1e293204a0bf1902e5954b6d2b32f3b6d3 Mon Sep 17 00:00:00 2001 From: Steffen van Bergerem Date: Sat, 5 Mar 2016 09:36:04 +0100 Subject: [PATCH 5/6] Move multiple participations test to rspec --- features/desktop/activity_stream.feature | 32 ------------------------ spec/lib/evil_query_spec.rb | 25 ++++++++++++++++++ 2 files changed, 25 insertions(+), 32 deletions(-) diff --git a/features/desktop/activity_stream.feature b/features/desktop/activity_stream.feature index 783a5e12c..1f671572e 100644 --- a/features/desktop/activity_stream.feature +++ b/features/desktop/activity_stream.feature @@ -41,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" diff --git a/spec/lib/evil_query_spec.rb b/spec/lib/evil_query_spec.rb index 0e98c5c78..f0cdb7f67 100644 --- a/spec/lib/evil_query_spec.rb +++ b/spec/lib/evil_query_spec.rb @@ -65,4 +65,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 From 9d591c64337fca10ffab0e424bebb03fdd519678 Mon Sep 17 00:00:00 2001 From: Steffen van Bergerem Date: Sat, 5 Mar 2016 11:10:31 +0100 Subject: [PATCH 6/6] Move some multi stream and person stream cukes to rspec --- features/desktop/connects_users.feature | 34 ------------------------- spec/lib/evil_query_spec.rb | 26 ++++++++++++++++++- spec/lib/stream/person_spec.rb | 17 +++++++++---- 3 files changed, 37 insertions(+), 40 deletions(-) diff --git a/features/desktop/connects_users.feature b/features/desktop/connects_users.feature index 41f2ffb9b..715d74004 100644 --- a/features/desktop/connects_users.feature +++ b/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