From 717554edac4b14eb93d29d16bbb9c5f2eb608266 Mon Sep 17 00:00:00 2001
From: cmrd Senya <senya@riseup.net>
Date: Fri, 27 May 2016 20:11:36 +0300
Subject: [PATCH 1/2] Fix possible duplication of AspectVisibility

No uniqueness control on AspectVisibility resulted in possible having
multiple AspectVisibility objects in the DB for the same aspect and
shareable which doesn't make sense. Introduce uniqueness validation
and fix up tests where duplication happened.
---
 app/models/aspect_visibility.rb       |  1 +
 spec/integration/mentioning_spec.rb   |  4 ++--
 spec/models/aspect_visibility_spec.rb | 32 +++++++++++++++++++++++++++
 spec/support/user_methods.rb          |  1 -
 4 files changed, 35 insertions(+), 3 deletions(-)
 create mode 100644 spec/models/aspect_visibility_spec.rb

diff --git a/app/models/aspect_visibility.rb b/app/models/aspect_visibility.rb
index a5593b834..88e645727 100644
--- a/app/models/aspect_visibility.rb
+++ b/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
diff --git a/spec/integration/mentioning_spec.rb b/spec/integration/mentioning_spec.rb
index d8e851660..1b127bea7 100644
--- a/spec/integration/mentioning_spec.rb
+++ b/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
diff --git a/spec/models/aspect_visibility_spec.rb b/spec/models/aspect_visibility_spec.rb
new file mode 100644
index 000000000..b27b2e984
--- /dev/null
+++ b/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
diff --git a/spec/support/user_methods.rb b/spec/support/user_methods.rb
index 3909c1645..30d38cbca 100644
--- a/spec/support/user_methods.rb
+++ b/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,

From fd975eeae5b248b63cf5dd35b2d851c4d88fd354 Mon Sep 17 00:00:00 2001
From: cmrd Senya <senya@riseup.net>
Date: Tue, 31 May 2016 20:48:07 +0300
Subject: [PATCH 2/2] Cleanup migration that removes duplicating
 AspectVisibilities

---
 ...0531170531_remove_duplicate_aspect_visibilities.rb | 11 +++++++++++
 db/schema.rb                                          |  2 +-
 2 files changed, 12 insertions(+), 1 deletion(-)
 create mode 100644 db/migrate/20160531170531_remove_duplicate_aspect_visibilities.rb

diff --git a/db/migrate/20160531170531_remove_duplicate_aspect_visibilities.rb b/db/migrate/20160531170531_remove_duplicate_aspect_visibilities.rb
new file mode 100644
index 000000000..8269606be
--- /dev/null
+++ b/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
diff --git a/db/schema.rb b/db/schema.rb
index 8fa4f0d5b..382d7ef2b 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: 20160327103605) do
+ActiveRecord::Schema.define(version: 20160531170531) do
 
   create_table "account_deletions", force: :cascade do |t|
     t.string   "diaspora_handle", limit: 255