Merge pull request #6844 from cmrd-senya/unique-aspect-visibilities

Fix possible duplication of AspectVisibility

app/models/aspect_visibility.rb

@@ -10,4 +10,5 @@ class AspectVisibility < ActiveRecord::Base
   belongs_to :shareable, :polymorphic => true
   validates :shareable, :presence => true
 
+  validates :aspect, uniqueness: {scope: %i(shareable_id shareable_type)}
 end

db/migrate/20160531170531_remove_duplicate_aspect_visibilities.rb

@@ -0,0 +1,11 @@
+class RemoveDuplicateAspectVisibilities < ActiveRecord::Migration
+  def up
+    where = "WHERE a1.aspect_id = a2.aspect_id AND a1.shareable_id = a2.shareable_id AND "\
+      "a1.shareable_type = a2.shareable_type AND a1.id > a2.id"
+    if AppConfig.postgres?
+      execute("DELETE FROM aspect_visibilities AS a1 USING aspect_visibilities AS a2 #{where}")
+    else
+      execute("DELETE a1 FROM aspect_visibilities a1, aspect_visibilities a2 #{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: 20160327103605) do
+ActiveRecord::Schema.define(version: 20160531170531) do
 
   create_table "account_deletions", force: :cascade do |t|
     t.string   "diaspora_handle", limit: 255

spec/integration/mentioning_spec.rb

@@ -43,9 +43,9 @@ describe 'mentioning', :type => :request do
     expect(users_connected?(@user1, @user3)).to be false
 
     status_msg = nil
-    expect do
+    expect {
       status_msg = @user1.post(:status_message, {text: text_mentioning(@user3), to: default_aspect})
-    end.to change(Post, :count).by(1)
+    }.to change(Post, :count).by(1).and change(AspectVisibility, :count).by(1)
 
     expect(status_msg).not_to be_nil
     expect(status_msg.public?).to be false

spec/models/aspect_visibility_spec.rb

@@ -0,0 +1,32 @@
+require "spec_helper"
+
+describe AspectVisibility, type: :model do
+  let(:status_message) { FactoryGirl.create(:status_message) }
+  let(:aspect) { FactoryGirl.create(:aspect) }
+  let(:status_message_in_aspect) { FactoryGirl.create(:status_message_in_aspect) }
+  let(:photo_with_same_id) {
+    Photo.find_by_id(status_message_in_aspect.id) || FactoryGirl.create(:photo, id: status_message_in_aspect.id)
+  }
+
+  describe ".create" do
+    it "creates object when attributes are fine" do
+      expect {
+        AspectVisibility.create(shareable: status_message, aspect: aspect)
+      }.to change(AspectVisibility, :count).by(1)
+    end
+
+    it "doesn't allow duplicating objects" do
+      expect {
+        AspectVisibility
+          .create(shareable: status_message_in_aspect, aspect: status_message_in_aspect.aspects.first)
+          .save!
+      }.to raise_error(ActiveRecord::RecordInvalid)
+    end
+
+    it "makes difference between shareable types" do
+      expect {
+        AspectVisibility.create(shareable: photo_with_same_id, aspect: status_message_in_aspect.aspects.first).save!
+      }.not_to raise_error
+    end
+  end
+end

spec/support/user_methods.rb

@@ -16,7 +16,6 @@ class User
       if p.save!
         self.aspects.reload
 
-        add_to_streams(p, aspects)
         dispatch_opts = {
           url: Rails.application.routes.url_helpers.post_url(
             p,