diff --git a/app/coffeescripts/conversations/Inbox.coffee b/app/coffeescripts/conversations/Inbox.coffee index f7ec8604415..6a280bc8b96 100644 --- a/app/coffeescripts/conversations/Inbox.coffee +++ b/app/coffeescripts/conversations/Inbox.coffee @@ -651,7 +651,15 @@ define [ @$form.formSubmit fileUpload: => return @$form.find(".file_input:visible").length > 0 - disableWhileLoading: true, + preparedFileUpload: true + context_code: "user_" + $("#identity .user_id").text() + folder_id: @options.FOLDER_ID + intent: 'message' + formDataTarget: 'url' + handle_files: (attachments, data) -> + data.attachment_ids = (a.attachment.id for a in attachments) + data + disableWhileLoading: true success: (data) => data = [data] unless data.length? for conversation in data diff --git a/app/controllers/conversations_controller.rb b/app/controllers/conversations_controller.rb index d3590f73ea9..1ca9d4aa6c0 100644 --- a/app/controllers/conversations_controller.rb +++ b/app/controllers/conversations_controller.rb @@ -159,7 +159,8 @@ class ConversationsController < ApplicationController :NOTES_ENABLED => notes_enabled, :CAN_ADD_NOTES_FOR_ACCOUNT => can_add_notes_for_account, :INITIAL_FILTER => @filter, - :SHOW_INTRO => !@current_user.watched_conversations_intro? + :SHOW_INTRO => !@current_user.watched_conversations_intro?, + :FOLDER_ID => @current_user.conversation_attachments_folder.id }) } format.json { render :json => @conversations_json } @@ -179,6 +180,13 @@ class ConversationsController < ApplicationController # recipient, defaults to false. If true, this will be a group conversation # (i.e. all recipients will see all messages and replies). If false, # individual private conversations will be started with each recipient. + # @argument attachment_ids[] An array of attachments ids. These must be + # files that have been previously uploaded to the sender's "conversation + # attachments" folder. + # @argument media_comment_id Media comment id of an audio of video file to + # be associated with this message. + # @argument media_comment_type ["audio"|"video"] Type of the associated + # media file def create return render_error('recipients', 'blank') if params[:recipients].blank? return render_error('recipients', 'invalid') if @recipients.blank? @@ -242,7 +250,7 @@ class ConversationsController < ApplicationController # body:: The actual message body # author_id:: The id of the user who sent the message (see audience, participants) # generated:: If true, indicates this is a system-generated message (e.g. "Bob added Alice to the conversation") - # media_comment:: Audio comment data for this message (if applicable). Fields include: display_name, content-type, media_id, media_type, url + # media_comment:: Audio/video comment data for this message (if applicable). Fields include: display_name, content-type, media_id, media_type, url # forwarded_messages:: If this message contains forwarded messages, they will be included here (same format as this list). Note that those messages may have forwarded messages of their own, etc. # attachments:: Array of attachments for this message. Fields include: display_name, content-type, filename, url # @response_field submissions Array of assignment submissions having @@ -437,6 +445,13 @@ class ConversationsController < ApplicationController # latest message (i.e. what we just sent) # # @argument body The message to be sent + # @argument attachment_ids[] An array of attachments ids. These must be + # files that have been previously uploaded to the sender's "conversation + # attachments" folder. + # @argument media_comment_id Media comment id of an audio of video file to + # be associated with this message. + # @argument media_comment_type ["audio"|"video"] Type of the associated + # media file # # @example_response # { @@ -901,13 +916,7 @@ class ConversationsController < ApplicationController end def create_message_on_conversation(conversation=@conversation, update_for_sender=true) - message = conversation.add_message(params[:body], :forwarded_message_ids => params[:forwarded_message_ids], :update_for_sender => update_for_sender, :context => @domain_root_account, :tags => @tags, :has_attachments => params[:attachments].present?) do |m| - if params[:attachments] - params[:attachments].sort_by{ |k,v| k.to_i }.each do |k,v| - m.attachments.create(:uploaded_data => v) if v.present? - end - end - + message = conversation.add_message(params[:body], :attachment_ids => params[:attachment_ids], :forwarded_message_ids => params[:forwarded_message_ids], :update_for_sender => update_for_sender, :context => @domain_root_account, :tags => @tags) do |m| media_id = params[:media_comment_id] media_type = params[:media_comment_type] if media_id.present? && media_type.present? diff --git a/app/controllers/files_controller.rb b/app/controllers/files_controller.rb index 16fe4223208..b83704d842c 100644 --- a/app/controllers/files_controller.rb +++ b/app/controllers/files_controller.rb @@ -401,7 +401,6 @@ class FilesController < ApplicationController elsif @context && intent == 'message' permission_object = @context permission = :send_messages - workflow_state = 'unattached_temporary' @check_quota = false elsif @context && intent && intent != 'upload' # In other cases (like unzipping a file, extracting a QTI, etc. diff --git a/app/models/conversation.rb b/app/models/conversation.rb index 9c1d96d595f..8d3eee647a4 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -30,7 +30,6 @@ class Conversation < ActiveRecord::Base :source => :user, :select => User::MESSAGEABLE_USER_COLUMN_SQL + ", NULL AS common_courses, NULL AS common_groups", :order => 'last_authored_at IS NULL, last_authored_at DESC, LOWER(COALESCE(short_name, name))' - has_many :attachments, :through => :conversation_messages attr_accessible @@ -114,7 +113,7 @@ class Conversation < ActiveRecord::Base end if options[:update_participants] - update_participants message, options.merge(:skip_attachments_and_media_comments => true) + update_participants message, options else conversation_participants.each{ |cp| cp.update_cached_data!(options.merge(:set_last_message_at => false)) } end @@ -189,6 +188,7 @@ class Conversation < ActiveRecord::Base message.generated = options[:generated] message.context = options[:context] message.asset = options[:asset] + message.attachment_ids = options[:attachment_ids] if options[:attachment_ids].present? if options[:forwarded_message_ids].present? messages = ConversationMessage.find_all_by_id(options[:forwarded_message_ids].map(&:to_i)) conversation_ids = messages.select(&:forwardable?).map(&:conversation_id).uniq @@ -303,10 +303,9 @@ class Conversation < ActiveRecord::Base ] updates << "workflow_state = CASE WHEN workflow_state = 'archived' THEN 'read' ELSE workflow_state END" if update_for_skips conversation_participants.update_all(updates.join(", "), ["user_id IN (?)", skip_ids]) - return if options[:skip_attachments_and_media_comments] updated = false - if options.has_key?(:has_attachments) ? options[:has_attachments] : message.attachments.present? + if message.attachment_ids.present? self.has_attachments = true conversation_participants.update_all({:has_attachments => true}, "NOT has_attachments") updated = true diff --git a/app/models/conversation_message.rb b/app/models/conversation_message.rb index 9467de1f25e..5667235da53 100644 --- a/app/models/conversation_message.rb +++ b/app/models/conversation_message.rb @@ -25,13 +25,15 @@ class ConversationMessage < ActiveRecord::Base belongs_to :author, :class_name => 'User' belongs_to :context, :polymorphic => true has_many :conversation_message_participants - has_many :attachments, :as => :context, :order => 'created_at, id' + has_many :attachment_associations, :as => :context + has_many :attachments, :through => :attachment_associations, :order => 'attachments.created_at, attachments.id' belongs_to :asset, :polymorphic => true, :types => :submission # TODO: move media comments into this delegate :participants, :to => :conversation delegate :subscribed_participants, :to => :conversation attr_accessible named_scope :human, :conditions => "NOT generated" + named_scope :with_attachments, :conditions => "attachment_ids <> ''" named_scope :with_media_comments, :conditions => "media_comment_id IS NOT NULL" named_scope :by_user, lambda { |user_or_id| user_or_id = user_or_id.id if user_or_id.is_a?(User) @@ -105,6 +107,15 @@ class ConversationMessage < ActiveRecord::Base self.media_comment_type = nil unless self.media_comment_id end + def attachment_ids + read_attribute :attachment_ids + end + + def attachment_ids=(ids) + self.attachments = author.conversation_attachments_folder.attachments.find_all_by_id(ids.map(&:to_i)) + write_attribute(:attachment_ids, attachments.map(&:id).join(',')) + end + def delete_from_participants conversation.conversation_participants.each do |p| p.remove_messages(self) # ensures cached stuff gets updated, etc. diff --git a/app/models/conversation_participant.rb b/app/models/conversation_participant.rb index 4e4ef9b7b62..b0f2d94e95e 100644 --- a/app/models/conversation_participant.rb +++ b/app/models/conversation_participant.rb @@ -94,21 +94,6 @@ class ConversationParticipant < ActiveRecord::Base }.with_indifferent_access end - [:attachments].each do |association| - class_eval <<-ASSOC - def #{association} - @#{association} ||= conversation.#{association}.scoped(:conditions => <<-SQL) - EXISTS ( - SELECT 1 - FROM conversation_message_participants - WHERE conversation_participant_id = \#{id} - AND conversation_message_id = conversation_messages.id - ) - SQL - end - ASSOC - end - def participants(options = {}) options = { :include_participant_contexts => false, @@ -239,7 +224,7 @@ class ConversationParticipant < ActiveRecord::Base older = times.reject!{ |t| t <= last_message_at} || [] older.first || times.reverse.first end - self.has_attachments = attachments.size > 0 + self.has_attachments = messages.with_attachments.size > 0 self.has_media_objects = messages.with_media_comments.size > 0 self.visible_last_authored_at = if latest.author_id == user_id latest.created_at diff --git a/app/models/folder.rb b/app/models/folder.rb index 26b913e40c7..ddd5740ea65 100644 --- a/app/models/folder.rb +++ b/app/models/folder.rb @@ -23,6 +23,7 @@ class Folder < ActiveRecord::Base ROOT_FOLDER_NAME = "course files" PROFILE_PICS_FOLDER_NAME = "profile pictures" MY_FILES_FOLDER_NAME = "my files" + CONVERSATION_ATTACHMENTS_FOLDER_NAME = "conversation attachments" belongs_to :context, :polymorphic => true belongs_to :cloned_item diff --git a/app/models/user.rb b/app/models/user.rb index 6a5b68af9c1..4264bc319b8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -879,7 +879,7 @@ class User < ActiveRecord::Base set_policy do given { |user| user == self } - can :rename and can :read and can :manage and can :manage_content and can :manage_files and can :manage_calendar + can :rename and can :read and can :manage and can :manage_content and can :manage_files and can :manage_calendar and can :send_messages given {|user| self.courses.any?{|c| c.user_is_teacher?(user)}} can :rename and can :create_user_notes and can :read_user_notes @@ -1753,9 +1753,17 @@ class User < ActiveRecord::Base end def profile_pics_folder - folder = self.active_folders.find_by_name(Folder::PROFILE_PICS_FOLDER_NAME) + initialize_default_folder(Folder::PROFILE_PICS_FOLDER_NAME) + end + + def conversation_attachments_folder + initialize_default_folder(Folder::CONVERSATION_ATTACHMENTS_FOLDER_NAME) + end + + def initialize_default_folder(name) + folder = self.active_folders.find_by_name(name) unless folder - folder = self.folders.create!(:name => Folder::PROFILE_PICS_FOLDER_NAME, + folder = self.folders.create!(:name => name, :parent_folder => Folder.root_folders(self).find {|f| f.name == Folder::MY_FILES_FOLDER_NAME }) end folder diff --git a/app/views/content_imports/quizzes.html.erb b/app/views/content_imports/quizzes.html.erb index c678fdabdff..2cf22fc6733 100644 --- a/app/views/content_imports/quizzes.html.erb +++ b/app/views/content_imports/quizzes.html.erb @@ -96,7 +96,7 @@ $(document).ready(function() { context_code: $("#current_context_code").text(), upload_only: true, uploadDataUrl: $qti_file_import_form.attr('action'), - postFormData: true, + formDataTarget: 'uploadDataUrl', beforeSubmit: function(data) { $(this).find(".submit_button").attr('disabled', true).text(<%= jt('messages.uploading_button', "Uploading QTI File...") %>); }, diff --git a/db/migrate/20120227192729_conversation_message_attachment_ids.rb b/db/migrate/20120227192729_conversation_message_attachment_ids.rb new file mode 100644 index 00000000000..0a42d916f66 --- /dev/null +++ b/db/migrate/20120227192729_conversation_message_attachment_ids.rb @@ -0,0 +1,11 @@ +class ConversationMessageAttachmentIds < ActiveRecord::Migration + tag :predeploy + + def self.up + add_column :conversation_messages, :attachment_ids, :text + end + + def self.down + remove_column :conversation_messages, :attachment_ids + end +end diff --git a/db/migrate/20120227194305_reassociate_conversation_attachments.rb b/db/migrate/20120227194305_reassociate_conversation_attachments.rb new file mode 100644 index 00000000000..aea5621080b --- /dev/null +++ b/db/migrate/20120227194305_reassociate_conversation_attachments.rb @@ -0,0 +1,65 @@ +class ReassociateConversationAttachments < ActiveRecord::Migration + tag :postdeploy + + def self.up + temp_table_options = adapter_name =~ /mysql/i ? 'engine=innodb' : 'AS' + + execute <<-SQL + CREATE TEMPORARY TABLE _conversation_message_attachments #{temp_table_options} + SELECT cm.id AS conversation_message_id, author_id, a.id AS attachment_id + FROM conversation_messages cm, attachments a + WHERE cm.id = a.context_id AND a.context_type = 'ConversationMessage' + SQL + add_index :_conversation_message_attachments, :conversation_message_id, :name => '_cma_cmid_index' + add_index :_conversation_message_attachments, :attachment_id, :name => '_cma_aid_index' + execute "ANALYZE _conversation_message_attachments" if adapter_name =~ /postgres/i + + # make sure users w/ conversation attachments have root folders + execute <<-SQL + INSERT INTO folders(context_id, context_type, name, full_name, workflow_state) + SELECT DISTINCT author_id, 'User', 'my files', 'my files', 'visible' + FROM _conversation_message_attachments + WHERE NOT EXISTS (SELECT 1 FROM folders WHERE context_id = author_id AND context_type = 'User' AND name = 'my files') + SQL + + # and conversation attachment folders + execute <<-SQL + INSERT INTO folders(context_id, context_type, name, full_name, workflow_state, parent_folder_id) + SELECT DISTINCT author_id, 'User', 'conversation attachments', 'conversation attachments', 'visible', folders.id + FROM _conversation_message_attachments, folders + WHERE folders.context_id = author_id AND folders.context_type = 'User' + AND NOT EXISTS (SELECT 1 FROM folders WHERE context_id = author_id AND context_type = 'User' AND name = 'conversation attachments') + SQL + + execute <<-SQL + INSERT INTO attachment_associations(attachment_id, context_id, context_type) + SELECT attachment_id, conversation_message_id, 'ConversationMessage' + FROM _conversation_message_attachments + SQL + + execute <<-SQL + UPDATE conversation_messages + SET attachment_ids = ( + SELECT #{connection.func(:group_concat, :attachment_id, ',')} + FROM _conversation_message_attachments + WHERE conversation_message_id = conversation_messages.id + ) + WHERE id IN ( + SELECT conversation_message_id FROM _conversation_message_attachments + ) + SQL + + execute <<-SQL + UPDATE attachments + SET context_type = 'User', + context_id = (SELECT author_id FROM _conversation_message_attachments WHERE attachment_id = attachments.id), + folder_id = (SELECT f.id FROM folders f, _conversation_message_attachments cma WHERE f.name = 'conversation attachments' AND f.context_type = 'User' AND f.context_id = cma.author_id AND cma.attachment_id = attachments.id LIMIT 1) + WHERE context_type = 'ConversationMessage' + AND id IN (SELECT attachment_id FROM _conversation_message_attachments) + SQL + end + + def self.down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/public/javascripts/content_migration.js b/public/javascripts/content_migration.js index e93895b5511..77a6f9dfd02 100644 --- a/public/javascripts/content_migration.js +++ b/public/javascripts/content_migration.js @@ -116,7 +116,7 @@ $(function(){ context_code: $("#current_context_code").text(), upload_only: true, uploadDataUrl: $migration_form.attr('action'), - postFormData: true + formDataTarget: 'uploadDataUrl' }, processData: function(data) { if($export_file_enabled.val() != '1'){ diff --git a/public/javascripts/jquery.ajaxJSON.js b/public/javascripts/jquery.ajaxJSON.js index de1fd92e01d..de1361cf55f 100644 --- a/public/javascripts/jquery.ajaxJSON.js +++ b/public/javascripts/jquery.ajaxJSON.js @@ -45,7 +45,7 @@ define([ } var attachments = []; var ready = function() { - var data = options.data; + var data = options.formDataTarget == 'url' ? options.formData : {}; if(options.handle_files) { var result = attachments; if(options.single_file) { @@ -91,7 +91,7 @@ define([ 'attachment[asset_string]': options.asset_string, 'attachment[filename]': item.name, 'attachment[context_code]': options.context_code - }, options.formData || {}), item); + }, options.formDataTarget == 'uploadDataUrl' ? options.formData : {}), item); } else { ready.call($this); } diff --git a/public/javascripts/jquery.disableWhileLoading.js b/public/javascripts/jquery.disableWhileLoading.js index 64db83dbf47..ef35f673006 100644 --- a/public/javascripts/jquery.disableWhileLoading.js +++ b/public/javascripts/jquery.disableWhileLoading.js @@ -33,7 +33,7 @@ define([ $.when.apply($, thingsToWaitOn).done(function() { var dataKey = 'disabled_' + $.guid++, $disabledArea = $this.add($this.next('.ui-dialog-buttonpane')), - $inputsToDisable = $disabledArea.find('*').andSelf().filter(':input').not(':disabled'), + $inputsToDisable = $disabledArea.find('*').andSelf().filter(':input').not(':disabled,[type=file]'), $foundSpinHolder = $this.find('.spin_holder'), $spinHolder = $foundSpinHolder.length ? $foundSpinHolder : $this, previousSpinHolderDisplay = $spinHolder.css('display'), diff --git a/public/javascripts/jquery.instructure_forms.js b/public/javascripts/jquery.instructure_forms.js index 6538bf0fde5..e0a1e915918 100644 --- a/public/javascripts/jquery.instructure_forms.js +++ b/public/javascripts/jquery.instructure_forms.js @@ -147,10 +147,11 @@ define([ asset_string: options.asset_string, intent: options.intent, folder_id: $.isFunction(options.folder_id) ? (options.folder_id.call($form)) : options.folder_id, - file_elements: $form.find("input[type='file']"), + file_elements: $form.find("input[type='file']:visible"), url: (options.upload_only ? null : action), uploadDataUrl: options.uploadDataUrl, - formData: options.postFormData ? formData : null, + formData: formData, + formDataTarget: options.formDataTarget, success: options.success, error: options.error }); diff --git a/spec/apis/v1/conversations_api_spec.rb b/spec/apis/v1/conversations_api_spec.rb index bc9513a7a81..b49f8b69c4e 100644 --- a/spec/apis/v1/conversations_api_spec.rb +++ b/spec/apis/v1/conversations_api_spec.rb @@ -394,7 +394,8 @@ describe ConversationsController, :type => :integration do it "should create a conversation with forwarded messages" do forwarded_message = conversation(@me, :sender => @bob).messages.first - attachment = forwarded_message.attachments.create(:uploaded_data => stub_png_data) + attachment = @me.conversation_attachments_folder.attachments.create!(:context => @me, :uploaded_data => stub_png_data) + forwarded_message.attachments << attachment json = api_call(:post, "/api/v1/conversations", { :controller => 'conversations', :action => 'create', :format => 'json' }, @@ -663,10 +664,9 @@ describe ConversationsController, :type => :integration do context "conversation" do it "should return the conversation" do conversation = conversation(@bob) - attachment = nil + attachment = @me.conversation_attachments_folder.attachments.create!(:context => @me, :filename => 'test.txt', :display_name => "test.txt", :uploaded_data => StringIO.new('test')) media_object = nil - conversation.add_message("another") do |message| - attachment = message.attachments.create(:filename => 'test.txt', :display_name => "test.txt", :uploaded_data => StringIO.new('test')) + message = conversation.add_message("another", :attachment_ids => [attachment.id]) do |message| media_object = MediaObject.new media_object.media_id = '0_12345678' media_object.media_type = 'audio' @@ -675,7 +675,7 @@ describe ConversationsController, :type => :integration do media_object.title = "test title" media_object.save! message.media_comment = media_object - message.save + message.save! end conversation.reload diff --git a/spec/controllers/conversations_controller_spec.rb b/spec/controllers/conversations_controller_spec.rb index f1cea33d16c..6c2e46fe7e5 100644 --- a/spec/controllers/conversations_controller_spec.rb +++ b/spec/controllers/conversations_controller_spec.rb @@ -452,11 +452,8 @@ describe ConversationsController do it "should include an attachment if one exists" do course_with_student conversation - @conversation.add_message('test attachment') do |message| - attachment_model(:filename => "somefile.doc") - @attachment.context = message - @attachment.save - end + attachment = @user.conversation_attachments_folder.attachments.create!(:filename => "somefile.doc", :context => @user, :uploaded_data => StringIO.new('test')) + @conversation.add_message('test attachment', :attachment_ids => [attachment.id]) HostUrl.stubs(:context_host).returns("test.host") get 'public_feed', :format => 'atom', :feed_code => @student.feed_code feed = Atom::Feed.load_feed(response.body) rescue nil diff --git a/spec/migrations/reassociate_conversation_attachments.rb b/spec/migrations/reassociate_conversation_attachments.rb new file mode 100644 index 00000000000..e5aea4c71cb --- /dev/null +++ b/spec/migrations/reassociate_conversation_attachments.rb @@ -0,0 +1,73 @@ +# +# Copyright (C) 2011 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 . +# + +require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb') +require 'db/migrate/20120227194305_reassociate_conversation_attachments.rb' + +describe ReassociateConversationAttachments do + describe "up" do + it "should work" do + c = Conversation.create! + + u1 = user + cm1 = c.conversation_messages.build + cm1.author_id = u1.id + cm1.body = '' + cm1.save_without_broadcasting! + a1 = attachment_obj_with_context(cm1) + a1.save_without_touching_context + + # this will set up the conversation attachments folder (and my files folder) + u1.conversation_attachments_folder + u1.folders.map(&:name).sort.should eql ["conversation attachments", "my files"] + + u2 = user + cm2 = c.conversation_messages.build + cm2.author_id = u2.id + cm2.body = '' + cm2.save_without_broadcasting! + a2 = attachment_obj_with_context(cm2) + a2.save_without_touching_context + a3 = attachment_obj_with_context(cm2) + a3.save_without_touching_context + u2.folders.should be_empty + + ReassociateConversationAttachments.up + + u1.reload + u1.folders.map(&:name).sort.should eql ["conversation attachments", "my files"] + u1.conversation_attachments_folder.attachments.should eql [a1] + cm1.reload + a1.reload + cm1.attachment_ids.should eql a1.id.to_s + cm1.attachments.should eql [a1] + a1.context.should eql u1 + + u2.reload + u2.folders.map(&:name).sort.should eql ["conversation attachments", "my files"] + u2.conversation_attachments_folder.attachments.map(&:id).sort.should eql [a2.id, a3.id] + cm2.reload + a2.reload + a3.reload + cm2.attachment_ids.split(',').map(&:to_i).sort.should eql [a2.id, a3.id] + cm2.attachments.should eql [a2, a3] + a2.context.should eql u2 + a3.context.should eql u2 + end + end +end diff --git a/spec/selenium/conversations_attachments_spec.rb b/spec/selenium/conversations_attachments_spec.rb index 1c1d012d07a..dd9f692ab0c 100644 --- a/spec/selenium/conversations_attachments_spec.rb +++ b/spec/selenium/conversations_attachments_spec.rb @@ -1,7 +1,8 @@ require File.expand_path(File.dirname(__FILE__) + '/common') require File.expand_path(File.dirname(__FILE__) + '/conversations_common') -describe "conversations attachments" do +shared_examples_for "conversations attachments selenium tests" do + it_should_behave_like "forked server selenium tests" it_should_behave_like "conversations selenium tests" it "should be able to add an attachment to the message form" do @@ -55,6 +56,22 @@ describe "conversations attachments" do file.read.should match data end + it "should save just one attachment when sending a bulk private message" do + student_in_course + @course.enroll_user(User.create(:name => "student1")) + @course.enroll_user(User.create(:name => "student2")) + @course.enroll_user(User.create(:name => "student3")) + + filename, fullpath, data = get_file("testfile1.txt") + new_conversation + add_recipient("student1") + add_recipient("student2") + add_recipient("student3") + expect { + submit_message_form(:attachments => [fullpath], :add_recipient => false, :group_conversation => false) + }.to change(Attachment, :count).by(1) + end + it "should save attachments on new messages on existing conversations" do student_in_course filename, fullpath, data = get_file("testfile1.txt") @@ -82,3 +99,23 @@ describe "conversations attachments" do find_with_jquery("#{message} .message_attachments li:last a .title").text.should == file2[0] end end + +describe "conversations attachments local tests" do + it_should_behave_like "conversations attachments selenium tests" + prepend_before (:each) do + Setting.set("file_storage_test_override", "local") + end + prepend_before (:all) do + Setting.set("file_storage_test_override", "local") + end +end + +describe "conversations attachments S3 tests" do + it_should_behave_like "conversations attachments selenium tests" + prepend_before (:each) do + Setting.set("file_storage_test_override", "s3") + end + prepend_before (:all) do + Setting.set("file_storage_test_override", "s3") + end +end diff --git a/spec/selenium/conversations_common.rb b/spec/selenium/conversations_common.rb index bc4397fce6e..af505b3a912 100644 --- a/spec/selenium/conversations_common.rb +++ b/spec/selenium/conversations_common.rb @@ -1,10 +1,4 @@ shared_examples_for "conversations selenium tests" do - it_should_behave_like "in-process server selenium tests" - - prepend_before(:each) do - Setting.set("file_storage_test_override", "local") - end - before(:each) do course_with_teacher_logged_in @@ -159,13 +153,15 @@ shared_examples_for "conversations selenium tests" do group_conversation_link.click if group_conversation_link.displayed? && opts[:group_conversation] expect { - driver.find_element(:id, "create_message_form").submit - wait_for_ajaximations - }.to change(ConversationMessage, :count).by(opts[:group_conversation] ? 1 : find_all_with_jquery('.token_input li').size) + # ensure that we've focused on the button, since file inputs go away + f('#create_message_form button[type=submit]').click + # file uploads can trigger multiple ajax requests, so we just wait for stuff to get reenabled + keep_trying_until{ f('#create_message_form textarea').enabled? } + }.to change(ConversationMessage, :count).by(opts[:group_conversation] ? 1 : ff('.token_input li').size) if opts[:group_conversation] message = ConversationMessage.last - driver.find_element(:id, "message_#{message.id}").should_not be_nil + f("#message_#{message.id}").should_not be_nil message end end diff --git a/spec/selenium/conversations_context_filtering_spec.rb b/spec/selenium/conversations_context_filtering_spec.rb index 452140f1ac1..01b4c57e79f 100644 --- a/spec/selenium/conversations_context_filtering_spec.rb +++ b/spec/selenium/conversations_context_filtering_spec.rb @@ -2,6 +2,7 @@ require File.expand_path(File.dirname(__FILE__) + '/common') require File.expand_path(File.dirname(__FILE__) + '/conversations_common') describe "conversations context filtering" do + it_should_behave_like "in-process server selenium tests" it_should_behave_like "conversations selenium tests" before (:each) do diff --git a/spec/selenium/conversations_group_spec.rb b/spec/selenium/conversations_group_spec.rb index b4fd8010e1a..c17152b3e21 100644 --- a/spec/selenium/conversations_group_spec.rb +++ b/spec/selenium/conversations_group_spec.rb @@ -2,6 +2,7 @@ require File.expand_path(File.dirname(__FILE__) + '/common') require File.expand_path(File.dirname(__FILE__) + '/conversations_common') describe "conversations group" do + it_should_behave_like "in-process server selenium tests" it_should_behave_like "conversations selenium tests" before(:each) do diff --git a/spec/selenium/conversations_recipient_finder_spec.rb b/spec/selenium/conversations_recipient_finder_spec.rb index 3542499f973..c5bbd048a3a 100644 --- a/spec/selenium/conversations_recipient_finder_spec.rb +++ b/spec/selenium/conversations_recipient_finder_spec.rb @@ -2,6 +2,7 @@ require File.expand_path(File.dirname(__FILE__) + '/common') require File.expand_path(File.dirname(__FILE__) + '/conversations_common') describe "conversations recipient finder" do + it_should_behave_like "in-process server selenium tests" it_should_behave_like "conversations selenium tests" before(:each) do diff --git a/spec/selenium/conversations_sent_filter_spec.rb b/spec/selenium/conversations_sent_filter_spec.rb index 8c632ebe95b..8c04677a2b7 100644 --- a/spec/selenium/conversations_sent_filter_spec.rb +++ b/spec/selenium/conversations_sent_filter_spec.rb @@ -2,6 +2,7 @@ require File.expand_path(File.dirname(__FILE__) + '/common') require File.expand_path(File.dirname(__FILE__) + '/conversations_common') describe "conversations sent filter" do + it_should_behave_like "in-process server selenium tests" it_should_behave_like "conversations selenium tests" before do diff --git a/spec/selenium/conversations_spec.rb b/spec/selenium/conversations_spec.rb index 0403dde2981..32a2bbb1baf 100644 --- a/spec/selenium/conversations_spec.rb +++ b/spec/selenium/conversations_spec.rb @@ -2,6 +2,7 @@ require File.expand_path(File.dirname(__FILE__) + '/common') require File.expand_path(File.dirname(__FILE__) + '/conversations_common') describe "conversations" do + it_should_behave_like "in-process server selenium tests" it_should_behave_like "conversations selenium tests" it "should not allow double form submissions" do @@ -19,6 +20,7 @@ describe "conversations" do name_input.send_keys(:return) f('#body').send_keys(new_message) 5.times { f('#create_message_form button[type=submit]').click } + keep_trying_until{ f('#create_message_form textarea').enabled? } }.to change(ConversationMessage, :count).by(1) end diff --git a/spec/selenium/conversations_submissions_spec.rb b/spec/selenium/conversations_submissions_spec.rb index 002deb47b19..602f3276d0a 100644 --- a/spec/selenium/conversations_submissions_spec.rb +++ b/spec/selenium/conversations_submissions_spec.rb @@ -2,6 +2,7 @@ require File.expand_path(File.dirname(__FILE__) + '/common') require File.expand_path(File.dirname(__FILE__) + '/conversations_common') describe "conversations submissions" do + it_should_behave_like "in-process server selenium tests" it_should_behave_like "conversations selenium tests" it "should list submission comments in the conversation" do diff --git a/spec/selenium/conversations_user_notes_spec.rb b/spec/selenium/conversations_user_notes_spec.rb index a046ed944fd..8b1c0b9e2d3 100644 --- a/spec/selenium/conversations_user_notes_spec.rb +++ b/spec/selenium/conversations_user_notes_spec.rb @@ -2,6 +2,7 @@ require File.expand_path(File.dirname(__FILE__) + '/common') require File.expand_path(File.dirname(__FILE__) + '/conversations_common') describe "conversations user notes" do + it_should_behave_like "in-process server selenium tests" it_should_behave_like "conversations selenium tests" before(:each) do