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
This commit is contained in:
Claire 2021-09-13 18:59:37 +02:00 committed by GitHub
parent 75027b51a4
commit db57bff11d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 7 additions and 14 deletions

View file

@ -255,7 +255,7 @@ class MediaAttachment < ApplicationRecord
after_commit :reset_parent_cache, on: :update after_commit :reset_parent_cache, on: :update
before_create :prepare_description, unless: :local? before_create :prepare_description, unless: :local?
before_create :set_shortcode before_create :set_unknown_type
before_create :set_processing before_create :set_processing
after_post_process :set_meta after_post_process :set_meta
@ -298,15 +298,8 @@ class MediaAttachment < ApplicationRecord
private private
def set_shortcode def set_unknown_type
self.type = :unknown if file.blank? && !type_changed? 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 end
def prepare_description def prepare_description

View file

@ -40,7 +40,7 @@ class REST::MediaAttachmentSerializer < ActiveModel::Serializer
end end
def text_url def text_url
object.local? ? medium_url(object) : nil object.local? && object.shortcode.present? ? medium_url(object) : nil
end end
def meta def meta

View file

@ -37,7 +37,7 @@
%p %p
- status.media_attachments.each do |a| - status.media_attachments.each do |a|
- if status.local? - 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 - else
= link_to a.remote_url, a.remote_url = link_to a.remote_url, a.remote_url

View file

@ -8,14 +8,14 @@ describe MediaController do
describe '#show' do describe '#show' do
it 'redirects to the file url when attached to a status' do it 'redirects to the file url when attached to a status' do
status = Fabricate(:status) 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 } get :show, params: { id: media_attachment.to_param }
expect(response).to redirect_to(media_attachment.file.url(:original)) expect(response).to redirect_to(media_attachment.file.url(:original))
end end
it 'responds with missing when there is not an attached status' do 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 } get :show, params: { id: media_attachment.to_param }
expect(response).to have_http_status(404) expect(response).to have_http_status(404)
@ -29,7 +29,7 @@ describe MediaController do
it 'raises when not permitted to view' do it 'raises when not permitted to view' do
status = Fabricate(:status, visibility: :direct) 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 } get :show, params: { id: media_attachment.to_param }
expect(response).to have_http_status(404) expect(response).to have_http_status(404)