Merge pull request #6812 from cmrd-senya/6811-visibilities-id-overlap

Don't include wrong shareable types in scopes

app/models/photo.rb

@@ -82,11 +82,7 @@ class Photo < ActiveRecord::Base
   end
 
   def self.diaspora_initialize(params = {})
-    photo = self.new params.to_hash.slice(:text, :pending)
-    photo.author = params[:author]
-    photo.public = params[:public] if params[:public]
-    photo.pending = params[:pending] if params[:pending]
-
+    photo = shareable_initialize(params)
     photo.random_string = SecureRandom.hex(10)
 
     if photo.author.local?

app/models/post.rb

@@ -133,9 +133,7 @@ class Post < ActiveRecord::Base
   #############
 
   def self.diaspora_initialize(params)
-    new(params.to_hash.stringify_keys.slice(*column_names)).tap do |new_post|
-      new_post.author = params[:author]
-    end
+    shareable_initialize(params)
   end
 
   # @return Returns true if this Post will accept updates (i.e. updates to the caption of a photo).

lib/diaspora/shareable.rb

@@ -21,11 +21,13 @@ module Diaspora
         scope :all_public, -> { where(public: true, pending: false) }
 
         scope :with_visibility, -> {
-          joins("LEFT OUTER JOIN share_visibilities ON share_visibilities.shareable_id = #{table_name}.id")
+          joins("LEFT OUTER JOIN share_visibilities ON share_visibilities.shareable_id = #{table_name}.id AND "\
+            "share_visibilities.shareable_type = '#{base_class}'")
         }
 
         scope :with_aspects, -> {
-          joins("LEFT OUTER JOIN aspect_visibilities ON aspect_visibilities.shareable_id = #{table_name}.id")
+          joins("LEFT OUTER JOIN aspect_visibilities ON aspect_visibilities.shareable_id = #{table_name}.id AND "\
+          " aspect_visibilities.shareable_type = '#{base_class}'")
         }
 
         def self.owned_or_visible_by_user(user)
@@ -55,9 +57,14 @@ module Diaspora
           user.person.send(table_name).where(pending: false)
         end
 
+        def self.shareable_initialize(params)
+          new(params.to_hash.stringify_keys.slice(*column_names)).tap do |new_shareable|
+            new_shareable.author = params[:author]
+          end
+        end
+
         def self.visible_by_user(user)
           ShareVisibility.arel_table[:user_id].eq(user.id)
-            .and(ShareVisibility.arel_table[:shareable_type].eq(base_class.to_s))
         end
         private_class_method :visible_by_user
       end

spec/lib/diaspora/shareable_spec.rb

@@ -17,5 +17,30 @@ describe Diaspora::Shareable do
         expect(Post.all_public.map(&:id)).to eq([])
       end
     end
+
+    context "having multiple objects with equal db IDs" do
+      before do
+        # Determine the next database key ID, free on both Photo and StatusMessage
+        id = [Photo, StatusMessage].map {|model| model.maximum(:id).try(:next).to_i }.push(1).max
+
+        alice.post(:status_message, id: id, text: "I'm #{alice.username}", to: alice.aspects.first.id, public: false)
+        alice.post(:photo, id: id, user_file: uploaded_photo, to: alice.aspects.first.id, public: false)
+        expect(StatusMessage.where(id: id)).to exist
+        expect(Photo.where(id: id)).to exist
+      end
+
+      {with_visibility: ShareVisibility, with_aspects: AspectVisibility}.each do |method, visibility_class|
+        describe ".#{method}" do
+          it "includes only object of a right type" do
+            [Photo, Post].each do |klass|
+              expect(klass.send(method).where(visibility_class.arel_table[:shareable_type].eq(klass.to_s)).count)
+                .not_to eq(0)
+              expect(klass.send(method).where.not(visibility_class.arel_table[:shareable_type].eq(klass.to_s)).count)
+                .to eq(0)
+            end
+          end
+        end
+      end
+    end
   end
 end