Allow subscriptions to be created without a project

parent 346f5824
...@@ -12,7 +12,7 @@ module Subscribable ...@@ -12,7 +12,7 @@ module Subscribable
has_many :subscriptions, dependent: :destroy, as: :subscribable has_many :subscriptions, dependent: :destroy, as: :subscribable
end end
def subscribed?(user, project) def subscribed?(user, project = nil)
if subscription = subscriptions.find_by(user: user, project: project) if subscription = subscriptions.find_by(user: user, project: project)
subscription.subscribed subscription.subscribed
else else
...@@ -27,20 +27,22 @@ module Subscribable ...@@ -27,20 +27,22 @@ module Subscribable
end end
def subscribers(project) def subscribers(project)
subscriptions.where(project: project, subscribed: true).map(&:user) subscriptions_available(project).
where(subscribed: true).
map(&:user)
end end
def toggle_subscription(user, project) def toggle_subscription(user, project = nil)
find_or_initialize_subscription(user, project). find_or_initialize_subscription(user, project).
update(subscribed: !subscribed?(user, project)) update(subscribed: !subscribed?(user, project))
end end
def subscribe(user, project) def subscribe(user, project = nil)
find_or_initialize_subscription(user, project). find_or_initialize_subscription(user, project).
update(subscribed: true) update(subscribed: true)
end end
def unsubscribe(user, project) def unsubscribe(user, project = nil)
find_or_initialize_subscription(user, project). find_or_initialize_subscription(user, project).
update(subscribed: false) update(subscribed: false)
end end
...@@ -49,6 +51,13 @@ module Subscribable ...@@ -49,6 +51,13 @@ module Subscribable
def find_or_initialize_subscription(user, project) def find_or_initialize_subscription(user, project)
subscriptions. subscriptions.
find_or_initialize_by(user_id: user.id, project_id: project.id) find_or_initialize_by(user_id: user.id, project_id: project.try(:id))
end
def subscriptions_available(project)
t = Subscription.arel_table
subscriptions.
where(t[:project_id].eq(nil).or(t[:project_id].eq(project.try(:id))))
end end
end end
...@@ -3,9 +3,7 @@ class Subscription < ActiveRecord::Base ...@@ -3,9 +3,7 @@ class Subscription < ActiveRecord::Base
belongs_to :project belongs_to :project
belongs_to :subscribable, polymorphic: true belongs_to :subscribable, polymorphic: true
validates :user, :project, :subscribable, presence: true validates :user, :subscribable, presence: true
validates :project_id, validates :project_id, uniqueness: { scope: [:subscribable_id, :subscribable_type, :user_id] }
uniqueness: { scope: [:subscribable_id, :subscribable_type, :user_id] },
presence: true
end end
...@@ -3,23 +3,43 @@ require 'spec_helper' ...@@ -3,23 +3,43 @@ require 'spec_helper'
describe Subscribable, 'Subscribable' do describe Subscribable, 'Subscribable' do
let(:project) { create(:empty_project) } let(:project) { create(:empty_project) }
let(:resource) { create(:issue, project: project) } let(:resource) { create(:issue, project: project) }
let(:user) { create(:user) } let(:user_1) { create(:user) }
describe '#subscribed?' do describe '#subscribed?' do
it 'returns false when no subcription exists' do context 'without project' do
expect(resource.subscribed?(user, project)).to be_falsey it 'returns false when no subscription exists' do
expect(resource.subscribed?(user_1)).to be_falsey
end end
it 'returns true when a subcription exists and subscribed is true' do it 'returns true when a subcription exists and subscribed is true' do
resource.subscriptions.create(user: user, project: project, subscribed: true) resource.subscriptions.create(user: user_1, subscribed: true)
expect(resource.subscribed?(user, project)).to be_truthy expect(resource.subscribed?(user_1)).to be_truthy
end end
it 'returns false when a subcription exists and subscribed is false' do it 'returns false when a subcription exists and subscribed is false' do
resource.subscriptions.create(user: user, project: project, subscribed: false) resource.subscriptions.create(user: user_1, subscribed: false)
expect(resource.subscribed?(user, project)).to be_falsey expect(resource.subscribed?(user_1)).to be_falsey
end
end
context 'with project' do
it 'returns false when no subscription exists' do
expect(resource.subscribed?(user_1, project)).to be_falsey
end
it 'returns true when a subcription exists and subscribed is true' do
resource.subscriptions.create(user: user_1, project: project, subscribed: true)
expect(resource.subscribed?(user_1, project)).to be_truthy
end
it 'returns false when a subcription exists and subscribed is false' do
resource.subscriptions.create(user: user_1, project: project, subscribed: false)
expect(resource.subscribed?(user_1, project)).to be_falsey
end
end end
end end
...@@ -29,41 +49,80 @@ describe Subscribable, 'Subscribable' do ...@@ -29,41 +49,80 @@ describe Subscribable, 'Subscribable' do
end end
it 'returns the subscribed users' do it 'returns the subscribed users' do
resource.subscriptions.create(user: user, project: project, subscribed: true) user_2 = create(:user)
resource.subscriptions.create(user: user_1, subscribed: true)
resource.subscriptions.create(user: user_2, project: project, subscribed: true)
resource.subscriptions.create(user: create(:user), project: project, subscribed: false) resource.subscriptions.create(user: create(:user), project: project, subscribed: false)
expect(resource.subscribers(project)).to eq [user] expect(resource.subscribers(project)).to contain_exactly(user_1, user_2)
end end
end end
describe '#toggle_subscription' do describe '#toggle_subscription' do
context 'without project' do
it 'toggles the current subscription state for the given user' do it 'toggles the current subscription state for the given user' do
expect(resource.subscribed?(user, project)).to be_falsey expect(resource.subscribed?(user_1)).to be_falsey
resource.toggle_subscription(user, project) resource.toggle_subscription(user_1)
expect(resource.subscribed?(user, project)).to be_truthy expect(resource.subscribed?(user_1)).to be_truthy
end
end
context 'with project' do
it 'toggles the current subscription state for the given user' do
expect(resource.subscribed?(user_1, project)).to be_falsey
resource.toggle_subscription(user_1, project)
expect(resource.subscribed?(user_1, project)).to be_truthy
end
end end
end end
describe '#subscribe' do describe '#subscribe' do
context 'without project' do
it 'subscribes the given user' do
expect(resource.subscribed?(user_1)).to be_falsey
resource.subscribe(user_1)
expect(resource.subscribed?(user_1)).to be_truthy
end
end
context 'with project' do
it 'subscribes the given user' do it 'subscribes the given user' do
expect(resource.subscribed?(user, project)).to be_falsey expect(resource.subscribed?(user_1, project)).to be_falsey
resource.subscribe(user, project) resource.subscribe(user_1, project)
expect(resource.subscribed?(user, project)).to be_truthy expect(resource.subscribed?(user_1, project)).to be_truthy
end
end end
end end
describe '#unsubscribe' do describe '#unsubscribe' do
context 'without project' do
it 'unsubscribes the given current user' do
resource.subscriptions.create(user: user_1, subscribed: true)
expect(resource.subscribed?(user_1)).to be_truthy
resource.unsubscribe(user_1)
expect(resource.subscribed?(user_1)).to be_falsey
end
end
context 'with project' do
it 'unsubscribes the given current user' do it 'unsubscribes the given current user' do
resource.subscriptions.create(user: user, project: project, subscribed: true) resource.subscriptions.create(user: user_1, project: project, subscribed: true)
expect(resource.subscribed?(user, project)).to be_truthy expect(resource.subscribed?(user_1, project)).to be_truthy
resource.unsubscribe(user, project) resource.unsubscribe(user_1, project)
expect(resource.subscribed?(user, project)).to be_falsey expect(resource.subscribed?(user_1, project)).to be_falsey
end
end end
end end
end end
...@@ -8,7 +8,6 @@ describe Subscription, models: true do ...@@ -8,7 +8,6 @@ describe Subscription, models: true do
end end
describe 'validations' do describe 'validations' do
it { is_expected.to validate_presence_of(:project) }
it { is_expected.to validate_presence_of(:subscribable) } it { is_expected.to validate_presence_of(:subscribable) }
it { is_expected.to validate_presence_of(:user) } it { is_expected.to validate_presence_of(:user) }
......
...@@ -382,18 +382,21 @@ describe NotificationService, services: true do ...@@ -382,18 +382,21 @@ describe NotificationService, services: true do
user_1 = create(:user) user_1 = create(:user)
user_2 = create(:user) user_2 = create(:user)
user_3 = create(:user) user_3 = create(:user)
user_4 = create(:user)
label = create(:label, project: project, issues: [issue]) label = create(:label, project: project, issues: [issue])
group_label = create(:group_label, group: group, issues: [issue]) group_label = create(:group_label, group: group, issues: [issue])
issue.reload issue.reload
label.toggle_subscription(user_1, project) label.toggle_subscription(user_1, project)
group_label.toggle_subscription(user_2, project) group_label.toggle_subscription(user_2, project)
group_label.toggle_subscription(user_3, another_project) group_label.toggle_subscription(user_3, another_project)
group_label.toggle_subscription(user_4)
notification.new_issue(issue, @u_disabled) notification.new_issue(issue, @u_disabled)
should_email(user_1) should_email(user_1)
should_email(user_2) should_email(user_2)
should_not_email(user_3) should_not_email(user_3)
should_email(user_4)
end end
context 'confidential issues' do context 'confidential issues' do
...@@ -569,7 +572,8 @@ describe NotificationService, services: true do ...@@ -569,7 +572,8 @@ describe NotificationService, services: true do
let(:label_1) { create(:label, project: project, title: 'Label 1', issues: [issue]) } let(:label_1) { create(:label, project: project, title: 'Label 1', issues: [issue]) }
let(:label_2) { create(:label, project: project, title: 'Label 2') } let(:label_2) { create(:label, project: project, title: 'Label 2') }
let!(:subscriber_to_group_label_1) { create(:user) { |u| group_label_1.toggle_subscription(u, project) } } let!(:subscriber_to_group_label_1) { create(:user) { |u| group_label_1.toggle_subscription(u, project) } }
let!(:subscriber_to_group_label_2) { create(:user) { |u| group_label_2.toggle_subscription(u, project) } } let!(:subscriber_1_to_group_label_2) { create(:user) { |u| group_label_2.toggle_subscription(u, project) } }
let!(:subscriber_2_to_group_label_2) { create(:user) { |u| group_label_2.toggle_subscription(u) } }
let!(:subscriber_to_group_label_2_on_another_project) { create(:user) { |u| group_label_2.toggle_subscription(u, another_project) } } let!(:subscriber_to_group_label_2_on_another_project) { create(:user) { |u| group_label_2.toggle_subscription(u, another_project) } }
let!(:subscriber_to_label_1) { create(:user) { |u| label_1.toggle_subscription(u, project) } } let!(:subscriber_to_label_1) { create(:user) { |u| label_1.toggle_subscription(u, project) } }
let!(:subscriber_to_label_2) { create(:user) { |u| label_2.toggle_subscription(u, project) } } let!(:subscriber_to_label_2) { create(:user) { |u| label_2.toggle_subscription(u, project) } }
...@@ -580,7 +584,8 @@ describe NotificationService, services: true do ...@@ -580,7 +584,8 @@ describe NotificationService, services: true do
should_not_email(subscriber_to_label_1) should_not_email(subscriber_to_label_1)
should_not_email(subscriber_to_group_label_1) should_not_email(subscriber_to_group_label_1)
should_not_email(subscriber_to_group_label_2_on_another_project) should_not_email(subscriber_to_group_label_2_on_another_project)
should_email(subscriber_to_group_label_2) should_email(subscriber_1_to_group_label_2)
should_email(subscriber_2_to_group_label_2)
should_email(subscriber_to_label_2) should_email(subscriber_to_label_2)
end end
...@@ -599,7 +604,8 @@ describe NotificationService, services: true do ...@@ -599,7 +604,8 @@ describe NotificationService, services: true do
should_not_email(subscriber_to_label_1) should_not_email(subscriber_to_label_1)
should_not_email(subscriber_to_group_label_1) should_not_email(subscriber_to_group_label_1)
should_not_email(subscriber_to_group_label_2_on_another_project) should_not_email(subscriber_to_group_label_2_on_another_project)
should_email(subscriber_to_group_label_2) should_email(subscriber_1_to_group_label_2)
should_email(subscriber_2_to_group_label_2)
should_email(subscriber_to_label_2) should_email(subscriber_to_label_2)
end end
...@@ -784,17 +790,20 @@ describe NotificationService, services: true do ...@@ -784,17 +790,20 @@ describe NotificationService, services: true do
user_1 = create(:user) user_1 = create(:user)
user_2 = create(:user) user_2 = create(:user)
user_3 = create(:user) user_3 = create(:user)
user_4 = create(:user)
label = create(:label, project: project, merge_requests: [merge_request]) label = create(:label, project: project, merge_requests: [merge_request])
group_label = create(:group_label, group: group, merge_requests: [merge_request]) group_label = create(:group_label, group: group, merge_requests: [merge_request])
label.toggle_subscription(user_1, project) label.toggle_subscription(user_1, project)
group_label.toggle_subscription(user_2, project) group_label.toggle_subscription(user_2, project)
group_label.toggle_subscription(user_3, another_project) group_label.toggle_subscription(user_3, another_project)
group_label.toggle_subscription(user_4)
notification.new_merge_request(merge_request, @u_disabled) notification.new_merge_request(merge_request, @u_disabled)
should_email(user_1) should_email(user_1)
should_email(user_2) should_email(user_2)
should_not_email(user_3) should_not_email(user_3)
should_email(user_4)
end end
context 'participating' do context 'participating' do
...@@ -893,7 +902,8 @@ describe NotificationService, services: true do ...@@ -893,7 +902,8 @@ describe NotificationService, services: true do
let(:label_1) { create(:label, project: project, title: 'Label 1', merge_requests: [merge_request]) } let(:label_1) { create(:label, project: project, title: 'Label 1', merge_requests: [merge_request]) }
let(:label_2) { create(:label, project: project, title: 'Label 2') } let(:label_2) { create(:label, project: project, title: 'Label 2') }
let!(:subscriber_to_group_label_1) { create(:user) { |u| group_label_1.toggle_subscription(u, project) } } let!(:subscriber_to_group_label_1) { create(:user) { |u| group_label_1.toggle_subscription(u, project) } }
let!(:subscriber_to_group_label_2) { create(:user) { |u| group_label_2.toggle_subscription(u, project) } } let!(:subscriber_1_to_group_label_2) { create(:user) { |u| group_label_2.toggle_subscription(u, project) } }
let!(:subscriber_2_to_group_label_2) { create(:user) { |u| group_label_2.toggle_subscription(u) } }
let!(:subscriber_to_group_label_2_on_another_project) { create(:user) { |u| group_label_2.toggle_subscription(u, another_project) } } let!(:subscriber_to_group_label_2_on_another_project) { create(:user) { |u| group_label_2.toggle_subscription(u, another_project) } }
let!(:subscriber_to_label_1) { create(:user) { |u| label_1.toggle_subscription(u, project) } } let!(:subscriber_to_label_1) { create(:user) { |u| label_1.toggle_subscription(u, project) } }
let!(:subscriber_to_label_2) { create(:user) { |u| label_2.toggle_subscription(u, project) } } let!(:subscriber_to_label_2) { create(:user) { |u| label_2.toggle_subscription(u, project) } }
...@@ -904,7 +914,8 @@ describe NotificationService, services: true do ...@@ -904,7 +914,8 @@ describe NotificationService, services: true do
should_not_email(subscriber_to_label_1) should_not_email(subscriber_to_label_1)
should_not_email(subscriber_to_group_label_1) should_not_email(subscriber_to_group_label_1)
should_not_email(subscriber_to_group_label_2_on_another_project) should_not_email(subscriber_to_group_label_2_on_another_project)
should_email(subscriber_to_group_label_2) should_email(subscriber_1_to_group_label_2)
should_email(subscriber_2_to_group_label_2)
should_email(subscriber_to_label_2) should_email(subscriber_to_label_2)
end end
...@@ -923,7 +934,8 @@ describe NotificationService, services: true do ...@@ -923,7 +934,8 @@ describe NotificationService, services: true do
should_not_email(subscriber_to_label_1) should_not_email(subscriber_to_label_1)
should_not_email(subscriber_to_group_label_1) should_not_email(subscriber_to_group_label_1)
should_not_email(subscriber_to_group_label_2_on_another_project) should_not_email(subscriber_to_group_label_2_on_another_project)
should_email(subscriber_to_group_label_2) should_email(subscriber_1_to_group_label_2)
should_email(subscriber_2_to_group_label_2)
should_email(subscriber_to_label_2) should_email(subscriber_to_label_2)
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment