diff --git a/AGENTS.md b/AGENTS.md index df764e7b9f..8ff3d4bd69 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -186,6 +186,7 @@ end - `WorkshopVariationFromIdeaService` — Variation creation from ideas - `TaggingSearchService` — Search and filter tagging data - `PersonFromUserService` — Create Person from User account +- `PersonArchivalService` — Soft-delete (discard) / restore a person together with their user - `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 diff --git a/Gemfile b/Gemfile index 7e976e9002..8f94ce7040 100644 --- a/Gemfile +++ b/Gemfile @@ -64,6 +64,9 @@ gem "action_policy", "~> 0.7.6" gem "active_storage_validations", "~> 3.0" +# Soft-delete / archive support (kept/discarded scopes, no default scope) +gem "discard", "~> 2.0" + gem "solid_cache" # Payments diff --git a/Gemfile.lock b/Gemfile.lock index 24ae78e5a2..35be6b5b79 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -184,6 +184,8 @@ GEM responders warden (~> 1.2.3) diff-lcs (1.6.2) + discard (2.0.0) + activerecord (>= 7.0, < 9.0) docile (1.4.1) domain_name (0.6.20240107) dotenv (3.2.0) @@ -788,6 +790,7 @@ DEPENDENCIES country_select debug (~> 1.11) devise (~> 5.0.4) + discard (~> 2.0) dotenv-rails draper factory_bot_rails @@ -892,6 +895,7 @@ CHECKSUMS device_detector (1.1.3) sha256=c5fe3fe42cab2e8aa01f193b2074b8bb1510373ce47127206f28c7dea75a9c79 devise (5.0.4) sha256=d605f2b85854e74e56ee789e2d398702bc2d06e6bcd894717a670a3199c74cc1 diff-lcs (1.6.2) sha256=9ae0d2cba7d4df3075fe8cd8602a8604993efc0dfa934cff568969efb1909962 + discard (2.0.0) sha256=0fc520786b4d7b9b1ea8b2b3dadbb13c3e41d2ce7d297d04869baf1c9e30d2c0 docile (1.4.1) sha256=96159be799bfa73cdb721b840e9802126e4e03dfc26863db73647204c727f21e domain_name (0.6.20240107) sha256=5f693b2215708476517479bf2b3802e49068ad82167bcd2286f899536a17d933 dotenv (3.2.0) sha256=e375b83121ea7ca4ce20f214740076129ab8514cd81378161f11c03853fe619d diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index e698aa54eb..a96c0685f9 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 archive unarchive workshop_logs checkout bio ] def index authorize! @@ -14,6 +14,7 @@ def index affiliations: :organization, categorizable_items: { category: :category_type } ).references(:user)) + base_scope = ActiveModel::Type::Boolean.new.cast(params[:archived]) ? base_scope.discarded : base_scope.kept filtered = base_scope.search_by_params(params.to_unsafe_h) .order(:first_name, :last_name) @count_display = filtered.count @@ -227,6 +228,24 @@ def destroy end end + def archive + authorize! @person, to: :archive? + PersonArchivalService.new(@person).archive! + + respond_to do |format| + format.html { redirect_to @person, status: :see_other, notice: "Person was archived." } + end + end + + def unarchive + authorize! @person, to: :archive? + PersonArchivalService.new(@person).restore! + + respond_to do |format| + format.html { redirect_to @person, status: :see_other, notice: "Person was restored." } + end + end + def check_duplicates authorize! diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index cb828a465c..5e87b30d05 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -12,6 +12,7 @@ def index per_page = params[:number_of_items_per_page].presence || 25 base_scope = authorized_scope(User.includes(:created_by, :updated_by, person: { avatar_attachment: :blob })) + base_scope = ActiveModel::Type::Boolean.new.cast(params[:archived]) ? base_scope.discarded : base_scope.kept filtered = base_scope.search_by_params(params).order(:first_name, :last_name) @users_count = filtered.count @users = filtered.paginate(page: params[:page], per_page: per_page) diff --git a/app/models/person.rb b/app/models/person.rb index 424dd07d15..398897cee4 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -1,4 +1,5 @@ class Person < ApplicationRecord + include Discard::Model include RemoteSearchable, TagFilterable, Trendable, WindowsTypeFilterable, SectorsTaggable, AgeGroupTaggable pay_customer default_payment_processor: :stripe diff --git a/app/models/user.rb b/app/models/user.rb index 39000e1c27..9d3037b0f2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -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? - super && !inactive? + super && !inactive? && !discarded? + end + + def inactive_message + discarded? ? :archived : super end # Instance-level mirror of the `has_access` scope: the account can sign in — diff --git a/app/policies/person_policy.rb b/app/policies/person_policy.rb index 1f12418ba9..b3d47c22f4 100644 --- a/app/policies/person_policy.rb +++ b/app/policies/person_policy.rb @@ -29,6 +29,11 @@ def destroy? admin? && record.persisted? && !has_associated_data? end + # Soft-delete (archive) the person and their user. + def archive? + admin? && record.persisted? + end + def search? admin? end @@ -38,7 +43,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 diff --git a/app/services/person_archival_service.rb b/app/services/person_archival_service.rb new file mode 100644 index 0000000000..d8ccc2ec0f --- /dev/null +++ b/app/services/person_archival_service.rb @@ -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 diff --git a/app/views/people/index.html.erb b/app/views/people/index.html.erb index 30120897f0..56b50d78ab 100644 --- a/app/views/people/index.html.erb +++ b/app/views/people/index.html.erb @@ -5,7 +5,12 @@

People

-
+
+ <% if ActiveModel::Type::Boolean.new.cast(params[:archived]) %> + <%= link_to "View active", people_path, class: "admin-only btn btn-secondary-outline" %> + <% else %> + <%= link_to "View archived", people_path(archived: true), class: "admin-only btn btn-secondary-outline" %> + <% end %> <% if allowed_to?(:new?, Person) %> <%= link_to "New Person", new_person_path, diff --git a/app/views/people/show.html.erb b/app/views/people/show.html.erb index d8fdc59d3b..fba70d9d8b 100644 --- a/app/views/people/show.html.erb +++ b/app/views/people/show.html.erb @@ -48,6 +48,24 @@ <%= render "bookmarks/editable_bookmark_button", resource: @person.object %>
+ <% if allowed_to?(:archive?, @person) %> +
+ <% if @person.discarded? %> + + Archived + + <%= link_to unarchive_person_path(@person), class: "btn btn-secondary px-3 py-1 text-sm", + data: { turbo_method: :patch } do %> + Restore + <% end %> + <% else %> + <%= link_to archive_person_path(@person), class: "btn btn-secondary px-3 py-1 text-sm", + data: { turbo_method: :patch } do %> + Archive + <% end %> + <% end %> +
+ <% end %>
diff --git a/config/locales/devise.en.yml b/config/locales/devise.en.yml index 61c351fd38..3b279e11d1 100644 --- a/config/locales/devise.en.yml +++ b/config/locales/devise.en.yml @@ -8,6 +8,7 @@ en: send_paranoid_instructions: "If your email address exists in our database, you will receive an email with instructions for how to confirm your email address in a few minutes." failure: already_authenticated: "You are already signed in." + archived: "Invalid email or password. Please email us or fill out our Contact Us form for assistance." inactive: "Invalid email or password. Please email us or fill out our Contact Us form for assistance." invalid: "Invalid email or password. Please email us or fill out our Contact Us form for assistance." locked: "Please email us or fill out our Contact Us form for assistance." diff --git a/config/routes.rb b/config/routes.rb index 529c0cc97e..7cffd6e9ac 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -172,6 +172,8 @@ get :workshop_logs get :checkout get :bio + patch :archive + patch :unarchive end resources :comments, only: [ :index, :create, :update ] end diff --git a/db/migrate/20260615120000_add_discarded_at_to_people_and_users.rb b/db/migrate/20260615120000_add_discarded_at_to_people_and_users.rb new file mode 100644 index 0000000000..efbe4ad6e6 --- /dev/null +++ b/db/migrate/20260615120000_add_discarded_at_to_people_and_users.rb @@ -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 diff --git a/db/schema.rb b/db/schema.rb index 87e2573c42..93ddd7efa6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -948,6 +948,7 @@ t.integer "created_by_id" t.string "credentials" t.date "date_of_birth" + t.datetime "discarded_at" t.string "display_name_preference" t.string "email" t.string "email_2" @@ -988,6 +989,7 @@ t.integer "updated_by_id" t.string "youtube_url" t.index ["created_by_id"], name: "index_people_on_created_by_id" + t.index ["discarded_at"], name: "index_people_on_discarded_at" t.index ["updated_by_id"], name: "index_people_on_updated_by_id" end @@ -1267,6 +1269,7 @@ t.integer "created_by_id" t.datetime "current_sign_in_at", precision: nil t.string "current_sign_in_ip" + t.datetime "discarded_at" t.string "email", default: "", null: false t.string "email_type" t.string "encrypted_password", default: "", null: false @@ -1307,6 +1310,7 @@ t.index ["agency_id"], name: "index_users_on_agency_id" t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true t.index ["created_by_id"], name: "index_users_on_created_by_id" + t.index ["discarded_at"], name: "index_users_on_discarded_at" t.index ["email"], name: "index_users_on_email", unique: true t.index ["favorite_event_id"], name: "index_users_on_favorite_event_id" t.index ["person_id"], name: "index_users_on_person_id" diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 5d85c58635..0b5f7976e9 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -231,6 +231,12 @@ user = create(:user, locked: true) expect(user.active_for_authentication?).to be false end + + it "returns false when discarded (archived)" do + user = create(:user) + user.discard! + expect(user.active_for_authentication?).to be false + end end describe "#first_name_or_email" do diff --git a/spec/policies/person_policy_spec.rb b/spec/policies/person_policy_spec.rb index c299741f0f..8d82a39967 100644 --- a/spec/policies/person_policy_spec.rb +++ b/spec/policies/person_policy_spec.rb @@ -111,6 +111,20 @@ def policy_for(record: nil, user:) end end + describe "#archive?" do + context "with an admin" do + subject { policy_for(record: create(:person), user: admin_user) } + + it { is_expected.to be_allowed_to(:archive?) } + end + + context "with a regular user" do + subject { policy_for(record: create(:person), user: regular_user) } + + it { is_expected.not_to be_allowed_to(:archive?) } + end + end + describe "relation_scope" do context "with admin user" do let(:policy) { policy_for(record: Person, user: admin_user) } diff --git a/spec/requests/people_archive_spec.rb b/spec/requests/people_archive_spec.rb new file mode 100644 index 0000000000..ec3852d3e2 --- /dev/null +++ b/spec/requests/people_archive_spec.rb @@ -0,0 +1,48 @@ +require "rails_helper" + +RSpec.describe "People archiving and deletion", type: :request do + let(:admin) { create(:user, :admin) } + + before { sign_in admin } + + describe "PATCH /people/:id/archive" do + it "archives the person and their user" do + person = create(:person) + + patch archive_person_path(person) + + expect(response).to redirect_to(person_path(person)) + expect(person.reload).to be_discarded + expect(person.user.reload).to be_discarded + end + end + + describe "PATCH /people/:id/unarchive" do + it "restores an archived person and their user" do + person = create(:person) + PersonArchivalService.new(person).archive! + + patch unarchive_person_path(person) + + expect(response).to redirect_to(person_path(person)) + expect(person.reload).to be_kept + expect(person.user.reload).to be_kept + end + end + + describe "GET /people (archived filter)" do + it "hides archived people by default and shows them when archived=true" do + active = create(:person, first_name: "Active", last_name: "Person") + archived = create(:person, first_name: "Archived", last_name: "Person") + PersonArchivalService.new(archived).archive! + + get people_path, headers: { "Turbo-Frame" => "people_results" } + expect(response.body).to include("Active Person") + expect(response.body).not_to include("Archived Person") + + get people_path(archived: true), headers: { "Turbo-Frame" => "people_results" } + expect(response.body).to include("Archived Person") + expect(response.body).not_to include("Active Person") + end + end +end diff --git a/spec/services/person_archival_service_spec.rb b/spec/services/person_archival_service_spec.rb new file mode 100644 index 0000000000..a644ac6e78 --- /dev/null +++ b/spec/services/person_archival_service_spec.rb @@ -0,0 +1,44 @@ +require "rails_helper" + +RSpec.describe PersonArchivalService do + describe "#archive!" do + it "discards the person and their user together" do + person = create(:person) + user = person.user + + described_class.new(person).archive! + + expect(person.reload).to be_discarded + expect(user.reload).to be_discarded + end + + it "discards a person without a user" do + person = create(:person, user: nil) + + described_class.new(person).archive! + + expect(person.reload).to be_discarded + end + + it "blocks a discarded user from authenticating" do + person = create(:person) + + described_class.new(person).archive! + + expect(person.user.reload.active_for_authentication?).to be false + end + end + + describe "#restore!" do + it "undiscards the person and their user together" do + person = create(:person) + user = person.user + described_class.new(person).archive! + + described_class.new(person).restore! + + expect(person.reload).to be_kept + expect(user.reload).to be_kept + end + end +end