From 1dbe13886f0f684182cfc429cf70586f5fa86d64 Mon Sep 17 00:00:00 2001 From: OsamaSayegh Date: Mon, 11 Jun 2018 07:59:21 +0300 Subject: [PATCH] REFACTOR: admin site texts controller specs to requests (#5958) --- config/routes.rb | 12 +- .../admin/site_texts_controller_spec.rb | 159 ------------------ .../admin/site_texts_controller_spec.rb | 143 +++++++++++++++- 3 files changed, 145 insertions(+), 169 deletions(-) delete mode 100644 spec/controllers/admin/site_texts_controller_spec.rb diff --git a/config/routes.rb b/config/routes.rb index 1ee17cc3e1b..1a1d68ee7c2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -213,11 +213,13 @@ Discourse::Application.routes.draw do get 'themes/:id' => 'themes#index' # They have periods in their URLs often: - get 'site_texts' => 'site_texts#index' - get 'site_texts/:id' => 'site_texts#show', constraints: { id: /[\w.\-\+]+/i } - put 'site_texts/:id.json' => 'site_texts#update', constraints: { id: /[\w.\-\+]+/i } - put 'site_texts/:id' => 'site_texts#update', constraints: { id: /[\w.\-\+]+/i } - delete 'site_texts/:id' => 'site_texts#revert', constraints: { id: /[\w.\-\+]+/i } + get 'site_texts' => 'site_texts#index' + get 'site_texts/:id.json' => 'site_texts#show', constraints: { id: /[\w.\-\+]+/i } + get 'site_texts/:id' => 'site_texts#show', constraints: { id: /[\w.\-\+]+/i } + put 'site_texts/:id.json' => 'site_texts#update', constraints: { id: /[\w.\-\+]+/i } + put 'site_texts/:id' => 'site_texts#update', constraints: { id: /[\w.\-\+]+/i } + delete 'site_texts/:id.json' => 'site_texts#revert', constraints: { id: /[\w.\-\+]+/i } + delete 'site_texts/:id' => 'site_texts#revert', constraints: { id: /[\w.\-\+]+/i } get 'email_templates' => 'email_templates#index' get 'email_templates/(:id)' => 'email_templates#show', constraints: { id: /[0-9a-z_.]+/ } diff --git a/spec/controllers/admin/site_texts_controller_spec.rb b/spec/controllers/admin/site_texts_controller_spec.rb deleted file mode 100644 index d9bce364879..00000000000 --- a/spec/controllers/admin/site_texts_controller_spec.rb +++ /dev/null @@ -1,159 +0,0 @@ -require 'rails_helper' - -describe Admin::SiteTextsController do - - it "is a subclass of AdminController" do - expect(Admin::SiteTextsController < Admin::AdminController).to eq(true) - end - - context 'while logged in as an admin' do - before do - @user = log_in(:admin) - end - - context '.index' do - it 'returns json' do - get :index, params: { q: 'title' }, format: :json - expect(response.status).to eq(200) - expect(::JSON.parse(response.body)).to be_present - end - end - - context '.show' do - it 'returns a site text for a key that exists' do - get :show, params: { id: 'title' }, format: :json - expect(response.status).to eq(200) - - json = ::JSON.parse(response.body) - expect(json).to be_present - - site_text = json['site_text'] - expect(site_text).to be_present - - expect(site_text['id']).to eq('title') - expect(site_text['value']).to eq(I18n.t(:title)) - end - - it 'returns not found for missing keys' do - get :show, params: { id: 'made_up_no_key_exists' }, format: :json - expect(response).not_to be_successful - end - end - - context '#update and #revert' do - after do - TranslationOverride.delete_all - I18n.reload! - end - - describe 'failure' do - before do - I18n.backend.store_translations(:en, some_key: '%{first} %{second}') - end - - it 'returns the right error message' do - put :update, params: { - id: 'some_key', site_text: { value: 'hello %{key} %{omg}' } - }, format: :json - - expect(response.status).to eq(422) - - body = JSON.parse(response.body) - - expect(body['message']).to eq(I18n.t( - 'activerecord.errors.models.translation_overrides.attributes.value.invalid_interpolation_keys', - keys: 'key, omg' - )) - end - end - - it 'updates and reverts the key' do - orig_title = I18n.t(:title) - - put :update, params: { id: 'title', site_text: { value: 'hello' } }, format: :json - expect(response.status).to eq(200) - - json = ::JSON.parse(response.body) - expect(json).to be_present - - site_text = json['site_text'] - expect(site_text).to be_present - - expect(site_text['id']).to eq('title') - expect(site_text['value']).to eq('hello') - - # Revert - put :revert, params: { id: 'title' }, format: :json - expect(response.status).to eq(200) - - json = ::JSON.parse(response.body) - expect(json).to be_present - - site_text = json['site_text'] - expect(site_text).to be_present - - expect(site_text['id']).to eq('title') - expect(site_text['value']).to eq(orig_title) - end - - it 'returns not found for missing keys' do - put :update, params: { - id: 'made_up_no_key_exists', site_text: { value: 'hello' } - }, format: :json - - expect(response).not_to be_successful - end - - it 'logs the change' do - original_title = I18n.t(:title) - - put :update, params: { - id: 'title', site_text: { value: 'yay' } - }, format: :json - - log = UserHistory.last - - expect(log.previous_value).to eq(original_title) - expect(log.new_value).to eq('yay') - expect(log.action).to eq(UserHistory.actions[:change_site_text]) - - put :revert, params: { id: 'title' }, format: :json - - log = UserHistory.last - - expect(log.previous_value).to eq('yay') - expect(log.new_value).to eq(original_title) - expect(log.action).to eq(UserHistory.actions[:change_site_text]) - end - - it 'returns site texts for the correct locale' do - SiteSetting.default_locale = :ru - - ru_title = 'title ru' - ru_mf_text = 'ru {NUM_RESULTS, plural, one {1 result} other {many} }' - - put :update, params: { id: 'title', site_text: { value: ru_title } }, format: :json - put :update, params: { id: 'js.topic.read_more_MF', site_text: { value: ru_mf_text } }, format: :json - - get :show, params: { id: 'title' }, format: :json - json = ::JSON.parse(response.body) - expect(json['site_text']['value']).to eq(ru_title) - - get :show, params: { id: 'js.topic.read_more_MF' }, format: :json - json = ::JSON.parse(response.body) - expect(json['site_text']['value']).to eq(ru_mf_text) - - SiteSetting.default_locale = :en - - get :show, params: { id: 'title' }, format: :json - json = ::JSON.parse(response.body) - expect(json['site_text']['value']).to_not eq(ru_title) - - get :show, params: { id: 'js.topic.read_more_MF' }, format: :json - json = ::JSON.parse(response.body) - expect(json['site_text']['value']).to_not eq(ru_mf_text) - end - end - end - -end diff --git a/spec/requests/admin/site_texts_controller_spec.rb b/spec/requests/admin/site_texts_controller_spec.rb index f19c6015c8f..0ee4383e23e 100644 --- a/spec/requests/admin/site_texts_controller_spec.rb +++ b/spec/requests/admin/site_texts_controller_spec.rb @@ -9,7 +9,11 @@ RSpec.describe Admin::SiteTextsController do I18n.reload! end - context "#update" do + it "is a subclass of AdminController" do + expect(Admin::SiteTextsController < Admin::AdminController).to eq(true) + end + + context "when not logged in as an admin" do it "raises an error if you aren't logged in" do put '/admin/customize/site_texts/some_key.json', params: { site_text: { value: 'foo' } @@ -21,18 +25,47 @@ RSpec.describe Admin::SiteTextsController do it "raises an error if you aren't an admin" do sign_in(user) - put '/admin/customize/site_texts/some_key', params: { + put "/admin/customize/site_texts/some_key.json", params: { site_text: { value: 'foo' } } expect(response.status).to eq(404) end + end - context "when logged in as admin" do - before do - sign_in(admin) + context "when logged in as amin" do + before do + sign_in(admin) + end + + describe '#index' do + it 'returns json' do + get "/admin/customize/site_texts.json", params: { q: 'title' } + expect(response.status).to eq(200) + expect(::JSON.parse(response.body)).to be_present + end + end + + describe '#show' do + it 'returns a site text for a key that exists' do + get "/admin/customize/site_texts/js.topic.list.json" + expect(response.status).to eq(200) + + json = ::JSON.parse(response.body) + + site_text = json['site_text'] + + expect(site_text['id']).to eq('js.topic.list') + expect(site_text['value']).to eq(I18n.t("js.topic.list")) end + it 'returns not found for missing keys' do + get "/admin/customize/site_texts/made_up_no_key_exists.json" + expect(response.status).to eq(404) + end + end + + describe '#update & #revert' do it "returns 'not found' when an unknown key is used" do put '/admin/customize/site_texts/some_key.json', params: { site_text: { value: 'foo' } @@ -68,6 +101,106 @@ RSpec.describe Admin::SiteTextsController do json = ::JSON.parse(response.body) expect(json['error_type']).to eq('not_found') end + + it "returns the right error message" do + I18n.backend.store_translations(:en, some_key: '%{first} %{second}') + + put "/admin/customize/site_texts/some_key.json", params: { + site_text: { value: 'hello %{key} %{omg}' } + } + + expect(response.status).to eq(422) + + body = JSON.parse(response.body) + + expect(body['message']).to eq(I18n.t( + 'activerecord.errors.models.translation_overrides.attributes.value.invalid_interpolation_keys', + keys: 'key, omg' + )) + end + + it 'logs the change' do + original_title = I18n.t(:title) + + put "/admin/customize/site_texts/title.json", params: { + site_text: { value: 'yay' } + } + expect(response.status).to eq(200) + + log = UserHistory.last + + expect(log.previous_value).to eq(original_title) + expect(log.new_value).to eq('yay') + expect(log.action).to eq(UserHistory.actions[:change_site_text]) + + delete "/admin/customize/site_texts/title.json" + expect(response.status).to eq(200) + + log = UserHistory.last + + expect(log.previous_value).to eq('yay') + expect(log.new_value).to eq(original_title) + expect(log.action).to eq(UserHistory.actions[:change_site_text]) + end + + it 'updates and reverts the key' do + orig_title = I18n.t(:title) + + put "/admin/customize/site_texts/title.json", params: { site_text: { value: 'hello' } } + expect(response.status).to eq(200) + + json = ::JSON.parse(response.body) + + site_text = json['site_text'] + + expect(site_text['id']).to eq('title') + expect(site_text['value']).to eq('hello') + + # Revert + delete "/admin/customize/site_texts/title.json" + expect(response.status).to eq(200) + + json = ::JSON.parse(response.body) + + site_text = json['site_text'] + + expect(site_text['id']).to eq('title') + expect(site_text['value']).to eq(orig_title) + end + + it 'returns site texts for the correct locale' do + SiteSetting.default_locale = :ru + + ru_title = 'title ru' + ru_mf_text = 'ru {NUM_RESULTS, plural, one {1 result} other {many} }' + + put "/admin/customize/site_texts/title.json", params: { site_text: { value: ru_title } } + expect(response.status).to eq(200) + put "/admin/customize/site_texts/js.topic.read_more_MF.json", params: { site_text: { value: ru_mf_text } } + expect(response.status).to eq(200) + + get "/admin/customize/site_texts/title.json" + expect(response.status).to eq(200) + json = ::JSON.parse(response.body) + expect(json['site_text']['value']).to eq(ru_title) + + get "/admin/customize/site_texts/js.topic.read_more_MF.json" + expect(response.status).to eq(200) + json = ::JSON.parse(response.body) + expect(json['site_text']['value']).to eq(ru_mf_text) + + SiteSetting.default_locale = :en + + get "/admin/customize/site_texts/title.json" + expect(response.status).to eq(200) + json = ::JSON.parse(response.body) + expect(json['site_text']['value']).to_not eq(ru_title) + + get "/admin/customize/site_texts/js.topic.read_more_MF.json" + expect(response.status).to eq(200) + json = ::JSON.parse(response.body) + expect(json['site_text']['value']).to_not eq(ru_mf_text) + end end end end