-
Notifications
You must be signed in to change notification settings - Fork 24
HOLD: Add archive and deletion options for people #1686
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,6 @@ | ||
| class User < ApplicationRecord | ||
| include Discard::Model | ||
|
|
||
| # Include default devise modules. Others available are: | ||
| # :confirmable, :timeoutable and :omniauthable | ||
| devise :database_authenticatable, :recoverable, :confirmable, | ||
|
|
@@ -82,7 +84,7 @@ class User < ApplicationRecord | |
| attributes user: "organizations.name" | ||
| end | ||
|
|
||
| scope :has_access, -> { where(locked_at: nil, inactive: [ false, nil ]).where.not(confirmed_at: nil) } | ||
| scope :has_access, -> { kept.where(locked_at: nil, inactive: [ false, nil ]).where.not(confirmed_at: nil) } | ||
|
|
||
| def self.search_by_params(params) | ||
| results = is_a?(ActiveRecord::Relation) ? self : all | ||
|
|
@@ -118,7 +120,11 @@ def remote_search_label | |
| end | ||
|
|
||
| def active_for_authentication? | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 From Claude: Discarded (archived) users are blocked from authenticating here in addition to the existing inactive check, so archiving an account immediately revokes login. |
||
| super && !inactive? | ||
| super && !inactive? && !discarded? | ||
| end | ||
|
|
||
| def inactive_message | ||
| discarded? ? :archived : super | ||
| end | ||
|
|
||
| def bookmark_for(record) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,8 +25,21 @@ def update? | |
| admin? | ||
| end | ||
|
|
||
| # Plain delete: allowed only when the person has nothing of substance beyond a | ||
| # user account (and trivial profile data, which cascades away on destroy). | ||
| def destroy? | ||
| admin? && record.persisted? && !has_associated_data? | ||
| admin? && record.persisted? && !has_significant_associated_data? | ||
| end | ||
|
|
||
| # Full purge of person + user + all associated data. Used to clean test data | ||
| # off production; the UI requires a browser confirmation. | ||
| def full_destroy? | ||
| admin? && record.persisted? | ||
| end | ||
|
|
||
| # Soft-delete (archive) the person and their user. | ||
| def archive? | ||
| admin? && record.persisted? | ||
| end | ||
|
|
||
| def search? | ||
|
|
@@ -38,7 +51,7 @@ def search? | |
|
|
||
| relation_scope do |relation| | ||
| next relation if admin? | ||
| relation.searchable.with_active_affiliations.where_user_not_locked | ||
| relation.kept.searchable.with_active_affiliations.where_user_not_locked | ||
| end | ||
|
|
||
| private | ||
|
|
@@ -48,9 +61,13 @@ def owner? | |
| record.user == user | ||
| end | ||
|
|
||
| def has_associated_data? | ||
| record.user.present? || | ||
| record.affiliations.exists? || | ||
| record.stories_as_spotlighted_facilitator.exists? | ||
| def has_significant_associated_data? | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 From Claude: A bare user account is intentionally not "significant" here — that's exactly the "person only has a user" case we want plain Delete to allow. Affiliations/registrations/scholarships/etc. still block it. |
||
| record.affiliations.exists? || | ||
| record.stories_as_spotlighted_facilitator.exists? || | ||
| record.event_registrations.exists? || | ||
| record.event_staffs.exists? || | ||
| record.scholarships.exists? || | ||
| record.grants.exists? || | ||
| record.form_submissions.exists? | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| class PersonArchivalService | ||
| def initialize(person) | ||
| @person = person | ||
| end | ||
|
|
||
| # Soft-delete (archive) the person and their user together. A discarded user | ||
| # is blocked from authenticating and hidden from default listings. | ||
| def archive! | ||
| ActiveRecord::Base.transaction do | ||
| @person.user&.discard! | ||
| @person.discard! | ||
| end | ||
| end | ||
|
|
||
| # Reverse an archive, bringing the person and their user back. | ||
| def restore! | ||
| ActiveRecord::Base.transaction do | ||
| @person.user&.undiscard! | ||
| @person.undiscard! | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| class PersonDeletionService | ||
| def initialize(person) | ||
| @person = person | ||
| end | ||
|
|
||
| # Permanently delete a person whose only associated record is a user account. | ||
| # Gated by PersonPolicy#destroy?, which guarantees no significant associated | ||
| # data exists, so the person's dependent: :destroy cascades are trivial. The | ||
| # user and its ahoy tracking records (visits/events) are removed alongside it. | ||
| def destroy_with_user! | ||
| ActiveRecord::Base.transaction do | ||
| destroy_user_with_tracking(@person.user) | ||
| @person.destroy! | ||
| end | ||
| end | ||
|
|
||
| # Permanently delete the person, their user, and ALL associated data. Intended | ||
| # for purging test records from production, so it clears the associations that | ||
| # would otherwise block a destroy (spotlighted stories use restrict_with_error) | ||
| # before cascading the rest via dependent: :destroy. | ||
| def full_destroy! | ||
| ActiveRecord::Base.transaction do | ||
| # Intentional bulk nullify: detach the person from any stories that | ||
| # spotlight them so the restrict_with_error guard does not block deletion. | ||
| @person.stories_as_spotlighted_facilitator.update_all(spotlighted_facilitator_id: nil) | ||
| destroy_user_with_tracking(@person.user) | ||
| @person.destroy! | ||
| end | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def destroy_user_with_tracking(user) | ||
| return if user.nil? | ||
|
|
||
| purge_ahoy_records(user) | ||
| user.destroy! | ||
| end | ||
|
|
||
| # Ahoy visits/events reference the user by id without an association, so remove | ||
| # them explicitly. Events are deleted both by user_id and by their parent visit | ||
| # so none are left orphaned. | ||
| def purge_ahoy_records(user) | ||
| visit_ids = Ahoy::Visit.where(user_id: user.id).pluck(:id) | ||
| Ahoy::Event.where(user_id: user.id).or(Ahoy::Event.where(visit_id: visit_ids)).delete_all | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 From Claude: Ahoy events are deleted by both |
||
| Ahoy::Visit.where(user_id: user.id).delete_all | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| class AddDiscardedAtToPeopleAndUsers < ActiveRecord::Migration[8.1] | ||
| def up | ||
| add_column :people, :discarded_at, :datetime unless column_exists?(:people, :discarded_at) | ||
| add_index :people, :discarded_at unless index_exists?(:people, :discarded_at) | ||
|
|
||
| add_column :users, :discarded_at, :datetime unless column_exists?(:users, :discarded_at) | ||
| add_index :users, :discarded_at unless index_exists?(:users, :discarded_at) | ||
| end | ||
|
|
||
| def down | ||
| remove_index :people, :discarded_at if index_exists?(:people, :discarded_at) | ||
| remove_column :people, :discarded_at if column_exists?(:people, :discarded_at) | ||
|
|
||
| remove_index :users, :discarded_at if index_exists?(:users, :discarded_at) | ||
| remove_column :users, :discarded_at if column_exists?(:users, :discarded_at) | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 From Claude:
?admin=trueselects the full-purge path (person + user + all associated data) and authorizes againstfull_destroy?; the UI gates it behind a browser confirmation.