summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGrzegorz Bizon <grzesiek.bizon@gmail.com>2016-11-17 20:19:16 +0100
committerGrzegorz Bizon <grzesiek.bizon@gmail.com>2016-11-17 20:19:16 +0100
commitd9f9be3c9548e8e99e071d764163c47c073c3c28 (patch)
tree879d15c8d794a2f0265debbb3dd76a73e1eb4432
parent8a7860fd7fe91a9d2642dab9e66be5b22012901d (diff)
parent3344e1e8dbcb3e53e388511a661f6769b375b748 (diff)
downloadgitlab-ce-d9f9be3c9548e8e99e071d764163c47c073c3c28.tar.gz
Merge branch 'master' into 22539-display-folders
* master: (26 commits) Remove extra subscribable_type filter on migration Add feature spec for labels subscription Avoid code duplication for label subscription status on label partial Rename LabelSubscription javascript to ProjectLabelSubscription Fix label subscription menu on small screens resolution Allow users to subscribe to a group label at group or project level Use button instead of an icon to subscribe/unsubscribe to labels Add CHANGELOG entry Add toggle_subscription action to Groups::LabelsController Allow subscriptions to be created without a project Use subqueries instead of joins to migrate subscriptions Add helper method to toggle label subscription on labels controller spec Remove default value for `project` argument on subscribable concern Use @project as default on ToggleSubscriptionAction concern Allow users to subscribe to group labels at project-level Pass project to Entities::Label to check if user is subscribed Fix specs to pass a project when creating subscriptions Refactoring label subscription toggle button text to accept a project Refactoring label subscription status to accept a project Refactoring notification service to find subscriptions per project ...
-rw-r--r--app/assets/javascripts/group_label_subscription.js.es653
-rw-r--r--app/assets/javascripts/project_label_subscription.js.es653
-rw-r--r--app/assets/stylesheets/pages/labels.scss10
-rw-r--r--app/controllers/concerns/toggle_subscription_action.rb6
-rw-r--r--app/controllers/groups/labels_controller.rb7
-rw-r--r--app/controllers/projects/labels_controller.rb7
-rw-r--r--app/controllers/sent_notifications_controller.rb2
-rw-r--r--app/helpers/labels_helper.rb30
-rw-r--r--app/models/concerns/issuable.rb2
-rw-r--r--app/models/concerns/subscribable.rb64
-rw-r--r--app/models/issue.rb2
-rw-r--r--app/models/subscription.rb7
-rw-r--r--app/services/issuable_base_service.rb4
-rw-r--r--app/services/notification_service.rb28
-rw-r--r--app/services/slash_commands/interpret_service.rb4
-rw-r--r--app/views/shared/_label.html.haml55
-rw-r--r--app/views/shared/issuable/_sidebar.html.haml2
-rw-r--r--changelogs/unreleased/feature-subscribe-to-group-level-labels.yml4
-rw-r--r--config/routes/group.rb5
-rw-r--r--db/migrate/20161031171301_add_project_id_to_subscriptions.rb14
-rw-r--r--db/migrate/20161031174110_migrate_subscriptions_project_id.rb44
-rw-r--r--db/migrate/20161031181638_add_unique_index_to_subscriptions.rb18
-rw-r--r--db/schema.rb4
-rw-r--r--lib/api/entities.rb6
-rw-r--r--lib/api/issues.rb10
-rw-r--r--lib/api/merge_requests.rb10
-rw-r--r--lib/api/milestones.rb2
-rw-r--r--lib/api/subscriptions.rb12
-rw-r--r--spec/controllers/groups/labels_controller_spec.rb22
-rw-r--r--spec/controllers/projects/boards/issues_controller_spec.rb2
-rw-r--r--spec/controllers/projects/labels_controller_spec.rb30
-rw-r--r--spec/controllers/sent_notifications_controller_spec.rb18
-rw-r--r--spec/factories/subscriptions.rb7
-rw-r--r--spec/features/projects/labels/subscription_spec.rb74
-rw-r--r--spec/features/unsubscribe_links_spec.rb14
-rw-r--r--spec/models/concerns/issuable_spec.rb22
-rw-r--r--spec/models/concerns/subscribable_spec.rb117
-rw-r--r--spec/models/subscription_spec.rb20
-rw-r--r--spec/requests/api/issues_spec.rb4
-rw-r--r--spec/requests/api/labels_spec.rb6
-rw-r--r--spec/services/issuable/bulk_update_service_spec.rb6
-rw-r--r--spec/services/issues/update_service_spec.rb2
-rw-r--r--spec/services/merge_requests/update_service_spec.rb2
-rw-r--r--spec/services/notification_service_spec.rb156
-rw-r--r--spec/services/slash_commands/interpret_service_spec.rb4
-rw-r--r--spec/support/features/issuable_slash_commands_shared_examples.rb10
46 files changed, 754 insertions, 227 deletions
diff --git a/app/assets/javascripts/group_label_subscription.js.es6 b/app/assets/javascripts/group_label_subscription.js.es6
new file mode 100644
index 00000000000..eea6cd40859
--- /dev/null
+++ b/app/assets/javascripts/group_label_subscription.js.es6
@@ -0,0 +1,53 @@
+/* eslint-disable */
+(function(global) {
+ class GroupLabelSubscription {
+ constructor(container) {
+ const $container = $(container);
+ this.$dropdown = $container.find('.dropdown');
+ this.$subscribeButtons = $container.find('.js-subscribe-button');
+ this.$unsubscribeButtons = $container.find('.js-unsubscribe-button');
+
+ this.$subscribeButtons.on('click', this.subscribe.bind(this));
+ this.$unsubscribeButtons.on('click', this.unsubscribe.bind(this));
+ }
+
+ unsubscribe(event) {
+ event.preventDefault();
+
+ const url = this.$unsubscribeButtons.attr('data-url');
+
+ $.ajax({
+ type: 'POST',
+ url: url
+ }).done(() => {
+ this.toggleSubscriptionButtons();
+ this.$unsubscribeButtons.removeAttr('data-url');
+ });
+ }
+
+ subscribe(event) {
+ event.preventDefault();
+
+ const $btn = $(event.currentTarget);
+ const url = $btn.attr('data-url');
+
+ this.$unsubscribeButtons.attr('data-url', url);
+
+ $.ajax({
+ type: 'POST',
+ url: url
+ }).done(() => {
+ this.toggleSubscriptionButtons();
+ });
+ }
+
+ toggleSubscriptionButtons() {
+ this.$dropdown.toggleClass('hidden');
+ this.$subscribeButtons.toggleClass('hidden');
+ this.$unsubscribeButtons.toggleClass('hidden');
+ }
+ }
+
+ global.GroupLabelSubscription = GroupLabelSubscription;
+
+})(window.gl || (window.gl = {}));
diff --git a/app/assets/javascripts/project_label_subscription.js.es6 b/app/assets/javascripts/project_label_subscription.js.es6
new file mode 100644
index 00000000000..03a115cb35b
--- /dev/null
+++ b/app/assets/javascripts/project_label_subscription.js.es6
@@ -0,0 +1,53 @@
+/* eslint-disable */
+(function(global) {
+ class ProjectLabelSubscription {
+ constructor(container) {
+ this.$container = $(container);
+ this.$buttons = this.$container.find('.js-subscribe-button');
+
+ this.$buttons.on('click', this.toggleSubscription.bind(this));
+ }
+
+ toggleSubscription(event) {
+ event.preventDefault();
+
+ const $btn = $(event.currentTarget);
+ const $span = $btn.find('span');
+ const url = $btn.attr('data-url');
+ const oldStatus = $btn.attr('data-status');
+
+ $btn.addClass('disabled');
+ $span.toggleClass('hidden');
+
+ $.ajax({
+ type: 'POST',
+ url: url
+ }).done(() => {
+ let newStatus, newAction;
+
+ if (oldStatus === 'unsubscribed') {
+ [newStatus, newAction] = ['subscribed', 'Unsubscribe'];
+ } else {
+ [newStatus, newAction] = ['unsubscribed', 'Subscribe'];
+ }
+
+ $span.toggleClass('hidden');
+ $btn.removeClass('disabled');
+
+ this.$buttons.attr('data-status', newStatus);
+ this.$buttons.find('> span').text(newAction);
+
+ for (let button of this.$buttons) {
+ let $button = $(button);
+
+ if ($button.attr('data-original-title')) {
+ $button.tooltip('hide').attr('data-original-title', newAction).tooltip('fixTitle');
+ }
+ }
+ });
+ }
+ }
+
+ global.ProjectLabelSubscription = ProjectLabelSubscription;
+
+})(window.gl || (window.gl = {}));
diff --git a/app/assets/stylesheets/pages/labels.scss b/app/assets/stylesheets/pages/labels.scss
index 397f89f501a..e39ce19f846 100644
--- a/app/assets/stylesheets/pages/labels.scss
+++ b/app/assets/stylesheets/pages/labels.scss
@@ -90,7 +90,7 @@
@media (min-width: $screen-sm-min) {
display: inline-block;
- width: 40%;
+ width: 30%;
margin-left: 10px;
margin-bottom: 0;
vertical-align: middle;
@@ -222,6 +222,14 @@
width: 100%;
}
+.label-subscription {
+ vertical-align: middle;
+
+ .dropdown-group-label a {
+ cursor: pointer;
+ }
+}
+
.label-subscribe-button {
.label-subscribe-button-icon {
&[disabled] {
diff --git a/app/controllers/concerns/toggle_subscription_action.rb b/app/controllers/concerns/toggle_subscription_action.rb
index 9e3b9be2ff4..92cb534343e 100644
--- a/app/controllers/concerns/toggle_subscription_action.rb
+++ b/app/controllers/concerns/toggle_subscription_action.rb
@@ -4,13 +4,17 @@ module ToggleSubscriptionAction
def toggle_subscription
return unless current_user
- subscribable_resource.toggle_subscription(current_user)
+ subscribable_resource.toggle_subscription(current_user, subscribable_project)
head :ok
end
private
+ def subscribable_project
+ @project || raise(NotImplementedError)
+ end
+
def subscribable_resource
raise NotImplementedError
end
diff --git a/app/controllers/groups/labels_controller.rb b/app/controllers/groups/labels_controller.rb
index 29528b2cfaa..587898a8634 100644
--- a/app/controllers/groups/labels_controller.rb
+++ b/app/controllers/groups/labels_controller.rb
@@ -1,4 +1,6 @@
class Groups::LabelsController < Groups::ApplicationController
+ include ToggleSubscriptionAction
+
before_action :label, only: [:edit, :update, :destroy]
before_action :authorize_admin_labels!, only: [:new, :create, :edit, :update, :destroy]
before_action :save_previous_label_path, only: [:edit]
@@ -69,6 +71,11 @@ class Groups::LabelsController < Groups::ApplicationController
def label
@label ||= @group.labels.find(params[:id])
end
+ alias_method :subscribable_resource, :label
+
+ def subscribable_project
+ nil
+ end
def label_params
params.require(:label).permit(:title, :description, :color)
diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb
index 42fd09e9b7e..824ed7be73e 100644
--- a/app/controllers/projects/labels_controller.rb
+++ b/app/controllers/projects/labels_controller.rb
@@ -3,7 +3,7 @@ class Projects::LabelsController < Projects::ApplicationController
before_action :module_enabled
before_action :label, only: [:edit, :update, :destroy]
- before_action :find_labels, only: [:index, :set_priorities, :remove_priority]
+ before_action :find_labels, only: [:index, :set_priorities, :remove_priority, :toggle_subscription]
before_action :authorize_read_label!
before_action :authorize_admin_labels!, only: [:new, :create, :edit, :update,
:generate, :destroy, :remove_priority,
@@ -123,7 +123,10 @@ class Projects::LabelsController < Projects::ApplicationController
def label
@label ||= @project.labels.find(params[:id])
end
- alias_method :subscribable_resource, :label
+
+ def subscribable_resource
+ @available_labels.find(params[:id])
+ end
def find_labels
@available_labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute
diff --git a/app/controllers/sent_notifications_controller.rb b/app/controllers/sent_notifications_controller.rb
index 3085ff33aba..04c36b3ebfe 100644
--- a/app/controllers/sent_notifications_controller.rb
+++ b/app/controllers/sent_notifications_controller.rb
@@ -12,7 +12,7 @@ class SentNotificationsController < ApplicationController
def unsubscribe_and_redirect
noteable = @sent_notification.noteable
- noteable.unsubscribe(@sent_notification.recipient)
+ noteable.unsubscribe(@sent_notification.recipient, @sent_notification.project)
flash[:notice] = "You have been unsubscribed from this thread."
diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb
index 221a84b042f..4f180456b16 100644
--- a/app/helpers/labels_helper.rb
+++ b/app/helpers/labels_helper.rb
@@ -68,14 +68,6 @@ module LabelsHelper
end
end
- def toggle_subscription_data(label)
- return unless label.is_a?(ProjectLabel)
-
- {
- url: toggle_subscription_namespace_project_label_path(label.project.namespace, label.project, label)
- }
- end
-
def render_colored_label(label, label_suffix = '', tooltip: true)
label_color = label.color || Label::DEFAULT_COLOR
text_color = text_color_for_bg(label_color)
@@ -148,20 +140,24 @@ module LabelsHelper
end
end
- def label_subscription_status(label)
- case label
- when GroupLabel then 'Subscribing to group labels is currently not supported.'
- when ProjectLabel then label.subscribed?(current_user) ? 'subscribed' : 'unsubscribed'
- end
+ def label_subscription_status(label, project)
+ return 'project-level' if label.subscribed?(current_user, project)
+ return 'group-level' if label.subscribed?(current_user)
+
+ 'unsubscribed'
end
- def label_subscription_toggle_button_text(label)
- case label
- when GroupLabel then 'Subscribing to group labels is currently not supported.'
- when ProjectLabel then label.subscribed?(current_user) ? 'Unsubscribe' : 'Subscribe'
+ def group_label_unsubscribe_path(label, project)
+ case label_subscription_status(label, project)
+ when 'project-level' then toggle_subscription_namespace_project_label_path(@project.namespace, @project, label)
+ when 'group-level' then toggle_subscription_group_label_path(label.group, label)
end
end
+ def label_subscription_toggle_button_text(label, project)
+ label.subscribed?(current_user, project) ? 'Unsubscribe' : 'Subscribe'
+ end
+
def label_deletion_confirm_text(label)
case label
when GroupLabel then 'Remove this label? This will affect all projects within the group. Are you sure?'
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 664bb594aa9..ec9e7e5ae2b 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -215,7 +215,7 @@ module Issuable
end
end
- def subscribed_without_subscriptions?(user)
+ def subscribed_without_subscriptions?(user, project)
participants(user).include?(user)
end
diff --git a/app/models/concerns/subscribable.rb b/app/models/concerns/subscribable.rb
index 083257f1005..83daa9b1a64 100644
--- a/app/models/concerns/subscribable.rb
+++ b/app/models/concerns/subscribable.rb
@@ -12,39 +12,71 @@ module Subscribable
has_many :subscriptions, dependent: :destroy, as: :subscribable
end
- def subscribed?(user)
- if subscription = subscriptions.find_by_user_id(user.id)
+ def subscribed?(user, project = nil)
+ if subscription = subscriptions.find_by(user: user, project: project)
subscription.subscribed
else
- subscribed_without_subscriptions?(user)
+ subscribed_without_subscriptions?(user, project)
end
end
# Override this method to define custom logic to consider a subscribable as
# subscribed without an explicit subscription record.
- def subscribed_without_subscriptions?(user)
+ def subscribed_without_subscriptions?(user, project)
false
end
- def subscribers
- subscriptions.where(subscribed: true).map(&:user)
+ def subscribers(project)
+ subscriptions_available(project).
+ where(subscribed: true).
+ map(&:user)
end
- def toggle_subscription(user)
- subscriptions.
- find_or_initialize_by(user_id: user.id).
- update(subscribed: !subscribed?(user))
+ def toggle_subscription(user, project = nil)
+ unsubscribe_from_other_levels(user, project)
+
+ find_or_initialize_subscription(user, project).
+ update(subscribed: !subscribed?(user, project))
+ end
+
+ def subscribe(user, project = nil)
+ unsubscribe_from_other_levels(user, project)
+
+ find_or_initialize_subscription(user, project)
+ .update(subscribed: true)
+ end
+
+ def unsubscribe(user, project = nil)
+ unsubscribe_from_other_levels(user, project)
+
+ find_or_initialize_subscription(user, project)
+ .update(subscribed: false)
end
- def subscribe(user)
+ private
+
+ def unsubscribe_from_other_levels(user, project)
+ other_subscriptions = subscriptions.where(user: user)
+
+ other_subscriptions =
+ if project.blank?
+ other_subscriptions.where.not(project: nil)
+ else
+ other_subscriptions.where(project: nil)
+ end
+
+ other_subscriptions.update_all(subscribed: false)
+ end
+
+ def find_or_initialize_subscription(user, project)
subscriptions.
- find_or_initialize_by(user_id: user.id).
- update(subscribed: true)
+ find_or_initialize_by(user_id: user.id, project_id: project.try(:id))
end
- def unsubscribe(user)
+ def subscriptions_available(project)
+ t = Subscription.arel_table
+
subscriptions.
- find_or_initialize_by(user_id: user.id).
- update(subscribed: false)
+ where(t[:project_id].eq(nil).or(t[:project_id].eq(project.try(:id))))
end
end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 4a4017003d8..6e8f5d3c422 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -266,7 +266,7 @@ class Issue < ActiveRecord::Base
def as_json(options = {})
super(options).tap do |json|
- json[:subscribed] = subscribed?(options[:user]) if options.has_key?(:user) && options[:user]
+ json[:subscribed] = subscribed?(options[:user], project) if options.has_key?(:user) && options[:user]
if options.has_key?(:labels)
json[:labels] = labels.as_json(
diff --git a/app/models/subscription.rb b/app/models/subscription.rb
index 3b8aa1eb866..17869c8bac2 100644
--- a/app/models/subscription.rb
+++ b/app/models/subscription.rb
@@ -1,8 +1,9 @@
class Subscription < ActiveRecord::Base
belongs_to :user
+ belongs_to :project
belongs_to :subscribable, polymorphic: true
- validates :user_id,
- uniqueness: { scope: [:subscribable_id, :subscribable_type] },
- presence: true
+ validates :user, :subscribable, presence: true
+
+ validates :project_id, uniqueness: { scope: [:subscribable_id, :subscribable_type, :user_id] }
end
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index bb92cd80cc9..575795788de 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -212,9 +212,9 @@ class IssuableBaseService < BaseService
def change_subscription(issuable)
case params.delete(:subscription_event)
when 'subscribe'
- issuable.subscribe(current_user)
+ issuable.subscribe(current_user, project)
when 'unsubscribe'
- issuable.unsubscribe(current_user)
+ issuable.unsubscribe(current_user, project)
end
end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 6697840cc26..ecdcbf08ee1 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -75,7 +75,7 @@ class NotificationService
# * watchers of the issue's labels
#
def relabeled_issue(issue, added_labels, current_user)
- relabeled_resource_email(issue, added_labels, current_user, :relabeled_issue_email)
+ relabeled_resource_email(issue, issue.project, added_labels, current_user, :relabeled_issue_email)
end
# When create a merge request we should send an email to:
@@ -118,7 +118,7 @@ class NotificationService
# * watchers of the mr's labels
#
def relabeled_merge_request(merge_request, added_labels, current_user)
- relabeled_resource_email(merge_request, added_labels, current_user, :relabeled_merge_request_email)
+ relabeled_resource_email(merge_request, merge_request.target_project, added_labels, current_user, :relabeled_merge_request_email)
end
def close_mr(merge_request, current_user)
@@ -205,7 +205,7 @@ class NotificationService
recipients = reject_muted_users(recipients, note.project)
- recipients = add_subscribed_users(recipients, note.noteable)
+ recipients = add_subscribed_users(recipients, note.project, note.noteable)
recipients = reject_unsubscribed_users(recipients, note.noteable)
recipients = reject_users_without_access(recipients, note.noteable)
@@ -393,7 +393,7 @@ class NotificationService
)
end
- # Build a list of users based on project notifcation settings
+ # Build a list of users based on project notification settings
def select_project_member_setting(project, global_setting, users_global_level_watch)
users = notification_settings_for(project, :watch)
@@ -505,17 +505,17 @@ class NotificationService
end
end
- def add_subscribed_users(recipients, target)
+ def add_subscribed_users(recipients, project, target)
return recipients unless target.respond_to? :subscribers
- recipients + target.subscribers
+ recipients + target.subscribers(project)
end
- def add_labels_subscribers(recipients, target, labels: nil)
+ def add_labels_subscribers(recipients, project, target, labels: nil)
return recipients unless target.respond_to? :labels
(labels || target.labels).each do |label|
- recipients += label.subscribers
+ recipients += label.subscribers(project)
end
recipients
@@ -571,8 +571,8 @@ class NotificationService
end
end
- def relabeled_resource_email(target, labels, current_user, method)
- recipients = build_relabeled_recipients(target, current_user, labels: labels)
+ def relabeled_resource_email(target, project, labels, current_user, method)
+ recipients = build_relabeled_recipients(target, project, current_user, labels: labels)
label_names = labels.map(&:name)
recipients.each do |recipient|
@@ -608,10 +608,10 @@ class NotificationService
end
recipients = reject_muted_users(recipients, project)
- recipients = add_subscribed_users(recipients, target)
+ recipients = add_subscribed_users(recipients, project, target)
if [:new_issue, :new_merge_request].include?(custom_action)
- recipients = add_labels_subscribers(recipients, target)
+ recipients = add_labels_subscribers(recipients, project, target)
end
recipients = reject_unsubscribed_users(recipients, target)
@@ -622,8 +622,8 @@ class NotificationService
recipients.uniq
end
- def build_relabeled_recipients(target, current_user, labels:)
- recipients = add_labels_subscribers([], target, labels: labels)
+ def build_relabeled_recipients(target, project, current_user, labels:)
+ recipients = add_labels_subscribers([], project, target, labels: labels)
recipients = reject_unsubscribed_users(recipients, target)
recipients = reject_users_without_access(recipients, target)
recipients.delete(current_user)
diff --git a/app/services/slash_commands/interpret_service.rb b/app/services/slash_commands/interpret_service.rb
index 5a81194a5f4..d75c5b1800e 100644
--- a/app/services/slash_commands/interpret_service.rb
+++ b/app/services/slash_commands/interpret_service.rb
@@ -193,7 +193,7 @@ module SlashCommands
desc 'Subscribe'
condition do
issuable.persisted? &&
- !issuable.subscribed?(current_user)
+ !issuable.subscribed?(current_user, project)
end
command :subscribe do
@updates[:subscription_event] = 'subscribe'
@@ -202,7 +202,7 @@ module SlashCommands
desc 'Unsubscribe'
condition do
issuable.persisted? &&
- issuable.subscribed?(current_user)
+ issuable.subscribed?(current_user, project)
end
command :unsubscribe do
@updates[:subscription_event] = 'unsubscribe'
diff --git a/app/views/shared/_label.html.haml b/app/views/shared/_label.html.haml
index 6ccdef0df46..db324d8868e 100644
--- a/app/views/shared/_label.html.haml
+++ b/app/views/shared/_label.html.haml
@@ -1,6 +1,7 @@
- label_css_id = dom_id(label)
- open_issues_count = label.open_issues_count(current_user)
- open_merge_requests_count = label.open_merge_requests_count(current_user)
+- status = label_subscription_status(label, @project).inquiry if current_user
- subject = local_assigns[:subject]
%li{id: label_css_id, data: { id: label.id } }
@@ -18,10 +19,19 @@
%li
= link_to_label(label, subject: subject) do
= pluralize open_issues_count, 'open issue'
- - if current_user
- %li.label-subscription{ data: toggle_subscription_data(label) }
- %a.js-subscribe-button.label-subscribe-button.subscription-status{ role: "button", href: "#", data: { toggle: "tooltip", status: label_subscription_status(label) } }
- %span= label_subscription_toggle_button_text(label)
+ - if current_user && defined?(@project)
+ %li.label-subscription
+ - if label.is_a?(ProjectLabel)
+ %a.js-subscribe-button.label-subscribe-button{ role: 'button', href: '#', data: { status: status, url: toggle_subscription_namespace_project_label_path(@project.namespace, @project, label) } }
+ %span= label_subscription_toggle_button_text(label, @project)
+ - else
+ %a.js-unsubscribe-button.label-subscribe-button{ role: 'button', href: '#', class: ('hidden' if status.unsubscribed?), data: { url: group_label_unsubscribe_path(label, @project) } }
+ %span Unsubscribe
+ %a.js-subscribe-button.label-subscribe-button{ role: 'button', href: '#', class: ('hidden' unless status.unsubscribed?), data: { url: toggle_subscription_namespace_project_label_path(@project.namespace, @project, label) } }
+ %span Subscribe at project level
+ %a.js-subscribe-button.label-subscribe-button{ role: 'button', href: '#', class: ('hidden' unless status.unsubscribed?), data: { url: toggle_subscription_group_label_path(label.group, label) } }
+ %span Subscribe at group level
+
- if can?(current_user, :admin_label, label)
%li
= link_to 'Edit', edit_label_path(label)
@@ -34,12 +44,27 @@
= link_to_label(label, subject: subject, css_class: 'btn btn-transparent btn-action') do
= pluralize open_issues_count, 'open issue'
- - if current_user
- .label-subscription.inline{ data: toggle_subscription_data(label) }
- %button.js-subscribe-button.label-subscribe-button.btn.btn-transparent.btn-action.subscription-status{ type: "button", title: label_subscription_toggle_button_text(label), data: { toggle: "tooltip", status: label_subscription_status(label) } }
- %span.sr-only= label_subscription_toggle_button_text(label)
- = icon('eye', class: 'label-subscribe-button-icon', disabled: label.is_a?(GroupLabel))
- = icon('spinner spin', class: 'label-subscribe-button-loading')
+ - if current_user && defined?(@project)
+ .label-subscription.inline
+ - if label.is_a?(ProjectLabel)
+ %button.js-subscribe-button.label-subscribe-button.btn.btn-default.btn-action{ type: 'button', title: label_subscription_toggle_button_text(label, @project), data: { toggle: 'tooltip', status: status, url: toggle_subscription_namespace_project_label_path(@project.namespace, @project, label) } }
+ %span= label_subscription_toggle_button_text(label, @project)
+ = icon('spinner spin', class: 'label-subscribe-button-loading')
+ - else
+ %button.js-unsubscribe-button.label-subscribe-button.btn.btn-default.btn-action{ type: 'button', class: ('hidden' if status.unsubscribed?), title: 'Unsubscribe', data: { toggle: 'tooltip', url: group_label_unsubscribe_path(label, @project) } }
+ %span Unsubscribe
+ = icon('spinner spin', class: 'label-subscribe-button-loading')
+
+ .dropdown.dropdown-group-label{ class: ('hidden' unless status.unsubscribed?) }
+ %button.dropdown-menu-toggle{type: 'button', 'data-toggle' => 'dropdown'}
+ %span Subscribe
+ = icon('chevron-down')
+ %ul.dropdown-menu
+ %li
+ %a.js-subscribe-button{ data: { url: toggle_subscription_namespace_project_label_path(@project.namespace, @project, label) } }
+ Project level
+ %a.js-subscribe-button{ data: { url: toggle_subscription_group_label_path(label.group, label) } }
+ Group level
- if can?(current_user, :admin_label, label)
= link_to edit_label_path(label), title: "Edit", class: 'btn btn-transparent btn-action', data: {toggle: "tooltip"} do
@@ -49,6 +74,10 @@
%span.sr-only Delete
= icon('trash-o')
- - if current_user && label.is_a?(ProjectLabel)
- :javascript
- new Subscription('##{dom_id(label)} .label-subscription');
+ - if current_user && defined?(@project)
+ - if label.is_a?(ProjectLabel)
+ :javascript
+ new gl.ProjectLabelSubscription('##{dom_id(label)} .label-subscription');
+ - else
+ :javascript
+ new gl.GroupLabelSubscription('##{dom_id(label)} .label-subscription');
diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml
index 7363ead09ff..f166fac105d 100644
--- a/app/views/shared/issuable/_sidebar.html.haml
+++ b/app/views/shared/issuable/_sidebar.html.haml
@@ -140,7 +140,7 @@
= render "shared/issuable/participants", participants: issuable.participants(current_user)
- if current_user
- - subscribed = issuable.subscribed?(current_user)
+ - subscribed = issuable.subscribed?(current_user, @project)
.block.light.subscription{data: {url: toggle_subscription_path(issuable)}}
.sidebar-collapsed-icon
= icon('rss')
diff --git a/changelogs/unreleased/feature-subscribe-to-group-level-labels.yml b/changelogs/unreleased/feature-subscribe-to-group-level-labels.yml
new file mode 100644
index 00000000000..ea336716dce
--- /dev/null
+++ b/changelogs/unreleased/feature-subscribe-to-group-level-labels.yml
@@ -0,0 +1,4 @@
+---
+title: Allow users to subscribe to group labels
+merge_request: 7215
+author:
diff --git a/config/routes/group.rb b/config/routes/group.rb
index 3c392f77ef6..068e0b6e843 100644
--- a/config/routes/group.rb
+++ b/config/routes/group.rb
@@ -30,7 +30,10 @@ scope(path: 'groups/:group_id', module: :groups, as: :group) do
resource :avatar, only: [:destroy]
resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :update, :new, :create]
- resources :labels, except: [:show], constraints: { id: /\d+/ }
+
+ resources :labels, except: [:show], constraints: { id: /\d+/ } do
+ post :toggle_subscription, on: :member
+ end
end
# Must be last route in this file
diff --git a/db/migrate/20161031171301_add_project_id_to_subscriptions.rb b/db/migrate/20161031171301_add_project_id_to_subscriptions.rb
new file mode 100644
index 00000000000..97534679b59
--- /dev/null
+++ b/db/migrate/20161031171301_add_project_id_to_subscriptions.rb
@@ -0,0 +1,14 @@
+class AddProjectIdToSubscriptions < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ def up
+ add_column :subscriptions, :project_id, :integer
+ add_foreign_key :subscriptions, :projects, column: :project_id, on_delete: :cascade
+ end
+
+ def down
+ remove_column :subscriptions, :project_id
+ end
+end
diff --git a/db/migrate/20161031174110_migrate_subscriptions_project_id.rb b/db/migrate/20161031174110_migrate_subscriptions_project_id.rb
new file mode 100644
index 00000000000..549145a0a65
--- /dev/null
+++ b/db/migrate/20161031174110_migrate_subscriptions_project_id.rb
@@ -0,0 +1,44 @@
+class MigrateSubscriptionsProjectId < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = true
+ DOWNTIME_REASON = 'Subscriptions will not work as expected until this migration is complete.'
+
+ def up
+ execute <<-EOF.strip_heredoc
+ UPDATE subscriptions
+ SET project_id = (
+ SELECT issues.project_id
+ FROM issues
+ WHERE issues.id = subscriptions.subscribable_id
+ )
+ WHERE subscriptions.subscribable_type = 'Issue';
+ EOF
+
+ execute <<-EOF.strip_heredoc
+ UPDATE subscriptions
+ SET project_id = (
+ SELECT merge_requests.target_project_id
+ FROM merge_requests
+ WHERE merge_requests.id = subscriptions.subscribable_id
+ )
+ WHERE subscriptions.subscribable_type = 'MergeRequest';
+ EOF
+
+ execute <<-EOF.strip_heredoc
+ UPDATE subscriptions
+ SET project_id = (
+ SELECT projects.id
+ FROM labels INNER JOIN projects ON projects.id = labels.project_id
+ WHERE labels.id = subscriptions.subscribable_id
+ )
+ WHERE subscriptions.subscribable_type = 'Label';
+ EOF
+ end
+
+ def down
+ execute <<-EOF.strip_heredoc
+ UPDATE subscriptions SET project_id = NULL;
+ EOF
+ end
+end
diff --git a/db/migrate/20161031181638_add_unique_index_to_subscriptions.rb b/db/migrate/20161031181638_add_unique_index_to_subscriptions.rb
new file mode 100644
index 00000000000..4b1b29e1265
--- /dev/null
+++ b/db/migrate/20161031181638_add_unique_index_to_subscriptions.rb
@@ -0,0 +1,18 @@
+class AddUniqueIndexToSubscriptions < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = true
+ DOWNTIME_REASON = 'This migration requires downtime because it changes a column to not accept null values.'
+
+ disable_ddl_transaction!
+
+ def up
+ add_concurrent_index :subscriptions, [:subscribable_id, :subscribable_type, :user_id, :project_id], { unique: true, name: 'index_subscriptions_on_subscribable_and_user_id_and_project_id' }
+ remove_index :subscriptions, name: 'subscriptions_user_id_and_ref_fields' if index_name_exists?(:subscriptions, 'subscriptions_user_id_and_ref_fields', false)
+ end
+
+ def down
+ add_concurrent_index :subscriptions, [:subscribable_id, :subscribable_type, :user_id], { unique: true, name: 'subscriptions_user_id_and_ref_fields' }
+ remove_index :subscriptions, name: 'index_subscriptions_on_subscribable_and_user_id_and_project_id' if index_name_exists?(:subscriptions, 'index_subscriptions_on_subscribable_and_user_id_and_project_id', false)
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 7b04ade3400..cdd80d1f092 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1070,9 +1070,10 @@ ActiveRecord::Schema.define(version: 20161113184239) do
t.boolean "subscribed"
t.datetime "created_at"
t.datetime "updated_at"
+ t.integer "project_id"
end
- add_index "subscriptions", ["subscribable_id", "subscribable_type", "user_id"], name: "subscriptions_user_id_and_ref_fields", unique: true, using: :btree
+ add_index "subscriptions", ["subscribable_id", "subscribable_type", "user_id", "project_id"], name: "index_subscriptions_on_subscribable_and_user_id_and_project_id", unique: true, using: :btree
create_table "taggings", force: :cascade do |t|
t.integer "tag_id"
@@ -1268,6 +1269,7 @@ ActiveRecord::Schema.define(version: 20161113184239) do
add_foreign_key "personal_access_tokens", "users"
add_foreign_key "protected_branch_merge_access_levels", "protected_branches"
add_foreign_key "protected_branch_push_access_levels", "protected_branches"
+ add_foreign_key "subscriptions", "projects", on_delete: :cascade
add_foreign_key "trending_projects", "projects", on_delete: :cascade
add_foreign_key "u2f_registrations", "users"
end
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 6e370e961c4..33cb6fd3704 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -218,7 +218,7 @@ module API
expose :assignee, :author, using: Entities::UserBasic
expose :subscribed do |issue, options|
- issue.subscribed?(options[:current_user])
+ issue.subscribed?(options[:current_user], options[:project] || issue.project)
end
expose :user_notes_count
expose :upvotes, :downvotes
@@ -248,7 +248,7 @@ module API
expose :diff_head_sha, as: :sha
expose :merge_commit_sha
expose :subscribed do |merge_request, options|
- merge_request.subscribed?(options[:current_user])
+ merge_request.subscribed?(options[:current_user], options[:project])
end
expose :user_notes_count
expose :should_remove_source_branch?, as: :should_remove_source_branch
@@ -454,7 +454,7 @@ module API
end
expose :subscribed do |label, options|
- label.subscribed?(options[:current_user])
+ label.subscribed?(options[:current_user], options[:project])
end
end
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index c9689e6f8ef..eea5b91d4f9 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -120,7 +120,7 @@ module API
issues = issues.reorder(issuable_order_by => issuable_sort)
- present paginate(issues), with: Entities::Issue, current_user: current_user
+ present paginate(issues), with: Entities::Issue, current_user: current_user, project: user_project
end
# Get a single project issue
@@ -132,7 +132,7 @@ module API
# GET /projects/:id/issues/:issue_id
get ":id/issues/:issue_id" do
@issue = find_project_issue(params[:issue_id])
- present @issue, with: Entities::Issue, current_user: current_user
+ present @issue, with: Entities::Issue, current_user: current_user, project: user_project
end
# Create a new project issue
@@ -174,7 +174,7 @@ module API
end
if issue.valid?
- present issue, with: Entities::Issue, current_user: current_user
+ present issue, with: Entities::Issue, current_user: current_user, project: user_project
else
render_validation_error!(issue)
end
@@ -217,7 +217,7 @@ module API
issue = ::Issues::UpdateService.new(user_project, current_user, attrs).execute(issue)
if issue.valid?
- present issue, with: Entities::Issue, current_user: current_user
+ present issue, with: Entities::Issue, current_user: current_user, project: user_project
else
render_validation_error!(issue)
end
@@ -239,7 +239,7 @@ module API
begin
issue = ::Issues::MoveService.new(user_project, current_user).execute(issue, new_project)
- present issue, with: Entities::Issue, current_user: current_user
+ present issue, with: Entities::Issue, current_user: current_user, project: user_project
rescue ::Issues::MoveService::MoveError => error
render_api_error!(error.message, 400)
end
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index f9720786e63..4176c7eec06 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -60,7 +60,7 @@ module API
end
merge_requests = merge_requests.reorder(issuable_order_by => issuable_sort)
- present paginate(merge_requests), with: Entities::MergeRequest, current_user: current_user
+ present paginate(merge_requests), with: Entities::MergeRequest, current_user: current_user, project: user_project
end
desc 'Create a merge request' do
@@ -87,7 +87,7 @@ module API
merge_request = ::MergeRequests::CreateService.new(user_project, current_user, mr_params).execute
if merge_request.valid?
- present merge_request, with: Entities::MergeRequest, current_user: current_user
+ present merge_request, with: Entities::MergeRequest, current_user: current_user, project: user_project
else
handle_merge_request_errors! merge_request.errors
end
@@ -120,7 +120,7 @@ module API
get path do
merge_request = user_project.merge_requests.find(params[:merge_request_id])
authorize! :read_merge_request, merge_request
- present merge_request, with: Entities::MergeRequest, current_user: current_user
+ present merge_request, with: Entities::MergeRequest, current_user: current_user, project: user_project
end
desc 'Get the commits of a merge request' do
@@ -167,7 +167,7 @@ module API
merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, mr_params).execute(merge_request)
if merge_request.valid?
- present merge_request, with: Entities::MergeRequest, current_user: current_user
+ present merge_request, with: Entities::MergeRequest, current_user: current_user, project: user_project
else
handle_merge_request_errors! merge_request.errors
end
@@ -212,7 +212,7 @@ module API
execute(merge_request)
end
- present merge_request, with: Entities::MergeRequest, current_user: current_user
+ present merge_request, with: Entities::MergeRequest, current_user: current_user, project: user_project
end
desc 'Cancel merge if "Merge when build succeeds" is enabled' do
diff --git a/lib/api/milestones.rb b/lib/api/milestones.rb
index ba4a84275bc..937c118779d 100644
--- a/lib/api/milestones.rb
+++ b/lib/api/milestones.rb
@@ -114,7 +114,7 @@ module API
}
issues = IssuesFinder.new(current_user, finder_params).execute
- present paginate(issues), with: Entities::Issue, current_user: current_user
+ present paginate(issues), with: Entities::Issue, current_user: current_user, project: user_project
end
end
end
diff --git a/lib/api/subscriptions.rb b/lib/api/subscriptions.rb
index 00a79c24f96..10749b34004 100644
--- a/lib/api/subscriptions.rb
+++ b/lib/api/subscriptions.rb
@@ -24,11 +24,11 @@ module API
post ":id/#{type}/:subscribable_id/subscription" do
resource = instance_exec(params[:subscribable_id], &finder)
- if resource.subscribed?(current_user)
+ if resource.subscribed?(current_user, user_project)
not_modified!
else
- resource.subscribe(current_user)
- present resource, with: entity_class, current_user: current_user
+ resource.subscribe(current_user, user_project)
+ present resource, with: entity_class, current_user: current_user, project: user_project
end
end
@@ -38,11 +38,11 @@ module API
delete ":id/#{type}/:subscribable_id/subscription" do
resource = instance_exec(params[:subscribable_id], &finder)
- if !resource.subscribed?(current_user)
+ if !resource.subscribed?(current_user, user_project)
not_modified!
else
- resource.unsubscribe(current_user)
- present resource, with: entity_class, current_user: current_user
+ resource.unsubscribe(current_user, user_project)
+ present resource, with: entity_class, current_user: current_user, project: user_project
end
end
end
diff --git a/spec/controllers/groups/labels_controller_spec.rb b/spec/controllers/groups/labels_controller_spec.rb
new file mode 100644
index 00000000000..899d8ebd12b
--- /dev/null
+++ b/spec/controllers/groups/labels_controller_spec.rb
@@ -0,0 +1,22 @@
+require 'spec_helper'
+
+describe Groups::LabelsController do
+ let(:group) { create(:group) }
+ let(:user) { create(:user) }
+
+ before do
+ group.add_owner(user)
+
+ sign_in(user)
+ end
+
+ describe 'POST #toggle_subscription' do
+ it 'allows user to toggle subscription on group labels' do
+ label = create(:group_label, group: group)
+
+ post :toggle_subscription, group_id: group.to_param, id: label.to_param
+
+ expect(response).to have_http_status(200)
+ end
+ end
+end
diff --git a/spec/controllers/projects/boards/issues_controller_spec.rb b/spec/controllers/projects/boards/issues_controller_spec.rb
index cbe0417a4a7..299d2c981d3 100644
--- a/spec/controllers/projects/boards/issues_controller_spec.rb
+++ b/spec/controllers/projects/boards/issues_controller_spec.rb
@@ -25,7 +25,7 @@ describe Projects::Boards::IssuesController do
create(:labeled_issue, project: project, labels: [planning])
create(:labeled_issue, project: project, labels: [development], due_date: Date.tomorrow)
create(:labeled_issue, project: project, labels: [development], assignee: johndoe)
- issue.subscribe(johndoe)
+ issue.subscribe(johndoe, project)
list_issues user: user, board: board, list: list2
diff --git a/spec/controllers/projects/labels_controller_spec.rb b/spec/controllers/projects/labels_controller_spec.rb
index 8faecec0063..ec6cea5c0f4 100644
--- a/spec/controllers/projects/labels_controller_spec.rb
+++ b/spec/controllers/projects/labels_controller_spec.rb
@@ -72,14 +72,8 @@ describe Projects::LabelsController do
end
describe 'POST #generate' do
- let(:admin) { create(:admin) }
-
- before do
- sign_in(admin)
- end
-
context 'personal project' do
- let(:personal_project) { create(:empty_project) }
+ let(:personal_project) { create(:empty_project, namespace: user.namespace) }
it 'creates labels' do
post :generate, namespace_id: personal_project.namespace.to_param, project_id: personal_project.to_param
@@ -96,4 +90,26 @@ describe Projects::LabelsController do
end
end
end
+
+ describe 'POST #toggle_subscription' do
+ it 'allows user to toggle subscription on project labels' do
+ label = create(:label, project: project)
+
+ toggle_subscription(label)
+
+ expect(response).to have_http_status(200)
+ end
+
+ it 'allows user to toggle subscription on group labels' do
+ group_label = create(:group_label, group: group)
+
+ toggle_subscription(group_label)
+
+ expect(response).to have_http_status(200)
+ end
+
+ def toggle_subscription(label)
+ post :toggle_subscription, namespace_id: project.namespace.to_param, project_id: project.to_param, id: label.to_param
+ end
+ end
end
diff --git a/spec/controllers/sent_notifications_controller_spec.rb b/spec/controllers/sent_notifications_controller_spec.rb
index 191e290a118..954fc2eaf21 100644
--- a/spec/controllers/sent_notifications_controller_spec.rb
+++ b/spec/controllers/sent_notifications_controller_spec.rb
@@ -3,11 +3,11 @@ require 'rails_helper'
describe SentNotificationsController, type: :controller do
let(:user) { create(:user) }
let(:project) { create(:empty_project) }
- let(:sent_notification) { create(:sent_notification, noteable: issue, recipient: user) }
+ let(:sent_notification) { create(:sent_notification, project: project, noteable: issue, recipient: user) }
let(:issue) do
create(:issue, project: project, author: user) do |issue|
- issue.subscriptions.create(user: user, subscribed: true)
+ issue.subscriptions.create(user: user, project: project, subscribed: true)
end
end
@@ -17,7 +17,7 @@ describe SentNotificationsController, type: :controller do
before { get(:unsubscribe, id: sent_notification.reply_key, force: true) }
it 'unsubscribes the user' do
- expect(issue.subscribed?(user)).to be_falsey
+ expect(issue.subscribed?(user, project)).to be_falsey
end
it 'sets the flash message' do
@@ -33,7 +33,7 @@ describe SentNotificationsController, type: :controller do
before { get(:unsubscribe, id: sent_notification.reply_key) }
it 'does not unsubscribe the user' do
- expect(issue.subscribed?(user)).to be_truthy
+ expect(issue.subscribed?(user, project)).to be_truthy
end
it 'does not set the flash message' do
@@ -53,7 +53,7 @@ describe SentNotificationsController, type: :controller do
before { get(:unsubscribe, id: sent_notification.reply_key.reverse) }
it 'does not unsubscribe the user' do
- expect(issue.subscribed?(user)).to be_truthy
+ expect(issue.subscribed?(user, project)).to be_truthy
end
it 'does not set the flash message' do
@@ -69,7 +69,7 @@ describe SentNotificationsController, type: :controller do
before { get(:unsubscribe, id: sent_notification.reply_key, force: true) }
it 'unsubscribes the user' do
- expect(issue.subscribed?(user)).to be_falsey
+ expect(issue.subscribed?(user, project)).to be_falsey
end
it 'sets the flash message' do
@@ -85,14 +85,14 @@ describe SentNotificationsController, type: :controller do
context 'when the force param is not passed' do
let(:merge_request) do
create(:merge_request, source_project: project, author: user) do |merge_request|
- merge_request.subscriptions.create(user: user, subscribed: true)
+ merge_request.subscriptions.create(user: user, project: project, subscribed: true)
end
end
- let(:sent_notification) { create(:sent_notification, noteable: merge_request, recipient: user) }
+ let(:sent_notification) { create(:sent_notification, project: project, noteable: merge_request, recipient: user) }
before { get(:unsubscribe, id: sent_notification.reply_key) }
it 'unsubscribes the user' do
- expect(merge_request.subscribed?(user)).to be_falsey
+ expect(merge_request.subscribed?(user, project)).to be_falsey
end
it 'sets the flash message' do
diff --git a/spec/factories/subscriptions.rb b/spec/factories/subscriptions.rb
new file mode 100644
index 00000000000..b11b0a0a17b
--- /dev/null
+++ b/spec/factories/subscriptions.rb
@@ -0,0 +1,7 @@
+FactoryGirl.define do
+ factory :subscription do
+ user
+ project factory: :empty_project
+ subscribable factory: :issue
+ end
+end
diff --git a/spec/features/projects/labels/subscription_spec.rb b/spec/features/projects/labels/subscription_spec.rb
new file mode 100644
index 00000000000..3130d87fba5
--- /dev/null
+++ b/spec/features/projects/labels/subscription_spec.rb
@@ -0,0 +1,74 @@
+require 'spec_helper'
+
+feature 'Labels subscription', feature: true do
+ let(:user) { create(:user) }
+ let(:group) { create(:group) }
+ let(:project) { create(:empty_project, :public, namespace: group) }
+ let!(:bug) { create(:label, project: project, title: 'bug') }
+ let!(:feature) { create(:group_label, group: group, title: 'feature') }
+
+ context 'when signed in' do
+ before do
+ project.team << [user, :developer]
+ login_as user
+ end
+
+ scenario 'users can subscribe/unsubscribe to labels', js: true do
+ visit namespace_project_labels_path(project.namespace, project)
+
+ expect(page).to have_content('bug')
+ expect(page).to have_content('feature')
+
+ within "#project_label_#{bug.id}" do
+ expect(page).not_to have_button 'Unsubscribe'
+
+ click_button 'Subscribe'
+
+ expect(page).not_to have_button 'Subscribe'
+ expect(page).to have_button 'Unsubscribe'
+
+ click_button 'Unsubscribe'
+
+ expect(page).to have_button 'Subscribe'
+ expect(page).not_to have_button 'Unsubscribe'
+ end
+
+ within "#group_label_#{feature.id}" do
+ expect(page).not_to have_button 'Unsubscribe'
+
+ click_link_on_dropdown('Group level')
+
+ expect(page).not_to have_selector('.dropdown-group-label')
+ expect(page).to have_button 'Unsubscribe'
+
+ click_button 'Unsubscribe'
+
+ expect(page).to have_selector('.dropdown-group-label')
+
+ click_link_on_dropdown('Project level')
+
+ expect(page).not_to have_selector('.dropdown-group-label')
+ expect(page).to have_button 'Unsubscribe'
+ end
+ end
+ end
+
+ context 'when not signed in' do
+ it 'users can not subscribe/unsubscribe to labels' do
+ visit namespace_project_labels_path(project.namespace, project)
+
+ expect(page).to have_content 'bug'
+ expect(page).to have_content 'feature'
+ expect(page).not_to have_button('Subscribe')
+ expect(page).not_to have_selector('.dropdown-group-label')
+ end
+ end
+
+ def click_link_on_dropdown(text)
+ find('.dropdown-group-label').click
+
+ page.within('.dropdown-group-label') do
+ find('a.js-subscribe-button', text: text).click
+ end
+ end
+end
diff --git a/spec/features/unsubscribe_links_spec.rb b/spec/features/unsubscribe_links_spec.rb
index 33b52d1547e..e2d9cfdd0b0 100644
--- a/spec/features/unsubscribe_links_spec.rb
+++ b/spec/features/unsubscribe_links_spec.rb
@@ -26,11 +26,11 @@ describe 'Unsubscribe links', feature: true do
expect(current_path).to eq unsubscribe_sent_notification_path(SentNotification.last)
expect(page).to have_text(%(Unsubscribe from issue #{issue.title} (#{issue.to_reference})))
expect(page).to have_text(%(Are you sure you want to unsubscribe from issue #{issue.title} (#{issue.to_reference})?))
- expect(issue.subscribed?(recipient)).to be_truthy
+ expect(issue.subscribed?(recipient, project)).to be_truthy
click_link 'Unsubscribe'
- expect(issue.subscribed?(recipient)).to be_falsey
+ expect(issue.subscribed?(recipient, project)).to be_falsey
expect(current_path).to eq new_user_session_path
end
@@ -38,11 +38,11 @@ describe 'Unsubscribe links', feature: true do
visit body_link
expect(current_path).to eq unsubscribe_sent_notification_path(SentNotification.last)
- expect(issue.subscribed?(recipient)).to be_truthy
+ expect(issue.subscribed?(recipient, project)).to be_truthy
click_link 'Cancel'
- expect(issue.subscribed?(recipient)).to be_truthy
+ expect(issue.subscribed?(recipient, project)).to be_truthy
expect(current_path).to eq new_user_session_path
end
end
@@ -51,7 +51,7 @@ describe 'Unsubscribe links', feature: true do
visit header_link
expect(page).to have_text('unsubscribed')
- expect(issue.subscribed?(recipient)).to be_falsey
+ expect(issue.subscribed?(recipient, project)).to be_falsey
end
end
@@ -62,14 +62,14 @@ describe 'Unsubscribe links', feature: true do
visit body_link
expect(page).to have_text('unsubscribed')
- expect(issue.subscribed?(recipient)).to be_falsey
+ expect(issue.subscribed?(recipient, project)).to be_falsey
end
it 'unsubscribes from the issue when visiting the link from the header' do
visit header_link
expect(page).to have_text('unsubscribed')
- expect(issue.subscribed?(recipient)).to be_falsey
+ expect(issue.subscribed?(recipient, project)).to be_falsey
end
end
end
diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb
index 6e987967ca5..6f84bffe046 100644
--- a/spec/models/concerns/issuable_spec.rb
+++ b/spec/models/concerns/issuable_spec.rb
@@ -176,23 +176,25 @@ describe Issue, "Issuable" do
end
describe '#subscribed?' do
+ let(:project) { issue.project }
+
context 'user is not a participant in the issue' do
before { allow(issue).to receive(:participants).with(user).and_return([]) }
it 'returns false when no subcription exists' do
- expect(issue.subscribed?(user)).to be_falsey
+ expect(issue.subscribed?(user, project)).to be_falsey
end
it 'returns true when a subcription exists and subscribed is true' do
- issue.subscriptions.create(user: user, subscribed: true)
+ issue.subscriptions.create(user: user, project: project, subscribed: true)
- expect(issue.subscribed?(user)).to be_truthy
+ expect(issue.subscribed?(user, project)).to be_truthy
end
it 'returns false when a subcription exists and subscribed is false' do
- issue.subscriptions.create(user: user, subscribed: false)
+ issue.subscriptions.create(user: user, project: project, subscribed: false)
- expect(issue.subscribed?(user)).to be_falsey
+ expect(issue.subscribed?(user, project)).to be_falsey
end
end
@@ -200,19 +202,19 @@ describe Issue, "Issuable" do
before { allow(issue).to receive(:participants).with(user).and_return([user]) }
it 'returns false when no subcription exists' do
- expect(issue.subscribed?(user)).to be_truthy
+ expect(issue.subscribed?(user, project)).to be_truthy
end
it 'returns true when a subcription exists and subscribed is true' do
- issue.subscriptions.create(user: user, subscribed: true)
+ issue.subscriptions.create(user: user, project: project, subscribed: true)
- expect(issue.subscribed?(user)).to be_truthy
+ expect(issue.subscribed?(user, project)).to be_truthy
end
it 'returns false when a subcription exists and subscribed is false' do
- issue.subscriptions.create(user: user, subscribed: false)
+ issue.subscriptions.create(user: user, project: project, subscribed: false)
- expect(issue.subscribed?(user)).to be_falsey
+ expect(issue.subscribed?(user, project)).to be_falsey
end
end
end
diff --git a/spec/models/concerns/subscribable_spec.rb b/spec/models/concerns/subscribable_spec.rb
index b7fc5a92497..58f5c164116 100644
--- a/spec/models/concerns/subscribable_spec.rb
+++ b/spec/models/concerns/subscribable_spec.rb
@@ -1,67 +1,128 @@
require 'spec_helper'
describe Subscribable, 'Subscribable' do
- let(:resource) { create(:issue) }
- let(:user) { create(:user) }
+ let(:project) { create(:empty_project) }
+ let(:resource) { create(:issue, project: project) }
+ let(:user_1) { create(:user) }
describe '#subscribed?' do
- it 'returns false when no subcription exists' do
- expect(resource.subscribed?(user)).to be_falsey
- end
+ context 'without project' do
+ it 'returns false when no subscription exists' do
+ expect(resource.subscribed?(user_1)).to be_falsey
+ end
+
+ it 'returns true when a subcription exists and subscribed is true' do
+ resource.subscriptions.create(user: user_1, subscribed: true)
+
+ expect(resource.subscribed?(user_1)).to be_truthy
+ end
- it 'returns true when a subcription exists and subscribed is true' do
- resource.subscriptions.create(user: user, subscribed: true)
+ it 'returns false when a subcription exists and subscribed is false' do
+ resource.subscriptions.create(user: user_1, subscribed: false)
- expect(resource.subscribed?(user)).to be_truthy
+ expect(resource.subscribed?(user_1)).to be_falsey
+ end
end
- it 'returns false when a subcription exists and subscribed is false' do
- resource.subscriptions.create(user: user, subscribed: false)
+ 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
- expect(resource.subscribed?(user)).to be_falsey
+ 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
+
describe '#subscribers' do
it 'returns [] when no subcribers exists' do
- expect(resource.subscribers).to be_empty
+ expect(resource.subscribers(project)).to be_empty
end
it 'returns the subscribed users' do
- resource.subscriptions.create(user: user, subscribed: true)
- resource.subscriptions.create(user: create(:user), subscribed: false)
+ 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)
- expect(resource.subscribers).to eq [user]
+ expect(resource.subscribers(project)).to contain_exactly(user_1, user_2)
end
end
describe '#toggle_subscription' do
- it 'toggles the current subscription state for the given user' do
- expect(resource.subscribed?(user)).to be_falsey
+ context 'without project' do
+ it 'toggles the current subscription state for the given user' do
+ expect(resource.subscribed?(user_1)).to be_falsey
- resource.toggle_subscription(user)
+ resource.toggle_subscription(user_1)
- expect(resource.subscribed?(user)).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
describe '#subscribe' do
- it 'subscribes the given user' do
- expect(resource.subscribed?(user)).to be_falsey
+ 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
+ expect(resource.subscribed?(user_1, project)).to be_falsey
- resource.subscribe(user)
+ resource.subscribe(user_1, project)
- expect(resource.subscribed?(user)).to be_truthy
+ expect(resource.subscribed?(user_1, project)).to be_truthy
+ end
end
end
describe '#unsubscribe' do
- it 'unsubscribes the given current user' do
- resource.subscriptions.create(user: user, subscribed: true)
- expect(resource.subscribed?(user)).to be_truthy
+ 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
+ resource.subscriptions.create(user: user_1, project: project, subscribed: true)
+ expect(resource.subscribed?(user_1, project)).to be_truthy
- resource.unsubscribe(user)
+ resource.unsubscribe(user_1, project)
- expect(resource.subscribed?(user)).to be_falsey
+ expect(resource.subscribed?(user_1, project)).to be_falsey
+ end
end
end
end
diff --git a/spec/models/subscription_spec.rb b/spec/models/subscription_spec.rb
new file mode 100644
index 00000000000..9ab112bb2ee
--- /dev/null
+++ b/spec/models/subscription_spec.rb
@@ -0,0 +1,20 @@
+require 'spec_helper'
+
+describe Subscription, models: true do
+ describe 'relationships' do
+ it { is_expected.to belong_to(:project) }
+ it { is_expected.to belong_to(:subscribable) }
+ it { is_expected.to belong_to(:user) }
+ end
+
+ describe 'validations' do
+ it { is_expected.to validate_presence_of(:subscribable) }
+ it { is_expected.to validate_presence_of(:user) }
+
+ it 'validates uniqueness of project_id scoped to subscribable_id, subscribable_type, and user_id' do
+ create(:subscription)
+
+ expect(subject).to validate_uniqueness_of(:project_id).scoped_to([:subscribable_id, :subscribable_type, :user_id])
+ end
+ end
+end
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index beed53d1e5c..7bae055b241 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -637,7 +637,7 @@ describe API::API, api: true do
it "sends notifications for subscribers of newly added labels" do
label = project.labels.first
- label.toggle_subscription(user2)
+ label.toggle_subscription(user2, project)
perform_enqueued_jobs do
post api("/projects/#{project.id}/issues", user),
@@ -828,7 +828,7 @@ describe API::API, api: true do
it "sends notifications for subscribers of newly added labels when issue is updated" do
label = create(:label, title: 'foo', color: '#FFAABB', project: project)
- label.toggle_subscription(user2)
+ label.toggle_subscription(user2, project)
perform_enqueued_jobs do
put api("/projects/#{project.id}/issues/#{issue.id}", user),
diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb
index 77dfebf1a98..aaf41639277 100644
--- a/spec/requests/api/labels_spec.rb
+++ b/spec/requests/api/labels_spec.rb
@@ -339,7 +339,7 @@ describe API::API, api: true do
end
context "when user is already subscribed to label" do
- before { label1.subscribe(user) }
+ before { label1.subscribe(user, project) }
it "returns 304" do
post api("/projects/#{project.id}/labels/#{label1.id}/subscription", user)
@@ -358,7 +358,7 @@ describe API::API, api: true do
end
describe "DELETE /projects/:id/labels/:label_id/subscription" do
- before { label1.subscribe(user) }
+ before { label1.subscribe(user, project) }
context "when label_id is a label title" do
it "unsubscribes from the label" do
@@ -381,7 +381,7 @@ describe API::API, api: true do
end
context "when user is already unsubscribed from label" do
- before { label1.unsubscribe(user) }
+ before { label1.unsubscribe(user, project) }
it "returns 304" do
delete api("/projects/#{project.id}/labels/#{label1.id}/subscription", user)
diff --git a/spec/services/issuable/bulk_update_service_spec.rb b/spec/services/issuable/bulk_update_service_spec.rb
index 6f7ce8ca992..5f3020b6525 100644
--- a/spec/services/issuable/bulk_update_service_spec.rb
+++ b/spec/services/issuable/bulk_update_service_spec.rb
@@ -260,14 +260,14 @@ describe Issuable::BulkUpdateService, services: true do
it 'subscribes the given user' do
bulk_update(issues, subscription_event: 'subscribe')
- expect(issues).to all(be_subscribed(user))
+ expect(issues).to all(be_subscribed(user, project))
end
end
describe 'unsubscribe from issues' do
let(:issues) do
create_list(:closed_issue, 2, project: project) do |issue|
- issue.subscriptions.create(user: user, subscribed: true)
+ issue.subscriptions.create(user: user, project: project, subscribed: true)
end
end
@@ -275,7 +275,7 @@ describe Issuable::BulkUpdateService, services: true do
bulk_update(issues, subscription_event: 'unsubscribe')
issues.each do |issue|
- expect(issue).not_to be_subscribed(user)
+ expect(issue).not_to be_subscribed(user, project)
end
end
end
diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index 1638a46ed51..4777a90639e 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -215,7 +215,7 @@ describe Issues::UpdateService, services: true do
let!(:subscriber) do
create(:user).tap do |u|
- label.toggle_subscription(u)
+ label.toggle_subscription(u, project)
project.team << [u, :developer]
end
end
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index 2433a7dad06..cb5d7cdb467 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -199,7 +199,7 @@ describe MergeRequests::UpdateService, services: true do
context 'when the issue is relabeled' do
let!(:non_subscriber) { create(:user) }
- let!(:subscriber) { create(:user).tap { |u| label.toggle_subscription(u) } }
+ let!(:subscriber) { create(:user) { |u| label.toggle_subscription(u, project) } }
before do
project.team << [non_subscriber, :developer]
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 8ce35354c22..8726c9eaa55 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -342,7 +342,9 @@ describe NotificationService, services: true do
end
describe 'Issues' do
- let(:project) { create(:empty_project, :public) }
+ let(:group) { create(:group) }
+ let(:project) { create(:empty_project, :public, namespace: group) }
+ let(:another_project) { create(:empty_project, :public, namespace: group) }
let(:issue) { create :issue, project: project, assignee: create(:user), description: 'cc @participant' }
before do
@@ -377,13 +379,24 @@ describe NotificationService, services: true do
end
it "emails subscribers of the issue's labels" do
- subscriber = create(:user)
- label = create(:label, issues: [issue])
+ user_1 = create(:user)
+ user_2 = create(:user)
+ user_3 = create(:user)
+ user_4 = create(:user)
+ label = create(:label, project: project, issues: [issue])
+ group_label = create(:group_label, group: group, issues: [issue])
issue.reload
- label.toggle_subscription(subscriber)
+ label.toggle_subscription(user_1, project)
+ group_label.toggle_subscription(user_2, project)
+ group_label.toggle_subscription(user_3, another_project)
+ group_label.toggle_subscription(user_4)
+
notification.new_issue(issue, @u_disabled)
- should_email(subscriber)
+ should_email(user_1)
+ should_email(user_2)
+ should_not_email(user_3)
+ should_email(user_4)
end
context 'confidential issues' do
@@ -399,14 +412,14 @@ describe NotificationService, services: true do
project.team << [member, :developer]
project.team << [guest, :guest]
- label = create(:label, issues: [confidential_issue])
+ label = create(:label, project: project, issues: [confidential_issue])
confidential_issue.reload
- label.toggle_subscription(non_member)
- label.toggle_subscription(author)
- label.toggle_subscription(assignee)
- label.toggle_subscription(member)
- label.toggle_subscription(guest)
- label.toggle_subscription(admin)
+ label.toggle_subscription(non_member, project)
+ label.toggle_subscription(author, project)
+ label.toggle_subscription(assignee, project)
+ label.toggle_subscription(member, project)
+ label.toggle_subscription(guest, project)
+ label.toggle_subscription(admin, project)
reset_delivered_emails!
@@ -554,20 +567,30 @@ describe NotificationService, services: true do
end
describe '#relabeled_issue' do
- let(:label) { create(:label, issues: [issue]) }
- let(:label2) { create(:label) }
- let!(:subscriber_to_label) { create(:user).tap { |u| label.toggle_subscription(u) } }
- let!(:subscriber_to_label2) { create(:user).tap { |u| label2.toggle_subscription(u) } }
+ let(:group_label_1) { create(:group_label, group: group, title: 'Group Label 1', issues: [issue]) }
+ let(:group_label_2) { create(:group_label, group: group, title: 'Group Label 2') }
+ let(:label_1) { create(:label, project: project, title: 'Label 1', issues: [issue]) }
+ 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_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_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) } }
it "emails subscribers of the issue's added labels only" do
- notification.relabeled_issue(issue, [label2], @u_disabled)
-
- should_not_email(subscriber_to_label)
- should_email(subscriber_to_label2)
+ notification.relabeled_issue(issue, [group_label_2, label_2], @u_disabled)
+
+ should_not_email(subscriber_to_label_1)
+ should_not_email(subscriber_to_group_label_1)
+ should_not_email(subscriber_to_group_label_2_on_another_project)
+ should_email(subscriber_1_to_group_label_2)
+ should_email(subscriber_2_to_group_label_2)
+ should_email(subscriber_to_label_2)
end
it "doesn't send email to anyone but subscribers of the given labels" do
- notification.relabeled_issue(issue, [label2], @u_disabled)
+ notification.relabeled_issue(issue, [group_label_2, label_2], @u_disabled)
should_not_email(issue.assignee)
should_not_email(issue.author)
@@ -578,8 +601,12 @@ describe NotificationService, services: true do
should_not_email(@watcher_and_subscriber)
should_not_email(@unsubscriber)
should_not_email(@u_participating)
- should_not_email(subscriber_to_label)
- should_email(subscriber_to_label2)
+ should_not_email(subscriber_to_label_1)
+ should_not_email(subscriber_to_group_label_1)
+ should_not_email(subscriber_to_group_label_2_on_another_project)
+ should_email(subscriber_1_to_group_label_2)
+ should_email(subscriber_2_to_group_label_2)
+ should_email(subscriber_to_label_2)
end
context 'confidential issues' do
@@ -590,19 +617,19 @@ describe NotificationService, services: true do
let(:guest) { create(:user) }
let(:admin) { create(:admin) }
let(:confidential_issue) { create(:issue, :confidential, project: project, title: 'Confidential issue', author: author, assignee: assignee) }
- let!(:label_1) { create(:label, issues: [confidential_issue]) }
- let!(:label_2) { create(:label) }
+ let!(:label_1) { create(:label, project: project, issues: [confidential_issue]) }
+ let!(:label_2) { create(:label, project: project) }
it "emails subscribers of the issue's labels that can read the issue" do
project.team << [member, :developer]
project.team << [guest, :guest]
- label_2.toggle_subscription(non_member)
- label_2.toggle_subscription(author)
- label_2.toggle_subscription(assignee)
- label_2.toggle_subscription(member)
- label_2.toggle_subscription(guest)
- label_2.toggle_subscription(admin)
+ label_2.toggle_subscription(non_member, project)
+ label_2.toggle_subscription(author, project)
+ label_2.toggle_subscription(assignee, project)
+ label_2.toggle_subscription(member, project)
+ label_2.toggle_subscription(guest, project)
+ label_2.toggle_subscription(admin, project)
reset_delivered_emails!
@@ -725,7 +752,9 @@ describe NotificationService, services: true do
end
describe 'Merge Requests' do
- let(:project) { create(:project, :public) }
+ let(:group) { create(:group) }
+ let(:project) { create(:project, :public, namespace: group) }
+ let(:another_project) { create(:empty_project, :public, namespace: group) }
let(:merge_request) { create :merge_request, source_project: project, assignee: create(:user), description: 'cc @participant' }
before do
@@ -758,12 +787,23 @@ describe NotificationService, services: true do
end
it "emails subscribers of the merge request's labels" do
- subscriber = create(:user)
- label = create(:label, merge_requests: [merge_request])
- label.toggle_subscription(subscriber)
+ user_1 = create(:user)
+ user_2 = create(:user)
+ user_3 = create(:user)
+ user_4 = create(:user)
+ label = create(:label, project: project, merge_requests: [merge_request])
+ group_label = create(:group_label, group: group, merge_requests: [merge_request])
+ label.toggle_subscription(user_1, project)
+ group_label.toggle_subscription(user_2, project)
+ group_label.toggle_subscription(user_3, another_project)
+ group_label.toggle_subscription(user_4)
+
notification.new_merge_request(merge_request, @u_disabled)
- should_email(subscriber)
+ should_email(user_1)
+ should_email(user_2)
+ should_not_email(user_3)
+ should_email(user_4)
end
context 'participating' do
@@ -857,20 +897,30 @@ describe NotificationService, services: true do
end
describe '#relabel_merge_request' do
- let(:label) { create(:label, merge_requests: [merge_request]) }
- let(:label2) { create(:label) }
- let!(:subscriber_to_label) { create(:user).tap { |u| label.toggle_subscription(u) } }
- let!(:subscriber_to_label2) { create(:user).tap { |u| label2.toggle_subscription(u) } }
+ let(:group_label_1) { create(:group_label, group: group, title: 'Group Label 1', merge_requests: [merge_request]) }
+ let(:group_label_2) { create(:group_label, group: group, title: 'Group Label 2') }
+ 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!(:subscriber_to_group_label_1) { create(:user) { |u| group_label_1.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_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) } }
it "emails subscribers of the merge request's added labels only" do
- notification.relabeled_merge_request(merge_request, [label2], @u_disabled)
-
- should_not_email(subscriber_to_label)
- should_email(subscriber_to_label2)
+ notification.relabeled_merge_request(merge_request, [group_label_2, label_2], @u_disabled)
+
+ should_not_email(subscriber_to_label_1)
+ should_not_email(subscriber_to_group_label_1)
+ should_not_email(subscriber_to_group_label_2_on_another_project)
+ should_email(subscriber_1_to_group_label_2)
+ should_email(subscriber_2_to_group_label_2)
+ should_email(subscriber_to_label_2)
end
it "doesn't send email to anyone but subscribers of the given labels" do
- notification.relabeled_merge_request(merge_request, [label2], @u_disabled)
+ notification.relabeled_merge_request(merge_request, [group_label_2, label_2], @u_disabled)
should_not_email(merge_request.assignee)
should_not_email(merge_request.author)
@@ -881,8 +931,12 @@ describe NotificationService, services: true do
should_not_email(@unsubscriber)
should_not_email(@u_participating)
should_not_email(@u_lazy_participant)
- should_not_email(subscriber_to_label)
- should_email(subscriber_to_label2)
+ should_not_email(subscriber_to_label_1)
+ should_not_email(subscriber_to_group_label_1)
+ should_not_email(subscriber_to_group_label_2_on_another_project)
+ should_email(subscriber_1_to_group_label_2)
+ should_email(subscriber_2_to_group_label_2)
+ should_email(subscriber_to_label_2)
end
end
@@ -1290,10 +1344,10 @@ describe NotificationService, services: true do
project.team << [@unsubscriber, :master]
project.team << [@watcher_and_subscriber, :master]
- issuable.subscriptions.create(user: @subscriber, subscribed: true)
- issuable.subscriptions.create(user: @subscribed_participant, subscribed: true)
- issuable.subscriptions.create(user: @unsubscriber, subscribed: false)
+ issuable.subscriptions.create(user: @subscriber, project: project, subscribed: true)
+ issuable.subscriptions.create(user: @subscribed_participant, project: project, subscribed: true)
+ issuable.subscriptions.create(user: @unsubscriber, project: project, subscribed: false)
# Make the watcher a subscriber to detect dupes
- issuable.subscriptions.create(user: @watcher_and_subscriber, subscribed: true)
+ issuable.subscriptions.create(user: @watcher_and_subscriber, project: project, subscribed: true)
end
end
diff --git a/spec/services/slash_commands/interpret_service_spec.rb b/spec/services/slash_commands/interpret_service_spec.rb
index b57e338b782..becf627a4f5 100644
--- a/spec/services/slash_commands/interpret_service_spec.rb
+++ b/spec/services/slash_commands/interpret_service_spec.rb
@@ -169,7 +169,7 @@ describe SlashCommands::InterpretService, services: true do
shared_examples 'unsubscribe command' do
it 'populates subscription_event: "unsubscribe" if content contains /unsubscribe' do
- issuable.subscribe(developer)
+ issuable.subscribe(developer, project)
_, updates = service.execute(content, issuable)
expect(updates).to eq(subscription_event: 'unsubscribe')
@@ -321,7 +321,7 @@ describe SlashCommands::InterpretService, services: true do
it_behaves_like 'multiple label with same argument' do
let(:content) { %(/label ~"#{inprogress.title}" \n/label ~#{inprogress.title}) }
let(:issuable) { issue }
- end
+ end
it_behaves_like 'unlabel command' do
let(:content) { %(/unlabel ~"#{inprogress.title}") }
diff --git a/spec/support/features/issuable_slash_commands_shared_examples.rb b/spec/support/features/issuable_slash_commands_shared_examples.rb
index 5e3b8f2b23e..194620d0a68 100644
--- a/spec/support/features/issuable_slash_commands_shared_examples.rb
+++ b/spec/support/features/issuable_slash_commands_shared_examples.rb
@@ -230,31 +230,31 @@ shared_examples 'issuable record that supports slash commands in its description
context "with a note subscribing to the #{issuable_type}" do
it "creates a new todo for the #{issuable_type}" do
- expect(issuable.subscribed?(master)).to be_falsy
+ expect(issuable.subscribed?(master, project)).to be_falsy
write_note("/subscribe")
expect(page).not_to have_content '/subscribe'
expect(page).to have_content 'Your commands have been executed!'
- expect(issuable.subscribed?(master)).to be_truthy
+ expect(issuable.subscribed?(master, project)).to be_truthy
end
end
context "with a note unsubscribing to the #{issuable_type} as done" do
before do
- issuable.subscribe(master)
+ issuable.subscribe(master, project)
end
it "creates a new todo for the #{issuable_type}" do
- expect(issuable.subscribed?(master)).to be_truthy
+ expect(issuable.subscribed?(master, project)).to be_truthy
write_note("/unsubscribe")
expect(page).not_to have_content '/unsubscribe'
expect(page).to have_content 'Your commands have been executed!'
- expect(issuable.subscribed?(master)).to be_falsy
+ expect(issuable.subscribed?(master, project)).to be_falsy
end
end
end