Create share visibilities for photos attached to a private post

Also fixed the "fix public photos"-migration, because it didn't work
with migration-models :/

fixes #6177

app/models/share_visibility.rb

@@ -10,6 +10,10 @@ class ShareVisibility < ActiveRecord::Base
     where(user_id: user.id)
   }
 
+  scope :for_shareable, ->(shareable) {
+    where(shareable_id: shareable.id, shareable_type: shareable.class.base_class.to_s)
+  }
+
   validate :not_public
 
   # Perform a batch import, given a set of users and a shareable
@@ -18,8 +22,17 @@ class ShareVisibility < ActiveRecord::Base
   # @param share [Shareable]
   # @return [void]
   def self.batch_import(user_ids, share)
-    return false unless ShareVisibility.new(shareable_id: share.id, shareable_type: share.class.to_s).valid?
+    return false unless ShareVisibility.new(shareable_id: share.id, shareable_type: share.class.base_class.to_s).valid?
 
+    user_ids -= ShareVisibility.for_shareable(share).where(user_id: user_ids).pluck(:user_id)
+    return false if user_ids.empty?
+
+    create_visilities(user_ids, share)
+  end
+
+  private
+
+  private_class_method def self.create_visilities(user_ids, share)
     if AppConfig.postgres?
       user_ids.each do |user_id|
         ShareVisibility.find_or_create_by(
@@ -36,8 +49,6 @@ class ShareVisibility < ActiveRecord::Base
     end
   end
 
-  private
-
   def not_public
     errors[:base] << "Cannot create visibility for a public object" if shareable.public?
   end

app/models/status_message.rb

@@ -120,6 +120,12 @@ class StatusMessage < Post
     }
   end
 
+  def receive(recipient_user_ids)
+    super(recipient_user_ids)
+
+    photos.each {|photo| photo.receive(recipient_user_ids) }
+  end
+
   private
 
   def presence_of_content

db/migrate/20160509232726_cleanup_duplicates_and_add_unique_indexes.rb

@@ -1,17 +1,4 @@
 class CleanupDuplicatesAndAddUniqueIndexes < ActiveRecord::Migration
-  class Post < ActiveRecord::Base
-  end
-
-  class StatusMessage < Post
-  end
-
-  class Photo < ActiveRecord::Base
-    belongs_to :status_message, foreign_key: :status_message_guid, primary_key: :guid
-  end
-
-  class ShareVisibility < ActiveRecord::Base
-  end
-
   def up
     # temporary index to speed up the migration
     add_index :photos, :guid, length: 191
@@ -34,12 +21,6 @@ class CleanupDuplicatesAndAddUniqueIndexes < ActiveRecord::Migration
     %i(conversations messages photos polls poll_answers poll_participations).each do |table|
       delete_duplicates_and_create_unique_index(table)
     end
-
-    # fix photo public flag again ...
-    Photo.joins(:status_message).where(posts: {public: true}).update_all(public: true)
-
-    ShareVisibility.joins("INNER JOIN photos ON photos.id = share_visibilities.shareable_id")
-                   .where(shareable_type: "Photo", photos: {public: true}).delete_all
   end
 
   def down

db/migrate/20160906225138_fix_photos_share_visibilities.rb

@@ -0,0 +1,42 @@
+class FixPhotosShareVisibilities < ActiveRecord::Migration
+  class Photo < ActiveRecord::Base
+  end
+
+  class ShareVisibility < ActiveRecord::Base
+  end
+
+  def up
+    Photo.joins("INNER JOIN posts ON posts.guid = photos.status_message_guid")
+         .where(posts: {type: "StatusMessage", public: true}).update_all(public: true)
+
+    ShareVisibility.joins("INNER JOIN photos ON photos.id = share_visibilities.shareable_id")
+                   .where(shareable_type: "Photo", photos: {public: true}).delete_all
+
+    remove_duplicates
+    remove_index :share_visibilities, name: :shareable_and_user_id
+    add_index :share_visibilities, %i(shareable_id shareable_type user_id), name: :shareable_and_user_id, unique: true
+
+    execute "INSERT INTO share_visibilities (user_id, shareable_id, shareable_type) " \
+            "SELECT post_visibility.user_id, photos.id, 'Photo' FROM photos " \
+            "INNER JOIN posts ON posts.guid = photos.status_message_guid AND posts.type = 'StatusMessage' " \
+            "LEFT OUTER JOIN share_visibilities ON share_visibilities.shareable_id = photos.id " \
+            "INNER JOIN share_visibilities AS post_visibility ON post_visibility.shareable_id = posts.id " \
+            "WHERE photos.public = 0 AND share_visibilities.shareable_id IS NULL " \
+            "AND post_visibility.shareable_type = 'Post'"
+  end
+
+  def down
+    remove_index :share_visibilities, name: :shareable_and_user_id
+    add_index :share_visibilities, %i(shareable_id shareable_type user_id), name: :shareable_and_user_id
+  end
+
+  def remove_duplicates
+    where = "WHERE s1.user_id = s2.user_id AND s1.shareable_id = s2.shareable_id AND "\
+      "s1.shareable_type = s2.shareable_type AND s1.id > s2.id"
+    if AppConfig.postgres?
+      execute("DELETE FROM share_visibilities AS s1 USING share_visibilities AS s2 #{where}")
+    else
+      execute("DELETE s1 FROM share_visibilities s1, share_visibilities s2 #{where}")
+    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: 20160902180630) do
+ActiveRecord::Schema.define(version: 20160906225138) do
 
   create_table "account_deletions", force: :cascade do |t|
     t.string   "diaspora_handle", limit: 255
@@ -538,7 +538,7 @@ ActiveRecord::Schema.define(version: 20160902180630) do
   end
 
   add_index "share_visibilities", ["shareable_id", "shareable_type", "hidden", "user_id"], name: "shareable_and_hidden_and_user_id", using: :btree
-  add_index "share_visibilities", ["shareable_id", "shareable_type", "user_id"], name: "shareable_and_user_id", using: :btree
+  add_index "share_visibilities", ["shareable_id", "shareable_type", "user_id"], name: "shareable_and_user_id", unique: true, using: :btree
   add_index "share_visibilities", ["shareable_id"], name: "index_post_visibilities_on_post_id", using: :btree
   add_index "share_visibilities", ["user_id"], name: "index_share_visibilities_on_user_id", using: :btree
 

spec/models/share_visibility_spec.rb

@@ -4,7 +4,7 @@
 
 require "spec_helper"
 
-describe ShareVisibility, :type => :model do
+describe ShareVisibility, type: :model do
   describe ".batch_import" do
     let(:post) { FactoryGirl.create(:status_message, author: alice.person) }
 
@@ -29,6 +29,13 @@ describe ShareVisibility, :type => :model do
       }.not_to raise_error
     end
 
+    it "does not create visibilities for a public shareable" do
+      public_post = FactoryGirl.create(:status_message, author: alice.person, public: true)
+
+      ShareVisibility.batch_import([bob.id], public_post)
+      expect(ShareVisibility.where(user_id: bob.id, shareable_id: post.id, shareable_type: "Post")).not_to exist
+    end
+
     context "scopes" do
       before do
         alice.post(:status_message, text: "Hey", to: alice.aspects.first)

spec/models/status_message_spec.rb

@@ -302,4 +302,33 @@ describe StatusMessage, type: :model do
       end
     end
   end
+
+  describe "#receive" do
+    let(:post) { FactoryGirl.create(:status_message, author: alice.person) }
+
+    it "receives attached photos" do
+      photo = FactoryGirl.create(:photo, status_message: post)
+
+      post.receive([bob.id])
+
+      expect(ShareVisibility.where(user_id: bob.id, shareable_id: post.id, shareable_type: "Post").count).to eq(1)
+      expect(ShareVisibility.where(user_id: bob.id, shareable_id: photo.id, shareable_type: "Photo").count).to eq(1)
+    end
+
+    it "works without attached photos" do
+      post.receive([bob.id])
+
+      expect(ShareVisibility.where(user_id: bob.id, shareable_id: post.id, shareable_type: "Post").count).to eq(1)
+    end
+
+    it "works with already received attached photos" do
+      photo = FactoryGirl.create(:photo, status_message: post)
+
+      photo.receive([bob.id])
+      post.receive([bob.id])
+
+      expect(ShareVisibility.where(user_id: bob.id, shareable_id: post.id, shareable_type: "Post").count).to eq(1)
+      expect(ShareVisibility.where(user_id: bob.id, shareable_id: photo.id, shareable_type: "Photo").count).to eq(1)
+    end
+  end
 end