Skip to content

Commit

Permalink
Merge pull request #3992 from evanrolfe/feature/watchlist_notifications
Browse files Browse the repository at this point in the history
[webui] Add notifications for projects in watchlist.
  • Loading branch information
Evan Rolfe committed Oct 13, 2017
2 parents 7e4f575 + 18bfe51 commit 72fc9c4
Show file tree
Hide file tree
Showing 10 changed files with 135 additions and 44 deletions.
5 changes: 5 additions & 0 deletions src/api/app/models/event/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,11 @@ def readers
ret
end

def watchers
project = ::Project.find_by_name(payload['project'])
project.watched_projects.map(&:user)
end

def _roles(role, project, package = nil)
return [] unless project
p = nil
Expand Down
2 changes: 1 addition & 1 deletion src/api/app/models/event/build.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class Event::BuildFail < Event::Build
include BuildLogSupport

self.description = 'Package has failed to build'
receiver_roles :maintainer, :bugowner, :reader
receiver_roles :maintainer, :bugowner, :reader, :watcher
after_commit :send_to_bus

def self.message_bus_queue
Expand Down
6 changes: 3 additions & 3 deletions src/api/app/models/event/comment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def custom_headers

class Event::CommentForProject < ::Event::Project
include CommentEvent
receiver_roles :maintainer
receiver_roles :maintainer, :watcher
after_commit :send_to_bus

def self.message_bus_queue
Expand All @@ -47,7 +47,7 @@ def subject

class Event::CommentForPackage < ::Event::Package
include CommentEvent
receiver_roles :maintainer
receiver_roles :maintainer, :watcher
after_commit :send_to_bus

def self.message_bus_queue
Expand All @@ -65,7 +65,7 @@ class Event::CommentForRequest < ::Event::Request
include CommentEvent
self.description = 'New comment for request created'
payload_keys :request_number
receiver_roles :source_maintainer, :target_maintainer, :creator, :reviewer
receiver_roles :source_maintainer, :target_maintainer, :creator, :reviewer, :source_watcher, :target_watcher
after_commit :send_to_bus

def self.message_bus_queue
Expand Down
20 changes: 18 additions & 2 deletions src/api/app/models/event/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,22 @@ def target_maintainers
def source_maintainers
action_maintainers('sourceproject', 'sourcepackage')
end

def target_watchers
find_watchers('targetproject')
end

def source_watchers
find_watchers('sourceproject')
end

private

def find_watchers(project_key)
project_names = payload['actions'].map { |action| action[project_key] }.uniq
projects = Project.where(name: project_names).joins(watched_projects: :user)
projects.flat_map { |project| project.watched_projects.map(&:user) }
end
end

class Event::RequestChange < Event::Request
Expand All @@ -132,7 +148,7 @@ def self.message_bus_queue

class Event::RequestCreate < Event::Request
self.description = 'Request created'
receiver_roles :source_maintainer, :target_maintainer
receiver_roles :source_maintainer, :target_maintainer, :source_watcher, :target_watcher
after_commit :send_to_bus

def self.message_bus_queue
Expand Down Expand Up @@ -168,7 +184,7 @@ def self.message_bus_queue
class Event::RequestStatechange < Event::Request
self.description = 'Request state was changed'
payload_keys :oldstate
receiver_roles :source_maintainer, :target_maintainer, :creator, :reviewer
receiver_roles :source_maintainer, :target_maintainer, :creator, :reviewer, :source_watcher, :target_watcher
after_commit :send_to_bus

def self.message_bus_queue
Expand Down
14 changes: 13 additions & 1 deletion src/api/app/models/event_subscription.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,19 @@ class EventSubscription < ApplicationRecord
belongs_to :user, inverse_of: :event_subscriptions
belongs_to :group, inverse_of: :event_subscriptions

validates :receiver_role, inclusion: { in: %i(maintainer bugowner reader source_maintainer target_maintainer reviewer commenter creator) }
validates :receiver_role, inclusion: {
in: %i(maintainer
bugowner
reader
source_maintainer
target_maintainer
reviewer
commenter
creator
watcher
source_watcher
target_watcher)
}

scope :for_eventtype, ->(eventtype) { where(eventtype: eventtype) }
scope :defaults, ->{ where(user_id: nil, group_id: nil) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
= fields_for "subscriptions[#{i}]", subscription do |f|
%li
= f.label :receive, subscription.receiver_role
= f.select :channel, EventSubscription.channels.keys
= f.select :channel, EventSubscription.channels.keys, {}, { data: { eventtype: subscription.eventtype, receiver_role: subscription.receiver_role } }
= f.hidden_field :eventtype
= f.hidden_field :receiver_role
- i += 1
13 changes: 7 additions & 6 deletions src/api/spec/features/webui/notifications_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@

expect(page).to have_content('Events to get email for')

%w(subscriptions_8_channel
subscriptions_7_channel
subscriptions_14_channel
subscriptions_15_channel
).each do |select_id|
select('instant_email', from: select_id)
[
['Event::CommentForPackage', 'commenter'],
['Event::CommentForProject', 'maintainer'],
['Event::CommentForRequest', 'reviewer'],
['Event::BuildFail', 'maintainer']
].each do |eventtype, receiver_role|
find("select[data-eventtype='#{eventtype}'][data-receiver-role='#{receiver_role}']").find(:option, 'instant_email').select_option
end

click_button 'Update'
Expand Down
105 changes: 83 additions & 22 deletions src/api/spec/models/event_subscription/find_for_event_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
require 'rails_helper'

RSpec::Matchers.define :be_like_subscription do |expected_subscription|
match do |actual_subscription|
actual_subscription.eventtype == expected_subscription.eventtype &&
actual_subscription.receiver_role == expected_subscription.receiver_role &&
actual_subscription.channel == expected_subscription.channel
end
end

RSpec.shared_context 'it returns subscriptions for an event' do
let(:maintainer_subscription_result) { subject.find { |subscription| subscription.subscriber == maintainer } }

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) }
Expand All @@ -9,9 +19,7 @@
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)
expect(maintainer_subscription_result).to eq(subscription)
end
end

Expand All @@ -36,12 +44,7 @@
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)
expect(maintainer_subscription_result).to be_like_subscription(default_subscription)
end
end

Expand All @@ -68,9 +71,7 @@
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)
expect(maintainer_subscription_result).to eq(subscription)
end
end

Expand All @@ -88,12 +89,56 @@

RSpec.describe EventSubscription::FindForEvent do
describe '#subscribers' do
subject do
event = Event::CommentForProject.first
EventSubscription::FindForEvent.new(event).subscriptions
context 'with a request' do
let!(:watcher) { create(:confirmed_user) }
let!(:watcher2) { create(:confirmed_user) }
let!(:source_project) { create(:project, name: 'TheSource') }
let!(:target_project) { create(:project, name: 'TheTarget') }
let!(:source_package) { create(:package) }
let!(:target_package) { create(:package) }
let(:request) do
create(
:bs_request_with_submit_action,
source_project: source_project.name,
target_project: target_project.name,
source_package: source_package.name,
target_package: target_package.name
)
end
let!(:default_subscription) do
create(
:event_subscription,
eventtype: 'Event::RequestCreate',
receiver_role: 'source_watcher',
user: nil,
group: nil,
channel: :instant_email
)
end

before do
watcher.add_watched_project(source_project.name)
request
end

subject do
event = Event::RequestCreate.first
EventSubscription::FindForEvent.new(event).subscriptions
end

it 'returns a new subscription for the watcher based on the default subscription' do
result_subscription = subject.find { |subscription| subscription.subscriber == watcher }

expect(result_subscription).to be_like_subscription(default_subscription)
end
end

context 'with a comment for a project' do
subject do
event = Event::CommentForProject.first
EventSubscription::FindForEvent.new(event).subscriptions
end

context 'with no maintainers' do
let!(:project) { create(:project) }
let!(:comment) { create(:comment_project, commentable: project) }
Expand All @@ -111,6 +156,26 @@
let!(:maintainer) { create(:group) }
end

context 'with a user who watching the project' do
let!(:watcher) { create(:confirmed_user) }
let!(:project) { create(:project) }
let!(:comment) { create(:comment_project, commentable: project) }

let!(:default_subscription) do
create(:event_subscription_comment_for_project, receiver_role: 'watcher', user: nil, group: nil)
end

before do
watcher.add_watched_project(project.name)
end

it 'returns a new subscription for the watcher based on the default subscription' do
result_subscription = subject.find { |subscription| subscription.subscriber == watcher }

expect(result_subscription).to be_like_subscription(default_subscription)
end
end

context 'with a user who is a maintainer and a commenter' do
context 'and the user has a maintainer and commenter subscriptions, which are both enabled' do
let!(:maintainer) { create(:confirmed_user) }
Expand All @@ -128,7 +193,6 @@
result_subscription = subject.find { |subscription| subscription.subscriber == maintainer }

# It doesn't matter if the maintainer or commenter subscription is returned
expect(result_subscription.id).not_to be_nil
expect(result_subscription.eventtype).to eq(subscription_maintainer.eventtype)
expect(result_subscription.channel).to eq(subscription_maintainer.channel)
end
Expand All @@ -148,6 +212,8 @@
let!(:project) { create(:project, maintainer: [group]) }
let!(:comment) { create(:comment_project, commentable: project) }

let(:user_subscription_result) { subject.find { |subscription| subscription.subscriber == user } }

before do
group.users << user
end
Expand Down Expand Up @@ -195,12 +261,7 @@
end

it 'returns a new subscription for that user/group based on the default subscription' do
result = subject.find { |subscription| subscription.subscriber == user }

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)
expect(user_subscription_result).to be_like_subscription(default_subscription)
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@
it 'includes the users subscriptions first' do
subscriptions = subject[Event::CommentForProject]

expect(subscriptions.count).to eq(2)
expect(subscriptions.count).to eq(3)
expect(subscriptions).to include(user_subscription1, user_subscription2)
end

it 'includes the default subscriptions second' do
subscriptions = subject[Event::CommentForRequest]

expect(subscriptions.count).to eq(5)
expect(subscriptions.count).to eq(7)
expect(subscriptions).to include(default_subscription3, default_subscription4)
end

Expand All @@ -41,10 +41,10 @@
let(:subscriber) { nil }

it 'includes the default subscriptions first' do
expect(subject[Event::CommentForProject].count).to eq(2)
expect(subject[Event::CommentForProject].count).to eq(3)
expect(subject[Event::CommentForProject]).to include(default_subscription1, default_subscription2)

expect(subject[Event::CommentForRequest].count).to eq(5)
expect(subject[Event::CommentForRequest].count).to eq(7)
expect(subject[Event::CommentForRequest]).to include(default_subscription3, default_subscription4)
end

Expand Down
4 changes: 0 additions & 4 deletions src/api/test/models/event_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ def setup
assert_equal [], e.receiver_roles
end

test 'receive roles for build failure' do
assert_equal %i(maintainer bugowner reader), events(:build_fails_with_deleted_user_and_request).receiver_roles
end

def users_for_event(e)
users = EventSubscription::FindForEvent.new(e).subscriptions.map(&:subscriber)
User.where(id: users).pluck(:login).sort
Expand Down

0 comments on commit 72fc9c4

Please sign in to comment.