From ea2ef16ea42d61b34653246a76ab2a5ba767ec15 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Tue, 23 May 2017 22:04:23 +0900 Subject: [PATCH] Cover StreamEntriesController more and remove redundant instructions (#3257) * Cover StreamEntriesController more * Remove redundant instructions in StreamEntriesController --- app/controllers/stream_entries_controller.rb | 10 +-- .../stream_entries_controller_spec.rb | 83 +++++++++++++++++-- 2 files changed, 80 insertions(+), 13 deletions(-) diff --git a/app/controllers/stream_entries_controller.rb b/app/controllers/stream_entries_controller.rb index a9ee73500..baff4317a 100644 --- a/app/controllers/stream_entries_controller.rb +++ b/app/controllers/stream_entries_controller.rb @@ -11,12 +11,8 @@ class StreamEntriesController < ApplicationController def show respond_to do |format| format.html do - return gone if @stream_entry.activity.nil? - - if @stream_entry.activity_type == 'Status' - @ancestors = @stream_entry.activity.reply? ? cache_collection(@stream_entry.activity.ancestors(current_account), Status) : [] - @descendants = cache_collection(@stream_entry.activity.descendants(current_account), Status) - end + @ancestors = @stream_entry.activity.reply? ? cache_collection(@stream_entry.activity.ancestors(current_account), Status) : [] + @descendants = cache_collection(@stream_entry.activity.descendants(current_account), Status) end format.atom do @@ -46,7 +42,7 @@ class StreamEntriesController < ApplicationController @stream_entry = @account.stream_entries.where(activity_type: 'Status').find(params[:id]) @type = @stream_entry.activity_type.downcase - raise ActiveRecord::RecordNotFound if @stream_entry.activity.nil? || (@stream_entry.hidden? && (@stream_entry.activity_type != 'Status' || (@stream_entry.activity_type == 'Status' && !@stream_entry.activity.permitted?(current_account)))) + raise ActiveRecord::RecordNotFound if @stream_entry.activity.nil? || (@stream_entry.hidden? && !@stream_entry.activity.permitted?(current_account)) end def check_account_suspension diff --git a/spec/controllers/stream_entries_controller_spec.rb b/spec/controllers/stream_entries_controller_spec.rb index 71de60604..db7c52037 100644 --- a/spec/controllers/stream_entries_controller_spec.rb +++ b/spec/controllers/stream_entries_controller_spec.rb @@ -3,24 +3,95 @@ require 'rails_helper' RSpec.describe StreamEntriesController, type: :controller do render_views - let(:alice) { Fabricate(:account, username: 'alice') } - let(:status) { Fabricate(:status, account: alice) } + shared_examples 'before_action' do |route| + context 'when account is not suspended anbd stream_entry is available' do + it 'assigns instance variables' do + status = Fabricate(:status) + + get route, params: { account_username: status.account.username, id: status.stream_entry.id } + + expect(assigns(:account)).to eq status.account + expect(assigns(:stream_entry)).to eq status.stream_entry + expect(assigns(:type)).to eq 'status' + end + + it 'sets Link headers' do + alice = Fabricate(:account, username: 'alice') + status = Fabricate(:status, account: alice) + + get route, params: { account_username: alice.username, id: status.stream_entry.id } + + expect(response.headers['Link'].to_s).to eq "; rel=\"alternate\"; type=\"application/atom+xml\"" + end + end + + context 'when account is suspended' do + it 'returns http status 410' do + account = Fabricate(:account, suspended: true) + status = Fabricate(:status, account: account) + + get route, params: { account_username: account.username, id: status.stream_entry.id } + + expect(response).to have_http_status(410) + end + end + + context 'when activity is nil' do + it 'raises ActiveRecord::RecordNotFound' do + account = Fabricate(:account) + stream_entry = Fabricate.build(:stream_entry, account: account, activity: nil, activity_type: 'Status') + stream_entry.save!(validate: false) + + get route, params: { account_username: account.username, id: stream_entry.id } + + expect(response).to have_http_status(404) + end + end + + context 'when it is hidden and it is not permitted' do + it 'raises ActiveRecord::RecordNotFound' do + status = Fabricate(:status) + user = Fabricate(:user) + status.account.block!(user.account) + status.stream_entry.update!(hidden: true) + + sign_in(user) + get route, params: { account_username: status.account.username, id: status.stream_entry.id } + + expect(response).to have_http_status(404) + end + end + end describe 'GET #show' do - it 'returns http success with HTML' do - get :show, params: { account_username: alice.username, id: status.stream_entry.id } + include_examples 'before_action', :show + + it 'renders with HTML' do + ancestor = Fabricate(:status) + status = Fabricate(:status, in_reply_to_id: ancestor.id) + descendant = Fabricate(:status, in_reply_to_id: status.id) + + get :show, params: { account_username: status.account.username, id: status.stream_entry.id } + + expect(assigns(:ancestors)).to match_array([ancestor]) + expect(assigns(:descendants)).to match_array([descendant]) expect(response).to have_http_status(:success) end it 'returns http success with Atom' do - get :show, params: { account_username: alice.username, id: status.stream_entry.id }, format: 'atom' + status = Fabricate(:status) + get :show, params: { account_username: status.account.username, id: status.stream_entry.id }, format: 'atom' expect(response).to have_http_status(:success) end end describe 'GET #embed' do + include_examples 'before_action', :embed + it 'returns embedded view of status' do - get :embed, params: { account_username: alice.username, id: status.stream_entry.id } + status = Fabricate(:status) + + get :embed, params: { account_username: status.account.username, id: status.stream_entry.id } expect(response).to have_http_status(:success) expect(response.headers['X-Frame-Options']).to eq 'ALLOWALL'