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] 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,