From 89b5071fde2f111eaa53ce404712423de25a593d Mon Sep 17 00:00:00 2001 From: Sasha Sorokin <10401817+Brawaru@users.noreply.github.com> Date: Wed, 13 Oct 2021 09:59:31 +0700 Subject: Add single option votes tooltip in polls (#16849) Adds a tooltip when hovering over the percentages in a poll showing how many votes were cast for that option. Also removes descriptions from messages here since they are not picked up by Crowdin anyway and might be confusing to other developers :) Fixes #16820 --- app/javascript/mastodon/components/poll.js | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) (limited to 'app') diff --git a/app/javascript/mastodon/components/poll.js b/app/javascript/mastodon/components/poll.js index 477f56e13..85aa28816 100644 --- a/app/javascript/mastodon/components/poll.js +++ b/app/javascript/mastodon/components/poll.js @@ -12,8 +12,18 @@ import RelativeTimestamp from './relative_timestamp'; import Icon from 'mastodon/components/icon'; const messages = defineMessages({ - closed: { id: 'poll.closed', defaultMessage: 'Closed' }, - voted: { id: 'poll.voted', defaultMessage: 'You voted for this answer', description: 'Tooltip of the "voted" checkmark in polls' }, + closed: { + id: 'poll.closed', + defaultMessage: 'Closed', + }, + voted: { + id: 'poll.voted', + defaultMessage: 'You voted for this answer', + }, + votes: { + id: 'poll.votes', + defaultMessage: '{votes, plural, one {# vote} other {# votes}}', + }, }); const makeEmojiMap = record => record.get('emojis').reduce((obj, emoji) => { @@ -148,9 +158,16 @@ class Poll extends ImmutablePureComponent { data-index={optionIndex} /> )} - {showResults && - {Math.round(percent)}% - } + {showResults && ( + + {Math.round(percent)}% + + )} Date: Wed, 13 Oct 2021 15:27:19 +0200 Subject: 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 --- app/controllers/media_controller.rb | 7 +++- app/models/media_attachment.rb | 2 +- spec/controllers/media_controller_spec.rb | 63 +++++++++++++++++++++--------- spec/models/media_attachment_spec.rb | 20 ++++++++-- spec/views/statuses/show.html.haml_spec.rb | 44 +++++++++++++++------ 5 files changed, 100 insertions(+), 36 deletions(-) (limited to 'app') 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 -- cgit From 5169b374c64ee48a820c04d7c6e7f1f9235d2417 Mon Sep 17 00:00:00 2001 From: Sasha Sorokin <10401817+Brawaru@users.noreply.github.com> Date: Wed, 13 Oct 2021 09:59:31 +0700 Subject: [Glitch] Add single option votes tooltip in polls Port 89b5071fde2f111eaa53ce404712423de25a593d to glitch-soc Signed-off-by: Claire --- app/javascript/flavours/glitch/components/poll.js | 27 ++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) (limited to 'app') diff --git a/app/javascript/flavours/glitch/components/poll.js b/app/javascript/flavours/glitch/components/poll.js index f230823cc..970b00705 100644 --- a/app/javascript/flavours/glitch/components/poll.js +++ b/app/javascript/flavours/glitch/components/poll.js @@ -12,8 +12,18 @@ import RelativeTimestamp from './relative_timestamp'; import Icon from 'flavours/glitch/components/icon'; const messages = defineMessages({ - closed: { id: 'poll.closed', defaultMessage: 'Closed' }, - voted: { id: 'poll.voted', defaultMessage: 'You voted for this answer', description: 'Tooltip of the "voted" checkmark in polls' }, + closed: { + id: 'poll.closed', + defaultMessage: 'Closed', + }, + voted: { + id: 'poll.voted', + defaultMessage: 'You voted for this answer', + }, + votes: { + id: 'poll.votes', + defaultMessage: '{votes, plural, one {# vote} other {# votes}}', + }, }); const makeEmojiMap = record => record.get('emojis').reduce((obj, emoji) => { @@ -148,9 +158,16 @@ class Poll extends ImmutablePureComponent { data-index={optionIndex} /> )} - {showResults && - {Math.round(percent)}% - } + {showResults && ( + + {Math.round(percent)}% + + )}