From 77b59b54ce343f6ca63224d0e0e53891e8cfc65a Mon Sep 17 00:00:00 2001 From: Scott Albertson Date: Fri, 8 Nov 2013 11:11:41 -0800 Subject: [PATCH] Refactor UsersController#invited * Add test coverage * Simplify controller action * Move finder code to Invite class --- app/controllers/users_controller.rb | 26 ++-- app/models/invite.rb | 24 ++++ spec/controllers/users_controller_spec.rb | 137 +++++++++++++++++++++- spec/models/invite_spec.rb | 46 ++++++++ spec/support/helpers.rb | 6 + 5 files changed, 215 insertions(+), 24 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index abbed4486..61d852e72 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -66,29 +66,17 @@ class UsersController < ApplicationController end def invited - params.require(:username) - params.permit(:filter) + inviter = fetch_user_from_params - by_user = fetch_user_from_params - - invited = Invite.where(invited_by_id: by_user.id) - .includes(:user => :user_stat) - .order('CASE WHEN invites.user_id IS NOT NULL THEN 0 ELSE 1 END', - 'user_stats.time_read DESC', - 'invites.redeemed_at DESC') - .limit(SiteSetting.invites_shown) - .references('user_stats') - - unless guardian.can_see_pending_invites_from?(by_user) - invited = invited.where('invites.user_id IS NOT NULL') + invites = if guardian.can_see_pending_invites_from?(inviter) + Invite.find_all_invites_from(inviter) + else + Invite.find_redeemed_invites_from(inviter) end - if params[:filter].present? - invited = invited.where('(LOWER(invites.email) LIKE :filter) or (LOWER(users.username) LIKE :filter)', filter: "%#{params[:filter].downcase}%") - .references(:users) - end + invites = invites.filter_by(params[:filter]) - render_serialized(invited.to_a, InviteSerializer) + render_serialized(invites.to_a, InviteSerializer) end def is_local_username diff --git a/app/models/invite.rb b/app/models/invite.rb index 72deddc9d..3cf736144 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -68,6 +68,30 @@ class Invite < ActiveRecord::Base invite end + def self.find_all_invites_from(inviter) + Invite.where(invited_by_id: inviter.id) + .includes(:user => :user_stat) + .order('CASE WHEN invites.user_id IS NOT NULL THEN 0 ELSE 1 END', + 'user_stats.time_read DESC', + 'invites.redeemed_at DESC') + .limit(SiteSetting.invites_shown) + .references('user_stats') + end + + def self.find_redeemed_invites_from(inviter) + find_all_invites_from(inviter).where('invites.user_id IS NOT NULL') + end + + def self.filter_by(email_or_username) + if email_or_username + where( + '(LOWER(invites.email) LIKE :filter) or (LOWER(users.username) LIKE :filter)', + filter: "%#{email_or_username.downcase}%" + ) + else + scoped + end + end end # == Schema Information diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index be80e6bd6..41232e25c 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -813,15 +813,142 @@ describe UsersController do end end - describe '.invited' do - - let(:user) { Fabricate(:user) } - + describe '#invited' do it 'returns success' do + user = Fabricate(:user) + xhr :get, :invited, username: user.username - response.should be_success + + expect(response).to be_success end + it 'filters by email' do + inviter = Fabricate(:user) + invitee = Fabricate(:user) + invite = Fabricate( + :invite, + email: 'billybob@example.com', + invited_by: inviter, + user: invitee + ) + Fabricate( + :invite, + email: 'jimtom@example.com', + invited_by: inviter, + user: invitee + ) + + xhr :get, :invited, username: inviter.username, filter: 'billybob' + + invites = JSON.parse(response.body) + expect(invites).to have(1).item + expect(invites.first).to include('email' => 'billybob@example.com') + end + + it 'filters by username' do + inviter = Fabricate(:user) + invitee = Fabricate(:user, username: 'billybob') + invite = Fabricate( + :invite, + invited_by: inviter, + email: 'billybob@example.com', + user: invitee + ) + Fabricate( + :invite, + invited_by: inviter, + user: Fabricate(:user, username: 'jimtom') + ) + + xhr :get, :invited, username: inviter.username, filter: 'billybob' + + invites = JSON.parse(response.body) + expect(invites).to have(1).item + expect(invites.first).to include('email' => 'billybob@example.com') + end + + context 'with guest' do + context 'with pending invites' do + it 'does not return invites' do + inviter = Fabricate(:user) + Fabricate(:invite, invited_by: inviter) + + xhr :get, :invited, username: inviter.username + + invites = JSON.parse(response.body) + expect(invites).to be_empty + end + end + + context 'with redeemed invites' do + it 'returns invites' do + inviter = Fabricate(:user) + invitee = Fabricate(:user) + invite = Fabricate(:invite, invited_by: inviter, user: invitee) + + xhr :get, :invited, username: inviter.username + + invites = JSON.parse(response.body) + expect(invites).to have(1).item + expect(invites.first).to include('email' => invite.email) + end + end + end + + context 'with authenticated user' do + context 'with pending invites' do + context 'with permission to see pending invites' do + it 'returns invites' do + user = log_in + inviter = Fabricate(:user) + invite = Fabricate(:invite, invited_by: inviter) + stub_guardian(user) do |guardian| + guardian.stubs(:can_see_pending_invites_from?). + with(inviter).returns(true) + end + + xhr :get, :invited, username: inviter.username + + invites = JSON.parse(response.body) + expect(invites).to have(1).item + expect(invites.first).to include("email" => invite.email) + end + end + + context 'without permission to see pending invites' do + it 'does not return invites' do + user = log_in + inviter = Fabricate(:user) + invitee = Fabricate(:user) + Fabricate(:invite, invited_by: inviter) + stub_guardian(user) do |guardian| + guardian.stubs(:can_see_pending_invites_from?). + with(inviter).returns(false) + end + + xhr :get, :invited, username: inviter.username + + invites = JSON.parse(response.body) + expect(invites).to be_empty + end + end + end + + context 'with redeemed invites' do + it 'returns invites' do + user = log_in + inviter = Fabricate(:user) + invitee = Fabricate(:user) + invite = Fabricate(:invite, invited_by: inviter, user: invitee) + + xhr :get, :invited, username: inviter.username + + invites = JSON.parse(response.body) + expect(invites).to have(1).item + expect(invites.first).to include('email' => invite.email) + end + end + end end describe '#update' do diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index 8b84a8f4a..3df25bc76 100644 --- a/spec/models/invite_spec.rb +++ b/spec/models/invite_spec.rb @@ -276,4 +276,50 @@ describe Invite do end end + describe '.find_all_invites_from' do + context 'with user that has invited' do + it 'returns invites' do + inviter = Fabricate(:user) + invite = Fabricate(:invite, invited_by: inviter) + + invites = Invite.find_all_invites_from(inviter) + + expect(invites).to include invite + end + end + + context 'with user that has not invited' do + it 'does not return invites' do + user = Fabricate(:user) + invite = Fabricate(:invite) + + invites = Invite.find_all_invites_from(user) + + expect(invites).to be_empty + end + end + end + + describe '.find_redeemed_invites_from' do + it 'returns redeemed invites only' do + inviter = Fabricate(:user) + pending_invite = Fabricate( + :invite, + invited_by: inviter, + user_id: nil, + email: 'pending@example.com' + ) + redeemed_invite = Fabricate( + :invite, + invited_by: inviter, + user_id: 123, + email: 'redeemed@example.com' + ) + + invites = Invite.find_redeemed_invites_from(inviter) + + expect(invites).to have(1).items + expect(invites.first).to eq redeemed_invite + end + end end diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb index 414a719f4..a0ddcf8ea 100644 --- a/spec/support/helpers.rb +++ b/spec/support/helpers.rb @@ -43,4 +43,10 @@ module Helpers range = [*'a'..'z'] Array.new(length){range.sample}.join end + + def stub_guardian(user) + guardian = Guardian.new(user) + yield(guardian) if block_given? + Guardian.stubs(new: guardian).with(user) + end end