diff --git a/app/models/article.rb b/app/models/article.rb index b0db1b2d6..f653a0d66 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -4,6 +4,7 @@ class Article < ApplicationRecord include Storext.model include Reactable include Searchable + include UserSubscriptionSourceable SEARCH_SERIALIZER = Search::ArticleSerializer SEARCH_CLASS = Search::FeedContent @@ -28,8 +29,6 @@ class Article < ApplicationRecord counter_culture :organization has_many :comments, as: :commentable, inverse_of: :commentable - has_many :user_subscriptions, as: :user_subscription_sourceable - has_many :sourced_subscribers, class_name: "User", through: :user_subscriptions, source: :subscriber, foreign_key: :user_id has_many :top_comments, -> { where("comments.score > ? AND ancestry IS NULL and hidden_by_commentable_user is FALSE and deleted is FALSE", 10).order("comments.score DESC") }, as: :commentable, diff --git a/app/models/concerns/user_subscription_sourceable.rb b/app/models/concerns/user_subscription_sourceable.rb new file mode 100644 index 000000000..ddce09ec4 --- /dev/null +++ b/app/models/concerns/user_subscription_sourceable.rb @@ -0,0 +1,33 @@ +module UserSubscriptionSourceable + extend ActiveSupport::Concern + + # This all assumes there's an association with User under the column user_id. + + included do + has_many :user_subscriptions, as: :user_subscription_sourceable + has_many :sourced_subscribers, + class_name: "User", + through: :user_subscriptions, + source: :subscriber, + foreign_key: :user_id + end + + def build_user_subscription(subscriber) + UserSubscription.new(user_subscription_attributes(subscriber)) + end + + def create_user_subscription(subscriber) + UserSubscription.create(user_subscription_attributes(subscriber)) + end + + private + + def user_subscription_attributes(subscriber) + { + user_subscription_sourceable: self, + author_id: user_id, + subscriber_id: subscriber&.id, + subscriber_email: subscriber&.email + } + end +end diff --git a/app/models/user.rb b/app/models/user.rb index c3e9732e9..1c5de99b8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -45,6 +45,10 @@ class User < ApplicationRecord acts_as_followable acts_as_follower + has_many :source_authored_user_subscriptions, class_name: "UserSubscription", foreign_key: :author_id, inverse_of: :author, dependent: :destroy + has_many :subscribers, through: :source_authored_user_subscriptions, dependent: :destroy + has_many :subscribed_to_user_subscriptions, class_name: "UserSubscription", foreign_key: :subscriber_id, inverse_of: :subscriber, dependent: :destroy + has_many :access_grants, class_name: "Doorkeeper::AccessGrant", foreign_key: :resource_owner_id, inverse_of: :resource_owner, dependent: :delete_all has_many :access_tokens, class_name: "Doorkeeper::AccessToken", foreign_key: :resource_owner_id, inverse_of: :resource_owner, dependent: :delete_all has_many :affected_feedback_messages, class_name: "FeedbackMessage", inverse_of: :affected, foreign_key: :affected_id, dependent: :nullify @@ -67,8 +71,6 @@ class User < ApplicationRecord has_many :display_ad_events, dependent: :destroy has_many :email_authorizations, dependent: :delete_all has_many :email_messages, class_name: "Ahoy::Message", dependent: :destroy - has_many :user_subscriptions, foreign_key: :author_id, inverse_of: :author, dependent: :destroy - has_many :subscribers, through: :user_subscriptions, dependent: :destroy has_many :field_test_memberships, class_name: "FieldTest::Membership", as: :participant, dependent: :destroy has_many :github_repos, dependent: :destroy has_many :html_variants, dependent: :destroy diff --git a/app/models/user_subscription.rb b/app/models/user_subscription.rb index ddb8ba39b..8ebeff98f 100644 --- a/app/models/user_subscription.rb +++ b/app/models/user_subscription.rb @@ -4,12 +4,30 @@ class UserSubscription < ApplicationRecord ALLOWED_TYPES = %w[Article].freeze - belongs_to :author, class_name: "User", inverse_of: :user_subscriptions - belongs_to :subscriber, class_name: "User", inverse_of: :user_subscriptions + belongs_to :author, class_name: "User", inverse_of: :source_authored_user_subscriptions + belongs_to :subscriber, class_name: "User", inverse_of: :subscribed_to_user_subscriptions belongs_to :user_subscription_sourceable, polymorphic: true validates :author_id, presence: true - validates :subscriber_id, presence: true, uniqueness: { scope: %i[user_subscription_sourceable_type user_subscription_sourceable_id] } + validates :subscriber_email, presence: true + validates :subscriber_id, presence: true, uniqueness: { scope: %i[subscriber_email user_subscription_sourceable_type user_subscription_sourceable_id] } validates :user_subscription_sourceable_id, presence: true validates :user_subscription_sourceable_type, presence: true, inclusion: { in: ALLOWED_TYPES } + + def self.build(source:, subscriber:) + new(build_attributes(source, subscriber)) + end + + def self.make(source:, subscriber:) + create(build_attributes(source, subscriber)) + end + + def self.build_attributes(source, subscriber) + { + user_subscription_sourceable: source, + author_id: source&.user_id, + subscriber_id: subscriber&.id, + subscriber_email: subscriber&.email + } + end end diff --git a/db/migrate/20200617014320_add_subscriber_email_to_user_subscriptions.rb b/db/migrate/20200617014320_add_subscriber_email_to_user_subscriptions.rb new file mode 100644 index 000000000..a446b239f --- /dev/null +++ b/db/migrate/20200617014320_add_subscriber_email_to_user_subscriptions.rb @@ -0,0 +1,5 @@ +class AddSubscriberEmailToUserSubscriptions < ActiveRecord::Migration[6.0] + def change + add_column :user_subscriptions, :subscriber_email, :string, null: false + end +end diff --git a/db/migrate/20200617014509_add_subscriber_email_index_to_user_subscriptions.rb b/db/migrate/20200617014509_add_subscriber_email_index_to_user_subscriptions.rb new file mode 100644 index 000000000..408213d29 --- /dev/null +++ b/db/migrate/20200617014509_add_subscriber_email_index_to_user_subscriptions.rb @@ -0,0 +1,45 @@ +class AddSubscriberEmailIndexToUserSubscriptions < ActiveRecord::Migration[6.0] + disable_ddl_transaction! + + def up + if index_exists?(:user_subscriptions, %i[subscriber_id user_subscription_sourceable_id user_subscription_sourceable_type]) + remove_index :user_subscriptions, + column: %i[subscriber_id user_subscription_sourceable_id user_subscription_sourceable_type], + algorithm: :concurrently + end + + unless index_exists?(:user_subscriptions, :subscriber_email) + add_index :user_subscriptions, + :subscriber_email, + algorithm: :concurrently + end + + unless index_exists?(:user_subscriptions, %i[subscriber_id subscriber_email user_subscription_sourceable_type user_subscription_sourceable_id]) + add_index :user_subscriptions, + %i[subscriber_id subscriber_email user_subscription_sourceable_type user_subscription_sourceable_id], + name: "index_subscriber_id_and_email_with_user_subscription_source", + unique: true, + algorithm: :concurrently + end + end + + def down + unless index_exists?(:user_subscriptions, %i[subscriber_id user_subscription_sourceable_id user_subscription_sourceable_type]) + add_index :user_subscriptions, + %i[subscriber_id user_subscription_sourceable_id user_subscription_sourceable_type], + unique: true, + name: :index_on_subscriber_id_user_subscription_sourceable_type_and_id, + algorithm: :concurrently + end + + if index_exists?(:user_subscriptions, :subscriber_email) + remove_index :user_subscriptions, column: :subscriber_email, algorithm: :concurrently + end + + if index_exists?(:user_subscriptions, %i[subscriber_id subscriber_email user_subscription_sourceable_type user_subscription_sourceable_id]) + remove_index :user_subscriptions, + column: %i[subscriber_id subscriber_email user_subscription_sourceable_type user_subscription_sourceable_id], + algorithm: :concurrently + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 62ea794e0..2c9cbcf8c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_06_16_200005) do +ActiveRecord::Schema.define(version: 2020_06_17_014509) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1134,12 +1134,14 @@ ActiveRecord::Schema.define(version: 2020_06_16_200005) do create_table "user_subscriptions", force: :cascade do |t| t.bigint "author_id", null: false t.datetime "created_at", precision: 6, null: false + t.string "subscriber_email", null: false t.bigint "subscriber_id", null: false t.datetime "updated_at", precision: 6, null: false t.bigint "user_subscription_sourceable_id", null: false t.string "user_subscription_sourceable_type", null: false t.index ["author_id"], name: "index_user_subscriptions_on_author_id" - t.index ["subscriber_id", "user_subscription_sourceable_id", "user_subscription_sourceable_type"], name: "index_on_subscriber_id_user_subscription_sourceable_type_and_id", unique: true + t.index ["subscriber_email"], name: "index_user_subscriptions_on_subscriber_email" + t.index ["subscriber_id", "subscriber_email", "user_subscription_sourceable_type", "user_subscription_sourceable_id"], name: "index_subscriber_id_and_email_with_user_subscription_source", unique: true t.index ["subscriber_id"], name: "index_user_subscriptions_on_subscriber_id" t.index ["user_subscription_sourceable_type", "user_subscription_sourceable_id"], name: "index_on_user_subscription_sourcebable_type_and_id" end diff --git a/spec/factories/user_subscription.rb b/spec/factories/user_subscription.rb index 2cebe5a1b..47889c971 100644 --- a/spec/factories/user_subscription.rb +++ b/spec/factories/user_subscription.rb @@ -3,6 +3,7 @@ FactoryBot.define do association :subscriber, factory: :user, strategy: :create association :user_subscription_sourceable, factory: :article - author { user_subscription_sourceable.user } + author { user_subscription_sourceable.user } + subscriber_email { subscriber.email } end end diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index a467d695b..92ff25d82 100644 --- a/spec/models/article_spec.rb +++ b/spec/models/article_spec.rb @@ -11,6 +11,7 @@ RSpec.describe Article, type: :model do let!(:article) { create(:article, user: user) } include_examples "#sync_reactions_count", :article + it_behaves_like "UserSubscriptionSourceable" describe "validations" do it { is_expected.to validate_uniqueness_of(:canonical_url).allow_blank } diff --git a/spec/models/shared_examples/user_subscription_sourceable_spec.rb b/spec/models/shared_examples/user_subscription_sourceable_spec.rb new file mode 100644 index 000000000..bede2d6ef --- /dev/null +++ b/spec/models/shared_examples/user_subscription_sourceable_spec.rb @@ -0,0 +1,42 @@ +RSpec.shared_examples "UserSubscriptionSourceable" do + let(:model) { described_class } + let(:source) { create(model.to_s.underscore.to_sym) } + let(:subscriber) { create(:user) } + + describe "#build_user_subscription" do + it "returns a new UserSubcription with the correct attributes" do + new_user_subscription = UserSubscription.new( + user_subscription_sourceable: source, + author_id: source.user_id, + subscriber_id: subscriber.id, + subscriber_email: subscriber.email, + ) + + factory_user_subscription = source.build_user_subscription(subscriber) + + factory_user_subscription.attributes.each do |name, val| + expect(new_user_subscription[name]).to eq val + end + end + end + + describe "#create_user_subscription" do + it "returns a created UserSubcription with the correct attributes" do + user_subscription_fields = %w[author_id subsciber_id subscriber_email user_subscription_sourceable_id user_susbcription_sourceable_type] + + user_subscription = create( + :user_subscription, + user_subscription_sourceable: source, + author_id: source.user_id, + subscriber_id: subscriber.id, + subscriber_email: subscriber.email, + ) + + factory_user_subscription = source.create_user_subscription(subscriber) + + user_subscription_fields.each do |field| + expect(factory_user_subscription[field]).to eq user_subscription[field] + end + end + end +end diff --git a/spec/models/user_subscription_spec.rb b/spec/models/user_subscription_spec.rb index 6ffe5e065..373135d98 100644 --- a/spec/models/user_subscription_spec.rb +++ b/spec/models/user_subscription_spec.rb @@ -3,12 +3,51 @@ require "rails_helper" RSpec.describe UserSubscription, type: :model do subject { build(:user_subscription) } + let(:source) { create(:article) } + let(:subscriber) { create(:user) } + describe "validations" do it { is_expected.to validate_presence_of(:user_subscription_sourceable_id) } it { is_expected.to validate_presence_of(:user_subscription_sourceable_type) } it { is_expected.to validate_presence_of(:subscriber_id) } + it { is_expected.to validate_presence_of(:subscriber_email) } it { is_expected.to validate_presence_of(:author_id) } it { is_expected.to validate_inclusion_of(:user_subscription_sourceable_type).in_array(%w[Article]) } - it { is_expected.to validate_uniqueness_of(:subscriber_id).scoped_to(:user_subscription_sourceable_type, :user_subscription_sourceable_id) } + it { is_expected.to validate_uniqueness_of(:subscriber_id).scoped_to(%i[subscriber_email user_subscription_sourceable_type user_subscription_sourceable_id]) } + end + + describe "#build" do + it "returns a new UserSubcription with the correct attributes" do + new_user_subscription = described_class.new( + user_subscription_sourceable: source, + author_id: source.user_id, + subscriber_id: subscriber.id, + subscriber_email: subscriber.email, + ) + + factory_user_subscription = described_class.build(source: source, subscriber: subscriber) + + factory_user_subscription.attributes.each do |name, val| + expect(new_user_subscription[name]).to eq val + end + end + end + + describe "#make" do + it "returns a created UserSubcription with the correct attributes" do + user_subscription_fields = %w[author_id subsciber_id subscriber_email user_subscription_sourceable_id user_susbcription_sourceable_type] + + user_subscription = create(:user_subscription, + user_subscription_sourceable: source, + author_id: source.user_id, + subscriber_id: subscriber.id, + subscriber_email: subscriber.email) + + factory_user_subscription = described_class.make(source: source, subscriber: subscriber) + + user_subscription_fields.each do |field| + expect(factory_user_subscription[field]).to eq user_subscription[field] + end + end end end