Add self-service account deletion (#2246)

* Add self-service account deletion

* Disable access to cancelled accounts & implement Stripe interactions

* Add tests

* Fix failing tests

* Remove cancelled accounts from lists

* Fix incorrect redirect

* Use _path instead of _url for consistency

* Don't track how far the inicieration job got

We still want the step tracking so that the job can be interrupted. But, since the scope is idempotent, we don't need to track how far it got.

* Specify the exact time when the data will be deleted

* Fix crash due to unadvancable cursor

* Rename up_for_incineration to due_for_incineration

* Regenrate the schema

* Fix incorrect path check

* Migrate the SQLite schema

* Only show the cancel button on cancellable accounts

* Check that a subscirption method exists before calling it

* Ignore job failures due to missing records when an account gets deleted

* Skip sending notifications on cancelled accounts

* Use collbacks to integrate

* Add a blank line between queue_as and discard_on

* Break checks into methods

* Inline methods

* Rename Account::IncinerateJob

* Run migrations

* Migrate SQLite

test/controllers/account/cancellations_controller_test.rb

@@ -0,0 +1,46 @@
+require "test_helper"
+
+class Account::CancellationsControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    @account = accounts(:"37s")
+    @user = users(:jason)
+    sign_in_as @user
+
+    if @account.respond_to?(:subscription)
+      Account.any_instance.stubs(:subscription).returns(nil)
+    end
+  end
+
+  test "an owner can cancel the account" do
+    assert_difference -> { Account::Cancellation.count }, 1 do
+      assert_enqueued_emails 1 do
+        post account_cancellation_url
+      end
+    end
+
+    assert_redirected_to session_menu_path(script_name: nil)
+    assert_equal "Account deleted", flash[:notice]
+    assert @account.reload.cancelled?
+    assert_equal @user, @account.cancellation.initiated_by
+  end
+
+  test "non-owner cannot cancel the account" do
+    logout_and_sign_in_as users(:david)
+
+    assert_no_difference -> { Account::Cancellation.count } do
+      post account_cancellation_url
+    end
+
+    assert_response :forbidden
+  end
+
+  test "cancelling an account while in single-tenant mode does nothing" do
+    with_multi_tenant_mode(false) do
+      assert_no_difference -> { Account::Cancellation.count } do
+        post account_cancellation_url
+      end
+
+      assert_not @account.reload.cancelled?
+    end
+  end
+end

test/controllers/my/menus_controller_test.rb

@@ -78,4 +78,20 @@ class My::MenusControllerTest < ActionDispatch::IntegrationTest
     get my_menu_path, headers: { "If-None-Match" => etag }
     assert_response :not_modified
   end
+
+  test "show excludes cancelled accounts" do
+    # Create another account for the same identity
+    another_account = Account.create!(external_account_id: 9999996, name: "Cancelled Account")
+    another_user = @user.identity.users.create!(account: another_account, name: "Kevin", role: "owner")
+
+    # Cancel the other account
+    another_account.cancel(initiated_by: another_user)
+
+    get my_menu_path
+    assert_response :success
+
+    # The response should include active account but not cancelled one
+    assert_select "a[href*='#{@account.slug}']"
+    assert_select "a[href*='#{another_account.slug}']", count: 0
+  end
 end

test/controllers/sessions/menus_controller_test.rb

@@ -47,4 +47,24 @@ class Sessions::MenusControllerTest < ActionDispatch::IntegrationTest
 
     assert_response :success
   end
+
+  test "show excludes cancelled accounts" do
+    sign_in_as @identity
+    @identity.users.delete_all
+    account1 = Account.create!(external_account_id: 9999994, name: "Active Account")
+    account2 = Account.create!(external_account_id: 9999995, name: "Cancelled Account")
+    user1 = @identity.users.create!(account: account1, name: "Kevin", role: "owner")
+    user2 = @identity.users.create!(account: account2, name: "Kevin", role: "owner")
+
+    # Cancel one account
+    account2.cancel(initiated_by: user2)
+
+    untenanted do
+      get session_menu_url
+    end
+
+    # Should redirect to the only active account
+    assert_response :redirect
+    assert_redirected_to root_url(script_name: account1.slug)
+  end
 end

test/fixtures/accounts.yml

@@ -9,3 +9,9 @@ initech:
   name: Initech LLC
   external_account_id: <%= ActiveRecord::FixtureSet.identify("initech") %>
   cards_count: 0
+
+acme:
+  id: <%= ActiveRecord::FixtureSet.identify("acme", :uuid) %>
+  name: ACME
+  external_account_id: <%= ActiveRecord::FixtureSet.identify("acme") %>
+  cards_count: 0

test/jobs/account/incinerate_due_job_test.rb

@@ -0,0 +1,67 @@
+require "test_helper"
+
+class Account::IncinerateDueJobTest < ActiveJob::TestCase
+  setup do
+    @account = accounts(:"37s")
+    @user = users(:david)
+
+    # Stub Stripe methods only in SaaS mode
+    if defined?(Stripe::Subscription)
+      Stripe::Subscription.stubs(:update).returns(true)
+      Stripe::Subscription.stubs(:cancel).returns(true)
+    end
+  end
+
+  test "finds accounts up for incineration" do
+    @account.cancel(initiated_by: @user)
+    @account.cancellation.update!(created_at: 31.days.ago)
+
+    Account.any_instance.expects(:incinerate).once
+
+    Account::IncinerateDueJob.perform_now
+  end
+
+  test "incinerates each old cancelled account" do
+    # Cancel the test account
+    @account.cancel(initiated_by: @user)
+    @account.cancellation.update!(created_at: 31.days.ago)
+
+    # Just verify it gets incinerated
+    assert_difference -> { Account.count }, -1 do
+      Account::IncinerateDueJob.perform_now
+    end
+  end
+
+  test "skips recent cancellations" do
+    @account.cancel(initiated_by: @user)
+    @account.cancellation.update!(created_at: 29.days.ago)
+
+    assert_no_difference -> { Account.count } do
+      Account::IncinerateDueJob.perform_now
+    end
+  end
+
+  test "handles cursor pagination correctly" do
+    # Cancel and age the account
+    @account.cancel(initiated_by: @user)
+    @account.cancellation.update!(created_at: 31.days.ago)
+
+    # Verify it processes accounts in the scope
+    assert_difference -> { Account.count }, -1 do
+      Account::IncinerateDueJob.perform_now
+    end
+  end
+
+  test "only incinerates accounts past grace period" do
+    # Account at 29 days (within grace period - should not be incinerated)
+    @account.cancel(initiated_by: @user)
+    @account.cancellation.update!(created_at: 29.days.ago)
+
+    assert_no_difference -> { Account.count } do
+      Account::IncinerateDueJob.perform_now
+    end
+
+    # The account should still exist
+    assert Account.exists?(@account.id)
+  end
+end

test/mailers/account_mailer_test.rb

@@ -0,0 +1,57 @@
+require "test_helper"
+
+class AccountMailerTest < ActionMailer::TestCase
+  setup do
+    @account = accounts(:"37s")
+    @user = users(:david)
+    @account.cancel(initiated_by: @user)
+    @cancellation = @account.cancellation
+  end
+
+  test "cancellation sends to initiating user" do
+    email = AccountMailer.cancellation(@cancellation)
+
+    assert_emails 1 do
+      email.deliver_now
+    end
+
+    assert_equal [ @user.identity.email_address ], email.to
+  end
+
+  test "cancellation includes account name" do
+    email = AccountMailer.cancellation(@cancellation)
+
+    assert_match @account.name, email.body.encoded
+  end
+
+  test "cancellation includes support email" do
+    email = AccountMailer.cancellation(@cancellation)
+
+    assert_match "support@fizzy.do", email.body.encoded
+  end
+
+  test "cancellation has correct subject" do
+    email = AccountMailer.cancellation(@cancellation)
+
+    assert_equal "Your Fizzy account was cancelled", email.subject
+  end
+
+  test "cancellation has both HTML and text parts" do
+    email = AccountMailer.cancellation(@cancellation)
+
+    assert email.html_part.present?, "Email should have HTML part"
+    assert email.text_part.present?, "Email should have text part"
+  end
+
+  test "cancellation mentions account access is removed" do
+    email = AccountMailer.cancellation(@cancellation)
+
+    assert_match /no one can access/i, email.body.encoded
+  end
+
+  test "cancellation mentions data will be deleted" do
+    email = AccountMailer.cancellation(@cancellation)
+
+    assert_match /deleted/i, email.body.encoded
+  end
+end

test/models/account/cancellable_test.rb

@@ -0,0 +1,79 @@
+require "test_helper"
+
+class Account::CancellableTest < ActiveSupport::TestCase
+  setup do
+    @account = accounts(:"37s")
+    @user = users(:david)
+  end
+
+  test "cancel" do
+    assert_difference -> { Account::Cancellation.count }, 1 do
+      assert_enqueued_with(job: ActionMailer::MailDeliveryJob) do
+        @account.cancel(initiated_by: @user)
+      end
+    end
+
+    assert @account.cancelled?
+    assert_equal @user, @account.cancellation.initiated_by
+  end
+
+  test "cancel does nothing if already cancelled" do
+    @account.cancel(initiated_by: @user)
+
+    assert_no_changes -> { @account.cancellation.reload.created_at } do
+      @account.cancel(initiated_by: @user)
+    end
+  end
+
+  test "cancel does nothing when in single-tenant mode" do
+    Account.stubs(:accepting_signups?).returns(false)
+
+    assert_no_difference -> { Account::Cancellation.count } do
+      @account.cancel(initiated_by: @user)
+    end
+
+    assert_not @account.cancelled?
+  end
+
+  test "cancelled? returns true when cancellation exists" do
+    assert_not @account.cancelled?
+
+    @account.cancel(initiated_by: @user)
+
+    assert @account.cancelled?
+  end
+
+  test "reactivate" do
+    @account.cancel(initiated_by: @user)
+
+    assert @account.cancelled?
+
+    @account.reactivate
+    @account.reload
+
+    assert_not @account.cancelled?
+    assert_nil @account.cancellation
+  end
+
+  test "reactivate does nothing if not cancelled" do
+    assert_not @account.cancelled?
+
+    assert_nothing_raised do
+      @account.reactivate
+    end
+
+    assert_not @account.cancelled?
+  end
+
+  test "active scope excludes cancelled accounts" do
+    account2 = accounts(:initech)
+
+    initial_active_count = Account.active.count
+
+    @account.cancel(initiated_by: @user)
+
+    assert_equal initial_active_count - 1, Account.active.count
+    assert_not_includes Account.active, @account
+    assert_includes Account.active, account2
+  end
+end

test/models/account/cancellation_test.rb

@@ -0,0 +1,7 @@
+require "test_helper"
+
+class Account::CancellationTest < ActiveSupport::TestCase
+  # test "the truth" do
+  #   assert true
+  # end
+end

test/models/account/incineratable_test.rb

@@ -0,0 +1,26 @@
+require "test_helper"
+
+class Account::IncineratableTest < ActiveSupport::TestCase
+  setup do
+    @account = accounts(:"37s")
+    @user = users(:david)
+  end
+
+  test "incinerate destroys account" do
+    assert_difference -> { Account.count }, -1 do
+      @account.incinerate
+    end
+
+    assert_not Account.exists?(@account.id)
+  end
+
+  test "due_for_incineration finds old cancellations" do
+    @account.cancel(initiated_by: @user)
+
+    @account.cancellation.update!(created_at: 31.days.ago)
+    assert_equal [ @account ], Account.due_for_incineration
+
+    @account.cancellation.update!(created_at: 29.days.ago)
+    assert Account.due_for_incineration.empty?
+  end
+end

test/models/notification/bundle_test.rb

@@ -1,6 +1,8 @@
 require "test_helper"
 
 class Notification::BundleTest < ActiveSupport::TestCase
+  include ActionMailer::TestHelper
+
   setup do
     @user = users(:david)
     @user.settings.bundle_email_every_few_hours!
@@ -160,4 +162,19 @@ class Notification::BundleTest < ActiveSupport::TestCase
     assert_includes @user.notification_bundles.last.notifications, first_notification
     assert_includes @user.notification_bundles.last.notifications, second_notification
   end
+
+  test "deliver does not send email for cancelled accounts" do
+    @user.notifications.create!(source: events(:logo_published), creator: @user)
+    bundle = @user.notification_bundles.pending.last
+
+    @user.account.cancel(initiated_by: @user)
+
+    assert_no_emails do
+      deliver_enqueued_emails do
+        bundle.deliver
+      end
+    end
+
+    assert bundle.delivered?, "Bundle should be marked as delivered even if not sent"
+  end
 end

test/models/notification_pusher_test.rb

@@ -0,0 +1,32 @@
+require "test_helper"
+
+class NotificationPusherTest < ActiveSupport::TestCase
+  setup do
+    @user = users(:david)
+    @notification = @user.notifications.create!(
+      source: events(:logo_published),
+      creator: users(:jason)
+    )
+    @pusher = NotificationPusher.new(@notification)
+
+    @user.push_subscriptions.create!(
+      endpoint: "https://fcm.googleapis.com/fcm/send/test123",
+      p256dh_key: "test_key",
+      auth_key: "test_auth"
+    )
+  end
+
+  test "push does not send notifications for cancelled accounts" do
+    @user.account.cancel(initiated_by: @user)
+
+    result = @pusher.push
+
+    assert_nil result, "Should not push notifications for cancelled accounts"
+  end
+
+  test "push sends notifications for active accounts with subscriptions" do
+    result = @pusher.push
+
+    assert_not_nil result, "Should push notifications for active accounts with subscriptions"
+  end
+end

test/models/webhook/triggerable_test.rb

@@ -0,0 +1,58 @@
+require "test_helper"
+
+class Webhook::TriggerableTest < ActiveSupport::TestCase
+  setup do
+    @account = accounts(:"37s")
+    @board = boards(:writebook)
+    @card = @board.cards.first
+    @webhook = @board.webhooks.create!(
+      name: "Test Webhook",
+      url: "https://example.com/webhook",
+      subscribed_actions: [ "card_published" ]
+    )
+    # Create a test event
+    @event = @board.events.create!(
+      creator: users(:david),
+      eventable: @card,
+      action: "card_published"
+    )
+    @user = users(:david)
+  end
+
+  test "trigger creates delivery for active accounts" do
+    assert_difference -> { Webhook::Delivery.count }, 1 do
+      @webhook.trigger(@event)
+    end
+
+    delivery = Webhook::Delivery.last
+    assert_equal @event, delivery.event
+    assert_equal @webhook, delivery.webhook
+  end
+
+  test "trigger skips cancelled accounts" do
+    @account.cancel(initiated_by: @user)
+
+    assert_no_difference -> { Webhook::Delivery.count } do
+      @webhook.trigger(@event)
+    end
+  end
+
+  test "triggered_by scope finds webhooks for event" do
+    other_webhook = @board.webhooks.create!(
+      name: "Other Webhook",
+      url: "https://example.com/other",
+      subscribed_actions: [ "card_closed" ]
+    )
+
+    matching_webhooks = Webhook.triggered_by(@event)
+
+    assert_includes matching_webhooks, @webhook
+    assert_not_includes matching_webhooks, other_webhook
+  end
+
+  test "active scope only returns active webhooks" do
+    @webhook.update!(active: false)
+
+    assert_not_includes Webhook.active, @webhook
+  end
+end