Stop setting a shortcode to newly-created media attachments (#16730)

* Stop setting a shortcode to newly-created media attachments

The WebUI has stopped using the “short media URL” in ages. This isn't used
anywhere except for mail notifications.

Deprecating it would allow us to eventually get rid of at least a database
column and corruption-prone index, as well as a controller.

* Fix tests

4 files changed, 7 insertions, 14 deletions

diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb
index 3515f6895..66d800b7b 100644
--- a/app/models/media_attachment.rb
+++ b/app/models/media_attachment.rb
@@ -255,7 +255,7 @@ class MediaAttachment < ApplicationRecord
   after_commit :reset_parent_cache, on: :update
 
   before_create :prepare_description, unless: :local?
-  before_create :set_shortcode
+  before_create :set_unknown_type
   before_create :set_processing
 
   after_post_process :set_meta
@@ -298,15 +298,8 @@ class MediaAttachment < ApplicationRecord
 
   private
 
-  def set_shortcode
+  def set_unknown_type
     self.type = :unknown if file.blank? && !type_changed?
-
-    return unless local?
-
-    loop do
-      self.shortcode = SecureRandom.urlsafe_base64(14)
-      break if MediaAttachment.find_by(shortcode: shortcode).nil?
-    end
   end
 
   def prepare_description
diff --git a/app/serializers/rest/media_attachment_serializer.rb b/app/serializers/rest/media_attachment_serializer.rb
index a24f95315..f27dda832 100644
--- a/app/serializers/rest/media_attachment_serializer.rb
+++ b/app/serializers/rest/media_attachment_serializer.rb
@@ -40,7 +40,7 @@ class REST::MediaAttachmentSerializer < ActiveModel::Serializer
   end
 
   def text_url
-    object.local? ? medium_url(object) : nil
+    object.local? && object.shortcode.present? ? medium_url(object) : nil
   end
 
   def meta
diff --git a/app/views/notification_mailer/_status.html.haml b/app/views/notification_mailer/_status.html.haml
index 9b7e1b65c..31460a76e 100644
--- a/app/views/notification_mailer/_status.html.haml
+++ b/app/views/notification_mailer/_status.html.haml
@@ -37,7 +37,7 @@
                                   %p
                                     - status.media_attachments.each do |a|
                                       - if status.local?
-                                        = link_to medium_url(a), medium_url(a)
+                                        = link_to full_asset_url(a.file.url(:original)), full_asset_url(a.file.url(:original))
                                       - else
                                         = link_to a.remote_url, a.remote_url
 
diff --git a/spec/controllers/media_controller_spec.rb b/spec/controllers/media_controller_spec.rb
index 2925aed59..6651e9d84 100644
--- a/spec/controllers/media_controller_spec.rb
+++ b/spec/controllers/media_controller_spec.rb
@@ -8,14 +8,14 @@ describe MediaController do
   describe '#show' do
     it 'redirects to the file url when attached to a status' do
       status = Fabricate(:status)
-      media_attachment = Fabricate(:media_attachment, status: status)
+      media_attachment = Fabricate(:media_attachment, status: status, shortcode: 'foo')
       get :show, params: { id: media_attachment.to_param }
 
       expect(response).to redirect_to(media_attachment.file.url(:original))
     end
 
     it 'responds with missing when there is not an attached status' do
-      media_attachment = Fabricate(:media_attachment, status: nil)
+      media_attachment = Fabricate(:media_attachment, status: nil, shortcode: 'foo')
       get :show, params: { id: media_attachment.to_param }
 
       expect(response).to have_http_status(404)
@@ -29,7 +29,7 @@ describe MediaController do
 
     it 'raises when not permitted to view' do
       status = Fabricate(:status, visibility: :direct)
-      media_attachment = Fabricate(:media_attachment, status: status)
+      media_attachment = Fabricate(:media_attachment, status: status, shortcode: 'foo')
       get :show, params: { id: media_attachment.to_param }
 
       expect(response).to have_http_status(404)