diff --git a/app/controllers/media_controller.rb b/app/controllers/media_controller.rb
index ce015dd1b..ee82625a0 100644
--- a/app/controllers/media_controller.rb
+++ b/app/controllers/media_controller.rb
@@ -27,7 +27,12 @@ class MediaController < ApplicationController
private
def set_media_attachment
- @media_attachment = MediaAttachment.attached.find_by!(shortcode: params[:id] || params[:medium_id])
+ id = params[:id] || params[:medium_id]
+ return if id.nil?
+
+ scope = MediaAttachment.local.attached
+ # If id is 19 characters long, it's a shortcode, otherwise it's an identifier
+ @media_attachment = id.size == 19 ? scope.find_by!(shortcode: id) : scope.find_by!(id: id)
end
def verify_permitted_status!
diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb
index 97d619f35..1741d20e9 100644
--- a/app/models/media_attachment.rb
+++ b/app/models/media_attachment.rb
@@ -217,7 +217,7 @@ class MediaAttachment < ApplicationRecord
end
def to_param
- shortcode
+ shortcode.presence || id&.to_s
end
def focus=(point)
diff --git a/spec/controllers/media_controller_spec.rb b/spec/controllers/media_controller_spec.rb
index 6651e9d84..efd15b5b4 100644
--- a/spec/controllers/media_controller_spec.rb
+++ b/spec/controllers/media_controller_spec.rb
@@ -6,33 +6,60 @@ describe MediaController do
render_views
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, shortcode: 'foo')
- get :show, params: { id: media_attachment.to_param }
+ it 'raises when shortcode cant be found' do
+ get :show, params: { id: 'missing' }
- expect(response).to redirect_to(media_attachment.file.url(:original))
+ expect(response).to have_http_status(404)
end
- it 'responds with missing when there is not an attached status' do
- media_attachment = Fabricate(:media_attachment, status: nil, shortcode: 'foo')
- get :show, params: { id: media_attachment.to_param }
+ context 'when the media attachment has a shortcode' do
+ it 'redirects to the file url when attached to a status' do
+ status = Fabricate(:status)
+ media_attachment = Fabricate(:media_attachment, status: status, shortcode: 'OI6IgDzG-nYTqvDQ994')
+ get :show, params: { id: media_attachment.to_param }
- expect(response).to have_http_status(404)
- end
+ expect(response).to redirect_to(media_attachment.file.url(:original))
+ end
- it 'raises when shortcode cant be found' do
- get :show, params: { id: 'missing' }
+ it 'responds with missing when there is not an attached status' do
+ media_attachment = Fabricate(:media_attachment, status: nil, shortcode: 'OI6IgDzG-nYTqvDQ994')
+ get :show, params: { id: media_attachment.to_param }
- expect(response).to have_http_status(404)
+ expect(response).to have_http_status(404)
+ end
+
+ it 'raises when not permitted to view' do
+ status = Fabricate(:status, visibility: :direct)
+ media_attachment = Fabricate(:media_attachment, status: status, shortcode: 'OI6IgDzG-nYTqvDQ994')
+ get :show, params: { id: media_attachment.to_param }
+
+ expect(response).to have_http_status(404)
+ end
end
- it 'raises when not permitted to view' do
- status = Fabricate(:status, visibility: :direct)
- media_attachment = Fabricate(:media_attachment, status: status, shortcode: 'foo')
- get :show, params: { id: media_attachment.to_param }
+ context 'when the media attachment has no shortcode' do
+ it 'redirects to the file url when attached to a status' do
+ status = Fabricate(:status)
+ media_attachment = Fabricate(:media_attachment, status: status)
+ get :show, params: { id: media_attachment.to_param }
- expect(response).to have_http_status(404)
+ 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)
+ get :show, params: { id: media_attachment.to_param }
+
+ expect(response).to have_http_status(404)
+ end
+
+ it 'raises when not permitted to view' do
+ status = Fabricate(:status, visibility: :direct)
+ media_attachment = Fabricate(:media_attachment, status: status)
+ get :show, params: { id: media_attachment.to_param }
+
+ expect(response).to have_http_status(404)
+ end
end
end
end
diff --git a/spec/models/media_attachment_spec.rb b/spec/models/media_attachment_spec.rb
index 5dbb3d206..7360b23cf 100644
--- a/spec/models/media_attachment_spec.rb
+++ b/spec/models/media_attachment_spec.rb
@@ -62,11 +62,23 @@ RSpec.describe MediaAttachment, type: :model do
end
describe '#to_param' do
- let(:media_attachment) { Fabricate(:media_attachment) }
- let(:shortcode) { media_attachment.shortcode }
+ let(:media_attachment) { Fabricate(:media_attachment, shortcode: shortcode) }
+ let(:shortcode) { nil }
- it 'returns shortcode' do
- expect(media_attachment.to_param).to eq shortcode
+ context 'when media attachment has a shortcode' do
+ let(:shortcode) { 'foo' }
+
+ it 'returns shortcode' do
+ expect(media_attachment.to_param).to eq shortcode
+ end
+ end
+
+ context 'when media attachment does not have a shortcode' do
+ let(:shortcode) { nil }
+
+ it 'returns string representation of id' do
+ expect(media_attachment.to_param).to eq media_attachment.id.to_s
+ end
end
end
diff --git a/spec/views/statuses/show.html.haml_spec.rb b/spec/views/statuses/show.html.haml_spec.rb
index dbda3b665..879a26959 100644
--- a/spec/views/statuses/show.html.haml_spec.rb
+++ b/spec/views/statuses/show.html.haml_spec.rb
@@ -16,10 +16,11 @@ describe 'statuses/show.html.haml', without_verify_partial_doubles: true do
end
it 'has valid author h-card and basic data for a detailed_status' do
- alice = Fabricate(:account, username: 'alice', display_name: 'Alice')
- bob = Fabricate(:account, username: 'bob', display_name: 'Bob')
- status = Fabricate(:status, account: alice, text: 'Hello World')
- reply = Fabricate(:status, account: bob, thread: status, text: 'Hello Alice')
+ alice = Fabricate(:account, username: 'alice', display_name: 'Alice')
+ bob = Fabricate(:account, username: 'bob', display_name: 'Bob')
+ status = Fabricate(:status, account: alice, text: 'Hello World')
+ media = Fabricate(:media_attachment, account: alice, status: status, type: :video)
+ reply = Fabricate(:status, account: bob, thread: status, text: 'Hello Alice')
assign(:status, status)
assign(:account, alice)
@@ -35,12 +36,13 @@ describe 'statuses/show.html.haml', without_verify_partial_doubles: true do
end
it 'has valid h-cites for p-in-reply-to and p-comment' do
- alice = Fabricate(:account, username: 'alice', display_name: 'Alice')
- bob = Fabricate(:account, username: 'bob', display_name: 'Bob')
- carl = Fabricate(:account, username: 'carl', display_name: 'Carl')
- status = Fabricate(:status, account: alice, text: 'Hello World')
- reply = Fabricate(:status, account: bob, thread: status, text: 'Hello Alice')
- comment = Fabricate(:status, account: carl, thread: reply, text: 'Hello Bob')
+ alice = Fabricate(:account, username: 'alice', display_name: 'Alice')
+ bob = Fabricate(:account, username: 'bob', display_name: 'Bob')
+ carl = Fabricate(:account, username: 'carl', display_name: 'Carl')
+ status = Fabricate(:status, account: alice, text: 'Hello World')
+ media = Fabricate(:media_attachment, account: alice, status: status, type: :video)
+ reply = Fabricate(:status, account: bob, thread: status, text: 'Hello Alice')
+ comment = Fabricate(:status, account: carl, thread: reply, text: 'Hello Bob')
assign(:status, reply)
assign(:account, alice)
@@ -62,8 +64,9 @@ describe 'statuses/show.html.haml', without_verify_partial_doubles: true do
end
it 'has valid opengraph tags' do
- alice = Fabricate(:account, username: 'alice', display_name: 'Alice')
- status = Fabricate(:status, account: alice, text: 'Hello World')
+ alice = Fabricate(:account, username: 'alice', display_name: 'Alice')
+ status = Fabricate(:status, account: alice, text: 'Hello World')
+ media = Fabricate(:media_attachment, account: alice, status: status, type: :video)
assign(:status, status)
assign(:account, alice)
@@ -78,4 +81,21 @@ describe 'statuses/show.html.haml', without_verify_partial_doubles: true do
expect(header_tags).to match(%r{})
expect(header_tags).to match(%r{})
end
+
+ it 'has twitter player tag' do
+ alice = Fabricate(:account, username: 'alice', display_name: 'Alice')
+ status = Fabricate(:status, account: alice, text: 'Hello World')
+ media = Fabricate(:media_attachment, account: alice, status: status, type: :video)
+
+ assign(:status, status)
+ assign(:account, alice)
+ assign(:descendant_threads, [])
+
+ render
+
+ header_tags = view.content_for(:header_tags)
+
+ expect(header_tags).to match(%r{})
+ expect(header_tags).to match(%r{})
+ end
end