From 71e6e50846ca797a43f7440b16f8dfdbaf0339e3 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 7 Apr 2026 05:58:37 -0400 Subject: [PATCH] Simplify media attachment lookup in show/player actions (#38565) --- app/controllers/media_controller.rb | 7 +------ app/models/media_attachment.rb | 6 ++++++ app/serializers/seo/social_media_posting_serializer.rb | 4 ++-- app/views/statuses/_og_image.html.haml | 4 ++-- config/routes.rb | 2 +- spec/requests/media_spec.rb | 2 +- spec/system/media_spec.rb | 6 +++--- 7 files changed, 16 insertions(+), 15 deletions(-) diff --git a/app/controllers/media_controller.rb b/app/controllers/media_controller.rb index 0590ea4027..2aa83717c3 100644 --- a/app/controllers/media_controller.rb +++ b/app/controllers/media_controller.rb @@ -24,12 +24,7 @@ class MediaController < ApplicationController private def set_media_attachment - 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(id) + @media_attachment = MediaAttachment.local.attached.identified(params[:id]) end def verify_permitted_status! diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index 7e3ec16dc9..ecfaca9de4 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -38,6 +38,8 @@ class MediaAttachment < ApplicationRecord enum :type, { image: 0, gifv: 1, video: 2, unknown: 3, audio: 4 } enum :processing, { queued: 0, in_progress: 1, complete: 2, failed: 3 }, prefix: true + SHORTCODE_LENGTH = 19 + MAX_DESCRIPTION_LENGTH = 1_500 MAX_DESCRIPTION_HARD_LENGTH_LIMIT = 10_000 @@ -300,6 +302,10 @@ class MediaAttachment < ApplicationRecord after_post_process :set_meta class << self + def identified(identifier) + identifier.size == SHORTCODE_LENGTH ? find_by!(shortcode: identifier) : find(identifier) + end + def supported_mime_types IMAGE_MIME_TYPES + VIDEO_MIME_TYPES + AUDIO_MIME_TYPES end diff --git a/app/serializers/seo/social_media_posting_serializer.rb b/app/serializers/seo/social_media_posting_serializer.rb index 72030e4173..7ed6dec160 100644 --- a/app/serializers/seo/social_media_posting_serializer.rb +++ b/app/serializers/seo/social_media_posting_serializer.rb @@ -97,7 +97,7 @@ class SEO::SocialMediaPostingSerializer < ActiveModel::Serializer upload_date: attachment.created_at.iso8601, content_url: full_asset_url(attachment.file.url(:original, false)), thumbnail_url: attachment.thumbnail.present? ? full_asset_url(attachment.thumbnail.url(:original)) : full_asset_url(attachment.file.url(:small)), - embed_url: medium_player_url(attachment), + embed_url: player_medium_url(attachment), description: attachment.description, } end @@ -112,7 +112,7 @@ class SEO::SocialMediaPostingSerializer < ActiveModel::Serializer upload_date: attachment.created_at.iso8601, content_url: full_asset_url(attachment.file.url(:original, false)), thumbnail_url: attachment.thumbnail.present? ? full_asset_url(attachment.thumbnail.url(:original)) : full_asset_url(attachment.file.url(:small)), - embed_url: medium_player_url(attachment), + embed_url: player_medium_url(attachment), description: attachment.description, } end diff --git a/app/views/statuses/_og_image.html.haml b/app/views/statuses/_og_image.html.haml index 1f7f57f156..7b6ca915c8 100644 --- a/app/views/statuses/_og_image.html.haml +++ b/app/views/statuses/_og_image.html.haml @@ -19,7 +19,7 @@ = opengraph 'og:video', full_asset_url(media.file.url(:original)) = opengraph 'og:video:secure_url', full_asset_url(media.file.url(:original)) = opengraph 'og:video:type', media.file_content_type - = opengraph 'twitter:player', medium_player_url(media) + = opengraph 'twitter:player', player_medium_url(media) = opengraph 'twitter:player:stream', full_asset_url(media.file.url(:original)) = opengraph 'twitter:player:stream:content_type', media.file_content_type - unless media.file.meta.nil? @@ -35,7 +35,7 @@ = opengraph 'og:audio', full_asset_url(media.file.url(:original)) = opengraph 'og:audio:secure_url', full_asset_url(media.file.url(:original)) = opengraph 'og:audio:type', media.file_content_type - = opengraph 'twitter:player', medium_player_url(media) + = opengraph 'twitter:player', player_medium_url(media) = opengraph 'twitter:player:stream', full_asset_url(media.file.url(:original)) = opengraph 'twitter:player:stream:content_type', media.file_content_type = opengraph 'twitter:player:width', '670' diff --git a/config/routes.rb b/config/routes.rb index 85bd79a1ce..e2b18cc48c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -193,7 +193,7 @@ Rails.application.routes.draw do end resources :media, only: [:show] do - get :player + member { get :player } end resources :tags, only: [:show] diff --git a/spec/requests/media_spec.rb b/spec/requests/media_spec.rb index 523c4689d6..a124c1d7c8 100644 --- a/spec/requests/media_spec.rb +++ b/spec/requests/media_spec.rb @@ -93,7 +93,7 @@ RSpec.describe 'Media' do let(:media) { Fabricate :media_attachment } it 'responds with not found' do - get medium_player_path(media) + get player_medium_path(media) expect(response) .to have_http_status(404) diff --git a/spec/system/media_spec.rb b/spec/system/media_spec.rb index ec069cdcaa..1ffd8b7381 100644 --- a/spec/system/media_spec.rb +++ b/spec/system/media_spec.rb @@ -15,7 +15,7 @@ RSpec.describe 'Media' do let(:media) { Fabricate :media_attachment, type: :video } it 'visits the player page and renders media' do - visit medium_player_path(media) + visit player_medium_path(media) expect(page) .to have_css('body', class: 'player') @@ -27,7 +27,7 @@ RSpec.describe 'Media' do let(:media) { Fabricate :media_attachment, type: :gifv } it 'visits the player page and renders media' do - visit medium_player_path(media) + visit player_medium_path(media) expect(page) .to have_css('body', class: 'player') @@ -39,7 +39,7 @@ RSpec.describe 'Media' do let(:media) { Fabricate :media_attachment, type: :audio } it 'visits the player page and renders media' do - visit medium_player_path(media) + visit player_medium_path(media) expect(page) .to have_css('body', class: 'player')