Skip to content

Commit 4f84071

Browse files
committed
load all the latest messages for conversations at once
Change-Id: I5df13199af77c52a17f172e24b87089a7f15775b Reviewed-on: https://gerrit.instructure.com/5143 Reviewed-by: Jon Jensen <jon@instructure.com> Tested-by: Hudson <hudson@instructure.com>
1 parent 11d27fe commit 4f84071

5 files changed

Lines changed: 28 additions & 11 deletions

File tree

app/controllers/conversations_controller.rb

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ class ConversationsController < ApplicationController
2727
add_crumb(lambda { I18n.t 'crumbs.messages', "Conversations" }) { |c| c.send :conversations_url }
2828

2929
def index
30-
@conversations = case params[:scope]
30+
conversations = case params[:scope]
3131
when 'unread'
3232
@view_name = I18n.t('index.inbox_views.unread', 'Unread')
3333
@no_messages = I18n.t('no_unread_messages', 'You have no unread messages')
@@ -55,8 +55,17 @@ def index
5555
@view_name = I18n.t('index.inbox_views.inbox', 'Inbox')
5656
@no_messages = I18n.t('no_messages', 'You have no messages')
5757
@current_user.conversations.default
58-
end.map{ |c| jsonify_conversation(c) }
58+
end
5959
@scope ||= params[:scope].to_sym
60+
# optimize loading the most recent messages for each conversation into a single query
61+
last_messages = ConversationMessage.latest_for_conversations(conversations).human.
62+
inject({}) { |hash, message|
63+
if !hash[message.conversation_id] || hash[message.conversation_id].id < message.id
64+
hash[message.conversation_id] = message
65+
end
66+
hash
67+
}
68+
@conversations_json = conversations.map{ |c| jsonify_conversation(c, last_messages[c.conversation_id]) }
6069
@user_cache = Hash[*jsonify_users([@current_user]).map{|u| [u[:id], u] }.flatten]
6170
end
6271

@@ -277,10 +286,10 @@ def create_message_on_conversation(conversation=@conversation, update_for_sender
277286
message
278287
end
279288

280-
def jsonify_conversation(conversation)
289+
def jsonify_conversation(conversation, last_message = nil)
281290
hash = {:audience => formatted_audience(conversation, 3)}
282291
hash[:avatar_url] = avatar_url_for(conversation)
283-
conversation.as_json.merge(hash)
292+
conversation.as_json(:last_message => last_message).merge(hash)
284293
end
285294

286295
def jsonify_users(users, blank_avatar_fallback = false)

app/models/conversation_message.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,14 @@ class ConversationMessage < ActiveRecord::Base
2929

3030
named_scope :human, :conditions => "NOT generated"
3131
named_scope :with_media_comments, :conditions => "media_comment_id IS NOT NULL"
32+
named_scope :latest_for_conversations, lambda{ |conversation_participants| {
33+
:select => 'conversation_messages.*',
34+
:joins => 'JOIN conversation_message_participants ON conversation_messages.id = conversation_message_id',
35+
:conditions => [
36+
"conversation_participant_id in (?) AND conversation_messages.created_at = (SELECT MAX(created_at) FROM conversation_messages cm2 WHERE cm2.conversation_id = conversation_messages.conversation_id)",
37+
conversation_participants.map(&:id)
38+
]
39+
}}
3240

3341
validates_length_of :body, :maximum => maximum_text_length
3442

app/models/conversation_participant.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ def self.labels
5656
validates_inclusion_of :label, :in => labels.map(&:first), :allow_nil => true
5757

5858
def as_json(options = {})
59-
latest = messages.human.first
59+
latest = options[:last_message] || messages.human.first
6060
{
6161
:id => conversation_id,
6262
:participants => participants(private?),
@@ -67,7 +67,7 @@ def as_json(options = {})
6767
:subscribed => subscribed?,
6868
:private => private?,
6969
:label => label,
70-
:properties => properties
70+
:properties => properties(latest)
7171
}
7272
end
7373

@@ -114,8 +114,8 @@ def infer_defaults
114114
self.has_media_objects = conversation.has_media_objects?
115115
end
116116

117-
def properties
118-
latest = messages.human.first
117+
def properties(latest = nil)
118+
latest ||= messages.human.first
119119
properties = []
120120
properties << :last_author if latest && latest.author_id == user_id
121121
properties << :attachments if has_attachments?

app/views/conversations/index.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@
216216
MessageInbox.user_id = <%= @current_user.id %>;
217217
MessageInbox.user_cache = <%= raw @user_cache.to_json %>;
218218
MessageInbox.contexts = <%= raw @contexts.to_json %>;
219-
MessageInbox.initial_conversations = <%= raw @conversations.to_json %>;
219+
MessageInbox.initial_conversations = <%= raw @conversations_json.to_json %>;
220220
MessageInbox.scope = <%= raw @scope.to_json %>;
221221
MessageInbox.label_scope = <%= raw @label.to_json %>;
222222
</script>

spec/controllers/conversations_controller_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ def conversation(num_other_users = 1)
4545

4646
get 'index'
4747
response.should be_success
48-
assigns[:conversations].map{|c|c[:id]}.should == @user.conversations.map(&:conversation_id)
48+
assigns[:conversations_json].map{|c|c[:id]}.should == @user.conversations.map(&:conversation_id)
4949
end
5050

5151
it "should work for an admin as well" do
@@ -56,7 +56,7 @@ def conversation(num_other_users = 1)
5656

5757
get 'index'
5858
response.should be_success
59-
assigns[:conversations].map{|c|c[:id]}.should == @user.conversations.map(&:conversation_id)
59+
assigns[:conversations_json].map{|c|c[:id]}.should == @user.conversations.map(&:conversation_id)
6060
end
6161
end
6262

0 commit comments

Comments
 (0)