Merge pull request #3952 from evanrolfe/refactor/event_find_subscript…

…ions_tests

[ci] Improve EventFindSubscriptions model spec.

src/api/spec/factories/event_subscriptions.rb

@@ -4,14 +4,20 @@
       eventtype 'Event::CommentForProject'
       receiver_role "commenter"
       channel :instant_email
+      user
+    end
+
+    factory :event_subscription_comment_for_project_without_subscriber do
+      eventtype 'Event::CommentForProject'
+      receiver_role "commenter"
+      channel :instant_email
     end
 
     factory :event_subscription_request_created do
       eventtype 'Event::RequestCreate'
       receiver_role "target_maintainer"
       channel :instant_email
+      user
     end
-
-    user
   end
 end

src/api/spec/models/event_find_subscriptions_spec.rb

@@ -1,37 +1,143 @@
 require 'rails_helper'
 
+RSpec.shared_context 'it returns subscriptions for an event' do
+  context 'with a maintainer user/group who has a maintainer subscription' do
+    let!(:project) { create(:project, maintainer: [maintainer]) }
+    let!(:comment) { create(:comment_project, commentable: project) }
+
+    context 'which is enabled' do
+      let!(:subscription) { create(:event_subscription_comment_for_project_without_subscriber, receiver_role: 'maintainer', subscriber: maintainer) }
+
+      it 'returns the subscription for that user/group' do
+        subscriber_result = subject.find { |subscription| subscription.subscriber == maintainer }
+
+        expect(subscriber_result).to eq(subscription)
+      end
+    end
+
+    context 'which is disabled' do
+      let!(:subscription) do
+        create(:event_subscription_comment_for_project_without_subscriber, receiver_role: 'maintainer', subscriber: maintainer, channel: 'disabled')
+      end
+
+      it 'does not include that user/group' do
+        expect(subject.map(&:subscriber)).not_to include(maintainer)
+      end
+    end
+  end
+
+  context 'with a maintainer user/group who has no subscriptions' do
+    let!(:project) { create(:project, maintainer: [maintainer]) }
+    let!(:comment) { create(:comment_project, commentable: project) }
+
+    context 'and a default maintainer subscription is enabled' do
+      let!(:default_subscription) do
+        create(:event_subscription_comment_for_project, receiver_role: 'maintainer', user: nil, group: nil)
+      end
+
+      it 'returns a new subscription for that user/group based on the default subscription' do
+        result = subject.find { |subscription| subscription.subscriber == maintainer }
+
+        expect(result.id).to be_nil
+        expect(result.eventtype).to eq(default_subscription.eventtype)
+        expect(result.receiver_role).to eq(default_subscription.receiver_role)
+        expect(result.channel).to eq(default_subscription.channel)
+      end
+    end
+
+    context 'and a default maintainer subscription is disabled' do
+      let!(:default_subscription) do
+        create(:event_subscription_comment_for_project, receiver_role: 'maintainer', user: nil, group: nil, channel: 'disabled')
+      end
+
+      it 'does not include that user/group' do
+        expect(subject.map(&:subscriber)).not_to include(maintainer)
+      end
+    end
+  end
+end
+
 RSpec.describe EventFindSubscriptions do
   describe '#subscribers' do
-    let!(:comment_author) { create(:confirmed_user) }
-    let!(:user1) { create(:confirmed_user) }
-    let!(:user2) { create(:confirmed_user) }
-    let!(:group1) { create(:group) }
-    let!(:group2) { create(:group) }
-    let!(:group3) { create(:group, email: '') }
-    let!(:project) { create(:project, name: 'comment_project', maintainer: [user1, user2, group1, group2, group3]) }
-
-    let!(:subscription1) { create(:event_subscription_comment_for_project, receiver_role: 'commenter', user: comment_author) }
-    let!(:subscription2) { create(:event_subscription_comment_for_project, receiver_role: 'maintainer', user: user1) }
-    let!(:subscription3) { create(:event_subscription_comment_for_project, receiver_role: 'maintainer', user: user2) }
-    let!(:subscription4) { create(:event_subscription_comment_for_project, receiver_role: 'maintainer', user: nil, group: group1) }
-    let!(:subscription5) { create(:event_subscription_comment_for_project, receiver_role: 'maintainer', user: nil, group: group2) }
-    let!(:subscription6) { create(:event_subscription_comment_for_project, receiver_role: 'maintainer', user: nil, group: group3) }
-
-    let!(:comment) { create(:comment_project, commentable: project, body: "Hey @#{user1.login} how are things?", user: comment_author) }
-    let(:event) { Event::CommentForProject.first }
-
-    subject! { EventFindSubscriptions.new(event).subscriptions }
-
-    it 'includes the users and groups subscribed to Event::CommentForProject' do
-      expect(subject).to include(subscription2, subscription3, subscription4, subscription5)
+    subject do
+      event = Event::CommentForProject.first
+      EventFindSubscriptions.new(event).subscriptions
     end
 
-    it 'does not include the author of the comment' do
-      expect(subject).not_to include(subscription1)
-    end
+    context 'with a comment for a project' do
+      context 'with no maintainers' do
+        let!(:project) { create(:project) }
+        let!(:comment) { create(:comment_project, commentable: project) }
+
+        it 'does not include the author of the comment' do
+          expect(subject.map(&:subscriber)).not_to include(comment.user)
+        end
+      end
+
+      it_behaves_like 'it returns subscriptions for an event' do
+        let!(:maintainer) { create(:confirmed_user) }
+      end
+
+      it_behaves_like 'it returns subscriptions for an event' do
+        let!(:maintainer) { create(:group) }
+      end
+
+      context 'with a maintainer group who has no email set and has a user as a member' do
+        let!(:group) { create(:group, email: nil) }
+        let!(:user) { create(:confirmed_user) }
+
+        let!(:project) { create(:project, maintainer: [group]) }
+        let!(:comment) { create(:comment_project, commentable: project) }
+
+        before do
+          group.users << user
+        end
+
+        context 'and the user has a maintainer subscription' do
+          context 'which is enabled' do
+            let!(:subscription) { create(:event_subscription_comment_for_project_without_subscriber, receiver_role: 'maintainer', subscriber: user) }
+
+            it 'returns the subscription for that user/group' do
+              subscriber_result = subject.find { |subscription| subscription.subscriber == user }
+
+              expect(subscriber_result).to eq(subscription)
+            end
+          end
+
+          context 'which is disabled' do
+            let!(:subscription) do
+              create(:event_subscription_comment_for_project_without_subscriber, receiver_role: 'maintainer', subscriber: user, channel: 'disabled')
+            end
+
+            it 'does not include that user/group' do
+              expect(subject.map(&:subscriber)).not_to include(user)
+            end
+          end
+        end
+
+        context 'and the user has no subscriptions' do
+          context 'and a default maintainer subscription is enabled' do
+            let!(:default_subscription) do
+              create(:event_subscription_comment_for_project, receiver_role: 'maintainer', user: nil, group: nil)
+            end
+
+            # TODO: This does not seem like the correct logic for this class.
+            it 'does not include that user/group' do
+              expect(subject.map(&:subscriber)).not_to include(user)
+            end
+          end
+
+          context 'and a default maintainer subscription is disabled' do
+            let!(:default_subscription) do
+              create(:event_subscription_comment_for_project, receiver_role: 'maintainer', user: nil, group: nil, channel: 'disabled')
+            end
 
-    it 'does not include the group with no email set' do
-      expect(subject).not_to include(subscription6)
+            it 'does not include that user/group' do
+              expect(subject.map(&:subscriber)).not_to include(user)
+            end
+          end
+        end
+      end
     end
   end
 end