Commit d9f9be3c authored by Grzegorz Bizon's avatar Grzegorz Bizon

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
  ...
parents 8a7860fd 3344e1e8
/* 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 = {}));
/* 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 = {}));
...@@ -90,7 +90,7 @@ ...@@ -90,7 +90,7 @@
@media (min-width: $screen-sm-min) { @media (min-width: $screen-sm-min) {
display: inline-block; display: inline-block;
width: 40%; width: 30%;
margin-left: 10px; margin-left: 10px;
margin-bottom: 0; margin-bottom: 0;
vertical-align: middle; vertical-align: middle;
...@@ -222,6 +222,14 @@ ...@@ -222,6 +222,14 @@
width: 100%; width: 100%;
} }
.label-subscription {
vertical-align: middle;
.dropdown-group-label a {
cursor: pointer;
}
}
.label-subscribe-button { .label-subscribe-button {
.label-subscribe-button-icon { .label-subscribe-button-icon {
&[disabled] { &[disabled] {
......
...@@ -4,13 +4,17 @@ module ToggleSubscriptionAction ...@@ -4,13 +4,17 @@ module ToggleSubscriptionAction
def toggle_subscription def toggle_subscription
return unless current_user return unless current_user
subscribable_resource.toggle_subscription(current_user) subscribable_resource.toggle_subscription(current_user, subscribable_project)
head :ok head :ok
end end
private private
def subscribable_project
@project || raise(NotImplementedError)
end
def subscribable_resource def subscribable_resource
raise NotImplementedError raise NotImplementedError
end end
......
class Groups::LabelsController < Groups::ApplicationController class Groups::LabelsController < Groups::ApplicationController
include ToggleSubscriptionAction
before_action :label, only: [:edit, :update, :destroy] before_action :label, only: [:edit, :update, :destroy]
before_action :authorize_admin_labels!, only: [:new, :create, :edit, :update, :destroy] before_action :authorize_admin_labels!, only: [:new, :create, :edit, :update, :destroy]
before_action :save_previous_label_path, only: [:edit] before_action :save_previous_label_path, only: [:edit]
...@@ -69,6 +71,11 @@ class Groups::LabelsController < Groups::ApplicationController ...@@ -69,6 +71,11 @@ class Groups::LabelsController < Groups::ApplicationController
def label def label
@label ||= @group.labels.find(params[:id]) @label ||= @group.labels.find(params[:id])
end end
alias_method :subscribable_resource, :label
def subscribable_project
nil
end
def label_params def label_params
params.require(:label).permit(:title, :description, :color) params.require(:label).permit(:title, :description, :color)
......
...@@ -3,7 +3,7 @@ class Projects::LabelsController < Projects::ApplicationController ...@@ -3,7 +3,7 @@ class Projects::LabelsController < Projects::ApplicationController
before_action :module_enabled before_action :module_enabled
before_action :label, only: [:edit, :update, :destroy] 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_read_label!
before_action :authorize_admin_labels!, only: [:new, :create, :edit, :update, before_action :authorize_admin_labels!, only: [:new, :create, :edit, :update,
:generate, :destroy, :remove_priority, :generate, :destroy, :remove_priority,
...@@ -123,7 +123,10 @@ class Projects::LabelsController < Projects::ApplicationController ...@@ -123,7 +123,10 @@ class Projects::LabelsController < Projects::ApplicationController
def label def label
@label ||= @project.labels.find(params[:id]) @label ||= @project.labels.find(params[:id])
end end
alias_method :subscribable_resource, :label
def subscribable_resource
@available_labels.find(params[:id])
end
def find_labels def find_labels
@available_labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute @available_labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute
......
...@@ -12,7 +12,7 @@ class SentNotificationsController < ApplicationController ...@@ -12,7 +12,7 @@ class SentNotificationsController < ApplicationController
def unsubscribe_and_redirect def unsubscribe_and_redirect
noteable = @sent_notification.noteable 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." flash[:notice] = "You have been unsubscribed from this thread."
......
...@@ -68,14 +68,6 @@ module LabelsHelper ...@@ -68,14 +68,6 @@ module LabelsHelper
end end
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) def render_colored_label(label, label_suffix = '', tooltip: true)
label_color = label.color || Label::DEFAULT_COLOR label_color = label.color || Label::DEFAULT_COLOR
text_color = text_color_for_bg(label_color) text_color = text_color_for_bg(label_color)
...@@ -148,20 +140,24 @@ module LabelsHelper ...@@ -148,20 +140,24 @@ module LabelsHelper
end end
end end
def label_subscription_status(label) def label_subscription_status(label, project)
case label return 'project-level' if label.subscribed?(current_user, project)
when GroupLabel then 'Subscribing to group labels is currently not supported.' return 'group-level' if label.subscribed?(current_user)
when ProjectLabel then label.subscribed?(current_user) ? 'subscribed' : 'unsubscribed'
end 'unsubscribed'
end end
def label_subscription_toggle_button_text(label) def group_label_unsubscribe_path(label, project)
case label case label_subscription_status(label, project)
when GroupLabel then 'Subscribing to group labels is currently not supported.' when 'project-level' then toggle_subscription_namespace_project_label_path(@project.namespace, @project, label)
when ProjectLabel then label.subscribed?(current_user) ? 'Unsubscribe' : 'Subscribe' when 'group-level' then toggle_subscription_group_label_path(label.group, label)
end end
end end
def label_subscription_toggle_button_text(label, project)
label.subscribed?(current_user, project) ? 'Unsubscribe' : 'Subscribe'
end
def label_deletion_confirm_text(label) def label_deletion_confirm_text(label)
case label case label
when GroupLabel then 'Remove this label? This will affect all projects within the group. Are you sure?' when GroupLabel then 'Remove this label? This will affect all projects within the group. Are you sure?'
......
...@@ -215,7 +215,7 @@ module Issuable ...@@ -215,7 +215,7 @@ module Issuable
end end
end end
def subscribed_without_subscriptions?(user) def subscribed_without_subscriptions?(user, project)
participants(user).include?(user) participants(user).include?(user)
end end
......
...@@ -12,39 +12,71 @@ module Subscribable ...@@ -12,39 +12,71 @@ module Subscribable
has_many :subscriptions, dependent: :destroy, as: :subscribable has_many :subscriptions, dependent: :destroy, as: :subscribable
end end
def subscribed?(user) def subscribed?(user, project = nil)
if subscription = subscriptions.find_by_user_id(user.id) if subscription = subscriptions.find_by(user: user, project: project)
subscription.subscribed subscription.subscribed
else else
subscribed_without_subscriptions?(user) subscribed_without_subscriptions?(user, project)
end end
end end
# Override this method to define custom logic to consider a subscribable as # Override this method to define custom logic to consider a subscribable as
# subscribed without an explicit subscription record. # subscribed without an explicit subscription record.
def subscribed_without_subscriptions?(user) def subscribed_without_subscriptions?(user, project)
false false
end end
def subscribers def subscribers(project)
subscriptions.where(subscribed: true).map(&:user) subscriptions_available(project).
where(subscribed: true).
map(&:user)
end end
def toggle_subscription(user) def toggle_subscription(user, project = nil)
subscriptions. unsubscribe_from_other_levels(user, project)
find_or_initialize_by(user_id: user.id).
update(subscribed: !subscribed?(user)) 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 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. subscriptions.
find_or_initialize_by(user_id: user.id). find_or_initialize_by(user_id: user.id, project_id: project.try(:id))
update(subscribed: true)
end end
def unsubscribe(user) def subscriptions_available(project)
t = Subscription.arel_table
subscriptions. subscriptions.
find_or_initialize_by(user_id: user.id). where(t[:project_id].eq(nil).or(t[:project_id].eq(project.try(:id))))
update(subscribed: false)
end end
end end
...@@ -266,7 +266,7 @@ class Issue < ActiveRecord::Base ...@@ -266,7 +266,7 @@ class Issue < ActiveRecord::Base
def as_json(options = {}) def as_json(options = {})
super(options).tap do |json| 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) if options.has_key?(:labels)
json[:labels] = labels.as_json( json[:labels] = labels.as_json(
......
class Subscription < ActiveRecord::Base class Subscription < ActiveRecord::Base
belongs_to :user belongs_to :user
belongs_to :project
belongs_to :subscribable, polymorphic: true belongs_to :subscribable, polymorphic: true
validates :user_id, validates :user, :subscribable, presence: true
uniqueness: { scope: [:subscribable_id, :subscribable_type] },
presence: true validates :project_id, uniqueness: { scope: [:subscribable_id, :subscribable_type, :user_id] }
end end
...@@ -212,9 +212,9 @@ class IssuableBaseService < BaseService ...@@ -212,9 +212,9 @@ class IssuableBaseService < BaseService
def change_subscription(issuable) def change_subscription(issuable)
case params.delete(:subscription_event) case params.delete(:subscription_event)
when 'subscribe' when 'subscribe'
issuable.subscribe(current_user) issuable.subscribe(current_user, project)
when 'unsubscribe' when 'unsubscribe'
issuable.unsubscribe(current_user) issuable.unsubscribe(current_user, project)
end end
end end
......
...@@ -75,7 +75,7 @@ class NotificationService ...@@ -75,7 +75,7 @@ class NotificationService
# * watchers of the issue's labels # * watchers of the issue's labels
# #
def relabeled_issue(issue, added_labels, current_user) 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 end
# When create a merge request we should send an email to: # When create a merge request we should send an email to:
...@@ -118,7 +118,7 @@ class NotificationService ...@@ -118,7 +118,7 @@ class NotificationService
# * watchers of the mr's labels # * watchers of the mr's labels
# #
def relabeled_merge_request(merge_request, added_labels, current_user) 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 end
def close_mr(merge_request, current_user) def close_mr(merge_request, current_user)
...@@ -205,7 +205,7 @@ class NotificationService ...@@ -205,7 +205,7 @@ class NotificationService
recipients = reject_muted_users(recipients, note.project) 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_unsubscribed_users(recipients, note.noteable)
recipients = reject_users_without_access(recipients, note.noteable) recipients = reject_users_without_access(recipients, note.noteable)
...@@ -393,7 +393,7 @@ class NotificationService ...@@ -393,7 +393,7 @@ class NotificationService
) )
end 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) def select_project_member_setting(project, global_setting, users_global_level_watch)
users = notification_settings_for(project, :watch) users = notification_settings_for(project, :watch)
...@@ -505,17 +505,17 @@ class NotificationService ...@@ -505,17 +505,17 @@ class NotificationService
end end
end end
def add_subscribed_users(recipients, target) def add_subscribed_users(recipients, project, target)
return recipients unless target.respond_to? :subscribers return recipients unless target.respond_to? :subscribers
recipients + target.subscribers recipients + target.subscribers(project)
end 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 return recipients unless target.respond_to? :labels
(labels || target.labels).each do |label| (labels || target.labels).each do |label|
recipients += label.subscribers recipients += label.subscribers(project)
end end
recipients recipients
...@@ -571,8 +571,8 @@ class NotificationService ...@@ -571,8 +571,8 @@ class NotificationService
end end
end end
def relabeled_resource_email(target, labels, current_user, method) def relabeled_resource_email(target, project, labels, current_user, method)
recipients = build_relabeled_recipients(target, current_user, labels: labels) recipients = build_relabeled_recipients(target, project, current_user, labels: labels)
label_names = labels.map(&:name) label_names = labels.map(&:name)
recipients.each do |recipient| recipients.each do |recipient|
...@@ -608,10 +608,10 @@ class NotificationService ...@@ -608,10 +608,10 @@ class NotificationService
end end
recipients = reject_muted_users(recipients, project) 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) if [:new_issue, :new_merge_request].include?(custom_action)
recipients = add_labels_subscribers(recipients, target) recipients = add_labels_subscribers(recipients, project, target)
end end
recipients = reject_unsubscribed_users(recipients, target) recipients = reject_unsubscribed_users(recipients, target)
...@@ -622,8 +622,8 @@ class NotificationService ...@@ -622,8 +622,8 @@ class NotificationService
recipients.uniq recipients.uniq
end end
def build_relabeled_recipients(target, current_user, labels:) def build_relabeled_recipients(target, project, current_user, labels:)
recipients = add_labels_subscribers([], target, labels: labels) recipients = add_labels_subscribers([], project, target, labels: labels)
recipients = reject_unsubscribed_users(recipients, target) recipients = reject_unsubscribed_users(recipients, target)
recipients = reject_users_without_access(recipients, target) recipients = reject_users_without_access(recipients, target)
recipients.delete(current_user) recipients.delete(current_user)
......
...@@ -193,7 +193,7 @@ module SlashCommands ...@@ -193,7 +193,7 @@ module SlashCommands
desc 'Subscribe' desc 'Subscribe'
condition do condition do
issuable.persisted? && issuable.persisted? &&
!issuable.subscribed?(current_user) !issuable.subscribed?(current_user, project)
end end
command :subscribe do command :subscribe do
@updates[:subscription_event] = 'subscribe' @updates[:subscription_event] = 'subscribe'
...@@ -202,7 +202,7 @@ module SlashCommands ...@@ -202,7 +202,7 @@ module SlashCommands
desc 'Unsubscribe' desc 'Unsubscribe'
condition do condition do
issuable.persisted? && issuable.persisted? &&
issuable.subscribed?(current_user) issuable.subscribed?(current_user, project)
end end
command :unsubscribe do command :unsubscribe do
@updates[:subscription_event] = 'unsubscribe' @updates[:subscription_event] = 'unsubscribe'
......
- label_css_id = dom_id(label) - label_css_id = dom_id(label)
- open_issues_count = label.open_issues_count(current_user) - open_issues_count = label.open_issues_count(current_user)
- open_merge_requests_count = label.open_merge_requests_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] - subject = local_assigns[:subject]
%li{id: label_css_id, data: { id: label.id } } %li{id: label_css_id, data: { id: label.id } }
...@@ -18,10 +19,19 @@ ...@@ -18,10 +19,19 @@
%li %li
= link_to_label(label, subject: subject) do = link_to_label(label, subject: subject) do
= pluralize open_issues_count, 'open issue' = pluralize open_issues_count, 'open issue'
- if current_user - if current_user && defined?(@project)
%li.label-subscription{ data: toggle_subscription_data(label) } %li.label-subscription
%a.js-subscribe-button.label-subscribe-button.subscription-status{ role: "button", href: "#", data: { toggle: "tooltip", status: label_subscription_status(label) } } - if label.is_a?(ProjectLabel)
%span= label_subscription_toggle_button_text(label) %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) - if can?(current_user, :admin_label, label)
%li %li
= link_to 'Edit', edit_label_path(label) = link_to 'Edit', edit_label_path(label)
...@@ -34,12 +44,27 @@ ...@@ -34,12 +44,27 @@
= link_to_label(label, subject: subject, css_class: 'btn btn-transparent btn-action') do = link_to_label(label, subject: subject, css_class: 'btn btn-transparent btn-action') do
= pluralize open_issues_count, 'open issue' = pluralize open_issues_count, 'open issue'
- if current_user - if current_user && defined?(@project)
.label-subscription.inline{ data: toggle_subscription_data(label) } .label-subscription.inline
%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) } } - if label.is_a?(ProjectLabel)
%span.sr-only= label_subscription_toggle_button_text(label) %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) } }
= icon('eye', class: 'label-subscribe-button-icon', disabled: label.is_a?(GroupLabel)) %span= label_subscription_toggle_button_text(label, @project)
= icon('spinner spin', class: 'label-subscribe-button-loading') = 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) - 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 = link_to edit_label_path(label), title: "Edit", class: 'btn btn-transparent btn-action', data: {toggle: "tooltip"} do
...@@ -49,6 +74,10 @@ ...@@ -49,6 +74,10 @@
%span.sr-only Delete %span.sr-only Delete
= icon('trash-o') = icon('trash-o')
- if current_user && label.is_a?(ProjectLabel) - if current_user && defined?(@project)
:javascript - if label.is_a?(ProjectLabel)
new Subscription('##{dom_id(label)} .label-subscription'); :javascript
new gl.ProjectLabelSubscription('##{dom_id(label)} .label-subscription');
- else
:javascript
new gl.GroupLabelSubscription('##{dom_id(label)} .label-subscription');
...@@ -140,7 +140,7 @@ ...@@ -140,7 +140,7 @@
= render "shared/issuable/participants", participants: issuable.participants(current_user) = render "shared/issuable/participants", participants: issuable.participants(current_user)
- if 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)}} .block.light.subscription{data: {url: toggle_subscription_path(issuable)}}
.sidebar-collapsed-icon .sidebar-collapsed-icon
= icon('rss') = icon('rss')
......
---
title: Allow users to subscribe to group labels
merge_request: 7215
author:
...@@ -30,7 +30,10 @@ scope(path: 'groups/:group_id', module: :groups, as: :group) do ...@@ -30,7 +30,10 @@ scope(path: 'groups/:group_id', module: :groups, as: :group) do
resource :avatar, only: [:destroy] resource :avatar, only: [:destroy]
resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :update, :new, :create] 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 end
# Must be last route in this file # Must be last route in this file
......
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
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
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
...@@ -1070,9 +1070,10 @@ ActiveRecord::Schema.define(version: 20161113184239) do ...@@ -1070,9 +1070,10 @@ ActiveRecord::Schema.define(version: 20161113184239) do
t.boolean "subscribed" t.boolean "subscribed"
t.datetime "created_at" t.datetime "created_at"
t.datetime "updated_at" t.datetime "updated_at"
t.integer "project_id"
end 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| create_table "taggings", force: :cascade do |t|
t.integer "tag_id" t.integer "tag_id"
...@@ -1268,6 +1269,7 @@ ActiveRecord::Schema.define(version: 20161113184239) do ...@@ -1268,6 +1269,7 @@ ActiveRecord::Schema.define(version: 20161113184239) do
add_foreign_key "personal_access_tokens", "users" add_foreign_key "personal_access_tokens", "users"
add_foreign_key "protected_branch_merge_access_levels", "protected_branches" add_foreign_key "protected_branch_merge_access_levels", "protected_branches"
add_foreign_key "protected_branch_push_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 "trending_projects", "projects", on_delete: :cascade
add_foreign_key "u2f_registrations", "users" add_foreign_key "u2f_registrations", "users"
end end
...@@ -218,7 +218,7 @@ module API ...@@ -218,7 +218,7 @@ module API
expose :assignee, :author, using: Entities::UserBasic expose :assignee, :author, using: Entities::UserBasic
expose :subscribed do |issue, options| expose :subscribed do |issue, options|
issue.subscribed?(options[:current_user]) issue.subscribed?(options[:current_user], options[:project] || issue.project)
end end
expose :user_notes_count expose :user_notes_count
expose :upvotes, :downvotes expose :upvotes, :downvotes
...@@ -248,7 +248,7 @@ module API ...@@ -248,7 +248,7 @@ module API
expose :diff_head_sha, as: :sha expose :diff_head_sha, as: :sha
expose :merge_commit_sha expose :merge_commit_sha
expose :subscribed do |merge_request, options| expose :subscribed do |merge_request, options|
merge_request.subscribed?(options[:current_user]) merge_request.subscribed?(options[:current_user], options[:project])
end end
expose :user_notes_count expose :user_notes_count
expose :should_remove_source_branch?, as: :should_remove_source_branch expose :should_remove_source_branch?, as: :should_remove_source_branch
...@@ -454,7 +454,7 @@ module API ...@@ -454,7 +454,7 @@ module API
end end
expose :subscribed do |label, options| expose :subscribed do |label, options|
label.subscribed?(options[:current_user]) label.subscribed?(options[:current_user], options[:project])
end end
end end
......
...@@ -120,7 +120,7 @@ module API ...@@ -120,7 +120,7 @@ module API
issues = issues.reorder(issuable_order_by => issuable_sort) 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 end
# Get a single project issue # Get a single project issue
...@@ -132,7 +132,7 @@ module API ...@@ -132,7 +132,7 @@ module API
# GET /projects/:id/issues/:issue_id # GET /projects/:id/issues/:issue_id
get ":id/issues/:issue_id" do get ":id/issues/:issue_id" do
@issue = find_project_issue(params[:issue_id]) @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 end
# Create a new project issue # Create a new project issue
...@@ -174,7 +174,7 @@ module API ...@@ -174,7 +174,7 @@ module API
end end
if issue.valid? 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 else
render_validation_error!(issue) render_validation_error!(issue)
end end
...@@ -217,7 +217,7 @@ module API ...@@ -217,7 +217,7 @@ module API
issue = ::Issues::UpdateService.new(user_project, current_user, attrs).execute(issue) issue = ::Issues::UpdateService.new(user_project, current_user, attrs).execute(issue)
if issue.valid? 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 else
render_validation_error!(issue) render_validation_error!(issue)
end end
...@@ -239,7 +239,7 @@ module API ...@@ -239,7 +239,7 @@ module API
begin begin
issue = ::Issues::MoveService.new(user_project, current_user).execute(issue, new_project) 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 rescue ::Issues::MoveService::MoveError => error
render_api_error!(error.message, 400) render_api_error!(error.message, 400)
end end
......
...@@ -60,7 +60,7 @@ module API ...@@ -60,7 +60,7 @@ module API
end end
merge_requests = merge_requests.reorder(issuable_order_by => issuable_sort) 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 end
desc 'Create a merge request' do desc 'Create a merge request' do
...@@ -87,7 +87,7 @@ module API ...@@ -87,7 +87,7 @@ module API
merge_request = ::MergeRequests::CreateService.new(user_project, current_user, mr_params).execute merge_request = ::MergeRequests::CreateService.new(user_project, current_user, mr_params).execute
if merge_request.valid? 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 else
handle_merge_request_errors! merge_request.errors handle_merge_request_errors! merge_request.errors
end end
...@@ -120,7 +120,7 @@ module API ...@@ -120,7 +120,7 @@ module API
get path do get path do
merge_request = user_project.merge_requests.find(params[:merge_request_id]) merge_request = user_project.merge_requests.find(params[:merge_request_id])
authorize! :read_merge_request, merge_request 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 end
desc 'Get the commits of a merge request' do desc 'Get the commits of a merge request' do
...@@ -167,7 +167,7 @@ module API ...@@ -167,7 +167,7 @@ module API
merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, mr_params).execute(merge_request) merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, mr_params).execute(merge_request)
if merge_request.valid? 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 else
handle_merge_request_errors! merge_request.errors handle_merge_request_errors! merge_request.errors
end end
...@@ -212,7 +212,7 @@ module API ...@@ -212,7 +212,7 @@ module API
execute(merge_request) execute(merge_request)
end 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 end
desc 'Cancel merge if "Merge when build succeeds" is enabled' do desc 'Cancel merge if "Merge when build succeeds" is enabled' do
......
...@@ -114,7 +114,7 @@ module API ...@@ -114,7 +114,7 @@ module API
} }
issues = IssuesFinder.new(current_user, finder_params).execute 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 end
end end
......
...@@ -24,11 +24,11 @@ module API ...@@ -24,11 +24,11 @@ module API
post ":id/#{type}/:subscribable_id/subscription" do post ":id/#{type}/:subscribable_id/subscription" do
resource = instance_exec(params[:subscribable_id], &finder) resource = instance_exec(params[:subscribable_id], &finder)
if resource.subscribed?(current_user) if resource.subscribed?(current_user, user_project)
not_modified! not_modified!
else else
resource.subscribe(current_user) resource.subscribe(current_user, user_project)
present resource, with: entity_class, current_user: current_user present resource, with: entity_class, current_user: current_user, project: user_project
end end
end end
...@@ -38,11 +38,11 @@ module API ...@@ -38,11 +38,11 @@ module API
delete ":id/#{type}/:subscribable_id/subscription" do delete ":id/#{type}/:subscribable_id/subscription" do
resource = instance_exec(params[:subscribable_id], &finder) resource = instance_exec(params[:subscribable_id], &finder)
if !resource.subscribed?(current_user) if !resource.subscribed?(current_user, user_project)
not_modified! not_modified!
else else
resource.unsubscribe(current_user) resource.unsubscribe(current_user, user_project)
present resource, with: entity_class, current_user: current_user present resource, with: entity_class, current_user: current_user, project: user_project
end end
end end
end end
......
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
...@@ -25,7 +25,7 @@ describe Projects::Boards::IssuesController do ...@@ -25,7 +25,7 @@ describe Projects::Boards::IssuesController do
create(:labeled_issue, project: project, labels: [planning]) 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], due_date: Date.tomorrow)
create(:labeled_issue, project: project, labels: [development], assignee: johndoe) create(:labeled_issue, project: project, labels: [development], assignee: johndoe)
issue.subscribe(johndoe) issue.subscribe(johndoe, project)
list_issues user: user, board: board, list: list2 list_issues user: user, board: board, list: list2
......
...@@ -72,14 +72,8 @@ describe Projects::LabelsController do ...@@ -72,14 +72,8 @@ describe Projects::LabelsController do
end end
describe 'POST #generate' do describe 'POST #generate' do
let(:admin) { create(:admin) }
before do
sign_in(admin)
end
context 'personal project' do context 'personal project' do
let(:personal_project) { create(:empty_project) } let(:personal_project) { create(:empty_project, namespace: user.namespace) }
it 'creates labels' do it 'creates labels' do
post :generate, namespace_id: personal_project.namespace.to_param, project_id: personal_project.to_param post :generate, namespace_id: personal_project.namespace.to_param, project_id: personal_project.to_param
...@@ -96,4 +90,26 @@ describe Projects::LabelsController do ...@@ -96,4 +90,26 @@ describe Projects::LabelsController do
end end
end 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 end
...@@ -3,11 +3,11 @@ require 'rails_helper' ...@@ -3,11 +3,11 @@ require 'rails_helper'
describe SentNotificationsController, type: :controller do describe SentNotificationsController, type: :controller do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:empty_project) } 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 let(:issue) do
create(:issue, project: project, author: user) do |issue| 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
end end
...@@ -17,7 +17,7 @@ describe SentNotificationsController, type: :controller do ...@@ -17,7 +17,7 @@ describe SentNotificationsController, type: :controller do
before { get(:unsubscribe, id: sent_notification.reply_key, force: true) } before { get(:unsubscribe, id: sent_notification.reply_key, force: true) }
it 'unsubscribes the user' do it 'unsubscribes the user' do
expect(issue.subscribed?(user)).to be_falsey expect(issue.subscribed?(user, project)).to be_falsey
end end
it 'sets the flash message' do it 'sets the flash message' do
...@@ -33,7 +33,7 @@ describe SentNotificationsController, type: :controller do ...@@ -33,7 +33,7 @@ describe SentNotificationsController, type: :controller do
before { get(:unsubscribe, id: sent_notification.reply_key) } before { get(:unsubscribe, id: sent_notification.reply_key) }
it 'does not unsubscribe the user' do it 'does not unsubscribe the user' do
expect(issue.subscribed?(user)).to be_truthy expect(issue.subscribed?(user, project)).to be_truthy
end end
it 'does not set the flash message' do it 'does not set the flash message' do
...@@ -53,7 +53,7 @@ describe SentNotificationsController, type: :controller do ...@@ -53,7 +53,7 @@ describe SentNotificationsController, type: :controller do
before { get(:unsubscribe, id: sent_notification.reply_key.reverse) } before { get(:unsubscribe, id: sent_notification.reply_key.reverse) }
it 'does not unsubscribe the user' do it 'does not unsubscribe the user' do
expect(issue.subscribed?(user)).to be_truthy expect(issue.subscribed?(user, project)).to be_truthy
end end
it 'does not set the flash message' do it 'does not set the flash message' do
...@@ -69,7 +69,7 @@ describe SentNotificationsController, type: :controller do ...@@ -69,7 +69,7 @@ describe SentNotificationsController, type: :controller do
before { get(:unsubscribe, id: sent_notification.reply_key, force: true) } before { get(:unsubscribe, id: sent_notification.reply_key, force: true) }
it 'unsubscribes the user' do it 'unsubscribes the user' do
expect(issue.subscribed?(user)).to be_falsey expect(issue.subscribed?(user, project)).to be_falsey
end end
it 'sets the flash message' do it 'sets the flash message' do
...@@ -85,14 +85,14 @@ describe SentNotificationsController, type: :controller do ...@@ -85,14 +85,14 @@ describe SentNotificationsController, type: :controller do
context 'when the force param is not passed' do context 'when the force param is not passed' do
let(:merge_request) do let(:merge_request) do
create(:merge_request, source_project: project, author: user) do |merge_request| 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
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) } before { get(:unsubscribe, id: sent_notification.reply_key) }
it 'unsubscribes the user' do it 'unsubscribes the user' do
expect(merge_request.subscribed?(user)).to be_falsey expect(merge_request.subscribed?(user, project)).to be_falsey
end end
it 'sets the flash message' do it 'sets the flash message' do
......
FactoryGirl.define do
factory :subscription do
user
project factory: :empty_project
subscribable factory: :issue
end
end
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
...@@ -26,11 +26,11 @@ describe 'Unsubscribe links', feature: true do ...@@ -26,11 +26,11 @@ describe 'Unsubscribe links', feature: true do
expect(current_path).to eq unsubscribe_sent_notification_path(SentNotification.last) 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(%(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(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' 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 expect(current_path).to eq new_user_session_path
end end
...@@ -38,11 +38,11 @@ describe 'Unsubscribe links', feature: true do ...@@ -38,11 +38,11 @@ describe 'Unsubscribe links', feature: true do
visit body_link visit body_link
expect(current_path).to eq unsubscribe_sent_notification_path(SentNotification.last) 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' 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 expect(current_path).to eq new_user_session_path
end end
end end
...@@ -51,7 +51,7 @@ describe 'Unsubscribe links', feature: true do ...@@ -51,7 +51,7 @@ describe 'Unsubscribe links', feature: true do
visit header_link visit header_link
expect(page).to have_text('unsubscribed') expect(page).to have_text('unsubscribed')
expect(issue.subscribed?(recipient)).to be_falsey expect(issue.subscribed?(recipient, project)).to be_falsey
end end
end end
...@@ -62,14 +62,14 @@ describe 'Unsubscribe links', feature: true do ...@@ -62,14 +62,14 @@ describe 'Unsubscribe links', feature: true do
visit body_link visit body_link
expect(page).to have_text('unsubscribed') expect(page).to have_text('unsubscribed')
expect(issue.subscribed?(recipient)).to be_falsey expect(issue.subscribed?(recipient, project)).to be_falsey
end end
it 'unsubscribes from the issue when visiting the link from the header' do it 'unsubscribes from the issue when visiting the link from the header' do
visit header_link visit header_link
expect(page).to have_text('unsubscribed') expect(page).to have_text('unsubscribed')
expect(issue.subscribed?(recipient)).to be_falsey expect(issue.subscribed?(recipient, project)).to be_falsey
end end
end end
end end
...@@ -176,23 +176,25 @@ describe Issue, "Issuable" do ...@@ -176,23 +176,25 @@ describe Issue, "Issuable" do
end end
describe '#subscribed?' do describe '#subscribed?' do
let(:project) { issue.project }
context 'user is not a participant in the issue' do context 'user is not a participant in the issue' do
before { allow(issue).to receive(:participants).with(user).and_return([]) } before { allow(issue).to receive(:participants).with(user).and_return([]) }
it 'returns false when no subcription exists' do it 'returns false when no subcription exists' do
expect(issue.subscribed?(user)).to be_falsey expect(issue.subscribed?(user, project)).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
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 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
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 end
...@@ -200,19 +202,19 @@ describe Issue, "Issuable" do ...@@ -200,19 +202,19 @@ describe Issue, "Issuable" do
before { allow(issue).to receive(:participants).with(user).and_return([user]) } before { allow(issue).to receive(:participants).with(user).and_return([user]) }
it 'returns false when no subcription exists' do it 'returns false when no subcription exists' do
expect(issue.subscribed?(user)).to be_truthy expect(issue.subscribed?(user, project)).to be_truthy
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
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 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
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 end
end end
......
require 'spec_helper' require 'spec_helper'
describe Subscribable, 'Subscribable' do describe Subscribable, 'Subscribable' do
let(:resource) { create(:issue) } let(:project) { create(:empty_project) }
let(:user) { create(:user) } let(:resource) { create(:issue, project: project) }
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)).to be_falsey it 'returns false when no subscription exists' do
end 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 it 'returns false when a subcription exists and subscribed is false' do
resource.subscriptions.create(user: user, subscribed: true) 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 end
it 'returns false when a subcription exists and subscribed is false' do context 'with project' do
resource.subscriptions.create(user: user, subscribed: false) 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
end end
describe '#subscribers' do describe '#subscribers' do
it 'returns [] when no subcribers exists' do it 'returns [] when no subcribers exists' do
expect(resource.subscribers).to be_empty expect(resource.subscribers(project)).to be_empty
end end
it 'returns the subscribed users' do it 'returns the subscribed users' do
resource.subscriptions.create(user: user, subscribed: true) user_2 = create(:user)
resource.subscriptions.create(user: create(:user), subscribed: false) 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
end end
describe '#toggle_subscription' do describe '#toggle_subscription' do
it 'toggles the current subscription state for the given user' do context 'without project' do
expect(resource.subscribed?(user)).to be_falsey 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
end end
describe '#subscribe' do describe '#subscribe' do
it 'subscribes the given user' do context 'without project' do
expect(resource.subscribed?(user)).to be_falsey 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
end end
describe '#unsubscribe' do describe '#unsubscribe' do
it 'unsubscribes the given current user' do context 'without project' do
resource.subscriptions.create(user: user, subscribed: true) it 'unsubscribes the given current user' do
expect(resource.subscribed?(user)).to be_truthy 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 end
end end
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
...@@ -637,7 +637,7 @@ describe API::API, api: true do ...@@ -637,7 +637,7 @@ describe API::API, api: true do
it "sends notifications for subscribers of newly added labels" do it "sends notifications for subscribers of newly added labels" do
label = project.labels.first label = project.labels.first
label.toggle_subscription(user2) label.toggle_subscription(user2, project)
perform_enqueued_jobs do perform_enqueued_jobs do
post api("/projects/#{project.id}/issues", user), post api("/projects/#{project.id}/issues", user),
...@@ -828,7 +828,7 @@ describe API::API, api: true do ...@@ -828,7 +828,7 @@ describe API::API, api: true do
it "sends notifications for subscribers of newly added labels when issue is updated" 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 = create(:label, title: 'foo', color: '#FFAABB', project: project)
label.toggle_subscription(user2) label.toggle_subscription(user2, project)
perform_enqueued_jobs do perform_enqueued_jobs do
put api("/projects/#{project.id}/issues/#{issue.id}", user), put api("/projects/#{project.id}/issues/#{issue.id}", user),
......
...@@ -339,7 +339,7 @@ describe API::API, api: true do ...@@ -339,7 +339,7 @@ describe API::API, api: true do
end end
context "when user is already subscribed to label" do context "when user is already subscribed to label" do
before { label1.subscribe(user) } before { label1.subscribe(user, project) }
it "returns 304" do it "returns 304" do
post api("/projects/#{project.id}/labels/#{label1.id}/subscription", user) post api("/projects/#{project.id}/labels/#{label1.id}/subscription", user)
...@@ -358,7 +358,7 @@ describe API::API, api: true do ...@@ -358,7 +358,7 @@ describe API::API, api: true do
end end
describe "DELETE /projects/:id/labels/:label_id/subscription" do 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 context "when label_id is a label title" do
it "unsubscribes from the label" do it "unsubscribes from the label" do
...@@ -381,7 +381,7 @@ describe API::API, api: true do ...@@ -381,7 +381,7 @@ describe API::API, api: true do
end end
context "when user is already unsubscribed from label" do context "when user is already unsubscribed from label" do
before { label1.unsubscribe(user) } before { label1.unsubscribe(user, project) }
it "returns 304" do it "returns 304" do
delete api("/projects/#{project.id}/labels/#{label1.id}/subscription", user) delete api("/projects/#{project.id}/labels/#{label1.id}/subscription", user)
......
...@@ -260,14 +260,14 @@ describe Issuable::BulkUpdateService, services: true do ...@@ -260,14 +260,14 @@ describe Issuable::BulkUpdateService, services: true do
it 'subscribes the given user' do it 'subscribes the given user' do
bulk_update(issues, subscription_event: 'subscribe') bulk_update(issues, subscription_event: 'subscribe')
expect(issues).to all(be_subscribed(user)) expect(issues).to all(be_subscribed(user, project))
end end
end end
describe 'unsubscribe from issues' do describe 'unsubscribe from issues' do
let(:issues) do let(:issues) do
create_list(:closed_issue, 2, project: project) do |issue| 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
end end
...@@ -275,7 +275,7 @@ describe Issuable::BulkUpdateService, services: true do ...@@ -275,7 +275,7 @@ describe Issuable::BulkUpdateService, services: true do
bulk_update(issues, subscription_event: 'unsubscribe') bulk_update(issues, subscription_event: 'unsubscribe')
issues.each do |issue| issues.each do |issue|
expect(issue).not_to be_subscribed(user) expect(issue).not_to be_subscribed(user, project)
end end
end end
end end
......
...@@ -215,7 +215,7 @@ describe Issues::UpdateService, services: true do ...@@ -215,7 +215,7 @@ describe Issues::UpdateService, services: true do
let!(:subscriber) do let!(:subscriber) do
create(:user).tap do |u| create(:user).tap do |u|
label.toggle_subscription(u) label.toggle_subscription(u, project)
project.team << [u, :developer] project.team << [u, :developer]
end end
end end
......
...@@ -199,7 +199,7 @@ describe MergeRequests::UpdateService, services: true do ...@@ -199,7 +199,7 @@ describe MergeRequests::UpdateService, services: true do
context 'when the issue is relabeled' do context 'when the issue is relabeled' do
let!(:non_subscriber) { create(:user) } 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 before do
project.team << [non_subscriber, :developer] project.team << [non_subscriber, :developer]
......
This diff is collapsed.
...@@ -169,7 +169,7 @@ describe SlashCommands::InterpretService, services: true do ...@@ -169,7 +169,7 @@ describe SlashCommands::InterpretService, services: true do
shared_examples 'unsubscribe command' do shared_examples 'unsubscribe command' do
it 'populates subscription_event: "unsubscribe" if content contains /unsubscribe' do it 'populates subscription_event: "unsubscribe" if content contains /unsubscribe' do
issuable.subscribe(developer) issuable.subscribe(developer, project)
_, updates = service.execute(content, issuable) _, updates = service.execute(content, issuable)
expect(updates).to eq(subscription_event: 'unsubscribe') expect(updates).to eq(subscription_event: 'unsubscribe')
...@@ -321,7 +321,7 @@ describe SlashCommands::InterpretService, services: true do ...@@ -321,7 +321,7 @@ describe SlashCommands::InterpretService, services: true do
it_behaves_like 'multiple label with same argument' do it_behaves_like 'multiple label with same argument' do
let(:content) { %(/label ~"#{inprogress.title}" \n/label ~#{inprogress.title}) } let(:content) { %(/label ~"#{inprogress.title}" \n/label ~#{inprogress.title}) }
let(:issuable) { issue } let(:issuable) { issue }
end end
it_behaves_like 'unlabel command' do it_behaves_like 'unlabel command' do
let(:content) { %(/unlabel ~"#{inprogress.title}") } let(:content) { %(/unlabel ~"#{inprogress.title}") }
......
...@@ -230,31 +230,31 @@ shared_examples 'issuable record that supports slash commands in its description ...@@ -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 context "with a note subscribing to the #{issuable_type}" do
it "creates a new todo for 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") write_note("/subscribe")
expect(page).not_to have_content '/subscribe' expect(page).not_to have_content '/subscribe'
expect(page).to have_content 'Your commands have been executed!' 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
end end
context "with a note unsubscribing to the #{issuable_type} as done" do context "with a note unsubscribing to the #{issuable_type} as done" do
before do before do
issuable.subscribe(master) issuable.subscribe(master, project)
end end
it "creates a new todo for the #{issuable_type}" do 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") write_note("/unsubscribe")
expect(page).not_to have_content '/unsubscribe' expect(page).not_to have_content '/unsubscribe'
expect(page).to have_content 'Your commands have been executed!' 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 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