From dc41f150971f11e5ce45f5775541806b84ca342f Mon Sep 17 00:00:00 2001 From: Georg Gadinger Date: Wed, 18 Oct 2023 21:42:19 +0200 Subject: [PATCH 1/4] fix deletion of inbox entries when deleting an user --- app/models/inbox.rb | 3 ++- spec/models/inbox_spec.rb | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 spec/models/inbox_spec.rb diff --git a/app/models/inbox.rb b/app/models/inbox.rb index 1a1a0066..b12beb29 100644 --- a/app/models/inbox.rb +++ b/app/models/inbox.rb @@ -22,7 +22,8 @@ class Inbox < ApplicationRecord end after_destroy do - user.touch(:inbox_updated_at) + # user might not exist at this point (account deleted, records are cleaned up async) + user&.touch(:inbox_updated_at) end def answer(answer_content, user) diff --git a/spec/models/inbox_spec.rb b/spec/models/inbox_spec.rb new file mode 100644 index 00000000..9a00635a --- /dev/null +++ b/spec/models/inbox_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe Inbox, type: :model do + describe "associations" do + it { should belong_to(:user) } + it { should belong_to(:question) } + end + + describe "before_destroy" do + let(:user) { FactoryBot.create(:user) } + let(:question) { FactoryBot.create(:question, author_is_anonymous: true) } + + it "does not fail if the user wants to delete their account" do + Inbox.create(user:, question:) + + # this deletes the User record and enqueues the deletion of all + # associated records in sidekiq + user.destroy! + + # so let's drain the queues + expect { Sidekiq::Worker.drain_all }.not_to raise_error + end + end +end From 8a26232fe6ce1290aacf99eae11686090d60450f Mon Sep 17 00:00:00 2001 From: Georg Gadinger Date: Wed, 18 Oct 2023 21:53:30 +0200 Subject: [PATCH 2/4] bad dog --- app/models/inbox.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/inbox.rb b/app/models/inbox.rb index b12beb29..ada94803 100644 --- a/app/models/inbox.rb +++ b/app/models/inbox.rb @@ -50,7 +50,7 @@ class Inbox < ApplicationRecord user.profile.anon_display_name || APP_CONFIG["anonymous_name"] else question.user.profile.safe_name - end + end, ), icon: notification_icon, body: question.content.truncate(Question::SHORT_QUESTION_MAX_LENGTH), From a85ce45b25ac921a440d7a94f068a16c73fd070e Mon Sep 17 00:00:00 2001 From: Georg Gadinger Date: Wed, 18 Oct 2023 21:57:09 +0200 Subject: [PATCH 3/4] appease the paw patrol --- app/models/inbox.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/inbox.rb b/app/models/inbox.rb index ada94803..96f3dc84 100644 --- a/app/models/inbox.rb +++ b/app/models/inbox.rb @@ -14,16 +14,16 @@ class Inbox < ApplicationRecord end after_create do - user.touch(:inbox_updated_at) + user.touch(:inbox_updated_at) # rubocop:disable Rails/SkipsModelValidations end after_update do - user.touch(:inbox_updated_at) + user.touch(:inbox_updated_at) # rubocop:disable Rails/SkipsModelValidations end after_destroy do # user might not exist at this point (account deleted, records are cleaned up async) - user&.touch(:inbox_updated_at) + user&.touch(:inbox_updated_at) # rubocop:disable Rails/SkipsModelValidations end def answer(answer_content, user) From c4da510fe71fb0d0d88b88ca730ebeee9d98276c Mon Sep 17 00:00:00 2001 From: Georg Gadinger Date: Wed, 18 Oct 2023 22:13:25 +0200 Subject: [PATCH 4/4] fix deletion of notification entries when deleting a user --- app/models/notification.rb | 7 ++++--- spec/models/notification_spec.rb | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 spec/models/notification_spec.rb diff --git a/app/models/notification.rb b/app/models/notification.rb index e36cad0a..76233c67 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -5,15 +5,16 @@ class Notification < ApplicationRecord belongs_to :target, polymorphic: true after_create do - recipient.touch(:notifications_updated_at) + recipient.touch(:notifications_updated_at) # rubocop:disable Rails/SkipsModelValidations end after_update do - recipient.touch(:notifications_updated_at) + recipient.touch(:notifications_updated_at) # rubocop:disable Rails/SkipsModelValidations end after_destroy do - recipient.touch(:notifications_updated_at) + # recipient might not exist at this point (account deleted, records are cleaned up async) + recipient&.touch(:notifications_updated_at) # rubocop:disable Rails/SkipsModelValidations end class << self diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb new file mode 100644 index 00000000..9bf3aea8 --- /dev/null +++ b/spec/models/notification_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe Notification, type: :model do + describe "associations" do + it { should belong_to(:recipient) } + it { should belong_to(:target) } + end + + describe "before_destroy" do + let(:user) { FactoryBot.create(:user) } + let(:answer) { FactoryBot.create(:answer, user: FactoryBot.create(:user)) } + + it "does not fail if the user wants to delete their account" do + Notification::QuestionAnswered.create(recipient: user, target: answer) + + # this deletes the User record and enqueues the deletion of all + # associated records in sidekiq + user.destroy! + + # so let's drain the queues + expect { Sidekiq::Worker.drain_all }.not_to raise_error + end + end +end