From 57f108899a909d7d4978da7ab2273de4ed69bbb7 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Sat, 30 Nov 2019 15:16:36 +0200 Subject: [PATCH] FEATURE: Make site texts controller handle pages and locales (#8408) Some endpoints are returning i18n keys instead of translated messages and with these changes, the site_texts endpoint can help translating those. Pagination part is needed for better wildcard support. For example, looking for 'js.notifications' would set 'has_more' to true, but return only the first 50 messages with no way of fetching the remaining. --- .../admin/site_texts_controller.rb | 19 ++++++++++--- .../admin/site_texts_controller_spec.rb | 27 +++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/app/controllers/admin/site_texts_controller.rb b/app/controllers/admin/site_texts_controller.rb index 0f9d7bad657..69d26650234 100644 --- a/app/controllers/admin/site_texts_controller.rb +++ b/app/controllers/admin/site_texts_controller.rb @@ -20,11 +20,15 @@ class Admin::SiteTextsController < Admin::AdminController extras = {} query = params[:q] || "" + + locale = params[:locale] || I18n.locale + raise Discourse::InvalidParameters.new(:locale) if !I18n.locale_available?(locale) + if query.blank? && !overridden extras[:recommended] = true - results = self.class.preferred_keys.map { |k| record_for(k) } + results = I18n.with_locale(locale) { self.class.preferred_keys.map { |k| record_for(k) } } else - results = find_translations(query, overridden) + results = I18n.with_locale(locale) { find_translations(query, overridden) } if results.any? extras[:regex] = I18n::Backend::DiscourseI18n.create_search_regexp(query, as_string: true) @@ -41,8 +45,15 @@ class Admin::SiteTextsController < Admin::AdminController end end - extras[:has_more] = true if results.size > 50 - render_serialized(results[0..49], SiteTextSerializer, root: 'site_texts', rest_serializer: true, extras: extras, overridden_keys: overridden_keys) + page = params[:page].to_i + raise Discourse::InvalidParameters.new(:page) if page < 0 + + per_page = 50 + first = page * per_page + last = first + per_page + + extras[:has_more] = true if results.size > last + render_serialized(results[first..last - 1], SiteTextSerializer, root: 'site_texts', rest_serializer: true, extras: extras, overridden_keys: overridden_keys) end def show diff --git a/spec/requests/admin/site_texts_controller_spec.rb b/spec/requests/admin/site_texts_controller_spec.rb index 356dcfc07cd..51e23127b9f 100644 --- a/spec/requests/admin/site_texts_controller_spec.rb +++ b/spec/requests/admin/site_texts_controller_spec.rb @@ -58,6 +58,33 @@ RSpec.describe Admin::SiteTextsController do expect(JSON.parse(response.body)['extras']['has_more']).to be_truthy end + it 'works with pages' do + texts = Set.new + + get "/admin/customize/site_texts.json", params: { q: 'e' } + JSON.parse(response.body)['site_texts'].each { |text| texts << text['id'] } + expect(texts.size).to eq(50) + + get "/admin/customize/site_texts.json", params: { q: 'e', page: 1 } + JSON.parse(response.body)['site_texts'].each { |text| texts << text['id'] } + expect(texts.size).to eq(100) + end + + it 'works with locales' do + get "/admin/customize/site_texts.json", params: { q: 'yes_value', locale: 'en' } + value = JSON.parse(response.body)['site_texts'].find { |text| text['id'] == 'js.yes_value' }['value'] + expect(value).to eq(I18n.with_locale(:en) { I18n.t('js.yes_value') }) + + get "/admin/customize/site_texts.json", params: { q: 'yes_value', locale: 'de' } + value = JSON.parse(response.body)['site_texts'].find { |text| text['id'] == 'js.yes_value' }['value'] + expect(value).to eq(I18n.with_locale(:de) { I18n.t('js.yes_value') }) + end + + it 'returns an error on invalid locale' do + get "/admin/customize/site_texts.json", params: { locale: '?' } + expect(response.status).to eq(400) + end + it 'normalizes quotes during search' do value = %q|“That’s a ‘magic’ sock.”| put "/admin/customize/site_texts/title.json", params: { site_text: { value: value } }