diff --git a/app/jobs/application_job.rb b/app/jobs/application_job.rb index d394c3d10..7cbf01d55 100644 --- a/app/jobs/application_job.rb +++ b/app/jobs/application_job.rb @@ -1,4 +1,6 @@ class ApplicationJob < ActiveJob::Base + prepend AccountTenanted + # Automatically retry jobs that encountered a deadlock # retry_on ActiveRecord::Deadlocked diff --git a/app/jobs/concerns/account_tenanted.rb b/app/jobs/concerns/account_tenanted.rb new file mode 100644 index 000000000..d8df4331c --- /dev/null +++ b/app/jobs/concerns/account_tenanted.rb @@ -0,0 +1,47 @@ +# Serializes the current account into job data so that jobs run +# within the correct account context via Current.with_account. +# +# Account resolution is deferred to an around_perform callback so +# that missing accounts raise DeserializationError inside the +# execution path where discard_on can handle it. +module AccountTenanted + extend ActiveSupport::Concern + + prepended do + attr_reader :account + around_perform :with_account_context + end + + def initialize(...) + super + @account = Current.account + end + + def serialize + super.merge({ "account" => @account&.to_gid }) + end + + def deserialize(job_data) + super + @account_gid = job_data["account"] + end + + private + def with_account_context(&block) + resolve_account! + + if account.present? + Current.with_account(account, &block) + else + yield + end + end + + def resolve_account! + if @account_gid + @account = GlobalID::Locator.locate(@account_gid) + end + rescue ActiveRecord::RecordNotFound + raise ActiveJob::DeserializationError + end +end diff --git a/app/models/access.rb b/app/models/access.rb index afdd6312f..3492317e7 100644 --- a/app/models/access.rb +++ b/app/models/access.rb @@ -19,6 +19,6 @@ class Access < ApplicationRecord end def clean_inaccessible_data_later - Board::CleanInaccessibleDataJob.perform_later(user, board) + Board::CleanInaccessibleDataJob.perform_later(user, board) unless user.destroyed? end end diff --git a/app/models/account.rb b/app/models/account.rb index 92590d9f9..687a46a39 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -1,5 +1,5 @@ class Account < ApplicationRecord - include Account::Storage, Cancellable, Entropic, Incineratable, MultiTenantable, Seedeable + include Account::Storage, Cancellable, Entropic, Incineratable, MultiTenantable, Searchable, Seedeable has_one :join_code, dependent: :destroy has_many :users, dependent: :destroy diff --git a/app/models/account/import.rb b/app/models/account/import.rb index 50ec3769d..2bf1b88e7 100644 --- a/app/models/account/import.rb +++ b/app/models/account/import.rb @@ -4,7 +4,7 @@ class Account::Import < ApplicationRecord belongs_to :account belongs_to :identity - has_one_attached :file + has_one_attached :file, dependent: :purge_later enum :status, %w[ pending processing completed failed ].index_by(&:itself), default: :pending enum :failure_reason, %w[ conflict invalid_export ].index_by(&:itself), prefix: :failed_due_to, scopes: false diff --git a/app/models/account/incineratable.rb b/app/models/account/incineratable.rb index c734c0768..25b26a9f9 100644 --- a/app/models/account/incineratable.rb +++ b/app/models/account/incineratable.rb @@ -11,7 +11,7 @@ module Account::Incineratable def incinerate run_callbacks :incinerate do - account.destroy + Storage::Entry.suppressing_recording { account.destroy } end end end diff --git a/app/models/account/searchable.rb b/app/models/account/searchable.rb new file mode 100644 index 000000000..b06a44ed3 --- /dev/null +++ b/app/models/account/searchable.rb @@ -0,0 +1,14 @@ +module Account::Searchable + extend ActiveSupport::Concern + + included do + has_many :search_queries, class_name: "Search::Query", dependent: :delete_all + + before_destroy :clear_search_records + end + + private + def clear_search_records + Search::Record.for(id).destroy_all + end +end diff --git a/app/models/account/storage.rb b/app/models/account/storage.rb index 8b432e8ce..f59ff88bb 100644 --- a/app/models/account/storage.rb +++ b/app/models/account/storage.rb @@ -2,7 +2,15 @@ module Account::Storage extend ActiveSupport::Concern include Storage::Totaled + included do + before_destroy :clear_storage_entries + end + private + def clear_storage_entries + Storage::Entry.where(account_id: id).delete_all + end + def calculate_real_storage_bytes boards.sum { |board| board.send(:calculate_real_storage_bytes) } end diff --git a/app/models/export.rb b/app/models/export.rb index 101c9a379..1ebdebda9 100644 --- a/app/models/export.rb +++ b/app/models/export.rb @@ -2,7 +2,7 @@ class Export < ApplicationRecord belongs_to :account belongs_to :user - has_one_attached :file + has_one_attached :file, dependent: :purge_later enum :status, %w[ pending processing completed failed ].index_by(&:itself), default: :pending diff --git a/app/models/identity.rb b/app/models/identity.rb index ae4f7a8cb..059e8ac9c 100644 --- a/app/models/identity.rb +++ b/app/models/identity.rb @@ -9,7 +9,7 @@ class Identity < ApplicationRecord has_many :users, dependent: :nullify has_many :accounts, through: :users - has_one_attached :avatar + has_one_attached :avatar, dependent: :purge_later before_destroy :deactivate_users, prepend: true diff --git a/app/models/storage/entry.rb b/app/models/storage/entry.rb index 768a4c6f8..cb1e25c6e 100644 --- a/app/models/storage/entry.rb +++ b/app/models/storage/entry.rb @@ -5,12 +5,22 @@ class Storage::Entry < ApplicationRecord scope :pending, ->(last_entry_id) { where.not(id: ..last_entry_id) if last_entry_id } + thread_mattr_accessor :recording, default: true + + def self.suppressing_recording(&block) + original, self.recording = self.recording, false + yield + ensure + self.recording = original + end + # Records may be destroyed (during cascading deletes) but .id still works. # Skip entirely if account is destroyed - no need to track storage for deleted accounts. # Skip materialize jobs for destroyed boards since there's nothing to update. def self.record(delta:, operation:, account:, board: nil, recordable: nil, blob: nil) return if delta.zero? return if account.destroyed? + return unless recording entry = create! \ account_id: account.id, diff --git a/app/models/user/avatar.rb b/app/models/user/avatar.rb index 970104d6f..02e437630 100644 --- a/app/models/user/avatar.rb +++ b/app/models/user/avatar.rb @@ -11,7 +11,7 @@ module User::Avatar ].freeze included do - has_one_attached :avatar do |attachable| + has_one_attached :avatar, dependent: :purge_later do |attachable| attachable.variant :thumb, resize_to_fill: [ 256, 256 ], process: :immediately end diff --git a/config/initializers/active_job.rb b/config/initializers/active_job.rb index d8eed109d..415fcc909 100644 --- a/config/initializers/active_job.rb +++ b/config/initializers/active_job.rb @@ -1,39 +1,15 @@ # frozen_string_literal: true -# inspired from code in ActiveRecord::Tenanted -module FizzyActiveJobExtensions - extend ActiveSupport::Concern - - prepended do - attr_reader :account - self.enqueue_after_transaction_commit = true - end - - def initialize(...) - super - @account = Current.account - end - - def serialize - super.merge({ "account" => @account&.to_gid }) - end - - def deserialize(job_data) - super - if _account = job_data.fetch("account", nil) - @account = GlobalID::Locator.locate(_account) - end - end - - def perform_now - if account.present? - Current.with_account(account) { super } - else - super - end - end -end - ActiveSupport.on_load(:active_job) do - prepend FizzyActiveJobExtensions + self.enqueue_after_transaction_commit = true +end + +ActiveSupport.on_load(:action_mailer) do + ActionMailer::MailDeliveryJob.prepend AccountTenanted +end + +Rails.application.config.after_initialize do + Turbo::Streams::ActionBroadcastJob.prepend AccountTenanted + Turbo::Streams::BroadcastJob.prepend AccountTenanted + Turbo::Streams::BroadcastStreamJob.prepend AccountTenanted end diff --git a/test/mailers/notification/bundle_mailer_test.rb b/test/mailers/notification/bundle_mailer_test.rb index 8aca366c9..d37ecaeb7 100644 --- a/test/mailers/notification/bundle_mailer_test.rb +++ b/test/mailers/notification/bundle_mailer_test.rb @@ -106,6 +106,19 @@ class Notification::BundleMailerTest < ActionMailer::TestCase "Should not generate a real email when all notifications are stale" end + test "deliver_later works with account context" do + # Give user a second account so the mailer includes the account name in the subject + second_account = Account.create!(name: "Second account") + second_account.users.create!(identity: @user.identity, role: :member, name: @user.name) + + create_notification(@user) + + Notification::BundleMailer.notification(@bundle).deliver_later + perform_enqueued_jobs + + assert_emails 1 + end + private def create_notification(user, source: events(:logo_published)) Notification.create!(user: user, creator: user, source: source, created_at: 30.minutes.ago) diff --git a/test/models/account/incineratable_test.rb b/test/models/account/incineratable_test.rb index 021f751ba..7f69bed0b 100644 --- a/test/models/account/incineratable_test.rb +++ b/test/models/account/incineratable_test.rb @@ -23,4 +23,157 @@ class Account::IncineratableTest < ActiveSupport::TestCase @account.cancellation.update!(created_at: 29.days.ago) assert Account.due_for_incineration.empty? end + + test "incinerate destroys all associated records" do + account = accounts(:initech) + board = boards(:miltons_wish_list) + card = cards(:radio) + + Current.account = account + Current.session = Session.new(identity: identities(:mike)) + + user = users(:mike) + user.create_settings!(bundle_email_frequency: :never) + + # Direct on Account + tag = Tag.create!(title: "urgent") + column = Column.create!(board: board, name: "Test", position: 0) + export = Account::Export.create!(account: account, user: user) + search_query = Search::Query.create!(user: user, terms: "test search") + storage_entry = Storage::Entry.create!(account: account, delta: 100, operation: "attach") + + # Via Board + publication = Board::Publication.create!(board: board) + webhook = Webhook.create!(board: board, name: "Test", url: "https://example.com/webhook") + event = Event.create!(board: board, creator: user, eventable: card, action: "card_published") + + # Via Webhook + delivery = Webhook::Delivery.create!(webhook: webhook, event: event) + # DelinquencyTracker is auto-created with Webhook + + # Via Card + comment = card.comments.create!(body: "Test comment") + step = Step.create!(card: card, content: "Test step") + assignment = Assignment.create!(card: card, assignee: user, assigner: user) + tagging = Tagging.create!(card: card, tag: tag) + watch = Watch.create!(card: card, user: user) + pin = Pin.create!(card: card, user: user) + reaction = Reaction.create!(reactable: card, content: "thumbs_up") + mention = Mention.create!(source: card, mentioner: user, mentionee: user) + closure = Closure.create!(card: cards(:paycheck), user: user) + goldness = Card::Goldness.create!(card: card) + not_now = Card::NotNow.create!(card: cards(:unfinished_thoughts), user: user) + activity_spike = Card::ActivitySpike.create!(card: card) + + # Via User + notification = Notification.create!(user: user, source: event, creator: user) + notification_bundle = Notification::Bundle.create!(user: user, starts_at: 1.hour.ago, ends_at: 1.hour.from_now) + filter = Filter.create!(creator: user, fields: { indexed_by: :all, sorted_by: :newest }.to_json, params_digest: SecureRandom.hex) + + # ActiveStorage (attach image to card) + card.image.attach(io: StringIO.new("fake image"), filename: "test.png", content_type: "image/png") + + account_id = account.id + user_ids = User.where(account_id: account_id).pluck(:id) + + # Confirm records exist before destroy + assert User.where(account_id: account_id).exists? + assert Board.where(account_id: account_id).exists? + assert Card.where(account_id: account_id).exists? + assert Tag.where(account_id: account_id).exists? + assert Column.where(account_id: account_id).exists? + assert Webhook.where(account_id: account_id).exists? + assert Access.where(account_id: account_id).exists? + assert Entropy.where(account_id: account_id).exists? + assert Account::JoinCode.where(account_id: account_id).exists? + assert Account::Export.where(account_id: account_id).exists? + assert Search::Query.where(account_id: account_id).exists? + assert Storage::Entry.where(account_id: account_id).exists? + assert Board::Publication.where(account_id: account_id).exists? + assert Event.where(account_id: account_id).exists? + assert Webhook::Delivery.where(account_id: account_id).exists? + assert Webhook::DelinquencyTracker.where(account_id: account_id).exists? + assert Comment.where(account_id: account_id).exists? + assert Step.where(account_id: account_id).exists? + assert Assignment.where(account_id: account_id).exists? + assert Tagging.where(account_id: account_id).exists? + assert Watch.where(account_id: account_id).exists? + assert Pin.where(account_id: account_id).exists? + assert Reaction.where(account_id: account_id).exists? + assert Mention.where(account_id: account_id).exists? + assert Closure.where(account_id: account_id).exists? + assert Card::Goldness.where(account_id: account_id).exists? + assert Card::NotNow.where(account_id: account_id).exists? + assert Card::ActivitySpike.where(account_id: account_id).exists? + assert Notification.where(account_id: account_id).exists? + assert Notification::Bundle.where(account_id: account_id).exists? + assert Filter.where(account_id: account_id).exists? + assert User::Settings.where(user_id: user.id).exists? + assert ActiveStorage::Attachment.where(account_id: account_id).exists? + assert ActiveStorage::Blob.where(account_id: account_id).exists? + assert ActionText::RichText.where(account_id: account_id).exists? + + # Flush jobs enqueued during setup (Turbo broadcasts, etc.) while records still exist + perform_enqueued_jobs + + account.incinerate + perform_enqueued_jobs + + # Confirm account is gone + assert_not Account.exists?(account_id) + + # Direct associations + assert_empty User.where(account_id: account_id) + assert_empty Board.where(account_id: account_id) + assert_empty Card.where(account_id: account_id) + assert_empty Webhook.where(account_id: account_id) + assert_empty Tag.where(account_id: account_id) + assert_empty Column.where(account_id: account_id) + assert_empty Entropy.where(account_id: account_id) + assert_empty Account::JoinCode.where(account_id: account_id) + assert_empty Account::Export.where(account_id: account_id) + assert_empty Account::Import.where(account_id: account_id) + assert_empty Search::Query.where(account_id: account_id) + + # Via Board + assert_empty Board::Publication.where(account_id: account_id) + assert_empty Access.where(account_id: account_id) + assert_empty Event.where(account_id: account_id) + + # Via Webhook + assert_empty Webhook::Delivery.where(account_id: account_id) + assert_empty Webhook::DelinquencyTracker.where(account_id: account_id) + + # Via Card + assert_empty Comment.where(account_id: account_id) + assert_empty Step.where(account_id: account_id) + assert_empty Assignment.where(account_id: account_id) + assert_empty Tagging.where(account_id: account_id) + assert_empty Watch.where(account_id: account_id) + assert_empty Pin.where(account_id: account_id) + assert_empty Reaction.where(account_id: account_id) + assert_empty Mention.where(account_id: account_id) + assert_empty Closure.where(account_id: account_id) + assert_empty Card::Goldness.where(account_id: account_id) + assert_empty Card::NotNow.where(account_id: account_id) + assert_empty Card::ActivitySpike.where(account_id: account_id) + + # Via User + assert_empty Notification.where(account_id: account_id) + assert_empty Notification::Bundle.where(account_id: account_id) + assert_empty Filter.where(account_id: account_id) + assert_empty User::Settings.where(user_id: user_ids) + + # Storage + assert_empty Storage::Entry.where(account_id: account_id) + assert_empty Storage::Total.where(owner_type: "Account", owner_id: account_id) + + # ActiveStorage / ActionText + assert_empty ActiveStorage::Attachment.where(account_id: account_id) + assert_empty ActiveStorage::Blob.where(account_id: account_id) + assert_empty ActionText::RichText.where(account_id: account_id) + + # Search records (sharded) + assert_empty Search::Record.for(account_id).where(account_id: account_id) + end end diff --git a/test/models/notification_test.rb b/test/models/notification_test.rb index 8d9809ba1..02dcbc7db 100644 --- a/test/models/notification_test.rb +++ b/test/models/notification_test.rb @@ -50,6 +50,23 @@ class NotificationTest < ActiveSupport::TestCase assert_turbo_stream_broadcasts [ kevin, :notifications ] end + test "broadcast job renders account-scoped URLs" do + kevin = users(:kevin) + account = kevin.account + notifications(:layout_commented_kevin).destroy + + # Enqueue the broadcast job but don't run it yet + notification = Notification.create!(user: kevin, source: events(:layout_commented), creator: users(:david)) + + broadcasts = capture_turbo_stream_broadcasts([ kevin, :notifications ]) do + perform_enqueued_jobs + end + + assert broadcasts.any?, "Expected at least one broadcast" + html = broadcasts.last.to_s + assert_includes html, "href=\"#{account.slug}/", "Broadcast should include account slug in URLs" + end + test "broadcasting on update when read removes" do notification = notifications(:layout_commented_kevin) diff --git a/test/test_helper.rb b/test/test_helper.rb index b1f813761..4d85c51f9 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -48,6 +48,16 @@ module ActiveSupport include ActionTextTestHelper, CachingTestHelper, CardTestHelper, ChangeTestHelper, SessionTestHelper include Turbo::Broadcastable::TestHelper + # Jobs must carry their own account context via AccountTenanted, + # not rely on Current.account leaking from the test setup. + def perform_enqueued_jobs(...) + saved_account = Current.account + Current.account = nil + super + ensure + Current.account = saved_account + end + setup do Current.account = accounts("37s") end