Merge pull request #4547 from Raven24/conversations

refactor conversations_controller a bit

Changelog.md

@@ -8,6 +8,7 @@
 * Ported fileuploader to Backbone and refactored publisher views [#4480](https://github.com/diaspora/diaspora/pull/4480)
 * Refactor 404.html, fix [#4078](https://github.com/diaspora/diaspora/issues/4078)
 * Remove the (now useless) last post link from the user profile. [#4540](https://github.com/diaspora/diaspora/pull/4540)
+* Refactor ConversationsController, move query building to User model. [#4547](https://github.com/diaspora/diaspora/pull/4547)
 
 ## Bug fixes
 * Highlight down arrow at the user menu on hover [#4441](https://github.com/diaspora/diaspora/pull/4441)

app/controllers/conversations_controller.rb

@@ -4,12 +4,11 @@ class ConversationsController < ApplicationController
   respond_to :html, :mobile, :json, :js
 
   def index
-    @conversations = Conversation.joins(:conversation_visibilities).where(
-      :conversation_visibilities => {:person_id => current_user.person_id}).paginate(
-      :page => params[:page], :per_page => 15, :order => 'updated_at DESC')
+    @conversations = current_user.conversations.paginate(
+      :page => params[:page], :per_page => 15)
 
-    @visibilities = ConversationVisibility.where(:person_id => current_user.person_id).paginate(
-      :page => params[:page], :per_page => 15, :order => 'updated_at DESC')
+    @visibilities = current_user.conversation_visibilities.paginate(
+      :page => params[:page], :per_page => 15)
 
     @conversation = Conversation.joins(:conversation_visibilities).where(
       :conversation_visibilities => {:person_id => current_user.person_id, :conversation_id => params[:conversation_id]}).first
@@ -60,8 +59,7 @@ class ConversationsController < ApplicationController
   end
 
   def show
-    if @conversation = Conversation.joins(:conversation_visibilities).where(:id => params[:id],
-                                                                            :conversation_visibilities => {:person_id => current_user.person_id}).first
+    if @conversation = current_user.conversations.where(id: params[:id]).first
 
       @first_unread_message_id = @conversation.first_unread_message(current_user).try(:id)
       if @visibility = ConversationVisibility.where(:conversation_id => params[:id], :person_id => current_user.person.id).first

app/models/person.rb

@@ -45,6 +45,7 @@ class Person < ActiveRecord::Base
   has_many :photos, :foreign_key => :author_id, :dependent => :destroy # This person's own photos
   has_many :comments, :foreign_key => :author_id, :dependent => :destroy # This person's own comments
   has_many :participations, :foreign_key => :author_id, :dependent => :destroy
+  has_many :conversation_visibilities
 
   has_many :roles
 

app/models/user.rb

@@ -61,6 +61,9 @@ class User < ActiveRecord::Base
   has_many :blocks
   has_many :ignored_people, :through => :blocks, :source => :person
 
+  has_many :conversation_visibilities, through: :person, order: 'updated_at DESC'
+  has_many :conversations, through: :conversation_visibilities, order: 'updated_at DESC'
+
   has_many :notifications, :foreign_key => :recipient_id
 
   before_save :guard_unconfirmed_email,

lib/account_deleter.rb

@@ -50,7 +50,7 @@ class AccountDeleter
   end
 
   def ignored_ar_user_associations
-    [:followed_tags, :invited_by, :contact_people, :aspect_memberships, :ignored_people]
+    [:followed_tags, :invited_by, :contact_people, :aspect_memberships, :ignored_people, :conversation_visibilities, :conversations]
   end
 
   def delete_standard_user_associations
@@ -107,6 +107,6 @@ class AccountDeleter
   end
 
   def ignored_or_special_ar_person_associations
-    [:comments, :contacts, :notification_actors, :notifications, :owner, :profile ]
+    [:comments, :contacts, :notification_actors, :notifications, :owner, :profile, :conversation_visibilities]
   end
 end

spec/factories.rb

@@ -208,6 +208,37 @@ FactoryGirl.define do
     association(:post, :factory => :status_message)
   end
 
+  factory(:conversation) do
+    association(:author, factory: :person)
+    sequence(:subject) { |n| "conversation ##{n}" }
+
+    after(:build) do |c|
+      c.participants << c.author
+    end
+  end
+
+  factory(:conversation_with_message, parent: :conversation) do
+    after(:build) do |c|
+      msg = FactoryGirl.build(:message)
+      msg.conversation_id = c.id
+      c.participants << msg.author
+      msg.save
+    end
+  end
+
+  factory(:message) do
+    association(:author, factory: :person)
+    sequence(:text) { |n| "message text ##{n}" }
+  end
+
+  factory(:message_with_conversation, parent: :message) do
+    after(:build) do |msg|
+      c = FactoryGirl.build(:conversation)
+      c.participants << msg.author
+      msg.conversation_id = c.id
+    end
+  end
+
   #templates
   factory(:status_with_photo_backdrop, :parent => :status_message_with_photo)
 

spec/models/user_spec.rb

@@ -5,6 +5,31 @@
 require 'spec_helper'
 
 describe User do
+  context "relations" do
+    context "#conversations" do
+      it "doesn't find anything when there is nothing to find" do
+        u = FactoryGirl.create(:user)
+        u.conversations.should be_empty
+      end
+
+      it "finds the users conversations" do
+        c = FactoryGirl.create(:conversation, { author: alice.person })
+
+        alice.conversations.should include c
+      end
+
+      it "doesn't find other users conversations" do
+        c1 = FactoryGirl.create(:conversation)
+        c2 = FactoryGirl.create(:conversation)
+        c_own = FactoryGirl.create(:conversation, { author: alice.person })
+
+        alice.conversations.should include c_own
+        alice.conversations.should_not include c1
+        alice.conversations.should_not include c2
+      end
+    end
+  end
+
   describe "private key" do
     it 'has a key' do
       alice.encryption_key.should_not be nil