update root_account_ids on conversations tables

fixes VICE-341
flag=none

/ ---- ---- \
| Test Plan |
\ ---- ---- /

- Create a conversation
- Grab that conversation and add another root account id to it
  - c = Conversation.last
  - c.root_account_ids = [1,2] # assumes normal account and site admin
  - c.save!
- Check that the conversation participants root_account_ids now includes
  the added root_account_id
  - c.conversation_participants.first.root_account_ids
- Check that the conversation messages root_account_ids now includes the
  added root_account_id
  - c.conversation_messages.first.root_account_ids
- Check that the conversation message participants root_account_ids now
  includes the added root_account_id
  - c.conversation_message_participants.first.root_account_ids

Change-Id: I949bd25001d18303d6e79bd07dc7d3494166b84c
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/240730
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
This commit is contained in:
Matthew Lemon 2020-06-17 11:58:34 -06:00
parent d5ddea29b1
commit ec8040c720
7 changed files with 102 additions and 11 deletions

View File

@ -22,6 +22,7 @@ class Conversation < ActiveRecord::Base
include SimpleTags
include ModelCache
include SendToStream
include ConversationHelper
has_many :conversation_participants, :dependent => :destroy
has_many :conversation_messages, -> { order("created_at DESC, id DESC") }, dependent: :delete_all
@ -29,6 +30,8 @@ class Conversation < ActiveRecord::Base
has_one :stream_item, :as => :asset
belongs_to :context, polymorphic: [:account, :course, :group]
before_save :update_root_account_ids
validates_length_of :subject, :maximum => maximum_string_length, :allow_nil => true
attr_accessor :latest_messages_from_stream_item
@ -72,7 +75,7 @@ class Conversation < ActiveRecord::Base
:workflow_state => 'read',
:has_attachments => has_attachments?,
:has_media_objects => has_media_objects?,
:root_account_ids => self.root_account_ids
:root_account_ids => self.root_account_ids.join(',')
}.merge(options)
ConversationParticipant.bulk_insert(user_ids.map{ |user_id|
options.merge({:user_id => user_id})
@ -648,13 +651,14 @@ class Conversation < ActiveRecord::Base
end
end
def root_account_ids
(read_attribute(:root_account_ids) || '').split(',').map(&:to_i)
end
def root_account_ids=(ids)
# ids must be sorted for the scope to work
write_attribute(:root_account_ids, ids.sort.join(','))
def update_root_account_ids
if root_account_ids_changed?
latest_ids = root_account_ids.join(',')
%w[conversation_participants conversation_messages conversation_message_participants].each do |assoc|
scope = self.send(assoc).where("#{assoc}.root_account_ids IS DISTINCT FROM ?", latest_ids).limit(1_000)
until scope.update_all(root_account_ids: latest_ids) < 1_000; end
end
end
end
# rails' has_many-:through preloading doesn't preserve :select or :order

View File

@ -22,7 +22,7 @@ class ConversationMessage < ActiveRecord::Base
self.ignored_columns = %i[root_account_id]
include HtmlTextHelper
include ConversationHelper
include Rails.application.routes.url_helpers
include SendToStream
include SimpleTags::ReaderInstanceMethods
@ -38,6 +38,7 @@ class ConversationMessage < ActiveRecord::Base
delegate :participants, :to => :conversation
delegate :subscribed_participants, :to => :conversation
before_create :set_root_account_ids
after_create :generate_user_note!
after_save :update_attachment_associations

View File

@ -21,6 +21,7 @@ class ConversationMessageParticipant < ActiveRecord::Base
include SimpleTags
include Workflow
include ConversationHelper
belongs_to :conversation_message
belongs_to :user
@ -28,6 +29,8 @@ class ConversationMessageParticipant < ActiveRecord::Base
belongs_to :conversation_participant
delegate :author, :author_id, :generated, :body, :to => :conversation_message
before_create :set_root_account_ids
scope :active, -> { where("(conversation_message_participants.workflow_state <> 'deleted' OR conversation_message_participants.workflow_state IS NULL)") }
scope :deleted, -> { where(workflow_state: 'deleted') }
@ -41,6 +44,10 @@ class ConversationMessageParticipant < ActiveRecord::Base
state :deleted
end
def conversation
conversation_message.conversation
end
def self.query_deleted(user_id, options={})
query = self.deleted.eager_load(:conversation_message).where(user_id: user_id).order(deleted_at: :desc)

View File

@ -23,6 +23,7 @@ class ConversationParticipant < ActiveRecord::Base
include TextHelper
include SimpleTags
include ModelCache
include ConversationHelper
belongs_to :conversation
belongs_to :user
@ -188,6 +189,7 @@ class ConversationParticipant < ActiveRecord::Base
delegate :context_name, :to => :conversation
delegate :context_components, :to => :conversation
before_create :set_root_account_ids
before_update :update_unread_count_for_update
before_destroy :update_unread_count_for_destroy

View File

@ -0,0 +1,36 @@
#
# Copyright (C) 2020 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
module ConversationHelper
def set_root_account_ids
write_attribute(:root_account_ids, conversation&.root_account_ids&.join(','))
end
def root_account_ids
(read_attribute(:root_account_ids) || '').split(',').map(&:to_i)
end
def root_account_ids=(ids)
# handle case when ids is a comma separated list
if ids.is_a? String
ids = ids.split(',')
end
# ids must be sorted for the scope to work
write_attribute(:root_account_ids, ids.sort.join(','))
end
end

View File

@ -388,7 +388,7 @@ describe ConversationsController do
json.each do |conv|
conversation = Conversation.find(conv['id'])
conversation.conversation_participants.each do |cp|
expect(cp.root_account_ids).to eq @account_id.to_s
expect(cp.root_account_ids).to eq [@account_id]
end
end
end
@ -401,7 +401,7 @@ describe ConversationsController do
json.each do |conv|
conversation = Conversation.find(conv['id'])
conversation.conversation_participants.each do |cp|
expect(cp.root_account_ids).to eq @account_id.to_s
expect(cp.root_account_ids).to eq [@account_id]
end
end
end

View File

@ -1025,6 +1025,47 @@ describe Conversation do
expect(conversation.root_account_ids).to eql [new_course.root_account_id]
end
it 'should update conversation participants root account ids when changed' do
a1 = Account.create!
a2 = Account.create!
users = create_users(2, return_type: :record)
conversation = Conversation.initiate(users, false)
conversation.root_account_ids = [a1.id, a2.id]
conversation.save!
expect(
conversation.reload.conversation_participants.first.root_account_ids
).to eq [a1.id, a2.id].sort
end
it 'should update conversation messages root account ids when changed' do
a1 = Account.create!
a2 = Account.create!
users = create_users(2, return_type: :record)
conversation = Conversation.initiate(users, false)
conversation.add_message(users[0], 'howdy partner')
conversation.root_account_ids = [a1.id, a2.id]
conversation.save!
expect(
conversation.reload.conversation_messages.first.root_account_ids
).to eq [a1.id, a2.id].sort
end
it 'should update conversation message participants root account ids when changed' do
a1 = Account.create!
a2 = Account.create!
users = create_users(2, return_type: :record)
conversation = Conversation.initiate(users, false)
conversation.add_message(users[0], 'howdy partner')
conversation.root_account_ids = [a1.id, a2.id]
conversation.save!
expect(
conversation.reload.conversation_message_participants.first.root_account_ids
).to eq [a1.id, a2.id].sort
end
context "sharding" do
specs_require_sharding