Refactor UsersController#invited

* Add test coverage
* Simplify controller action
* Move finder code to Invite class

Author: salbertson

app/controllers/users_controller.rb

@@ -66,29 +66,17 @@ def preferences
   end
 
   def invited
-    params.require(:username)
-    params.permit(:filter)
-
-    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')
+    inviter = fetch_user_from_params
 
-    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

app/models/invite.rb

@@ -68,6 +68,30 @@ def self.invite_by_email(email, invited_by, topic=nil)
     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

spec/controllers/users_controller_spec.rb

@@ -813,15 +813,142 @@ def post_user
     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

spec/models/invite_spec.rb

@@ -276,4 +276,50 @@
     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

spec/support/helpers.rb

@@ -43,4 +43,10 @@ def generate_username(length=10)
     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