From d3e610eed9e0b0b919dd639b92a5d8e14c686dcc Mon Sep 17 00:00:00 2001 From: OsamaSayegh Date: Tue, 5 Jun 2018 07:03:49 +0300 Subject: [PATCH] REFACTOR: topic controller (2) specs to requests (#5911) --- spec/controllers/topic_controller_spec.rb | 368 ---------------------- spec/requests/posts_controller_spec.rb | 69 +++- spec/requests/topics_controller_spec.rb | 247 ++++++++++++++- 3 files changed, 312 insertions(+), 372 deletions(-) delete mode 100644 spec/controllers/topic_controller_spec.rb diff --git a/spec/controllers/topic_controller_spec.rb b/spec/controllers/topic_controller_spec.rb deleted file mode 100644 index 16ecfc9e33f..00000000000 --- a/spec/controllers/topic_controller_spec.rb +++ /dev/null @@ -1,368 +0,0 @@ -require 'rails_helper' - -describe TopicsController do - before do - TopicUser.stubs(:track_visit!) - end - - let :topic do - Fabricate(:post).topic - end - - def set_referer(ref) - request.env['HTTP_REFERER'] = ref - end - - def set_accept_language(locale) - request.env['HTTP_ACCEPT_LANGUAGE'] = locale - end - - describe "themes" do - let :theme do - Theme.create!(user_id: -1, name: 'bob', user_selectable: true) - end - - let :theme2 do - Theme.create!(user_id: -1, name: 'bobbob', user_selectable: true) - end - - it "selects the theme the user has selected" do - user = log_in - user.user_option.update_columns(theme_key: theme.key) - - get :show, params: { id: 666 } - expect(controller.theme_key).to eq(theme.key) - - theme.update_attribute(:user_selectable, false) - - get :show, params: { id: 666 } - expect(controller.theme_key).not_to eq(theme.key) - end - - it "can be overridden with a cookie" do - user = log_in - user.user_option.update_columns(theme_key: theme.key) - - cookies['theme_key'] = "#{theme2.key},#{user.user_option.theme_key_seq}" - - get :show, params: { id: 666 } - expect(controller.theme_key).to eq(theme2.key) - - end - - it "cookie can fail back to user if out of sync" do - user = log_in - user.user_option.update_columns(theme_key: theme.key) - cookies['theme_key'] = "#{theme2.key},#{user.user_option.theme_key_seq - 1}" - - get :show, params: { id: 666 } - expect(controller.theme_key).to eq(theme.key) - end - end - - it "doesn't store an incoming link when there's no referer" do - expect { - get :show, params: { id: topic.id }, format: :json - }.not_to change(IncomingLink, :count) - end - - it "doesn't raise an error on a very long link" do - set_referer("http://#{'a' * 2000}.com") - - expect do - get :show, params: { id: topic.id }, format: :json - end.not_to raise_error - end - - describe "has_escaped_fragment?" do - render_views - - context "when the SiteSetting is disabled" do - - it "uses the application layout even with an escaped fragment param" do - SiteSetting.enable_escaped_fragments = false - - get :show, params: { - 'topic_id' => topic.id, - 'slug' => topic.slug, - '_escaped_fragment_' => 'true' - } - - body = response.body - - expect(body).to have_tag(:script, with: { src: '/assets/application.js' }) - expect(body).to_not have_tag(:meta, with: { name: 'fragment' }) - end - - end - - context "when the SiteSetting is enabled" do - before do - SiteSetting.enable_escaped_fragments = true - end - - it "uses the application layout when there's no param" do - get :show, params: { topic_id: topic.id, slug: topic.slug } - - body = response.body - - expect(body).to have_tag(:script, with: { src: '/assets/application.js' }) - expect(body).to have_tag(:meta, with: { name: 'fragment' }) - end - - it "uses the crawler layout when there's an _escaped_fragment_ param" do - get :show, params: { - topic_id: topic.id, - slug: topic.slug, - _escaped_fragment_: 'true' - } - - body = response.body - - expect(body).to have_tag(:body, with: { class: 'crawler' }) - expect(body).to_not have_tag(:meta, with: { name: 'fragment' }) - end - end - end - - describe "print" do - render_views - - context "when the SiteSetting is enabled" do - it "uses the application layout when there's no param" do - get :show, params: { topic_id: topic.id, slug: topic.slug } - - body = response.body - - expect(body).to have_tag(:script, src: '/assets/application.js') - expect(body).to have_tag(:meta, with: { name: 'fragment' }) - end - - it "uses the crawler layout when there's an print param" do - get :show, params: { topic_id: topic.id, slug: topic.slug, print: 'true' } - - body = response.body - - expect(body).to have_tag(:body, class: 'crawler') - expect(body).to_not have_tag(:meta, with: { name: 'fragment' }) - end - end - end - - describe 'clear_notifications' do - it 'correctly clears notifications if specified via cookie' do - notification = Fabricate(:notification) - log_in_user(notification.user) - - request.cookies['cn'] = "2828,100,#{notification.id}" - - get :show, params: { topic_id: 100, format: :json } - - expect(response.cookies['cn']).to eq nil - - notification.reload - expect(notification.read).to eq true - - end - - it 'correctly clears notifications if specified via header' do - notification = Fabricate(:notification) - log_in_user(notification.user) - - request.headers['Discourse-Clear-Notifications'] = "2828,100,#{notification.id}" - - get :show, params: { topic_id: 100, format: :json } - - notification.reload - expect(notification.read).to eq true - end - end - - describe "set_locale" do - context "allow_user_locale disabled" do - context "accept-language header differs from default locale" do - before do - SiteSetting.allow_user_locale = false - SiteSetting.default_locale = "en" - set_accept_language("fr") - end - - context "with an anonymous user" do - it "uses the default locale" do - get :show, params: { topic_id: topic.id, format: :json } - - expect(I18n.locale).to eq(:en) - end - end - - context "with a logged in user" do - it "it uses the default locale" do - user = Fabricate(:user, locale: :fr) - log_in_user(user) - - get :show, params: { topic_id: topic.id, format: :json } - - expect(I18n.locale).to eq(:en) - end - end - end - end - - context "set_locale_from_accept_language_header enabled" do - context "accept-language header differs from default locale" do - before do - SiteSetting.allow_user_locale = true - SiteSetting.set_locale_from_accept_language_header = true - SiteSetting.default_locale = "en" - set_accept_language("fr") - end - - context "with an anonymous user" do - it "uses the locale from the headers" do - get :show, params: { topic_id: topic.id, format: :json } - - expect(I18n.locale).to eq(:fr) - end - end - - context "with a logged in user" do - it "uses the user's preferred locale" do - user = Fabricate(:user, locale: :fr) - log_in_user(user) - - get :show, params: { topic_id: topic.id, format: :json } - - expect(I18n.locale).to eq(:fr) - end - end - end - - context "the preferred locale includes a region" do - it "returns the locale and region separated by an underscore" do - SiteSetting.allow_user_locale = true - SiteSetting.set_locale_from_accept_language_header = true - SiteSetting.default_locale = "en" - set_accept_language("zh-CN") - - get :show, params: { topic_id: topic.id, format: :json } - - expect(I18n.locale).to eq(:zh_CN) - end - end - - context 'accept-language header is not set' do - it 'uses the site default locale' do - SiteSetting.allow_user_locale = true - SiteSetting.default_locale = 'en' - set_accept_language('') - - get :show, params: { topic_id: topic.id, format: :json } - - expect(I18n.locale).to eq(:en) - end - end - end - end - - describe "read only header" do - it "returns no read only header by default" do - get :show, params: { topic_id: topic.id, format: :json } - expect(response.headers['Discourse-Readonly']).to eq(nil) - end - - it "returns a readonly header if the site is read only" do - Discourse.received_readonly! - get :show, params: { topic_id: topic.id, format: :json } - expect(response.headers['Discourse-Readonly']).to eq('true') - end - end -end - -describe 'api' do - - before do - ActionController::Base.allow_forgery_protection = true - end - - after do - ActionController::Base.allow_forgery_protection = false - end - - describe PostsController do - let(:user) do - Fabricate(:user) - end - - let(:post) do - Fabricate(:post) - end - - let(:api_key) { user.generate_api_key(user) } - let(:master_key) { ApiKey.create_master_key } - - # choosing an arbitrarily easy to mock trusted activity - it 'allows users with api key to bookmark posts' do - PostAction.expects(:act).with(user, post, PostActionType.types[:bookmark]).once - - put :bookmark, params: { - bookmarked: "true", - post_id: post.id, - api_key: api_key.key - }, format: :json - - expect(response).to be_success - end - - it 'raises an error with a user key that does not match an optionally specified username' do - PostAction.expects(:act).with(user, post, PostActionType.types[:bookmark]).never - - put :bookmark, params: { - bookmarked: "true", - post_id: post.id, - api_key: api_key.key, - api_username: 'made_up' - }, format: :json - - expect(response).not_to be_success - end - - it 'allows users with a master api key to bookmark posts' do - PostAction.expects(:act).with(user, post, PostActionType.types[:bookmark]).once - - put :bookmark, params: { - bookmarked: "true", - post_id: post.id, - api_key: master_key.key, - api_username: user.username - }, format: :json - - expect(response).to be_success - end - - it 'disallows phonies to bookmark posts' do - PostAction.expects(:act).with(user, post, PostActionType.types[:bookmark]).never - - put :bookmark, params: { - bookmarked: "true", - post_id: post.id, - api_key: SecureRandom.hex(32), - api_username: user.username - }, format: :json - - expect(response.code.to_i).to eq(403) - end - - it 'disallows blank api' do - PostAction.expects(:act).with(user, post, PostActionType.types[:bookmark]).never - - put :bookmark, params: { - bookmarked: "true", - post_id: post.id, - api_key: "", - api_username: user.username - }, format: :json - - expect(response.code.to_i).to eq(403) - end - end -end diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index f43a5b511f5..cd225dabf37 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/spec/requests/posts_controller_spec.rb @@ -356,14 +356,14 @@ describe PostsController do describe '#bookmark' do include_examples 'action requires login', :put, "/posts/2/bookmark.json" + let(:post) { Fabricate(:post, user: user) } + let(:user) { Fabricate(:user) } describe 'when logged in' do before do sign_in(user) end - let(:user) { Fabricate(:user) } - let(:post) { Fabricate(:post, user: user) } let(:private_message) { Fabricate(:private_message_post) } it "raises an error if the user doesn't have permission to see the post" do @@ -423,6 +423,71 @@ describe PostsController do end end end + + context "api" do + let(:api_key) { user.generate_api_key(user) } + let(:master_key) { ApiKey.create_master_key } + + # choosing an arbitrarily easy to mock trusted activity + it 'allows users with api key to bookmark posts' do + put "/posts/#{post.id}/bookmark.json", params: { + bookmarked: "true", + api_key: api_key.key + } + + expect(response).to be_success + expect(PostAction.where( + post: post, + user: user, + post_action_type_id: PostActionType.types[:bookmark] + ).count).to eq(1) + end + + it 'raises an error with a user key that does not match an optionally specified username' do + put "/posts/#{post.id}/bookmark.json", params: { + bookmarked: "true", + api_key: api_key.key, + api_username: 'made_up' + } + + expect(response.status).to eq(403) + end + + it 'allows users with a master api key to bookmark posts' do + put "/posts/#{post.id}/bookmark.json", params: { + bookmarked: "true", + api_key: master_key.key, + api_username: user.username + } + + expect(response).to be_success + expect(PostAction.where( + post: post, + user: user, + post_action_type_id: PostActionType.types[:bookmark] + ).count).to eq(1) + end + + it 'disallows phonies to bookmark posts' do + put "/posts/#{post.id}/bookmark.json", params: { + bookmarked: "true", + api_key: SecureRandom.hex(32), + api_username: user.username + } + + expect(response.status).to eq(403) + end + + it 'disallows blank api' do + put "/posts/#{post.id}/bookmark.json", params: { + bookmarked: "true", + api_key: "", + api_username: user.username + } + + expect(response.status).to eq(403) + end + end end describe '#wiki' do diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 597ced4be5c..9c471ee1ee1 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -1042,10 +1042,23 @@ RSpec.describe TopicsController do it 'renders the print view when enabled' do SiteSetting.max_prints_per_hour_per_user = 10 - - get "/t/#{topic.slug}/#{topic.id}/print" + get "/t/#{topic.slug}/#{topic.id}/print", headers: { HTTP_USER_AGENT: "Rails Testing" } expect(response).to be_successful + body = response.body + + expect(body).to have_tag(:body, class: 'crawler') + expect(body).to_not have_tag(:meta, with: { name: 'fragment' }) + end + + it "uses the application layout when there's no param" do + SiteSetting.max_prints_per_hour_per_user = 10 + get "/t/#{topic.slug}/#{topic.id}", headers: { HTTP_USER_AGENT: "Rails Testing" } + + body = response.body + + expect(body).to have_tag(:script, src: '/assets/application.js') + expect(body).to have_tag(:meta, with: { name: 'fragment' }) end end @@ -1168,6 +1181,236 @@ RSpec.describe TopicsController do expect(response.headers['X-Robots-Tag']).to eq(nil) end + + describe "themes" do + let(:theme) { Theme.create!(user_id: -1, name: 'bob', user_selectable: true) } + let(:theme2) { Theme.create!(user_id: -1, name: 'bobbob', user_selectable: true) } + + before do + sign_in(user) + end + + it "selects the theme the user has selected" do + user.user_option.update_columns(theme_key: theme.key) + + get "/t/#{topic.id}" + expect(response).to be_redirect + expect(controller.theme_key).to eq(theme.key) + + theme.update_attribute(:user_selectable, false) + + get "/t/#{topic.id}" + expect(response).to be_redirect + expect(controller.theme_key).not_to eq(theme.key) + end + + it "can be overridden with a cookie" do + user.user_option.update_columns(theme_key: theme.key) + + cookies['theme_key'] = "#{theme2.key},#{user.user_option.theme_key_seq}" + + get "/t/#{topic.id}" + expect(response).to be_redirect + expect(controller.theme_key).to eq(theme2.key) + end + + it "cookie can fail back to user if out of sync" do + user.user_option.update_columns(theme_key: theme.key) + cookies['theme_key'] = "#{theme2.key},#{user.user_option.theme_key_seq - 1}" + + get "/t/#{topic.id}" + expect(response).to be_redirect + expect(controller.theme_key).to eq(theme.key) + end + end + + it "doesn't store an incoming link when there's no referer" do + expect { + get "/t/#{topic.id}.json" + }.not_to change(IncomingLink, :count) + expect(response).to be_success + end + + it "doesn't raise an error on a very long link" do + get "/t/#{topic.id}.json", headers: { HTTP_REFERER: "http://#{'a' * 2000}.com" } + expect(response.status).to eq(200) + end + + describe "has_escaped_fragment?" do + context "when the SiteSetting is disabled" do + it "uses the application layout even with an escaped fragment param" do + SiteSetting.enable_escaped_fragments = false + + get "/t/#{topic.slug}/#{topic.id}", params: { + _escaped_fragment_: 'true' + } + + body = response.body + + expect(response).to be_success + expect(body).to have_tag(:script, with: { src: '/assets/application.js' }) + expect(body).to_not have_tag(:meta, with: { name: 'fragment' }) + end + end + + context "when the SiteSetting is enabled" do + before do + SiteSetting.enable_escaped_fragments = true + end + + it "uses the application layout when there's no param" do + get "/t/#{topic.slug}/#{topic.id}" + + body = response.body + + expect(body).to have_tag(:script, with: { src: '/assets/application.js' }) + expect(body).to have_tag(:meta, with: { name: 'fragment' }) + end + + it "uses the crawler layout when there's an _escaped_fragment_ param" do + get "/t/#{topic.slug}/#{topic.id}", params: { + _escaped_fragment_: true + }, headers: { HTTP_USER_AGENT: "Rails Testing" } + + body = response.body + + expect(response).to be_success + expect(body).to have_tag(:body, with: { class: 'crawler' }) + expect(body).to_not have_tag(:meta, with: { name: 'fragment' }) + end + end + end + + describe 'clear_notifications' do + it 'correctly clears notifications if specified via cookie' do + notification = Fabricate(:notification) + sign_in(notification.user) + + cookies['cn'] = "2828,100,#{notification.id}" + + get "/t/#{topic.id}.json" + + expect(response).to be_success + expect(response.cookies['cn']).to eq(nil) + + notification.reload + expect(notification.read).to eq(true) + end + + it 'correctly clears notifications if specified via header' do + notification = Fabricate(:notification) + sign_in(notification.user) + + get "/t/#{topic.id}.json", headers: { "Discourse-Clear-Notifications" => "2828,100,#{notification.id}" } + + expect(response).to be_success + notification.reload + expect(notification.read).to eq(true) + end + end + + describe "set_locale" do + def headers(locale) + { HTTP_ACCEPT_LANGUAGE: locale } + end + + context "allow_user_locale disabled" do + context "accept-language header differs from default locale" do + before do + SiteSetting.allow_user_locale = false + SiteSetting.default_locale = "en" + end + + context "with an anonymous user" do + it "uses the default locale" do + get "/t/#{topic.id}.json", headers: headers("fr") + + expect(response).to be_success + expect(I18n.locale).to eq(:en) + end + end + + context "with a logged in user" do + it "it uses the default locale" do + user = Fabricate(:user, locale: :fr) + sign_in(user) + + get "/t/#{topic.id}.json", headers: headers("fr") + + expect(response).to be_success + expect(I18n.locale).to eq(:en) + end + end + end + end + + context "set_locale_from_accept_language_header enabled" do + context "accept-language header differs from default locale" do + before do + SiteSetting.allow_user_locale = true + SiteSetting.set_locale_from_accept_language_header = true + SiteSetting.default_locale = "en" + end + + context "with an anonymous user" do + it "uses the locale from the headers" do + get "/t/#{topic.id}.json", headers: headers("fr") + expect(response).to be_success + expect(I18n.locale).to eq(:fr) + end + end + + context "with a logged in user" do + it "uses the user's preferred locale" do + user = Fabricate(:user, locale: :fr) + sign_in(user) + + get "/t/#{topic.id}.json", headers: headers("fr") + expect(response).to be_success + expect(I18n.locale).to eq(:fr) + end + end + end + + context "the preferred locale includes a region" do + it "returns the locale and region separated by an underscore" do + SiteSetting.allow_user_locale = true + SiteSetting.set_locale_from_accept_language_header = true + SiteSetting.default_locale = "en" + + get "/t/#{topic.id}.json", headers: headers("zh-CN") + expect(response).to be_success + expect(I18n.locale).to eq(:zh_CN) + end + end + + context 'accept-language header is not set' do + it 'uses the site default locale' do + SiteSetting.allow_user_locale = true + SiteSetting.default_locale = 'en' + + get "/t/#{topic.id}.json", headers: headers("") + expect(response).to be_success + expect(I18n.locale).to eq(:en) + end + end + end + end + + describe "read only header" do + it "returns no read only header by default" do + get "/t/#{topic.id}.json" + expect(response).to be_success + expect(response.headers['Discourse-Readonly']).to eq(nil) + end + + it "returns a readonly header if the site is read only" do + Discourse.received_readonly! + get "/t/#{topic.id}.json" + expect(response).to be_success + expect(response.headers['Discourse-Readonly']).to eq('true') + end + end end describe '#posts' do