REFACTOR: admin site texts controller specs to requests (#5958)

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'             => 'site_texts#index'
-      get 'site_texts/:id'      => 'site_texts#show',   constraints: { id: /[\w.\-\+]+/i }
+      get 'site_texts/:id.json'    => 'site_texts#show',   constraints: { id: /[\w.\-\+]+/i }
-      put 'site_texts/:id.json' => 'site_texts#update', constraints: { id: /[\w.\-\+]+/i }
+      get 'site_texts/:id'         => 'site_texts#show',   constraints: { id: /[\w.\-\+]+/i }
-      put 'site_texts/:id'      => 'site_texts#update', constraints: { id: /[\w.\-\+]+/i }
+      put 'site_texts/:id.json'    => 'site_texts#update', constraints: { id: /[\w.\-\+]+/i }
-      delete 'site_texts/:id'   => 'site_texts#revert', 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_.]+/ }

spec/controllers/admin/site_texts_controller_spec.rb

@@ -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

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
+  context "when logged in as amin" do
-      before do
+    before do
-        sign_in(admin)
+      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