diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 56edb7820e..18b1301e68 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -40,6 +40,7 @@ When changing a model or controller, check whether these related files need upda | Decorator | Decorator spec | | Mailer (add/remove) | Mailer spec, mailer preview (follow existing patterns) | | Add/remove model, concern, service, or gem | AGENTS.md | +| Add an association to `Person`/`User`, or a new model referencing them (`person_id`, `user_id`, `created_by_id`, `updated_by_id`, polymorphic `*able`) | Wire it into `PeopleRemover` (`app/services/people_remover.rb`) so a purge cascades/clears it (or deliberately blocks). `spec/services/people_remover_association_coverage_spec.rb` is the source of truth — it fails on new **associations** until they're handled and allowlisted there; it can't catch a new model that only *references* Person/User, so check that case by hand | ## Code Style diff --git a/AGENTS.md b/AGENTS.md index af9e50c08b..79506025ba 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -185,6 +185,7 @@ end - `WorkshopVariationFromIdeaService` — Variation creation from ideas - `TaggingSearchService` — Search and filter tagging data - `PersonFromUserService` — Create Person from User account +- `PeopleRemover` — Deletes a set of people and ALL their associated data, their linked User + the user's own data where relevant (real authored content is FK-restricted and blocks), plus the full PaperTrail/Ahoy audit graph; protects real/admin accounts (override with `force`). Used by the `data:remove_people` rake task and the admin-only "Delete person & all data" purge button on person edit (`purge_confirmation`/`purge` actions) - `BulkInviteService` — Bulk send welcome instructions and reset created_at for users - `FormBuilderService` — Builds configurable forms from composable sections with per-field visibility - `ModelDeduper` — Deduplication logic @@ -413,8 +414,9 @@ RuboCop linting on PRs and pushes to main. ## Rake Tasks -Located in `lib/tasks/` (4 files): +Located in `lib/tasks/` (9 files): - `dev.rake` — Development database seeding from XML/CSV - `rhino_migrator.rake` — Rich text editor migration - `attachment_report.rake` — Attachment reporting - `migrate_internal_id_to_filemaker_code.rake` — FileMaker code migration +- `remove_people.rake` (`data:remove_people`) — Removes fake form-submission people via `PeopleRemover` (dry run unless `CONFIRM=true`) diff --git a/CLAUDE.md b/CLAUDE.md index e28c34816e..9895483cdd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -40,6 +40,7 @@ When changing a model or controller, check whether these related files need upda | Decorator | Decorator spec | | Mailer (add/remove) | Mailer spec, mailer preview (follow existing patterns) | | Add/remove model, concern, service, or gem | AGENTS.md | +| Add an association to `Person`/`User`, or a new model referencing them (`person_id`, `user_id`, `created_by_id`, `updated_by_id`, polymorphic `*able`) | Wire it into `PeopleRemover` (`app/services/people_remover.rb`) so a purge cascades/clears it (or deliberately blocks). `spec/services/people_remover_association_coverage_spec.rb` is the source of truth — it fails on new **associations** until they're handled and allowlisted there; it can't catch a new model that only *references* Person/User, so check that case by hand | ## Code Style diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 7df2931117..18b9d2d84c 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -1,6 +1,6 @@ class PeopleController < ApplicationController include AhoyTracking, TagAssignable - before_action :set_person, only: %i[ show edit update destroy workshop_logs checkout bio ] + before_action :set_person, only: %i[ show edit update destroy workshop_logs checkout bio purge_confirmation purge ] def index authorize! @@ -227,6 +227,42 @@ def destroy end end + # Admin-only interstitial: shows exactly what would be deleted for this person + # (and why they're protected, if so) before anything is removed. + def purge_confirmation + authorize! @person, to: :purge? + @force = params[:force] == "true" + # force: true so the inventory reflects the full graph that WOULD be deleted, + # even when the person looks real — the page always shows what's at stake. + preview = PeopleRemover.new(person_ids: [ @person.id ], force: true) + @protections = preview.protection_reasons(@person) + @counts = preview.counts + # Everything removed with the linked account — only relevant when overriding, + # since that's when the account itself (and its content) gets destroyed. + @account_content = @force ? preview.account_content : {} + @person = @person.decorate + end + + # Admin-only deletion of a person and their whole data + audit graph. Skips a + # person who looks real unless force=true overrides the protections (see + # PeopleRemover). + def purge + authorize! @person, to: :purge? + remover = PeopleRemover.new(person_ids: [ @person.id ], force: params[:force] == "true") + + if remover.call.include?(@person.id) + redirect_to people_path, status: :see_other, + notice: "#{@person.full_name} and all their data were deleted." + else + reasons = remover.skipped.find { |skip| skip.person.id == @person.id }&.reasons + redirect_to edit_person_path(@person), status: :see_other, + alert: "Couldn't delete — this person looks real (#{reasons&.join(', ')}). Use “Delete anyway” to override." + end + rescue ActiveRecord::InvalidForeignKey + redirect_to edit_person_path(@person), status: :see_other, + alert: "Couldn't delete #{@person.full_name} — the linked User Account is still referenced by records it doesn't own (e.g. a shared banner or event it created). Reassign those first." + end + def check_duplicates authorize! diff --git a/app/policies/person_policy.rb b/app/policies/person_policy.rb index 1f12418ba9..59efe126f6 100644 --- a/app/policies/person_policy.rb +++ b/app/policies/person_policy.rb @@ -29,6 +29,14 @@ def destroy? admin? && record.persisted? && !has_associated_data? end + # Full purge of a person and their entire data graph. Bypasses the + # has_associated_data? guard on destroy? on purpose — purging exists precisely + # to remove people who have that data. The PeopleRemover service still + # refuses to touch anyone who looks real unless explicitly forced. + def purge? + admin? + end + def search? admin? end diff --git a/app/services/people_remover.rb b/app/services/people_remover.rb new file mode 100644 index 0000000000..7b0693b70c --- /dev/null +++ b/app/services/people_remover.rb @@ -0,0 +1,324 @@ +# Deletes a set of people along with ALL of their associated data, their linked +# User account and the user's own data (where relevant), and the audit trail for +# everything removed. Backs both the data:remove_people rake task and +# the admin-only "Delete person & all data" button on the person edit page. +# +# Person.destroy cascades everything a person owns — submissions, answers, +# affiliations, registrations, scholarships, addresses, contact methods, +# bookmarks, comments, sector/category tags, grants, event staffing. This also +# tears down what does NOT cascade off Person: payments (+ their refunds and +# allocations), bulk-payment notifications, Stripe Pay::Customer rows, and the +# PaperTrail + Ahoy audit trail for every record removed. +# +# The linked User and ALL of its own data are removed too: bookmarks, comments, +# user_forms, the user's notifications, Ahoy activity, AND everything it authored +# (workshops, workshop logs, reports, resources, stories, and the various ideas +# and variations). The purge preview lists all of this so the operator sees the +# full blast radius before confirming. +# +# People that look real are SKIPPED, never deleted: a grant donor, a spotlighted +# facilitator, or anyone with a User account — unless delete_users: true AND the +# account is clearly an unused auto-created one (never signed in, not a +# super_user, no Ahoy activity of its own, authored no other people). A real +# admin is always protected. +# +# force: true is a deliberate escape hatch that ignores ALL of the protections +# above and deletes anyway — the account and everything it authored. Records that +# merely *reference* the account without being owned by it (e.g. it created a +# shared banner or event) are FK-restricted; the transaction rolls back and the +# caller surfaces a clear error rather than orphaning or rewriting them. +# +# MAINTENANCE: the deletion graph is partly hardcoded (collected, audit_targets, +# authored_content_scopes, account_content). When an association is added to +# Person/User — or a model starts referencing them (person_id, user_id, +# created_by_id, updated_by_id, polymorphic *able) — wire it in here so purges +# keep cascading/clearing it instead of orphaning or blocking. +# spec/services/people_remover_association_coverage_spec.rb fails when a new +# Person/User association isn't accounted for. +class PeopleRemover + Skip = Data.define(:person, :reasons) + + # Only these models carry has_paper_trail. + VERSIONED_TYPES = %w[Payment Refund Allocation].freeze + + def initialize(person_ids: [], form_submission_ids: [], delete_users: false, force: false) + @person_ids = Array(person_ids).map(&:to_i).uniq + @form_submission_ids = Array(form_submission_ids).map(&:to_i).uniq + @delete_users = delete_users + @force = force + end + + # Why a person is protected, independent of delete_users/force — for showing + # the operator what they'd be overriding. Empty means the person is safe. + def protection_reasons(person) + reasons = [] + reasons << account_description(person.user) if person.user.present? + reasons << "is a spotlighted facilitator" if person.stories_as_spotlighted_facilitator.exists? + reasons << "is a grant donor" if person.grants.exists? + reasons + end + + # People that will be deleted (passed every safety check). + def deletable + @deletable ||= people.select { |person| reasons_for(person).empty? } + end + + # People that look real, with the reason(s) they were skipped. + def skipped + @skipped ||= people.filter_map do |person| + reasons = reasons_for(person) + Skip.new(person: person, reasons: reasons) if reasons.any? + end + end + + def missing_person_ids + target_person_ids - people.map(&:id) + end + + def missing_form_submission_ids + @form_submission_ids - FormSubmission.where(id: @form_submission_ids).pluck(:id) + end + + # Everything removed along with the destroyed User account(s) — its authored + # content plus the records that cascade with it — for the purge preview. Empty + # when no account is involved. + def account_content + user_ids = collected[:users].map(&:id) + return {} if user_ids.empty? + + counts = authored_content_scopes(user_ids).to_h { |label, relation| [ label, relation.count ] } + counts["Bookmarks"] = Bookmark.where(user_id: user_ids).count + counts["User forms"] = UserForm.where(user_id: user_ids).count + counts["Notifications"] = Notification.where(noticeable_type: "User", noticeable_id: user_ids).count + counts.select { |_, count| count.positive? } + end + + # Per-record-type counts for a dry-run preview. Does not mutate anything. + def counts + c = collected + { + people: deletable.size, + users: c[:users].size, + form_submissions: c[:submission_ids].size, + form_answers: c[:answer_ids].size, + affiliations: c[:affiliation_ids].size, + event_registrations: c[:registration_ids].size, + scholarships: c[:scholarship_ids].size, + payments: c[:payment_ids].size, + refunds: c[:refund_ids].size, + allocations: c[:allocation_ids].size, + notifications: c[:notification_ids].size, + pay_customers: c[:pay_customer_ids].size, + paper_trail_versions: version_count, + ahoy_events: ahoy_count + } + end + + # Deletes the graph in a single transaction. Returns the ids of the deleted + # people (empty when nothing was safe to delete). + def call + return [] if deletable.empty? + + # The destroys below buffer Ahoy "destroy" lifecycle events (the who/what/when + # deletion audit), but the buffer is flushed after the request — outside this + # transaction. Remember its size so a rollback can drop the events this purge + # added; otherwise the controller flushes phantom destroys for records that + # still exist. + buffered_lifecycle_events = Analytics::LifecycleBuffer.store.size + c = collected + user_ids = c[:users].map(&:id) + ActiveRecord::Base.transaction do + # Detach the user<->person link first. update_column bypasses the User + # validation that forbids nulling person_id and makes Person's has_one :user + # nullify a no-op — so we can delete the person before the user (next), even + # though the person references the user via created_by/updated_by. + c[:users].each { |user| user.update_column(:person_id, nil) } + deletable.each { |person| person.association(:user).reset } + + # Non-cascading person records, FK-safe order: allocations and refunds + # reference payments; payments reference submissions. + Allocation.where(id: c[:allocation_ids]).find_each(&:destroy!) + Refund.where(id: c[:refund_ids]).find_each(&:destroy!) + Payment.where(id: c[:payment_ids]).find_each(&:destroy!) + Notification.where(id: c[:notification_ids]).find_each(&:destroy!) + Pay::Customer.where(id: c[:pay_customer_ids]).find_each(&:destroy!) + + # Everything the account authored (FK-restricted, leaf → root) plus its own + # notifications — these don't cascade off the user and would block it. + authored_content_scopes(user_ids).each { |_, relation| relation.find_each(&:destroy!) } + Notification.where(noticeable_type: "User", noticeable_id: user_ids).delete_all + + # People BEFORE users: destroying a person removes its created_by/updated_by + # and affiliations (all FK to users), so the account is unreferenced by the + # time it's destroyed. + deletable.each(&:destroy!) + c[:users].each(&:destroy!) + + delete_audit_trail(c) + end + + deletable.map(&:id) + rescue + # Roll back the lifecycle events this purge buffered so a failed delete can't + # leave phantom Ahoy "destroy" records for records that still exist. + Analytics::LifecycleBuffer.store.slice!(buffered_lifecycle_events..) + raise + end + + private + + attr_reader :delete_users, :force + + def target_person_ids + @target_person_ids ||= begin + derived = FormSubmission.where(id: @form_submission_ids).distinct.pluck(:person_id) + (@person_ids + derived).uniq + end + end + + def people + @people ||= Person.where(id: target_person_ids).to_a + end + + # Content authored by the account(s), as [label, relation] in FK-safe delete + # order (leaf → root). created_by_id-owned with no dependent, so it must be + # destroyed explicitly before the account or its FKs block the deletion. + def authored_content_scopes(user_ids) + [ + [ "Comments", Comment.where(created_by_id: user_ids) ], + [ "Story ideas", StoryIdea.where(created_by_id: user_ids) ], + [ "Workshop ideas", WorkshopIdea.where(created_by_id: user_ids) ], + [ "Workshop variation ideas", WorkshopVariationIdea.where(created_by_id: user_ids) ], + [ "Workshop variations", WorkshopVariation.where(created_by_id: user_ids) ], + [ "Workshop logs", WorkshopLog.where(created_by_id: user_ids) ], + [ "Stories", Story.where(created_by_id: user_ids) ], + [ "Workshops", Workshop.where(created_by_id: user_ids) ], + [ "Reports", Report.where(created_by_id: user_ids) ], + [ "Resources", Resource.where(created_by_id: user_ids) ] + ] + end + + def reasons_for(person) + return [] if force + + reasons = [] + if person.user.present? && (real_user?(person.user) || !delete_users) + reasons << account_description(person.user) + end + reasons << "is a spotlighted facilitator" if person.stories_as_spotlighted_facilitator.exists? + reasons << "is a grant donor" if person.grants.exists? + reasons + end + + def account_description(user) + if real_user?(user) + "has an active/admin User account (signed in, super_user, or has its own activity)" + else + "has a User account" + end + end + + # A User is real (and must never be deleted) if it has ever signed in, is a + # super_user, has Ahoy activity of its own, or authored other people. This runs + # before any deletion, so cleaning the fake graph can't erase the signal. + def real_user?(user) + user.super_user? || + user.sign_in_count.to_i.positive? || + user.current_sign_in_at.present? || user.last_sign_in_at.present? || + Ahoy::Event.where(user_id: user.id).exists? || + Person.where(created_by_id: user.id).where.not(id: target_person_ids).exists? || + Person.where(updated_by_id: user.id).where.not(id: target_person_ids).exists? + end + + def collected + @collected ||= begin + person_ids = deletable.map(&:id) + submission_ids = FormSubmission.where(person_id: person_ids).pluck(:id) + registration_ids = EventRegistration.where(registrant_id: person_ids).pluck(:id) + scholarship_ids = Scholarship.where(recipient_id: person_ids).pluck(:id) + payment_ids = Payment.where(person_id: person_ids) + .or(Payment.where(form_submission_id: submission_ids)).pluck(:id) + { + person_ids: person_ids, + submission_ids: submission_ids, + registration_ids: registration_ids, + scholarship_ids: scholarship_ids, + affiliation_ids: Affiliation.where(person_id: person_ids).pluck(:id), + answer_ids: FormAnswer.where(form_submission_id: submission_ids).pluck(:id), + ero_ids: EventRegistrationOrganization.where(event_registration_id: registration_ids).pluck(:id), + address_ids: Address.where(addressable_type: "Person", addressable_id: person_ids).pluck(:id), + contact_method_ids: ContactMethod.where(contactable_type: "Person", contactable_id: person_ids).pluck(:id), + sectorable_item_ids: SectorableItem.where(sectorable_type: "Person", sectorable_id: person_ids).pluck(:id), + categorizable_item_ids: CategorizableItem.where(categorizable_type: "Person", categorizable_id: person_ids).pluck(:id), + payment_ids: payment_ids, + refund_ids: Refund.where(refundable_type: "Payment", refundable_id: payment_ids).pluck(:id), + allocation_ids: allocation_ids(payment_ids, scholarship_ids, registration_ids), + notification_ids: Notification.where(noticeable_type: "FormSubmission", noticeable_id: submission_ids).pluck(:id), + pay_customer_ids: Pay::Customer.where(owner_type: "Person", owner_id: person_ids).pluck(:id), + users: (delete_users || force) ? deletable.filter_map(&:user) : [] + } + end + end + + def allocation_ids(payment_ids, scholarship_ids, registration_ids) + Allocation + .where(source_type: "Payment", source_id: payment_ids) + .or(Allocation.where(source_type: "Scholarship", source_id: scholarship_ids)) + .or(Allocation.where(allocatable_type: "EventRegistration", allocatable_id: registration_ids)) + .pluck(:id) + end + + # resource_type => ids, covering every record this task removes — for the Ahoy + # event and PaperTrail version cleanup. + def audit_targets(c) + { + "Person" => c[:person_ids], + "FormSubmission" => c[:submission_ids], + "FormAnswer" => c[:answer_ids], + "Affiliation" => c[:affiliation_ids], + "EventRegistration" => c[:registration_ids], + "EventRegistrationOrganization" => c[:ero_ids], + "Scholarship" => c[:scholarship_ids], + "Address" => c[:address_ids], + "ContactMethod" => c[:contact_method_ids], + "SectorableItem" => c[:sectorable_item_ids], + "CategorizableItem" => c[:categorizable_item_ids], + "Payment" => c[:payment_ids], + "Refund" => c[:refund_ids], + "Allocation" => c[:allocation_ids], + "User" => c[:users].map(&:id) + }.reject { |_, ids| ids.empty? } + end + + def delete_audit_trail(c) + targets = audit_targets(c) + + # Includes the destroy-event versions PaperTrail just wrote for the + # versioned models. + targets.slice(*VERSIONED_TYPES).each do |type, ids| + PaperTrail::Version.where(item_type: type, item_id: ids).delete_all + end + targets.each do |type, ids| + Ahoy::Event.where(resource_type: type, resource_id: ids).delete_all + end + + user_ids = c[:users].map(&:id) + return if user_ids.empty? + + Ahoy::Event.where(user_id: user_ids).delete_all + Ahoy::Visit.where(user_id: user_ids).delete_all + end + + def version_count + audit_targets(collected).slice(*VERSIONED_TYPES) + .sum { |type, ids| PaperTrail::Version.where(item_type: type, item_id: ids).count } + end + + def ahoy_count + resource = audit_targets(collected) + .sum { |type, ids| Ahoy::Event.where(resource_type: type, resource_id: ids).count } + user_ids = collected[:users].map(&:id) + actor = user_ids.any? ? Ahoy::Event.where(user_id: user_ids).count : 0 + resource + actor + end +end diff --git a/app/views/people/edit.html.erb b/app/views/people/edit.html.erb index 1df94ef0e4..e7c4a3eb69 100644 --- a/app/views/people/edit.html.erb +++ b/app/views/people/edit.html.erb @@ -2,6 +2,11 @@
+ <% if params[:admin] == "true" && allowed_to?(:purge?, @person) %> + <%= link_to "Delete person & all data", + purge_confirmation_person_path(@person), + class: "btn btn-danger-outline px-2 py-1" %> + <% end %> <% unless @person.user %>
No user! diff --git a/app/views/people/purge_confirmation.html.erb b/app/views/people/purge_confirmation.html.erb new file mode 100644 index 0000000000..8a92a8a67d --- /dev/null +++ b/app/views/people/purge_confirmation.html.erb @@ -0,0 +1,72 @@ +<% content_for(:page_bg_class, "admin-only bg-blue-100") %> +
+
+ <%= link_to "← Back to edit", edit_person_path(@person), class: "text-sm text-gray-500 hover:text-gray-700" %> +
+ +

+ Delete person & all data: <%= @person.name %> +

+ + <% if @protections.any? %> +
+

This person looks real:

+
    + <% @protections.each do |reason| %> +
  • <%= reason %>
    • + <% end %> +
+
+ <% end %> + + <% if @force %> +
+

⚠️ Override: deleting despite the protections above.

+

+ The linked User Account and everything it created (listed below) will be + permanently deleted too. This cannot be undone. +

+
+ + <% if @account_content.any? %> +

Also removed with the User Account

+
+ <% @account_content.each do |label, count| %> +
"> +
<%= label %>
+
"><%= count.zero? ? "—" : count %>
+
+ <% end %> +
+ <% end %> + <% elsif @protections.empty? %> +
+

⚠️ This permanently deletes the person and everything below. This cannot be undone.

+
+ <% end %> + +

What will be deleted

+
+ <% @counts.each do |label, count| %> +
"> +
<%= label.to_s.tr("_", " ").capitalize %>
+
"><%= count.zero? ? "—" : count %>
+
+ <% end %> +
+ +
+ <%= link_to "Cancel", edit_person_path(@person), class: "btn btn-secondary-outline" %> + <% if @protections.any? && !@force %> + <%= link_to "Preview full delete", + purge_confirmation_person_path(@person, force: true), + class: "btn btn-danger-outline" %> + <% else %> + <%= button_to "Permanently delete", + purge_person_path(@person, force: @force), + method: :delete, + class: "btn btn-danger", + form: { data: { turbo_confirm: "Permanently delete #{@person.name} and ALL the data listed? This cannot be undone." } } %> + <% end %> +
+
diff --git a/config/routes.rb b/config/routes.rb index d5d8f257a9..9ee3605caf 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -170,6 +170,8 @@ get :workshop_logs get :checkout get :bio + get :purge_confirmation + delete :purge end resources :comments, only: [ :index, :create, :update ] end diff --git a/lib/tasks/remove_people.rake b/lib/tasks/remove_people.rake new file mode 100644 index 0000000000..a5edbd8204 --- /dev/null +++ b/lib/tasks/remove_people.rake @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +# Permanently remove people and ALL their associated data. Thin wrapper around +# PeopleRemover (app/services/people_remover.rb) — see it for the full deletion +# graph and the rules that protect real people. The original use case is cleaning +# up fake form-submission test data; FORM_SUBMISSION_IDS resolves those to their +# owning people. +# +# Usage (dry run — prints what WOULD be deleted, changes nothing): +# bin/rails data:remove_people PERSON_IDS=12,34,56 +# bin/rails data:remove_people FORM_SUBMISSION_IDS=78,90 +# +# To actually delete, add CONFIRM=true: +# bin/rails data:remove_people PERSON_IDS=12,34 CONFIRM=true +# +# DELETE_USERS=true also removes a person's unused auto-created account (never a +# super_user or anyone who has signed in / has activity). +# +# FORCE=true overrides ALL protections and deletes even people who look real +# (destroying any linked account). Use only for confirmed false positives. +# +# The deletion is attributed to an admin actor for the Ahoy audit trail: by +# default umberto.user@example.com, overridable with ACTOR_EMAIL=someone@example.com. +namespace :data do + desc "Permanently remove people and their full data + audit graph (dry run unless CONFIRM=true)" + task remove_people: :environment do + parse_ids = ->(raw) { raw.to_s.split(",").map(&:strip).reject(&:blank?).map(&:to_i).uniq } + + person_ids = parse_ids.call(ENV["PERSON_IDS"]) + submission_ids = parse_ids.call(ENV["FORM_SUBMISSION_IDS"]) + confirm = ENV["CONFIRM"] == "true" + + if person_ids.empty? && submission_ids.empty? + abort "Provide PERSON_IDS=1,2,3 and/or FORM_SUBMISSION_IDS=4,5,6" + end + + remover = PeopleRemover.new( + person_ids: person_ids, + form_submission_ids: submission_ids, + delete_users: ENV["DELETE_USERS"] == "true", + force: ENV["FORCE"] == "true" + ) + + puts "\n⚠️ WARNING: This permanently deletes each person and their ENTIRE data +" + puts " audit graph (and their User account where applicable). This cannot be" + puts " undone — verify every id against the dry-run output before re-running" + puts " with CONFIRM=true.\n" + + if remover.missing_form_submission_ids.any? + puts "Warning: FormSubmission ids not found: #{remover.missing_form_submission_ids.sort.join(', ')}" + end + if remover.missing_person_ids.any? + puts "Warning: Person ids not found: #{remover.missing_person_ids.sort.join(', ')}" + end + + if remover.skipped.any? + puts "\nSkipping #{remover.skipped.size} person(s) that look real:" + remover.skipped.each do |skip| + puts " ##{skip.person.id} #{skip.person.full_name_with_email} — #{skip.reasons.join(', ')}" + end + end + + if remover.deletable.empty? + puts "\nNothing to delete." + next + end + + puts "\n#{confirm ? 'DELETING' : 'DRY RUN — would delete'} for #{remover.deletable.size} person(s):" + remover.deletable.each { |person| puts " ##{person.id} #{person.full_name_with_email}" } + puts "\nRecords:" + remover.counts.each { |label, count| puts " #{label.to_s.tr('_', ' ').capitalize.ljust(21)}#{count}" } + + unless confirm + puts "\nDry run only. Re-run with CONFIRM=true to delete." + next + end + + # Attribute the deletion to an admin actor so the Ahoy "destroy" audit records + # who did it. Unlike the web flow there's no controller to flush the buffered + # lifecycle events, so we flush them here on success. + actor_email = ENV.fetch("ACTOR_EMAIL", "umberto.user@example.com") + actor = User.where(email: actor_email).last + puts "Warning: audit actor #{actor_email} not found — deletions won't be attributed." unless actor + + Current.user = actor + Current.source = "rake:data:remove_people" + begin + deleted = remover.call + flush_audit = -> { + events = Analytics::LifecycleBuffer.store + return if events.empty? + tracker = Ahoy::Tracker.new(user: actor) + events.each { |payload| tracker.track(payload[:name], payload[:properties]) } + events.clear + } + flush_audit.call + puts "\nDone. Deleted #{deleted.size} person(s) and their data + audit graph." + rescue ActiveRecord::InvalidForeignKey => e + abort "\nAborted (nothing deleted): a linked User Account owns content that " \ + "can't be orphaned (workshops, reports, stories, etc.). Reassign or " \ + "remove it first.\n#{e.message}" + ensure + Current.reset + end + end +end diff --git a/spec/policies/person_policy_spec.rb b/spec/policies/person_policy_spec.rb index c299741f0f..1efd3fb1a4 100644 --- a/spec/policies/person_policy_spec.rb +++ b/spec/policies/person_policy_spec.rb @@ -91,6 +91,20 @@ def policy_for(record: nil, user:) end end + describe "#purge?" do + context "with admin user" do + subject { policy_for(record: searchable_person, user: admin_user) } + + it { is_expected.to be_allowed_to(:purge?) } + end + + context "with regular user" do + subject { policy_for(record: searchable_person, user: regular_user) } + + it { is_expected.not_to be_allowed_to(:purge?) } + end + end + describe "#edit?" do context "with admin user" do subject { policy_for(record: searchable_person, user: admin_user) } diff --git a/spec/requests/people_purge_spec.rb b/spec/requests/people_purge_spec.rb new file mode 100644 index 0000000000..9a4b002db4 --- /dev/null +++ b/spec/requests/people_purge_spec.rb @@ -0,0 +1,129 @@ +require "rails_helper" + +RSpec.describe "People purge", type: :request do + let(:admin) { create(:user, :admin) } + let(:regular_user) { create(:user, :with_person) } + let!(:person) { create(:person, user: nil) } + let!(:submission) { create(:form_submission, person: person) } + + describe "GET /people/:id/purge_confirmation (preview)" do + context "as an admin" do + before { sign_in admin } + + it "shows the inventory and a delete button for a fake person" do + get purge_confirmation_person_path(person) + + expect(response).to have_http_status(:ok) + expect(response.body).to include("What will be deleted") + expect(response.body).to include("Permanently delete") + end + + it "shows the inventory and an override (not a direct delete) for a real person" do + real = create(:person) # factory gives them a User account + get purge_confirmation_person_path(real) + + expect(response.body).to include("looks real") + expect(response.body).to include("What will be deleted") + expect(response.body).to include("Preview full delete") + expect(response.body).not_to include("Permanently delete") + end + + it "shows the override warning, the account's content, and a delete button when force=true" do + real = create(:person) + create(:workshop, created_by: real.user) + + get purge_confirmation_person_path(real, force: true) + + expect(response.body).to include("Override") + expect(response.body).to include("Also removed with the User Account") + expect(response.body).to include("Workshops") + expect(response.body).to include("Permanently delete") + end + end + + context "as a regular user" do + before { sign_in regular_user } + + it "is forbidden" do + get purge_confirmation_person_path(person) + expect(response).not_to have_http_status(:ok) + end + end + end + + describe "DELETE /people/:id/purge" do + context "as an admin" do + before { sign_in admin } + + it "deletes the fake person and their data" do + delete purge_person_path(person) + + expect(response).to redirect_to(people_path) + expect(Person.exists?(person.id)).to be false + expect(FormSubmission.exists?(submission.id)).to be false + end + + it "skips a person who looks real and redirects back with an alert" do + real = create(:person) + delete purge_person_path(real) + + expect(response).to redirect_to(edit_person_path(real)) + expect(flash[:alert]).to be_present + expect(Person.exists?(real.id)).to be true + end + + it "deletes a person who looks real when force=true" do + real = create(:person) + real.user.update!(super_user: true) + + delete purge_person_path(real, force: true) + + expect(response).to redirect_to(people_path) + expect(Person.exists?(real.id)).to be false + end + + it "deletes the account's authored content too when force=true" do + real = create(:person) + workshop = create(:workshop, created_by: real.user) + + delete purge_person_path(real, force: true) + + expect(response).to redirect_to(people_path) + expect(Person.exists?(real.id)).to be false + expect(Workshop.exists?(workshop.id)).to be false + end + end + + context "as a regular user" do + before { sign_in regular_user } + + it "is forbidden and does not delete" do + delete purge_person_path(person) + + expect(response).not_to have_http_status(:ok) + expect(Person.exists?(person.id)).to be true + end + end + + context "as a visitor" do + it "redirects to sign in" do + delete purge_person_path(person) + + expect(response).to redirect_to(new_user_session_path) + expect(Person.exists?(person.id)).to be true + end + end + end + + describe "the edit-page button" do + before { sign_in admin } + + it "appears only when admin=true is in the query" do + get edit_person_path(person, admin: "true") + expect(response.body).to include("Delete person & all data") + + get edit_person_path(person) + expect(response.body).not_to include("Delete person & all data") + end + end +end diff --git a/spec/services/people_remover_association_coverage_spec.rb b/spec/services/people_remover_association_coverage_spec.rb new file mode 100644 index 0000000000..527098c547 --- /dev/null +++ b/spec/services/people_remover_association_coverage_spec.rb @@ -0,0 +1,50 @@ +require "rails_helper" + +# Drift guard: PeopleRemover hardcodes the person/user deletion graph, so adding +# an association to Person or User can silently leave a purge orphaning or +# blocking on the new data. This fails when a new association appears that isn't +# in the allowlist below — forcing a conscious decision to wire it into +# app/services/people_remover.rb (cascade it, tear it down explicitly, or make it +# a deliberate blocker) and then list it here. +RSpec.describe "PeopleRemover association coverage" do + # Every association currently accounted for. Keep alphabetized. + let(:person_associations) do + %w[ + addresses affiliations avatar_attachment avatar_blob bookmarks categories + categorizable_items comments communal_reports contact_methods created_by + event_registrations event_staffs events form_submissions grants organizations + pay_charges pay_customers pay_subscriptions payment_processor scholarships + sectorable_items sectors staffed_events stories_as_spotlighted_facilitator + updated_by user windows_types + ] + end + + let(:user_associations) do + %w[ + bookmarked_events bookmarked_resources bookmarked_video_recordings + bookmarked_workshops bookmarks comments created_by event_registrations events + favorite_event notifications organizations person reports resources + stories_as_creator story_ideas_as_creator updated_by user_form_form_fields + user_forms windows_types workshop_ideas_as_creator workshop_logs + workshop_variation_ideas_creator workshop_variations_as_creator workshops + ] + end + + it "accounts for every Person association" do + unhandled = Person.reflect_on_all_associations.map { |a| a.name.to_s } - person_associations + + expect(unhandled).to be_empty, + "New Person association(s) not accounted for: #{unhandled.join(', ')}.\n" \ + "Wire them into app/services/people_remover.rb (cascade, explicit teardown, " \ + "or a deliberate blocker), then add them to this allowlist." + end + + it "accounts for every User association" do + unhandled = User.reflect_on_all_associations.map { |a| a.name.to_s } - user_associations + + expect(unhandled).to be_empty, + "New User association(s) not accounted for: #{unhandled.join(', ')}.\n" \ + "Wire them into app/services/people_remover.rb (cascade, explicit teardown, " \ + "or a deliberate blocker), then add them to this allowlist." + end +end diff --git a/spec/services/people_remover_spec.rb b/spec/services/people_remover_spec.rb new file mode 100644 index 0000000000..cda6164efa --- /dev/null +++ b/spec/services/people_remover_spec.rb @@ -0,0 +1,297 @@ +require "rails_helper" + +RSpec.describe PeopleRemover do + # The :person factory associates a User by default; real fake-form people have + # none, so deletable candidates are built with user: nil. + + describe "#call deleting by person id" do + let!(:person) { create(:person, user: nil) } + let!(:other_person) { create(:person, user: nil) } + + let!(:submission) { create(:form_submission, person: person) } + let!(:form_field) { create(:form_field, form: submission.form) } + let!(:answer) { create(:form_answer, form_submission: submission, form_field: form_field) } + let!(:affiliation) { create(:affiliation, person: person) } + let!(:registration) { create(:event_registration, registrant: person) } + let!(:scholarship) { create(:scholarship, recipient: person) } + + # Records NOT cascaded by Person.destroy. + let!(:payment) { create(:payment, person: person, form_submission: submission) } + let!(:refund) { create(:refund, refundable: payment) } + let!(:scholarship_allocation) { create(:allocation, source: scholarship, allocatable: registration) } + let!(:bulk_notification) do + create(:notification, noticeable: submission, kind: :bulk_payment_confirmation, recipient_role: :person) + end + let!(:pay_customer) do + Pay::Customer.create!(owner: person, processor: "stripe", processor_id: "cus_fake_#{person.id}") + end + + # Audit trail for the removed records. + let!(:payment_version) { PaperTrail::Version.create!(item_type: "Payment", item_id: payment.id, event: "create") } + let!(:person_event) { create(:ahoy_event, name: "create.person", resource_type: "Person", resource_id: person.id) } + let!(:submission_event) do + create(:ahoy_event, name: "create.form_submission", resource_type: "FormSubmission", resource_id: submission.id) + end + + # An unrelated person's identical graph must survive untouched. + let!(:other_submission) { create(:form_submission, person: other_person) } + let!(:other_payment) { create(:payment, person: other_person) } + let!(:other_event) { create(:ahoy_event, name: "create.person", resource_type: "Person", resource_id: other_person.id) } + + def remove(**opts) + described_class.new(person_ids: [ person.id ], **opts).call + end + + it "returns the deleted person ids" do + expect(remove).to eq [ person.id ] + end + + it "destroys the person and the full cascaded graph" do + remove + + expect(Person.exists?(person.id)).to be false + expect(FormSubmission.exists?(submission.id)).to be false + expect(FormAnswer.exists?(answer.id)).to be false + expect(Affiliation.exists?(affiliation.id)).to be false + expect(EventRegistration.exists?(registration.id)).to be false + expect(Scholarship.exists?(scholarship.id)).to be false + end + + it "destroys records not cascaded by Person.destroy" do + remove + + expect(Payment.exists?(payment.id)).to be false + expect(Refund.exists?(refund.id)).to be false + expect(Allocation.exists?(scholarship_allocation.id)).to be false + expect(Notification.exists?(bulk_notification.id)).to be false + expect(Pay::Customer.exists?(pay_customer.id)).to be false + end + + it "deletes the PaperTrail and Ahoy audit trail for the removed records" do + remove + + expect(PaperTrail::Version.exists?(payment_version.id)).to be false + expect(Ahoy::Event.exists?(person_event.id)).to be false + expect(Ahoy::Event.exists?(submission_event.id)).to be false + end + + it "leaves unrelated people and their data untouched" do + remove + + expect(Person.exists?(other_person.id)).to be true + expect(FormSubmission.exists?(other_submission.id)).to be true + expect(Payment.exists?(other_payment.id)).to be true + expect(Ahoy::Event.exists?(other_event.id)).to be true + end + end + + describe "#call resolving by form submission id" do + let!(:person) { create(:person, user: nil) } + let!(:submission) { create(:form_submission, person: person) } + + it "resolves the owning person and deletes them" do + described_class.new(form_submission_ids: [ submission.id ]).call + + expect(Person.exists?(person.id)).to be false + expect(FormSubmission.exists?(submission.id)).to be false + end + end + + describe "protecting real-looking people" do + it "skips (does not delete) a person with a linked user account" do + person = create(:person) + create(:form_submission, person: person) + + remover = described_class.new(person_ids: [ person.id ]) + + expect(remover.call).to be_empty + expect(Person.exists?(person.id)).to be true + expect(remover.skipped.map { |s| s.person.id }).to include(person.id) + end + + it "skips a person who is a grant donor" do + person = create(:person, user: nil) + create(:grant, :donated_by_person, donor: person) + + described_class.new(person_ids: [ person.id ]).call + + expect(Person.exists?(person.id)).to be true + end + end + + describe "delete_users" do + it "destroys an unused auto-created account along with the person" do + person = create(:person) + user = person.user # factory default: not super_user, never signed in + + described_class.new(person_ids: [ person.id ], delete_users: true).call + + expect(Person.exists?(person.id)).to be false + expect(User.exists?(user.id)).to be false + end + + it "still protects a super_user" do + person = create(:person) + person.user.update!(super_user: true) + + described_class.new(person_ids: [ person.id ], delete_users: true).call + + expect(Person.exists?(person.id)).to be true + end + + it "still protects a user who has signed in" do + person = create(:person) + person.user.update!(sign_in_count: 3, current_sign_in_at: Time.current) + + described_class.new(person_ids: [ person.id ], delete_users: true).call + + expect(Person.exists?(person.id)).to be true + end + + it "protects a user with Ahoy activity of its own" do + person = create(:person) + create(:ahoy_event, user: person.user, name: "view.workshop") + + described_class.new(person_ids: [ person.id ], delete_users: true).call + + expect(Person.exists?(person.id)).to be true + end + end + + describe "force: true (override)" do + it "deletes a person with an active/admin account and destroys the account" do + person = create(:person) + person.user.update!(super_user: true) + user = person.user + + described_class.new(person_ids: [ person.id ], force: true).call + + expect(Person.exists?(person.id)).to be false + expect(User.exists?(user.id)).to be false + end + + it "deletes a grant donor that would otherwise be protected" do + person = create(:person, user: nil) + create(:grant, :donated_by_person, donor: person) + + described_class.new(person_ids: [ person.id ], force: true).call + + expect(Person.exists?(person.id)).to be false + end + + it "removes the user's own notifications when the account is destroyed" do + person = create(:person) + user = person.user + notification = create(:notification, noticeable: user) + + described_class.new(person_ids: [ person.id ], force: true).call + + expect(User.exists?(user.id)).to be false + expect(Notification.exists?(notification.id)).to be false + end + end + + describe "#protection_reasons" do + it "lists why a person looks real, regardless of force" do + person = create(:person) # factory gives a User account + create(:grant, :donated_by_person, donor: person) + + reasons = described_class.new(person_ids: [ person.id ], force: true).protection_reasons(person) + + expect(reasons).to include(a_string_matching(/User account/)) + expect(reasons).to include("is a grant donor") + end + + it "is empty for a plain fake person" do + person = create(:person, user: nil) + + expect(described_class.new(person_ids: [ person.id ]).protection_reasons(person)).to be_empty + end + end + + describe "#account_content (force)" do + it "lists the account's authored content and the records removed with it" do + person = create(:person) + user = person.user + create(:workshop, created_by: user) + create(:story, created_by: user) + create(:bookmark, user: user) + + content = described_class.new(person_ids: [ person.id ], force: true).account_content + + expect(content["Workshops"]).to eq 1 + expect(content["Stories"]).to eq 1 + expect(content["Bookmarks"]).to eq 1 + expect(content).not_to have_key("Reports") + end + + it "is empty when there is no linked account" do + person = create(:person, user: nil) + + expect(described_class.new(person_ids: [ person.id ], force: true).account_content).to be_empty + end + end + + describe "force delete when the person's own audit columns point to its user" do + it "deletes despite people.created_by_id/updated_by_id referencing the account" do + person = create(:person) + user = person.user + person.update_columns(created_by_id: user.id, updated_by_id: user.id) + + described_class.new(person_ids: [ person.id ], force: true).call + + expect(Person.exists?(person.id)).to be false + expect(User.exists?(user.id)).to be false + end + end + + describe "lifecycle (Ahoy) events on a rolled-back purge" do + before { Current.user = create(:user, :admin) } + after { Current.reset } + + it "discards the destroy events the purge buffered so no phantom audit is left" do + person = create(:person) + create(:banner, created_by: person.user) # FK-restricted ref the purge can't remove → rollback + + Analytics::LifecycleBuffer.store.clear + Analytics::LifecycleBuffer.push({ name: "sentinel", properties: {} }) + + expect do + described_class.new(person_ids: [ person.id ], force: true).call + end.to raise_error(ActiveRecord::InvalidForeignKey) + + expect(Analytics::LifecycleBuffer.store).to eq([ { name: "sentinel", properties: {} } ]) + ensure + Analytics::LifecycleBuffer.store.clear + end + end + + describe "force delete of an account that authored content" do + it "deletes the person, the account, and everything it authored" do + person = create(:person) + user = person.user + workshop = create(:workshop, created_by: user) + story = create(:story, created_by: user) + + described_class.new(person_ids: [ person.id ], force: true).call + + expect(Person.exists?(person.id)).to be false + expect(User.exists?(user.id)).to be false + expect(Workshop.exists?(workshop.id)).to be false + expect(Story.exists?(story.id)).to be false + end + end + + describe "#counts" do + it "reports per-record-type counts without mutating anything" do + person = create(:person, user: nil) + create(:form_submission, person: person) + + remover = described_class.new(person_ids: [ person.id ]) + + expect(remover.counts[:people]).to eq 1 + expect(remover.counts[:form_submissions]).to eq 1 + expect(Person.exists?(person.id)).to be true + end + end +end diff --git a/spec/tasks/remove_people_spec.rb b/spec/tasks/remove_people_spec.rb new file mode 100644 index 0000000000..33f1c78080 --- /dev/null +++ b/spec/tasks/remove_people_spec.rb @@ -0,0 +1,64 @@ +require "rails_helper" +require "rake" + +# Behaviour is covered in spec/services/people_remover_spec.rb. These specs just +# confirm the rake task wires ENV → PeopleRemover correctly. +RSpec.describe "data:remove_people" do + before(:all) do + Rails.application.load_tasks unless Rake::Task.task_defined?("data:remove_people") + end + + before { Rake::Task["data:remove_people"].reenable } + + def run_task(env = {}) + keys = %w[PERSON_IDS FORM_SUBMISSION_IDS CONFIRM DELETE_USERS FORCE] + saved = keys.index_with { |k| ENV[k] } + keys.each { |k| ENV.delete(k) } + env.each { |k, v| ENV[k.to_s] = v.to_s } + suppress_output { Rake::Task["data:remove_people"].invoke } + ensure + Rake::Task["data:remove_people"].reenable + keys.each { |k| ENV[k] = saved[k] } + end + + def suppress_output + original_stdout = $stdout + $stdout = StringIO.new + yield + ensure + $stdout = original_stdout + end + + it "aborts when no ids are given" do + expect { run_task }.to raise_error(SystemExit) + end + + it "deletes nothing without CONFIRM (dry run)" do + person = create(:person, user: nil) + create(:form_submission, person: person) + + expect { run_task(PERSON_IDS: person.id) }.not_to change(Person, :count) + expect(Person.exists?(person.id)).to be true + end + + it "deletes the targeted person with CONFIRM=true" do + person = create(:person, user: nil) + create(:form_submission, person: person) + + run_task(PERSON_IDS: person.id, CONFIRM: true) + + expect(Person.exists?(person.id)).to be false + end + + it "attributes the deletion audit to the admin actor" do + actor = create(:user, :admin, email: "umberto.user@example.com") + person = create(:person, user: nil) + + expect { run_task(PERSON_IDS: person.id, CONFIRM: true) } + .to change { Ahoy::Event.where(name: "destroy.person").count }.by(1) + + expect(Ahoy::Event.where(name: "destroy.person").last.user_id).to eq(actor.id) + ensure + Current.reset + end +end diff --git a/spec/views/page_bg_class_alignment_spec.rb b/spec/views/page_bg_class_alignment_spec.rb index 279b99147f..d9b32da5dd 100644 --- a/spec/views/page_bg_class_alignment_spec.rb +++ b/spec/views/page_bg_class_alignment_spec.rb @@ -149,6 +149,7 @@ "app/views/organizations/new.html.erb" => "admin-only bg-blue-100", "app/views/organization_statuses/new.html.erb" => "admin-only bg-blue-100", "app/views/people/new.html.erb" => "admin-only bg-blue-100", + "app/views/people/purge_confirmation.html.erb" => "admin-only bg-blue-100", "app/views/quotes/new.html.erb" => "admin-only bg-blue-100", "app/views/resources/new.html.erb" => "admin-only bg-blue-100", "app/views/sectors/new.html.erb" => "admin-only bg-blue-100",