diff --git a/app/models/concerns/user_subscription_sourceable.rb b/app/models/concerns/user_subscription_sourceable.rb index ddce09ec4..8da63d462 100644 --- a/app/models/concerns/user_subscription_sourceable.rb +++ b/app/models/concerns/user_subscription_sourceable.rb @@ -4,7 +4,7 @@ module UserSubscriptionSourceable # 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 :user_subscriptions, as: :user_subscription_sourceable, dependent: :nullify has_many :sourced_subscribers, class_name: "User", through: :user_subscriptions, diff --git a/app/models/user_subscription.rb b/app/models/user_subscription.rb index 9d306d250..29fb211cc 100644 --- a/app/models/user_subscription.rb +++ b/app/models/user_subscription.rb @@ -9,14 +9,23 @@ class UserSubscription < ApplicationRecord 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 + belongs_to :user_subscription_sourceable, polymorphic: true, optional: true validates :author_id, presence: true + 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 } + 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, on: :create + validates :user_subscription_sourceable_id, presence: true, on: :update, if: :user_subscription_sourceable_type + validates :user_subscription_sourceable_type, presence: true, on: :create + validates :user_subscription_sourceable_type, presence: true, on: :update, if: :user_subscription_sourceable_id + + validates :user_subscription_sourceable_type, inclusion: { in: ALLOWED_TYPES }, on: :create + validates :user_subscription_sourceable_type, + inclusion: { in: ALLOWED_TYPES }, on: :update, if: :user_subscription_sourceable_id validate :tag_enabled validate :non_apple_auth_subscriber diff --git a/db/migrate/20200827073520_set_user_subscription_sourceable_columns_to_null.rb b/db/migrate/20200827073520_set_user_subscription_sourceable_columns_to_null.rb new file mode 100644 index 000000000..de8119dad --- /dev/null +++ b/db/migrate/20200827073520_set_user_subscription_sourceable_columns_to_null.rb @@ -0,0 +1,6 @@ +class SetUserSubscriptionSourceableColumnsToNull < ActiveRecord::Migration[6.0] + def change + change_column_null :user_subscriptions, :user_subscription_sourceable_id, true + change_column_null :user_subscriptions, :user_subscription_sourceable_type, true + end +end diff --git a/db/schema.rb b/db/schema.rb index c1083a906..d63972c6a 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_08_22_092853) do +ActiveRecord::Schema.define(version: 2020_08_27_073520) do # These are extensions that must be enabled in order to support this database enable_extension "citext" @@ -1161,8 +1161,8 @@ ActiveRecord::Schema.define(version: 2020_08_22_092853) do 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.bigint "user_subscription_sourceable_id" + t.string "user_subscription_sourceable_type" t.index ["author_id"], name: "index_user_subscriptions_on_author_id" 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 diff --git a/spec/models/user_subscription_spec.rb b/spec/models/user_subscription_spec.rb index 5ec7581c6..5545804a4 100644 --- a/spec/models/user_subscription_spec.rb +++ b/spec/models/user_subscription_spec.rb @@ -44,6 +44,28 @@ RSpec.describe UserSubscription, type: :model do error = "Can't subscribe with an Apple private relay. Please update email." expect(user_subscription.errors[:subscriber_email]).to include(error) end + + describe "#user_subscription_sourceable" do + it "is required on creation" do + subscription = described_class.new( + user_subscription_sourceable: nil, subscriber: subscriber, subscriber_email: subscriber.email, + author: source.user + ) + subscription.save + + expect(subscription).not_to be_valid + expect(subscription.errors.messages.keys).to include( + :user_subscription_sourceable_id, :user_subscription_sourceable_type + ) + end + + it "can be nulled on update" do + subscription = described_class.make(source: source, subscriber: subscriber) + subscription.update(user_subscription_sourceable: nil) + + expect(subscription).to be_valid + end + end end describe "#build" do