Reduce factory creation (36 -> 12) in spec/controllers/oauth/* area (#32045)

spec/controllers/oauth/authorizations_controller_spec.rb

@@ -10,13 +10,6 @@ RSpec.describe Oauth::AuthorizationsController do
       get :new, params: { client_id: app.uid, response_type: 'code', redirect_uri: 'http://localhost/', scope: 'read' }
     end
 
-    shared_examples 'stores location for user' do
-      it 'stores location for user' do
-        subject
-        expect(controller.stored_location_for(:user)).to eq "/oauth/authorize?client_id=#{app.uid}&redirect_uri=http%3A%2F%2Flocalhost%2F&response_type=code&scope=read"
-      end
-    end
-
     context 'when signed in' do
       let!(:user) { Fabricate(:user) }
 
@@ -24,18 +17,17 @@ RSpec.describe Oauth::AuthorizationsController do
         sign_in user, scope: :user
       end
 
-      it 'returns http success' do
+      it 'returns http success and private cache control headers' do
         subject
-        expect(response).to have_http_status(200)
-      end
 
-      it 'returns private cache control headers' do
+        expect(response)
-        subject
+          .to have_http_status(200)
-        expect(response.headers['Cache-Control']).to include('private, no-store')
+        expect(response.headers['Cache-Control'])
+          .to include('private, no-store')
+        expect(controller.stored_location_for(:user))
+          .to eq authorize_path_for(app)
       end
 
-      include_examples 'stores location for user'
-
       context 'when app is already authorized' do
         before do
           Doorkeeper::AccessToken.find_or_create_for(
@@ -52,10 +44,12 @@ RSpec.describe Oauth::AuthorizationsController do
           expect(response).to redirect_to(/\A#{app.redirect_uri}/)
         end
 
-        it 'does not redirect to callback with force_login=true' do
+        context 'with `force_login` param true' do
+          subject do
             get :new, params: { client_id: app.uid, response_type: 'code', redirect_uri: 'http://localhost/', scope: 'read', force_login: 'true' }
+          end
 
-          expect(response).to have_http_status(:success)
+          it { is_expected.to have_http_status(:success) }
         end
       end
     end
@@ -63,10 +57,16 @@ RSpec.describe Oauth::AuthorizationsController do
     context 'when not signed in' do
       it 'redirects' do
         subject
-        expect(response).to redirect_to '/auth/sign_in'
+
+        expect(response)
+          .to redirect_to '/auth/sign_in'
+        expect(controller.stored_location_for(:user))
+          .to eq authorize_path_for(app)
+      end
     end
 
-      include_examples 'stores location for user'
+    def authorize_path_for(app)
+      "/oauth/authorize?client_id=#{app.uid}&redirect_uri=http%3A%2F%2Flocalhost%2F&response_type=code&scope=read"
     end
   end
 end

spec/controllers/oauth/authorized_applications_controller_spec.rb

@@ -10,38 +10,31 @@ RSpec.describe Oauth::AuthorizedApplicationsController do
       get :index
     end
 
-    shared_examples 'stores location for user' do
-      it 'stores location for user' do
-        subject
-        expect(controller.stored_location_for(:user)).to eq '/oauth/authorized_applications'
-      end
-    end
-
     context 'when signed in' do
       before do
         sign_in Fabricate(:user), scope: :user
       end
 
-      it 'returns http success' do
+      it 'returns http success with private cache control headers' do
         subject
-        expect(response).to have_http_status(200)
+        expect(response)
+          .to have_http_status(200)
+        expect(response.headers['Cache-Control'])
+          .to include('private, no-store')
+        expect(controller.stored_location_for(:user))
+          .to eq '/oauth/authorized_applications'
       end
-
-      it 'returns private cache control headers' do
-        subject
-        expect(response.headers['Cache-Control']).to include('private, no-store')
-      end
-
-      include_examples 'stores location for user'
     end
 
     context 'when not signed in' do
       it 'redirects' do
         subject
-        expect(response).to redirect_to '/auth/sign_in'
-      end
 
-      include_examples 'stores location for user'
+        expect(response)
+          .to redirect_to '/auth/sign_in'
+        expect(controller.stored_location_for(:user))
+          .to eq '/oauth/authorized_applications'
+      end
     end
   end
 
@@ -55,23 +48,19 @@ RSpec.describe Oauth::AuthorizedApplicationsController do
     before do
       sign_in user, scope: :user
       allow(redis).to receive(:pipelined).and_yield(redis_pipeline_stub)
+    end
+
+    it 'revokes access tokens for the application and removes subscriptions and sends kill payload to streaming' do
       post :destroy, params: { id: application.id }
-    end
 
-    it 'revokes access tokens for the application' do
+      expect(Doorkeeper::AccessToken.where(application: application).first.revoked_at)
-      expect(Doorkeeper::AccessToken.where(application: application).first.revoked_at).to_not be_nil
+        .to_not be_nil
-    end
+      expect(Web::PushSubscription.where(user: user).count)
-
+        .to eq(0)
-    it 'removes subscriptions for the application\'s access tokens' do
+      expect { web_push_subscription.reload }
-      expect(Web::PushSubscription.where(user: user).count).to eq 0
+        .to raise_error(ActiveRecord::RecordNotFound)
-    end
+      expect(redis_pipeline_stub)
-
+        .to have_received(:publish).with("timeline:access_token:#{access_token.id}", '{"event":"kill"}')
-    it 'removes the web_push_subscription' do
-      expect { web_push_subscription.reload }.to raise_error(ActiveRecord::RecordNotFound)
-    end
-
-    it 'sends a session kill payload to the streaming server' do
-      expect(redis_pipeline_stub).to have_received(:publish).with("timeline:access_token:#{access_token.id}", '{"event":"kill"}')
     end
   end
 end

spec/controllers/oauth/tokens_controller_spec.rb

@@ -9,20 +9,15 @@ RSpec.describe Oauth::TokensController do
     let!(:access_token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, application: application) }
     let!(:web_push_subscription) { Fabricate(:web_push_subscription, user: user, access_token: access_token) }
 
-    before do
+    it 'revokes the token and removes subscriptions' do
       post :revoke, params: { client_id: application.uid, token: access_token.token }
-    end
 
-    it 'revokes the token' do
+      expect(access_token.reload.revoked_at)
-      expect(access_token.reload.revoked_at).to_not be_nil
+        .to_not be_nil
-    end
+      expect(Web::PushSubscription.where(access_token: access_token).count)
-
+        .to eq(0)
-    it 'removes web push subscription for token' do
+      expect { web_push_subscription.reload }
-      expect(Web::PushSubscription.where(access_token: access_token).count).to eq 0
+        .to raise_error(ActiveRecord::RecordNotFound)
-    end
-
-    it 'removes the web_push_subscription' do
-      expect { web_push_subscription.reload }.to raise_error(ActiveRecord::RecordNotFound)
     end
   end
 end