Skip to content
Permalink
Browse files
Fix error when rendering public pages with media attachments (#16763)
* Add tests

* Fix error when rendering public pages with media attachments

* Add tests

* Fix tests

* Please CodeClimate
  • Loading branch information
ClearlyClaire committed Oct 13, 2021
1 parent 8818622 commit 5159ba26e485daaeded2288c1b02bd1e516e1ca6
@@ -27,7 +27,12 @@ def player
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!
@@ -217,7 +217,7 @@ def audio_or_video?
end

def to_param
shortcode
shortcode.presence || id&.to_s
end

def focus=(point)
@@ -6,33 +6,60 @@
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
@@ -62,11 +62,23 @@
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

@@ -16,10 +16,11 @@
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 @@
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 @@
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 @@
expect(header_tags).to match(%r{<meta content=".+" property="og:image" />})
expect(header_tags).to match(%r{<meta content="http://.+" property="og:url" />})
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{<meta content="http://.+/media/.+/player" property="twitter:player" />})
expect(header_tags).to match(%r{<meta content="player" property="twitter:card" />})
end
end

0 comments on commit 5159ba2

Please sign in to comment.