diff options
author | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2015-03-17 19:45:04 +0300 |
---|---|---|
committer | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2015-03-17 19:45:04 +0300 |
commit | 9162e34bb078be9f4fb35b7e43f89c926dc3bcd8 (patch) | |
tree | 7d93fd0f30f83fb2fb2e502a4891aa2f1571fbc7 | |
parent | 409097bd7e0f5857cf0bc5462bd47484980ec787 (diff) | |
parent | 22fcb2f418ed6a2c7e68c0cd3ec2d414510ad4ec (diff) |
Merge branch 'issue_subscription' into 'master'
Subscription to issue/mr
Fixes #1911 and #1909
![joxi_screenshot_1426601822159](https://dev.gitlab.org/gitlab/gitlabhq/uploads/53021bc5783271322ab2dfba7598eaa3/joxi_screenshot_1426601822159.png)
![joxi_screenshot_1426601836423](https://dev.gitlab.org/gitlab/gitlabhq/uploads/244ff360fbd6f30980f8dad699400814/joxi_screenshot_1426601836423.png)
See merge request !1702
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/assets/javascripts/subscription.js.coffee | 17 | ||||
-rw-r--r-- | app/controllers/projects/issues_controller.rb | 8 | ||||
-rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 8 | ||||
-rw-r--r-- | app/models/concerns/issuable.rb | 17 | ||||
-rw-r--r-- | app/models/subscription.rb | 21 | ||||
-rw-r--r-- | app/services/notification_service.rb | 29 | ||||
-rw-r--r-- | app/views/projects/issues/_issue_context.html.haml | 20 | ||||
-rw-r--r-- | app/views/projects/merge_requests/show/_context.html.haml | 20 | ||||
-rw-r--r-- | config/routes.rb | 4 | ||||
-rw-r--r-- | db/migrate/20150313012111_create_subscriptions_table.rb | 16 | ||||
-rw-r--r-- | db/schema.rb | 15 | ||||
-rw-r--r-- | features/project/issues/issues.feature | 8 | ||||
-rw-r--r-- | features/project/merge_requests.feature | 7 | ||||
-rw-r--r-- | features/steps/project/issues/issues.rb | 13 | ||||
-rw-r--r-- | features/steps/project/merge_requests.rb | 13 | ||||
-rw-r--r-- | spec/services/notification_service_spec.rb | 32 |
17 files changed, 245 insertions, 4 deletions
diff --git a/CHANGELOG b/CHANGELOG index bd66a92933d..1ee3b1a7d1b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -74,6 +74,7 @@ v 7.9.0 (unreleased) - Raise recommended number of unicorn workers from 2 to 3 - Use same layout and interactivity for project members as group members. - Prevent gitlab-shell character encoding issues by receiving its changes as raw data. + - Ability to unsubscribe/subscribe to issue or merge request v 7.8.4 - Fix issue_tracker_id substitution in custom issue trackers diff --git a/app/assets/javascripts/subscription.js.coffee b/app/assets/javascripts/subscription.js.coffee new file mode 100644 index 00000000000..7f41616d4e7 --- /dev/null +++ b/app/assets/javascripts/subscription.js.coffee @@ -0,0 +1,17 @@ +class @Subscription + constructor: (url) -> + $(".subscribe-button").unbind("click").click (event)=> + btn = $(event.currentTarget) + action = btn.find("span").text() + current_status = $(".subscription-status").attr("data-status") + btn.prop("disabled", true) + + $.post url, => + btn.prop("disabled", false) + status = if current_status == "subscribed" then "unsubscribed" else "subscribed" + $(".subscription-status").attr("data-status", status) + action = if status == "subscribed" then "Unsubscribe" else "Subscribe" + btn.find("span").text(action) + $(".subscription-status>div").toggleClass("hidden") + + diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 4266bcaef16..88302276b5e 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -1,6 +1,6 @@ class Projects::IssuesController < Projects::ApplicationController before_filter :module_enabled - before_filter :issue, only: [:edit, :update, :show] + before_filter :issue, only: [:edit, :update, :show, :toggle_subscription] # Allow read any issue before_filter :authorize_read_issue! @@ -97,6 +97,12 @@ class Projects::IssuesController < Projects::ApplicationController redirect_to :back, notice: "#{result[:count]} issues updated" end + def toggle_subscription + @issue.toggle_subscription(current_user) + + render nothing: true + end + protected def issue diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 93d79d81661..c63a9b0cd44 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -2,7 +2,7 @@ require 'gitlab/satellite/satellite' class Projects::MergeRequestsController < Projects::ApplicationController before_filter :module_enabled - before_filter :merge_request, only: [:edit, :update, :show, :diffs, :automerge, :automerge_check, :ci_status] + before_filter :merge_request, only: [:edit, :update, :show, :diffs, :automerge, :automerge_check, :ci_status, :toggle_subscription] before_filter :closes_issues, only: [:edit, :update, :show, :diffs] before_filter :validates_merge_request, only: [:show, :diffs] before_filter :define_show_vars, only: [:show, :diffs] @@ -174,6 +174,12 @@ class Projects::MergeRequestsController < Projects::ApplicationController render json: response end + def toggle_subscription + @merge_request.toggle_subscription(current_user) + + render nothing: true + end + protected def selected_target_project diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index f5e23e9dc2d..88ac83744df 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -15,6 +15,7 @@ module Issuable has_many :notes, as: :noteable, dependent: :destroy has_many :label_links, as: :target, dependent: :destroy has_many :labels, through: :label_links + has_many :subscriptions, dependent: :destroy, as: :subscribable validates :author, presence: true validates :title, presence: true, length: { within: 0..255 } @@ -132,6 +133,22 @@ module Issuable users.concat(mentions.reduce([], :|)).uniq end + def subscribed?(user) + subscription = subscriptions.find_by_user_id(user.id) + + if subscription + return subscription.subscribed + end + + participants.include?(user) + end + + def toggle_subscription(user) + subscriptions. + find_or_initialize_by(user_id: user.id). + update(subscribed: !subscribed?(user)) + end + def to_hook_data(user) { object_kind: self.class.name.underscore, diff --git a/app/models/subscription.rb b/app/models/subscription.rb new file mode 100644 index 00000000000..dd75d3ab8ba --- /dev/null +++ b/app/models/subscription.rb @@ -0,0 +1,21 @@ +# == Schema Information +# +# Table name: subscriptions +# +# id :integer not null, primary key +# user_id :integer +# subscribable_id :integer +# subscribable_type :string(255) +# subscribed :boolean +# created_at :datetime +# updated_at :datetime +# + +class Subscription < ActiveRecord::Base + belongs_to :user + belongs_to :subscribable, polymorphic: true + + validates :user_id, + uniqueness: { scope: [:subscribable_id, :subscribable_type] }, + presence: true +end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index fb411c3e23d..848ed77ebf8 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -92,6 +92,8 @@ class NotificationService # def merge_mr(merge_request, current_user) recipients = reject_muted_users([merge_request.author, merge_request.assignee], merge_request.target_project) + recipients = add_subscribed_users(recipients, merge_request) + recipients = reject_unsubscribed_users(recipients, merge_request) recipients = recipients.concat(project_watchers(merge_request.target_project)).uniq recipients.delete(current_user) @@ -151,6 +153,10 @@ class NotificationService # Reject mutes users recipients = reject_muted_users(recipients, note.project) + recipients = add_subscribed_users(recipients, note.noteable) + + recipients = reject_unsubscribed_users(recipients, note.noteable) + # Reject author recipients.delete(note.author) @@ -314,6 +320,27 @@ class NotificationService end end + def reject_unsubscribed_users(recipients, target) + return recipients unless target.respond_to? :subscriptions + + recipients.reject do |user| + subscription = target.subscriptions.find_by_user_id(user.id) + subscription && !subscription.subscribed + end + end + + def add_subscribed_users(recipients, target) + return recipients unless target.respond_to? :subscriptions + + subscriptions = target.subscriptions + + if subscriptions.any? + recipients + subscriptions.where(subscribed: true).map(&:user) + else + recipients + end + end + def new_resource_email(target, project, method) recipients = build_recipients(target, project) recipients.delete(target.author) @@ -361,7 +388,9 @@ class NotificationService recipients = reject_muted_users(recipients, project) recipients = reject_mention_users(recipients, project) + recipients = add_subscribed_users(recipients, target) recipients = recipients.concat(project_watchers(project)).uniq + recipients = reject_unsubscribed_users(recipients, target) recipients end diff --git a/app/views/projects/issues/_issue_context.html.haml b/app/views/projects/issues/_issue_context.html.haml index 4c7654354f4..cb4846a41d1 100644 --- a/app/views/projects/issues/_issue_context.html.haml +++ b/app/views/projects/issues/_issue_context.html.haml @@ -26,3 +26,23 @@ = f.select(:milestone_id, milestone_options(@issue), { include_blank: "Select milestone" }, {class: 'select2 select2-compact js-select2 js-milestone'}) = hidden_field_tag :issue_context = f.submit class: 'btn' + + %div.prepend-top-20.clearfix + .issuable-context-title + %label + Subscription: + %button.btn.btn-block.subscribe-button + %i.fa.fa-eye + %span= @issue.subscribed?(current_user) ? "Unsubscribe" : "Subscribe" + - subscribtion_status = @issue.subscribed?(current_user) ? "subscribed" : "unsubscribed" + .subscription-status{"data-status" => subscribtion_status} + .description-block.unsubscribed{class: ( "hidden" if @issue.subscribed?(current_user) )} + You're not receiving notifications from this thread. + .description-block.subscribed{class: ( "hidden" unless @issue.subscribed?(current_user) )} + You're receiving notifications because you're subscribed to this thread. + +:coffeescript + $ -> + new Subscription("#{toggle_subscription_namespace_project_issue_path(@issue.project.namespace, @project, @issue)}") + +
\ No newline at end of file diff --git a/app/views/projects/merge_requests/show/_context.html.haml b/app/views/projects/merge_requests/show/_context.html.haml index a74f3fb24e7..753c7e0e611 100644 --- a/app/views/projects/merge_requests/show/_context.html.haml +++ b/app/views/projects/merge_requests/show/_context.html.haml @@ -28,3 +28,23 @@ = f.select(:milestone_id, milestone_options(@merge_request), { include_blank: "Select milestone" }, {class: 'select2 select2-compact js-select2 js-milestone'}) = hidden_field_tag :merge_request_context = f.submit class: 'btn' + + %div.prepend-top-20.clearfix + .issuable-context-title + %label + Subscription: + %button.btn.btn-block.subscribe-button + %i.fa.fa-eye + %span= @merge_request.subscribed?(current_user) ? "Unsubscribe" : "Subscribe" + - subscribtion_status = @merge_request.subscribed?(current_user) ? "subscribed" : "unsubscribed" + .subscription-status{"data-status" => subscribtion_status} + .description-block.unsubscribed{class: ( "hidden" if @merge_request.subscribed?(current_user) )} + You're not receiving notifications from this thread. + .description-block.subscribed{class: ( "hidden" unless @merge_request.subscribed?(current_user) )} + You're receiving notifications because you're subscribed to this thread. + +:coffeescript + $ -> + new Subscription("#{toggle_subscription_namespace_project_merge_request_path(@merge_request.project.namespace, @project, @merge_request)}") + +
\ No newline at end of file diff --git a/config/routes.rb b/config/routes.rb index dd70ad2fa0d..e65ef30afb7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -404,6 +404,7 @@ Gitlab::Application.routes.draw do post :automerge get :automerge_check get :ci_status + post :toggle_subscription end collection do @@ -437,6 +438,9 @@ Gitlab::Application.routes.draw do end resources :issues, constraints: { id: /\d+/ }, except: [:destroy] do + member do + post :toggle_subscription + end collection do post :bulk_update end diff --git a/db/migrate/20150313012111_create_subscriptions_table.rb b/db/migrate/20150313012111_create_subscriptions_table.rb new file mode 100644 index 00000000000..a1d4d9dedc5 --- /dev/null +++ b/db/migrate/20150313012111_create_subscriptions_table.rb @@ -0,0 +1,16 @@ +class CreateSubscriptionsTable < ActiveRecord::Migration + def change + create_table :subscriptions do |t| + t.integer :user_id + t.references :subscribable, polymorphic: true + t.boolean :subscribed + + t.timestamps + end + + add_index :subscriptions, + [:subscribable_id, :subscribable_type, :user_id], + unique: true, + name: 'subscriptions_user_id_and_ref_fields' + end +end diff --git a/db/schema.rb b/db/schema.rb index 3dcc43803b9..e7dccbad4f9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20150306023112) do +ActiveRecord::Schema.define(version: 20150313012111) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -335,12 +335,12 @@ ActiveRecord::Schema.define(version: 20150306023112) do t.string "import_url" t.integer "visibility_level", default: 0, null: false t.boolean "archived", default: false, null: false + t.string "avatar" t.string "import_status" t.float "repository_size", default: 0.0 t.integer "star_count", default: 0, null: false t.string "import_type" t.string "import_source" - t.string "avatar" end add_index "projects", ["created_at", "id"], name: "index_projects_on_created_at_and_id", using: :btree @@ -398,6 +398,17 @@ ActiveRecord::Schema.define(version: 20150306023112) do add_index "snippets", ["project_id"], name: "index_snippets_on_project_id", using: :btree add_index "snippets", ["visibility_level"], name: "index_snippets_on_visibility_level", using: :btree + create_table "subscriptions", force: true do |t| + t.integer "user_id" + t.integer "subscribable_id" + t.string "subscribable_type" + t.boolean "subscribed" + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index "subscriptions", ["subscribable_id", "subscribable_type", "user_id"], name: "subscriptions_user_id_and_ref_fields", unique: true, using: :btree + create_table "taggings", force: true do |t| t.integer "tag_id" t.integer "taggable_id" diff --git a/features/project/issues/issues.feature b/features/project/issues/issues.feature index 283979204db..b9031f6f32b 100644 --- a/features/project/issues/issues.feature +++ b/features/project/issues/issues.feature @@ -202,3 +202,11 @@ Feature: Project Issues And I click link "Edit" for the issue And I preview a description text like "Bug fixed :smile:" Then I should see the Markdown write tab + + @javascript + Scenario: I can unsubscribe from issue + Given project "Shop" has "Tasks-open" open issue with task markdown + When I visit issue page "Tasks-open" + Then I should see that I am subscribed + When I click button "Unsubscribe" + Then I should see that I am unsubscribed diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature index adad100e56c..91dc576f8b4 100644 --- a/features/project/merge_requests.feature +++ b/features/project/merge_requests.feature @@ -225,3 +225,10 @@ Feature: Project Merge Requests When I fill in merge request search with "Fe" Then I should see "Feature NS-03" in merge requests And I should not see "Bug NS-04" in merge requests + + @javascript + Scenario: I can unsubscribe from merge request + Given I visit merge request page "Bug NS-04" + Then I should see that I am subscribed + When I click button "Unsubscribe" + Then I should see that I am unsubscribed diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb index 6d72c93ad13..e8ca3f7c176 100644 --- a/features/steps/project/issues/issues.rb +++ b/features/steps/project/issues/issues.rb @@ -18,10 +18,23 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps page.should_not have_content "Tweet control" end + step 'I should see that I am subscribed' do + find(".subscribe-button span").text.should == "Unsubscribe" + end + + step 'I should see that I am unsubscribed' do + sleep 0.2 + find(".subscribe-button span").text.should == "Subscribe" + end + step 'I click link "Closed"' do click_link "Closed" end + step 'I click button "Unsubscribe"' do + click_on "Unsubscribe" + end + step 'I should see "Release 0.3" in issues' do page.should have_content "Release 0.3" end diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index b67b2e58caf..6e2f60972b6 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -56,6 +56,19 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps page.should_not have_content "Bug NS-04" end + step 'I should see that I am subscribed' do + find(".subscribe-button span").text.should == "Unsubscribe" + end + + step 'I should see that I am unsubscribed' do + sleep 0.2 + find(".subscribe-button span").text.should == "Subscribe" + end + + step 'I click button "Unsubscribe"' do + click_on "Unsubscribe" + end + step 'I click link "Close"' do first(:css, '.close-mr-link').click end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 34737348d41..bfca2c88264 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -41,13 +41,18 @@ describe NotificationService do describe :new_note do it do + add_users_with_subscription(note.project, issue) + should_email(@u_watcher.id) should_email(note.noteable.author_id) should_email(note.noteable.assignee_id) should_email(@u_mentioned.id) + should_email(@subscriber.id) should_not_email(note.author_id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) + should_not_email(@unsubscriber.id) + notification.new_note(note) end @@ -191,6 +196,7 @@ describe NotificationService do before do build_team(issue.project) + add_users_with_subscription(issue.project, issue) end describe :new_issue do @@ -224,6 +230,8 @@ describe NotificationService do should_email(issue.assignee_id) should_email(@u_watcher.id) should_email(@u_participant_mentioned.id) + should_email(@subscriber.id) + should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) @@ -245,6 +253,8 @@ describe NotificationService do should_email(issue.author_id) should_email(@u_watcher.id) should_email(@u_participant_mentioned.id) + should_email(@subscriber.id) + should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) @@ -266,6 +276,8 @@ describe NotificationService do should_email(issue.author_id) should_email(@u_watcher.id) should_email(@u_participant_mentioned.id) + should_email(@subscriber.id) + should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) @@ -287,6 +299,7 @@ describe NotificationService do before do build_team(merge_request.target_project) + add_users_with_subscription(merge_request.target_project, merge_request) end describe :new_merge_request do @@ -311,6 +324,8 @@ describe NotificationService do it do should_email(merge_request.assignee_id) should_email(@u_watcher.id) + should_email(@subscriber.id) + should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) notification.reassigned_merge_request(merge_request, merge_request.author) @@ -329,6 +344,8 @@ describe NotificationService do it do should_email(merge_request.assignee_id) should_email(@u_watcher.id) + should_email(@subscriber.id) + should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) notification.close_mr(merge_request, @u_disabled) @@ -347,6 +364,8 @@ describe NotificationService do it do should_email(merge_request.assignee_id) should_email(@u_watcher.id) + should_email(@subscriber.id) + should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) notification.merge_mr(merge_request, @u_disabled) @@ -365,6 +384,8 @@ describe NotificationService do it do should_email(merge_request.assignee_id) should_email(@u_watcher.id) + should_email(@subscriber.id) + should_not_email(@unsubscriber.id) should_not_email(@u_participating.id) should_not_email(@u_disabled.id) notification.reopen_mr(merge_request, @u_disabled) @@ -420,4 +441,15 @@ describe NotificationService do project.team << [@u_mentioned, :master] project.team << [@u_committer, :master] end + + def add_users_with_subscription(project, issuable) + @subscriber = create :user + @unsubscriber = create :user + + project.team << [@subscriber, :master] + project.team << [@unsubscriber, :master] + + issuable.subscriptions.create(user: @subscriber, subscribed: true) + issuable.subscriptions.create(user: @unsubscriber, subscribed: false) + end end |