Replace Status#translatable? with language matrix in separate endpoint (#24037)

This commit is contained in:
Christian Schmidt 2023-03-16 11:07:24 +01:00 committed by GitHub
parent 630436ab2d
commit bd047acc35
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 165 additions and 180 deletions

View file

@ -484,7 +484,6 @@ RSpec/DescribedClass:
- 'spec/models/user_spec.rb' - 'spec/models/user_spec.rb'
- 'spec/policies/account_moderation_note_policy_spec.rb' - 'spec/policies/account_moderation_note_policy_spec.rb'
- 'spec/presenters/account_relationships_presenter_spec.rb' - 'spec/presenters/account_relationships_presenter_spec.rb'
- 'spec/presenters/instance_presenter_spec.rb'
- 'spec/presenters/status_relationships_presenter_spec.rb' - 'spec/presenters/status_relationships_presenter_spec.rb'
- 'spec/serializers/activitypub/note_serializer_spec.rb' - 'spec/serializers/activitypub/note_serializer_spec.rb'
- 'spec/serializers/activitypub/update_poll_serializer_spec.rb' - 'spec/serializers/activitypub/update_poll_serializer_spec.rb'

View file

@ -0,0 +1,23 @@
# frozen_string_literal: true
class Api::V1::Instances::TranslationLanguagesController < Api::BaseController
skip_before_action :require_authenticated_user!, unless: :whitelist_mode?
before_action :set_languages
def show
expires_in 1.day, public: true
render json: @languages
end
private
def set_languages
if TranslationService.configured?
@languages = Rails.cache.fetch('translation_service/languages', expires_in: 7.days, race_condition_ttl: 1.hour) { TranslationService.configured.languages }
@languages['und'] = @languages.delete(nil) if @languages.key?(nil)
else
@languages = {}
end
end
end

View file

@ -5,6 +5,10 @@ export const SERVER_FETCH_REQUEST = 'Server_FETCH_REQUEST';
export const SERVER_FETCH_SUCCESS = 'Server_FETCH_SUCCESS'; export const SERVER_FETCH_SUCCESS = 'Server_FETCH_SUCCESS';
export const SERVER_FETCH_FAIL = 'Server_FETCH_FAIL'; export const SERVER_FETCH_FAIL = 'Server_FETCH_FAIL';
export const SERVER_TRANSLATION_LANGUAGES_FETCH_REQUEST = 'SERVER_TRANSLATION_LANGUAGES_FETCH_REQUEST';
export const SERVER_TRANSLATION_LANGUAGES_FETCH_SUCCESS = 'SERVER_TRANSLATION_LANGUAGES_FETCH_SUCCESS';
export const SERVER_TRANSLATION_LANGUAGES_FETCH_FAIL = 'SERVER_TRANSLATION_LANGUAGES_FETCH_FAIL';
export const EXTENDED_DESCRIPTION_REQUEST = 'EXTENDED_DESCRIPTION_REQUEST'; export const EXTENDED_DESCRIPTION_REQUEST = 'EXTENDED_DESCRIPTION_REQUEST';
export const EXTENDED_DESCRIPTION_SUCCESS = 'EXTENDED_DESCRIPTION_SUCCESS'; export const EXTENDED_DESCRIPTION_SUCCESS = 'EXTENDED_DESCRIPTION_SUCCESS';
export const EXTENDED_DESCRIPTION_FAIL = 'EXTENDED_DESCRIPTION_FAIL'; export const EXTENDED_DESCRIPTION_FAIL = 'EXTENDED_DESCRIPTION_FAIL';
@ -37,6 +41,29 @@ const fetchServerFail = error => ({
error, error,
}); });
export const fetchServerTranslationLanguages = () => (dispatch, getState) => {
dispatch(fetchServerTranslationLanguagesRequest());
api(getState)
.get('/api/v1/instance/translation_languages').then(({ data }) => {
dispatch(fetchServerTranslationLanguagesSuccess(data));
}).catch(err => dispatch(fetchServerTranslationLanguagesFail(err)));
};
const fetchServerTranslationLanguagesRequest = () => ({
type: SERVER_TRANSLATION_LANGUAGES_FETCH_REQUEST,
});
const fetchServerTranslationLanguagesSuccess = translationLanguages => ({
type: SERVER_TRANSLATION_LANGUAGES_FETCH_SUCCESS,
translationLanguages,
});
const fetchServerTranslationLanguagesFail = error => ({
type: SERVER_TRANSLATION_LANGUAGES_FETCH_FAIL,
error,
});
export const fetchExtendedDescription = () => (dispatch, getState) => { export const fetchExtendedDescription = () => (dispatch, getState) => {
dispatch(fetchExtendedDescriptionRequest()); dispatch(fetchExtendedDescriptionRequest());

View file

@ -3,6 +3,7 @@ import ImmutablePropTypes from 'react-immutable-proptypes';
import PropTypes from 'prop-types'; import PropTypes from 'prop-types';
import { FormattedMessage, injectIntl } from 'react-intl'; import { FormattedMessage, injectIntl } from 'react-intl';
import { Link } from 'react-router-dom'; import { Link } from 'react-router-dom';
import { connect } from 'react-redux';
import classnames from 'classnames'; import classnames from 'classnames';
import PollContainer from 'mastodon/containers/poll_container'; import PollContainer from 'mastodon/containers/poll_container';
import Icon from 'mastodon/components/icon'; import Icon from 'mastodon/components/icon';
@ -47,7 +48,12 @@ class TranslateButton extends React.PureComponent {
} }
export default @injectIntl const mapStateToProps = state => ({
languages: state.getIn(['server', 'translationLanguages', 'items']),
});
export default @connect(mapStateToProps)
@injectIntl
class StatusContent extends React.PureComponent { class StatusContent extends React.PureComponent {
static contextTypes = { static contextTypes = {
@ -63,6 +69,7 @@ class StatusContent extends React.PureComponent {
onClick: PropTypes.func, onClick: PropTypes.func,
collapsable: PropTypes.bool, collapsable: PropTypes.bool,
onCollapsedToggle: PropTypes.func, onCollapsedToggle: PropTypes.func,
languages: ImmutablePropTypes.map,
intl: PropTypes.object, intl: PropTypes.object,
}; };
@ -220,7 +227,9 @@ class StatusContent extends React.PureComponent {
const hidden = this.props.onExpandedToggle ? !this.props.expanded : this.state.hidden; const hidden = this.props.onExpandedToggle ? !this.props.expanded : this.state.hidden;
const renderReadMore = this.props.onClick && status.get('collapsed'); const renderReadMore = this.props.onClick && status.get('collapsed');
const renderTranslate = this.props.onTranslate && status.get('translatable'); const contentLocale = intl.locale.replace(/[_-].*/, '');
const targetLanguages = this.props.languages?.get(status.get('language') || 'und');
const renderTranslate = this.props.onTranslate && this.context.identity.signedIn && ['public', 'unlisted'].includes(status.get('visibility')) && status.get('contentHtml').length > 0 && targetLanguages?.includes(contentLocale);
const content = { __html: status.get('translation') ? status.getIn(['translation', 'content']) : status.get('contentHtml') }; const content = { __html: status.get('translation') ? status.getIn(['translation', 'content']) : status.get('contentHtml') };
const spoilerContent = { __html: status.get('spoilerHtml') }; const spoilerContent = { __html: status.get('spoilerHtml') };

View file

@ -13,7 +13,7 @@ import { debounce } from 'lodash';
import { uploadCompose, resetCompose, changeComposeSpoilerness } from '../../actions/compose'; import { uploadCompose, resetCompose, changeComposeSpoilerness } from '../../actions/compose';
import { expandHomeTimeline } from '../../actions/timelines'; import { expandHomeTimeline } from '../../actions/timelines';
import { expandNotifications } from '../../actions/notifications'; import { expandNotifications } from '../../actions/notifications';
import { fetchServer } from '../../actions/server'; import { fetchServer, fetchServerTranslationLanguages } from '../../actions/server';
import { clearHeight } from '../../actions/height_cache'; import { clearHeight } from '../../actions/height_cache';
import { focusApp, unfocusApp, changeLayout } from 'mastodon/actions/app'; import { focusApp, unfocusApp, changeLayout } from 'mastodon/actions/app';
import { synchronouslySubmitMarkers, submitMarkers, fetchMarkers } from 'mastodon/actions/markers'; import { synchronouslySubmitMarkers, submitMarkers, fetchMarkers } from 'mastodon/actions/markers';
@ -399,6 +399,7 @@ class UI extends React.PureComponent {
this.props.dispatch(fetchMarkers()); this.props.dispatch(fetchMarkers());
this.props.dispatch(expandHomeTimeline()); this.props.dispatch(expandHomeTimeline());
this.props.dispatch(expandNotifications()); this.props.dispatch(expandNotifications());
this.props.dispatch(fetchServerTranslationLanguages());
setTimeout(() => this.props.dispatch(fetchServer()), 3000); setTimeout(() => this.props.dispatch(fetchServer()), 3000);
} }

View file

@ -2,6 +2,9 @@ import {
SERVER_FETCH_REQUEST, SERVER_FETCH_REQUEST,
SERVER_FETCH_SUCCESS, SERVER_FETCH_SUCCESS,
SERVER_FETCH_FAIL, SERVER_FETCH_FAIL,
SERVER_TRANSLATION_LANGUAGES_FETCH_REQUEST,
SERVER_TRANSLATION_LANGUAGES_FETCH_SUCCESS,
SERVER_TRANSLATION_LANGUAGES_FETCH_FAIL,
EXTENDED_DESCRIPTION_REQUEST, EXTENDED_DESCRIPTION_REQUEST,
EXTENDED_DESCRIPTION_SUCCESS, EXTENDED_DESCRIPTION_SUCCESS,
EXTENDED_DESCRIPTION_FAIL, EXTENDED_DESCRIPTION_FAIL,
@ -35,6 +38,12 @@ export default function server(state = initialState, action) {
return state.set('server', fromJS(action.server)).setIn(['server', 'isLoading'], false); return state.set('server', fromJS(action.server)).setIn(['server', 'isLoading'], false);
case SERVER_FETCH_FAIL: case SERVER_FETCH_FAIL:
return state.setIn(['server', 'isLoading'], false); return state.setIn(['server', 'isLoading'], false);
case SERVER_TRANSLATION_LANGUAGES_FETCH_REQUEST:
return state.setIn(['translationLanguages', 'isLoading'], true);
case SERVER_TRANSLATION_LANGUAGES_FETCH_SUCCESS:
return state.setIn(['translationLanguages', 'items'], fromJS(action.translationLanguages)).setIn(['translationLanguages', 'isLoading'], false);
case SERVER_TRANSLATION_LANGUAGES_FETCH_FAIL:
return state.setIn(['translationLanguages', 'isLoading'], false);
case EXTENDED_DESCRIPTION_REQUEST: case EXTENDED_DESCRIPTION_REQUEST:
return state.setIn(['extendedDescription', 'isLoading'], true); return state.setIn(['extendedDescription', 'isLoading'], true);
case EXTENDED_DESCRIPTION_SUCCESS: case EXTENDED_DESCRIPTION_SUCCESS:

View file

@ -21,8 +21,8 @@ class TranslationService
ENV['DEEPL_API_KEY'].present? || ENV['LIBRE_TRANSLATE_ENDPOINT'].present? ENV['DEEPL_API_KEY'].present? || ENV['LIBRE_TRANSLATE_ENDPOINT'].present?
end end
def supported?(_source_language, _target_language) def languages
false {}
end end
def translate(_text, _source_language, _target_language) def translate(_text, _source_language, _target_language)

View file

@ -17,23 +17,29 @@ class TranslationService::DeepL < TranslationService
end end
end end
def supported?(source_language, target_language) def languages
source_language.in?(languages('source')) && target_language.in?(languages('target')) source_languages = [nil] + fetch_languages('source')
# In DeepL, EN and PT are deprecated in favor of EN-GB/EN-US and PT-BR/PT-PT, so
# they are supported but not returned by the API.
target_languages = %w(en pt) + fetch_languages('target')
source_languages.index_with { |language| target_languages.without(nil, language) }
end end
private private
def languages(type) def fetch_languages(type)
Rails.cache.fetch("translation_service/deepl/languages/#{type}", expires_in: 7.days, race_condition_ttl: 1.minute) do
request(:get, "/v2/languages?type=#{type}") do |res| request(:get, "/v2/languages?type=#{type}") do |res|
# In DeepL, EN and PT are deprecated in favor of EN-GB/EN-US and PT-BR/PT-PT, so Oj.load(res.body_with_limit).map { |language| normalize_language(language['language']) }
# they are supported but not returned by the API. end
extra = type == 'source' ? [nil] : %w(en pt) end
languages = Oj.load(res.body_with_limit).map { |language| language['language'].downcase }
languages + extra def normalize_language(language)
end subtags = language.split(/[_-]/)
end subtags[0].downcase!
subtags[1]&.upcase!
subtags.join('-')
end end
def request(verb, path, **options) def request(verb, path, **options)

View file

@ -15,21 +15,17 @@ class TranslationService::LibreTranslate < TranslationService
end end
end end
def supported?(source_language, target_language)
languages.key?(source_language) && languages[source_language].include?(target_language)
end
private
def languages def languages
Rails.cache.fetch('translation_service/libre_translate/languages', expires_in: 7.days, race_condition_ttl: 1.minute) do
request(:get, '/languages') do |res| request(:get, '/languages') do |res|
languages = Oj.load(res.body_with_limit).to_h { |language| [language['code'], language['targets']] } languages = Oj.load(res.body_with_limit).to_h do |language|
languages[nil] = languages.values.flatten.uniq [language['code'], language['targets'].without(language['code'])]
end
languages[nil] = languages.values.flatten.uniq.sort
languages languages
end end
end end
end
private
def request(verb, path, **options) def request(verb, path, **options)
req = Request.new(verb, "#{@base_url}#{path}", allow_local: true, **options) req = Request.new(verb, "#{@base_url}#{path}", allow_local: true, **options)

View file

@ -232,16 +232,6 @@ class Status < ApplicationRecord
public_visibility? || unlisted_visibility? public_visibility? || unlisted_visibility?
end end
def translatable?
translate_target_locale = I18n.locale.to_s.split(/[_-]/).first
distributable? &&
content.present? &&
language != translate_target_locale &&
TranslationService.configured? &&
TranslationService.configured.supported?(language, translate_target_locale)
end
alias sign? distributable? alias sign? distributable?
def with_media? def with_media?

View file

@ -4,7 +4,7 @@ class REST::StatusSerializer < ActiveModel::Serializer
include FormattingHelper include FormattingHelper
attributes :id, :created_at, :in_reply_to_id, :in_reply_to_account_id, attributes :id, :created_at, :in_reply_to_id, :in_reply_to_account_id,
:sensitive, :spoiler_text, :visibility, :language, :translatable, :sensitive, :spoiler_text, :visibility, :language,
:uri, :url, :replies_count, :reblogs_count, :uri, :url, :replies_count, :reblogs_count,
:favourites_count, :edited_at :favourites_count, :edited_at
@ -50,10 +50,6 @@ class REST::StatusSerializer < ActiveModel::Serializer
object.account.user_shows_application? || (current_user? && current_user.account_id == object.account_id) object.account.user_shows_application? || (current_user? && current_user.account_id == object.account_id)
end end
def translatable
current_user? && object.translatable?
end
def visibility def visibility
# This visibility is masked behind "private" # This visibility is masked behind "private"
# to avoid API changes because there are no # to avoid API changes because there are no

View file

@ -6,19 +6,29 @@ class TranslateStatusService < BaseService
include FormattingHelper include FormattingHelper
def call(status, target_language) def call(status, target_language)
raise Mastodon::NotPermittedError unless status.translatable?
@status = status @status = status
@content = status_content_format(@status) @content = status_content_format(@status)
@target_language = target_language @target_language = target_language
raise Mastodon::NotPermittedError unless permitted?
Rails.cache.fetch("translations/#{@status.language}/#{@target_language}/#{content_hash}", expires_in: CACHE_TTL) { translation_backend.translate(@content, @status.language, @target_language) } Rails.cache.fetch("translations/#{@status.language}/#{@target_language}/#{content_hash}", expires_in: CACHE_TTL) { translation_backend.translate(@content, @status.language, @target_language) }
end end
private private
def translation_backend def translation_backend
TranslationService.configured @translation_backend ||= TranslationService.configured
end
def permitted?
return false unless @status.distributable? && @status.content.present? && TranslationService.configured?
languages[@status.language]&.include?(@target_language)
end
def languages
Rails.cache.fetch('translation_service/languages', expires_in: 7.days, race_condition_ttl: 1.hour) { TranslationService.configured.languages }
end end
def content_hash def content_hash

View file

@ -546,6 +546,7 @@ Rails.application.routes.draw do
resources :domain_blocks, only: [:index], controller: 'instances/domain_blocks' resources :domain_blocks, only: [:index], controller: 'instances/domain_blocks'
resource :privacy_policy, only: [:show], controller: 'instances/privacy_policies' resource :privacy_policy, only: [:show], controller: 'instances/privacy_policies'
resource :extended_description, only: [:show], controller: 'instances/extended_descriptions' resource :extended_description, only: [:show], controller: 'instances/extended_descriptions'
resource :translation_languages, only: [:show], controller: 'instances/translation_languages'
resource :activity, only: [:show], controller: 'instances/activity' resource :activity, only: [:show], controller: 'instances/activity'
end end

View file

@ -0,0 +1,31 @@
# frozen_string_literal: true
require 'rails_helper'
describe Api::V1::Instances::TranslationLanguagesController do
describe 'GET #show' do
context 'when no translation service is configured' do
it 'returns empty language matrix' do
get :show
expect(response).to have_http_status(200)
expect(body_as_json).to eq({})
end
end
context 'when a translation service is configured' do
before do
service = instance_double(TranslationService::DeepL, languages: { nil => %w(en de), 'en' => ['de'] })
allow(TranslationService).to receive(:configured?).and_return(true)
allow(TranslationService).to receive(:configured).and_return(service)
end
it 'returns language matrix' do
get :show
expect(response).to have_http_status(200)
expect(body_as_json).to eq({ und: %w(en de), en: ['de'] })
end
end
end
end

View file

@ -19,9 +19,10 @@ describe Api::V1::Statuses::TranslationsController do
before do before do
translation = TranslationService::Translation.new(text: 'Hello') translation = TranslationService::Translation.new(text: 'Hello')
service = instance_double(TranslationService::DeepL, translate: translation, supported?: true) service = instance_double(TranslationService::DeepL, translate: translation)
allow(TranslationService).to receive(:configured?).and_return(true) allow(TranslationService).to receive(:configured?).and_return(true)
allow(TranslationService).to receive(:configured).and_return(service) allow(TranslationService).to receive(:configured).and_return(service)
Rails.cache.write('translation_service/languages', { 'es' => ['en'] })
post :create, params: { status_id: status.id } post :create, params: { status_id: status.id }
end end

View file

@ -16,29 +16,6 @@ RSpec.describe TranslationService::DeepL do
) )
end end
describe '#supported?' do
it 'supports included languages as source and target languages' do
expect(service.supported?('uk', 'en')).to be true
end
it 'supports auto-detecting source language' do
expect(service.supported?(nil, 'en')).to be true
end
it 'supports "en" and "pt" as target languages though not included in language list' do
expect(service.supported?('uk', 'en')).to be true
expect(service.supported?('uk', 'pt')).to be true
end
it 'does not support non-included language as target language' do
expect(service.supported?('uk', 'nl')).to be false
end
it 'does not support non-included language as source language' do
expect(service.supported?('da', 'en')).to be false
end
end
describe '#translate' do describe '#translate' do
it 'returns translation with specified source language' do it 'returns translation with specified source language' do
stub_request(:post, 'https://api.deepl.com/v2/translate') stub_request(:post, 'https://api.deepl.com/v2/translate')
@ -63,13 +40,18 @@ RSpec.describe TranslationService::DeepL do
end end
end end
describe '#languages?' do describe '#languages' do
it 'returns source languages' do it 'returns source languages' do
expect(service.send(:languages, 'source')).to eq ['en', 'uk', nil] expect(service.languages.keys).to eq [nil, 'en', 'uk']
end end
it 'returns target languages' do it 'returns target languages for each source language' do
expect(service.send(:languages, 'target')).to eq %w(en-gb zh en pt) expect(service.languages['en']).to eq %w(pt en-GB zh)
expect(service.languages['uk']).to eq %w(en pt en-GB zh)
end
it 'returns target languages for auto-detection' do
expect(service.languages[nil]).to eq %w(en pt en-GB zh)
end end
end end

View file

@ -7,41 +7,24 @@ RSpec.describe TranslationService::LibreTranslate do
before do before do
stub_request(:get, 'https://libretranslate.example.com/languages').to_return( stub_request(:get, 'https://libretranslate.example.com/languages').to_return(
body: '[{"code": "en","name": "English","targets": ["de","es"]},{"code": "da","name": "Danish","targets": ["en","de"]}]' body: '[{"code": "en","name": "English","targets": ["de","en","es"]},{"code": "da","name": "Danish","targets": ["en","pt"]}]'
) )
end end
describe '#supported?' do
it 'supports included language pair' do
expect(service.supported?('en', 'de')).to be true
end
it 'does not support reversed language pair' do
expect(service.supported?('de', 'en')).to be false
end
it 'supports auto-detecting source language' do
expect(service.supported?(nil, 'de')).to be true
end
it 'does not support auto-detecting for unsupported target language' do
expect(service.supported?(nil, 'pt')).to be false
end
end
describe '#languages' do describe '#languages' do
subject(:languages) { service.send(:languages) } subject(:languages) { service.languages }
it 'includes supported source languages' do it 'returns source languages' do
expect(languages.keys).to eq ['en', 'da', nil] expect(languages.keys).to eq ['en', 'da', nil]
end end
it 'includes supported target languages for source language' do it 'returns target languages for each source language' do
expect(languages['en']).to eq %w(de es) expect(languages['en']).to eq %w(de es)
expect(languages['da']).to eq %w(en pt)
end end
it 'includes supported target languages for auto-detected language' do it 'returns target languages for auto-detected language' do
expect(languages[nil]).to eq %w(de es en) expect(languages[nil]).to eq %w(de en es pt)
end end
end end

View file

@ -114,85 +114,6 @@ RSpec.describe Status, type: :model do
end end
end end
describe '#translatable?' do
before do
allow(TranslationService).to receive(:configured?).and_return(true)
allow(TranslationService).to receive(:configured).and_return(TranslationService.new)
allow(TranslationService.configured).to receive(:supported?).with('es', 'en').and_return(true)
subject.language = 'es'
subject.visibility = :public
end
context 'all conditions are satisfied' do
it 'returns true' do
expect(subject.translatable?).to be true
end
end
context 'translation service is not configured' do
it 'returns false' do
allow(TranslationService).to receive(:configured?).and_return(false)
allow(TranslationService).to receive(:configured).and_raise(TranslationService::NotConfiguredError)
expect(subject.translatable?).to be false
end
end
context 'status language is nil' do
it 'returns true' do
subject.language = nil
allow(TranslationService.configured).to receive(:supported?).with(nil, 'en').and_return(true)
expect(subject.translatable?).to be true
end
end
context 'status language is same as default locale' do
it 'returns false' do
subject.language = I18n.locale
expect(subject.translatable?).to be false
end
end
context 'status language is unsupported' do
it 'returns false' do
subject.language = 'af'
allow(TranslationService.configured).to receive(:supported?).with('af', 'en').and_return(false)
expect(subject.translatable?).to be false
end
end
context 'default locale is unsupported' do
it 'returns false' do
allow(TranslationService.configured).to receive(:supported?).with('es', 'af').and_return(false)
I18n.with_locale('af') do
expect(subject.translatable?).to be false
end
end
end
context 'default locale has region' do
it 'returns true' do
I18n.with_locale('en-GB') do
expect(subject.translatable?).to be true
end
end
end
context 'status text is blank' do
it 'returns false' do
subject.text = ' '
expect(subject.translatable?).to be false
end
end
context 'status visiblity is hidden' do
it 'returns false' do
subject.visibility = 'limited'
expect(subject.translatable?).to be false
end
end
end
describe '#content' do describe '#content' do
it 'returns the text of the status if it is not a reblog' do it 'returns the text of the status if it is not a reblog' do
expect(subject.content).to eql subject.text expect(subject.content).to eql subject.text

View file

@ -3,7 +3,7 @@
require 'rails_helper' require 'rails_helper'
describe InstancePresenter do describe InstancePresenter do
let(:instance_presenter) { InstancePresenter.new } let(:instance_presenter) { described_class.new }
describe '#description' do describe '#description' do
around do |example| around do |example|