From 410d25c8ca8afabb25e5f89b36e3cfd09ffe6f87 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Mon, 16 Mar 2015 15:22:50 +0200 Subject: [PATCH] rename table subscribe; make it polymorfic --- app/controllers/projects/issues_controller.rb | 2 +- .../projects/merge_requests_controller.rb | 2 +- app/models/concerns/issuable.rb | 11 ++++++----- app/models/subscribe.rb | 6 ------ app/models/subscription.rb | 7 +++++++ app/services/notification_service.rb | 10 +++++----- .../projects/issues/_issue_context.html.haml | 4 ++-- .../merge_requests/show/_context.html.haml | 4 ++-- .../20150313012111_create_subscribes_table.rb | 16 ---------------- .../20150313012111_create_subscriptions_table.rb | 13 +++++++++++++ db/schema.rb | 10 ++++------ 11 files changed, 41 insertions(+), 44 deletions(-) delete mode 100644 app/models/subscribe.rb create mode 100644 app/models/subscription.rb delete mode 100644 db/migrate/20150313012111_create_subscribes_table.rb create mode 100644 db/migrate/20150313012111_create_subscriptions_table.rb diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 4eb5092b9d9..903b7a68dc9 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -100,7 +100,7 @@ class Projects::IssuesController < Projects::ApplicationController def set_subscription subscribed = params[:subscription] == "Subscribe" - sub = @issue.subscribes.find_or_create_by(user_id: current_user.id) + sub = @issue.subscriptions.find_or_create_by(user_id: current_user.id) sub.update(subscribed: subscribed) render nothing: true diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 5613eee35c0..51ac61c3273 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -177,7 +177,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController def set_subscription subscribed = params[:subscription] == "Subscribe" - sub = @merge_request.subscribes.find_or_create_by(user_id: current_user.id) + sub = @merge_request.subscriptions.find_or_create_by(user_id: current_user.id) sub.update(subscribed: subscribed) render nothing: true diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index c74d9cb9917..d1a35ca5294 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -15,7 +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 :subscribes, dependent: :destroy + has_many :subscriptions, dependent: :destroy, as: :subscribable validates :author, presence: true validates :title, presence: true, length: { within: 0..255 } @@ -133,10 +133,11 @@ module Issuable users.concat(mentions.reduce([], :|)).uniq end - def subscribe_status(user) - subscribe = subscribes.find_by_user_id(user.id) - if subscribe - return subscribe.subscribed + def subscription_status(user) + subscription = subscriptions.find_by_user_id(user.id) + + if subscription + return subscription.subscribed end participants.include?(user) diff --git a/app/models/subscribe.rb b/app/models/subscribe.rb deleted file mode 100644 index be8b9e7605d..00000000000 --- a/app/models/subscribe.rb +++ /dev/null @@ -1,6 +0,0 @@ -class Subscribe < ActiveRecord::Base - belongs_to :user - - validates :issue_id, uniqueness: { scope: :user_id, allow_nil: true } - validates :merge_request_id, uniqueness: { scope: :user_id, allow_nil: true } -end diff --git a/app/models/subscription.rb b/app/models/subscription.rb new file mode 100644 index 00000000000..7e57a8570ec --- /dev/null +++ b/app/models/subscription.rb @@ -0,0 +1,7 @@ +class Subscription < ActiveRecord::Base + 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 edfb62a4b18..e02418b7246 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -316,8 +316,8 @@ class NotificationService def reject_unsubscribed_users(recipients, target) recipients.reject do |user| - subscribe = target.subscribes.find_by_user_id(user.id) - subscribe && !subscribe.subscribed + subscription = target.subscriptions.find_by_user_id(user.id) + subscription && !subscription.subscribed end end @@ -375,9 +375,9 @@ class NotificationService end def add_subscribed_users(recipients, target) - subscribes = target.subscribes - if subscribes.any? - recipients.merge(subscribes.where("subscribed is true").map(&:user)) + subscriptions = target.subscriptions + if subscriptions.any? + recipients.merge(subscriptions.where("subscribed is true").map(&:user)) else recipients end diff --git a/app/views/projects/issues/_issue_context.html.haml b/app/views/projects/issues/_issue_context.html.haml index 09c531ac7fc..24bfbdd4c58 100644 --- a/app/views/projects/issues/_issue_context.html.haml +++ b/app/views/projects/issues/_issue_context.html.haml @@ -33,8 +33,8 @@ Subscription: %i.fa.fa-spinner.fa-spin.hidden.subscription %span.sub_status - = @issue.subscribe_status(current_user) ? "subscribed" : "unsubscribed" - - subscribe_action = @issue.subscribe_status(current_user) ? "Unsubscribe" : "Subscribe" + = @issue.subscription_status(current_user) ? "subscribed" : "unsubscribed" + - subscribe_action = @issue.subscription_status(current_user) ? "Unsubscribe" : "Subscribe" %input.btn.subscribe-button{:type => "button", :value => subscribe_action} :coffeescript diff --git a/app/views/projects/merge_requests/show/_context.html.haml b/app/views/projects/merge_requests/show/_context.html.haml index aae0aa24ed7..d0c00c1aeaf 100644 --- a/app/views/projects/merge_requests/show/_context.html.haml +++ b/app/views/projects/merge_requests/show/_context.html.haml @@ -35,8 +35,8 @@ Subscription: %i.fa.fa-spinner.fa-spin.hidden.subscription %span.sub_status - = @merge_request.subscribe_status(current_user) ? "subscribed" : "unsubscribed" - - subscribe_action = @merge_request.subscribe_status(current_user) ? "Unsubscribe" : "Subscribe" + = @merge_request.subscription_status(current_user) ? "subscribed" : "unsubscribed" + - subscribe_action = @merge_request.subscription_status(current_user) ? "Unsubscribe" : "Subscribe" %input.btn.subscribe-button{:type => "button", :value => subscribe_action} :coffeescript diff --git a/db/migrate/20150313012111_create_subscribes_table.rb b/db/migrate/20150313012111_create_subscribes_table.rb deleted file mode 100644 index ab0e9a2a5b5..00000000000 --- a/db/migrate/20150313012111_create_subscribes_table.rb +++ /dev/null @@ -1,16 +0,0 @@ -class CreateSubscribesTable < ActiveRecord::Migration - def change - create_table :subscribes do |t| - t.integer :user_id - t.integer :merge_request_id - t.integer :issue_id - t.boolean :subscribed - - t.timestamps - end - - add_index :subscribes, :user_id - add_index :subscribes, :issue_id - add_index :subscribes, :merge_request_id - end -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..78f7aeeaf7c --- /dev/null +++ b/db/migrate/20150313012111_create_subscriptions_table.rb @@ -0,0 +1,13 @@ +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 46663ad495b..3f808d5ac3f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -397,18 +397,16 @@ ActiveRecord::Schema.define(version: 20150313012111) 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 "subscribes", force: true do |t| + create_table "subscriptions", force: true do |t| t.integer "user_id" - t.integer "merge_request_id" - t.integer "issue_id" + t.integer "subscribable_id" + t.string "subscribable_type" t.boolean "subscribed" t.datetime "created_at" t.datetime "updated_at" end - add_index "subscribes", ["issue_id"], name: "index_subscribes_on_issue_id", using: :btree - add_index "subscribes", ["merge_request_id"], name: "index_subscribes_on_merge_request_id", using: :btree - add_index "subscribes", ["user_id"], name: "index_subscribes_on_user_id", using: :btree + 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"