UploadReference bug in many importers

Last week a client contacted us stating that a week after a migration we did for them, only the last image of every post was still showing. We dug into this and found an issue.

Last year, PostUpload changed to UploadReference and in commit DEV: Update importers from PostUpload to UploadReference (#23681) · discourse/discourse@8a5d97e · GitHub a lot of importers were changed to use UploadReference instead of PostUpload.

The pattern found in most importers is

def import_attachments
   # ... get uploads and their posts
   uploads.each do |upl|
     # ... upload file
     # ... find corresponding post
     UploadReference.ensure_exist!(upload_ids: [upload.id], target: post)
   end
end

which seems nice and fine.

However, UploadReference.ensure_exist! also makes sure that no other upload references exist to that post.

Hence, using UploadReference.ensure_exist! multiple times on a single post will only retain the last upload reference.

Given the name of the function it would probably be best to change the actual implementation (removing the delete_all) instead of rewriting the calling logic in all these places?

@david @nbianca

2 Likes