Clean up `organization_users` in `Organizations::HardDeleteService` before destroying the organization
## Context The foreign key `fk_8471abad75` on `organization_users(organization_id)` references `organizations(id)` with `ON DELETE RESTRICT`. This is intentional, per the design discussion in [!180028 (note_2357622733)](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180028#note_2357622733): > But I start to think, maybe it makes sense to remove the `ON CASCADE DELETE` and simply refuse to delete an organization if there are still users associated with it. > > Let's go with `DELETE RESTRICT` which will force handling of the deletion at application level. This is what happens with namespaces where the groups have to be recursively destroyed in depth first fashion. `Organization#empty?` (in `app/models/organizations/organization.rb:124`) deliberately checks only `groups.none? && projects.none?` — it does **not** check `organization_users`. The design intent is that **a soft-deleted organization can still have users associated with it**; user removal happens at hard-delete time, in `Organizations::HardDeleteService`, before the organization row is destroyed. This issue covers that cleanup. ## Proposal In `Organizations::HardDeleteService` (`app/services/organizations/hard_delete_service.rb`), inside `#organization_destroy`, delete the `organization_users` rows before calling `organization.destroy!`. This is the application-level cleanup the `ON DELETE RESTRICT` constraint forces. ### Suggested pattern Use `each_batch` + `delete_all` to keep memory bounded for large orgs and match the idiom used elsewhere in the codebase: ```ruby organization.organization_users.each_batch { |batch| batch.delete_all } ``` `organization_users` has no destroy-time side effects, so `delete_all` is safe — no `destroy_all` needed. ### Ordering The deletion must happen **before** `organization.destroy!` (the FK is `RESTRICT`, so the destroy would fail otherwise). It should happen **inside** the existing `#organization_destroy` rescue block, so a failure aborts the in-flight hard-deletion and rolls back the state via `abort_hard_deletion_safely`. Concretely: ```ruby def organization_destroy organization_path = organization.full_path organization.organization_users.each_batch { |batch| batch.delete_all } organization.destroy! log_event(organization_path) rescue StandardError => e abort_hard_deletion_safely log_failure('Organization hard deletion failed', e) raise end ``` ### Test coverage - Spec: hard-delete removes the `organization_users` rows in batches before destroying the organization. - Drop the `allow(organization).to receive(:destroy!).and_return(true)` stubs in `spec/services/organizations/hard_delete_service_spec.rb` and `ee/spec/services/ee/organizations/hard_delete_service_spec.rb`, and assert `Organizations::Organization.exists?(organization.id)` is false post-destroy. ## Explicit non-goal: do NOT tighten `Organization#empty?` `Organization#empty?` should continue to check only `groups.none? && projects.none?`. Soft-deleting an organization that still has members associated with it is a supported case — the design is to handle user removal at hard-delete time, not at soft-delete time. ## Out of scope - Other FKs referencing `organizations(id)` — tracked in #601743+s (audit per FK; remaining FKs may use `ON DELETE CASCADE`, application-level cleanup, or LFK, depending on intent). - Cross-database FK conversions to LFK — split into their own issues if/when needed. ## Notes - `Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/594310')` in `app/workers/organizations/hard_delete_worker.rb:18` was added in anticipation of this exact cleanup expanding the per-request query count beyond the default threshold; it should remain after this issue lands.
issue