From a920d6cf1c3b259cebf6d75cbd7c5a09a77f5445 Mon Sep 17 00:00:00 2001 From: Andy Zhao Date: Mon, 9 Jul 2018 15:54:09 -0400 Subject: [PATCH] Add dependent destroys to automate some deletion tasks (#554) * Add dependent destroys for dependency-less models * Add before destroy for empty dm channels * Fix typo * Add new tests for new dependent destroys * Remove failing test b/c rethinking relationships later * Use find_by to return nil instead of erroring out * Add before_destroy to destroy follows --- app/models/follow.rb | 2 +- app/models/user.rb | 36 ++++++++++++++++++++++++++---------- spec/models/user_spec.rb | 18 +++++++++++------- 3 files changed, 38 insertions(+), 18 deletions(-) diff --git a/app/models/follow.rb b/app/models/follow.rb index dbda56451..353827b25 100644 --- a/app/models/follow.rb +++ b/app/models/follow.rb @@ -48,7 +48,7 @@ class Follow < ApplicationRecord def remove_from_feed super if followable_type == "User" - User.find(followable.id)&.touch(:last_notification_activity) + User.find_by(id: followable.id)&.touch(:last_notification_activity) end end diff --git a/app/models/user.rb b/app/models/user.rb index c4f8f16c9..2b5052979 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -13,22 +13,22 @@ class User < ApplicationRecord belongs_to :organization, optional: true has_many :articles - has_many :badge_achievements + has_many :badge_achievements, dependent: :destroy has_many :badges, through: :badge_achievements - has_many :collections + has_many :collections, dependent: :destroy has_many :comments has_many :email_messages, class_name: "Ahoy::Message" - has_many :github_repos - has_many :identities - has_many :mentions + has_many :github_repos, dependent: :destroy + has_many :identities, dependent: :destroy + has_many :mentions, dependent: :destroy has_many :messages has_many :notes, as: :noteable - has_many :notifications - has_many :reactions - has_many :tweets - has_many :chat_channel_memberships + has_many :notifications, dependent: :destroy + has_many :reactions, dependent: :destroy + has_many :tweets, dependent: :destroy + has_many :chat_channel_memberships, dependent: :destroy has_many :chat_channels, through: :chat_channel_memberships - has_many :notification_subscriptions + has_many :push_notification_subscriptions, dependent: :destroy has_many :feedback_messages mount_uploader :profile_image, ProfileImageUploader @@ -88,6 +88,8 @@ class User < ApplicationRecord before_validation :downcase_email before_validation :check_for_username_change before_destroy :remove_from_algolia_index + before_destroy :destroy_empty_dm_channels + before_destroy :destroy_follows algoliasearch per_environment: true, enqueue: :trigger_delayed_index do attribute :name @@ -461,4 +463,18 @@ class User < ApplicationRecord def remove_from_algolia_index remove_from_index! end + + def destroy_empty_dm_channels + return if chat_channels.empty? || + chat_channels.where(channel_type: "direct").empty? + empty_dm_channels = chat_channels.where(channel_type: "direct"). + messages.select { |chat_channel| chat_channel.messages.empty? } + empty_dm_channels.destroy_all + end + + def destroy_follows + follower_relationships = Follow.where(followable_id: id, followable_type: "User") + follower_relationships.destroy_all + follows.destroy_all + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 65c225424..1169440fd 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -10,17 +10,21 @@ RSpec.describe User, type: :model do let (:second_org) { create(:organization) } it { is_expected.to have_many(:articles) } - it { is_expected.to have_many(:badge_achievements) } + it { is_expected.to have_many(:badge_achievements).dependent(:destroy) } it { is_expected.to have_many(:badges).through(:badge_achievements) } - it { is_expected.to have_many(:collections) } + it { is_expected.to have_many(:collections).dependent(:destroy) } it { is_expected.to have_many(:comments) } it { is_expected.to have_many(:email_messages).class_name("Ahoy::Message") } - it { is_expected.to have_many(:identities) } - it { is_expected.to have_many(:mentions) } + it { is_expected.to have_many(:identities).dependent(:destroy) } + it { is_expected.to have_many(:mentions).dependent(:destroy) } it { is_expected.to have_many(:notes) } - it { is_expected.to have_many(:notifications) } - it { is_expected.to have_many(:reactions) } - it { is_expected.to have_many(:tweets) } + it { is_expected.to have_many(:notifications).dependent(:destroy) } + it { is_expected.to have_many(:reactions).dependent(:destroy) } + it { is_expected.to have_many(:tweets).dependent(:destroy) } + it { is_expected.to have_many(:github_repos).dependent(:destroy) } + it { is_expected.to have_many(:chat_channel_memberships).dependent(:destroy) } + it { is_expected.to have_many(:chat_channels).through(:chat_channel_memberships) } + it { is_expected.to have_many(:push_notification_subscriptions).dependent(:destroy) } it { is_expected.to validate_uniqueness_of(:username).case_insensitive } it { is_expected.to validate_uniqueness_of(:github_username).allow_blank } it { is_expected.to validate_uniqueness_of(:twitter_username).allow_blank }