preserve submissions folders in user merge

previously, files moved in a user merge kept the same path
as before, which would *generally* do the right thing but
would end up moving submitted files outside submissions folders
if, for example, the target user doesn't have a Submissions folder
yet, or the target user's locale is different (so the folder
isn't named "Submissions")

test plan:
 - have a user who has submitted a file to an assignment
 - ensure it appears in their special Submissions folder
   (which is read-only and does not count against quota)
 - merge the user into a new user that has not submitted any
   assignments yet and doesn't have a submissions folder yet
 - as the destination user, submit a file to an assignment
 - both the pre-merge and post-merge submission files should
   exist in the same Submissions folder tree and should be
   an actual Submissions folder

extra credit:
 - in a target user who doesn't have a submissions folder yet,
   create a folder named "Submissions"
 - merge a user who has submitted files into this user
 - the target user should get a "Submissions 2" or somesuch
   that is the actual submissions folder, and the file should
   go in there
 - reverse the above situation: merge a user containing a file
   that is in a folder that is _not_ a submissions folder but
   is named the same as a submissions folder in the destination
   user, and ensure the file goes into a new folder

flag=none
fixes FOO-3487

Change-Id: I82adca1ac67d440ce49d3bd7f865c522a8db81bd
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/317282
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: August Thornton <august@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
This commit is contained in:
Jeremy Stanley 2023-04-28 14:27:09 -06:00
parent 5f632c66a2
commit ff532018e6
3 changed files with 43 additions and 4 deletions

View File

@ -2441,7 +2441,20 @@ class Attachment < ActiveRecord::Base
end
new_attachment
end
change_attachment.folder = Folder.assert_path(attachment.folder_path, to_context)
change_attachment.folder = if attachment.folder.submission_context_code
# source folder is a submissions folder; find the corresponding submissions
# folder in the destination context
submission_context = from_context.shard.activate do
Context.find_by_asset_string(attachment.folder.submission_context_code)
end
to_context.submissions_folder(submission_context)
else
# source folder is _not_ a submissions folder; ensure the file
# doesn't get placed in a submissions folder in the destination
Folder.assert_path(attachment.folder_path,
to_context,
conditions: { submission_context_code: nil })
end
change_attachment.save_without_broadcasting!
if match
change_attachment.folder.reload

View File

@ -393,7 +393,7 @@ class Folder < ActiveRecord::Base
# if a block is given, it'll be called with each new folder created by this
# method before the folder is saved
def self.assert_path(path, context)
def self.assert_path(path, context, conditions: {})
@@path_lookups ||= {}
key = [context.global_asset_string, path].join("//")
return @@path_lookups[key] if @@path_lookups[key]
@ -406,7 +406,7 @@ class Folder < ActiveRecord::Base
end
folders.each do |name|
sub_folder = @@path_lookups[[context.global_asset_string, current_folder.full_name + "/" + name].join("//")]
sub_folder ||= current_folder.sub_folders.active.where(name: name).first_or_initialize
sub_folder ||= current_folder.sub_folders.active.where({ name: name }.merge(conditions)).first_or_initialize
current_folder = sub_folder
if current_folder.new_record?
current_folder.context = context

View File

@ -2721,19 +2721,45 @@ describe Attachment do
expect(@user_1_file.context).to eq @merge_user_2
end
it "ensures files in submissions folders stay in submissions folders" do
@user_1_file.update! folder: @merge_user_1.submissions_folder(@course)
Attachment.migrate_attachments(@merge_user_1, @merge_user_2)
expect(@user_1_file.reload.folder.submission_context_code).to eq @course.asset_string
end
it "ensures files NOT in submissions folders don't end up in submissions folders" do
not_submissions_folder = Folder.assert_path("Submissions/#{@course.name}", @merge_user_1)
@user_1_file.update! folder: not_submissions_folder
sub_folder_2 = @merge_user_2.submissions_folder(@course)
Attachment.migrate_attachments(@merge_user_1, @merge_user_2)
dest_folder = @user_1_file.reload.folder
expect(dest_folder).not_to eq sub_folder_2
expect(dest_folder.submission_context_code).to be_nil
end
context "with sharding" do
specs_require_sharding
it "copies the attachment to the new shard and leaves the existing attachment" do
before :once do
@shard1.activate do
@merge_user_3 = user_model
end
end
it "copies the attachment to the new shard and leaves the existing attachment" do
Attachment.migrate_attachments(@merge_user_1, @merge_user_3)
expect(@user_1_file.reload.context).to eq @merge_user_1
expect(@user_1_file.user).to eq @merge_user_3
new_attachment = @merge_user_3.attachments.find_by(filename: @user_1_file.title, md5: @user_1_file.md5)
expect(new_attachment.full_display_path).to eq @user_1_file.full_display_path
end
it "translates a submission folder's context code" do
@user_1_file.update! folder: @merge_user_1.submissions_folder(@course)
Attachment.migrate_attachments(@merge_user_1, @merge_user_3)
new_attachment = @merge_user_3.attachments.find_by(filename: @user_1_file.title, md5: @user_1_file.md5)
expect(new_attachment.folder.submission_context_code).to eq @course.global_asset_string
end
end
end