Skip to content
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

Autodeletion #2923

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/models/archived_patient.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ class ArchivedPatient < ApplicationRecord
include PaperTrailable
include Exportable
include LastMenstrualPeriodMeasureable
include Autodeletable

# Relationships
belongs_to :clinic, optional: true
Expand Down
32 changes: 32 additions & 0 deletions app/models/concerns/autodeletable.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Allows models to be deleted after some period of time
# By making this a concern, we can reuse the same code for Patients and
# ArchivedPatients
module Autodeletable
extend ActiveSupport::Concern

# instance method
def delete_date
# if config is blank, we return nil here
initial_call_date + Config.days_until_delete if Config.days_until_delete.present?
end

# This funny business allows us to call both Patient.autodelete! and
# ArchivedPatient.autodelete! without any extra code
#
# (Any modules that include this Concern will get all methods in the
# ClassMethods module, below, as new class methods of their own!)
def self.included(base)
base.extend(ClassMethods)
end
Comment on lines +18 to +20
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this is to gnarly i can make a change ;)


module ClassMethods
def autodelete!
find_each do |p|
d = p.delete_date
# only delete if a delete date exists and is in the past.
p.destroy! if d.present? && d < Time.zone.today
end
end
end
end

14 changes: 12 additions & 2 deletions app/models/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ class Config < ApplicationRecord
hide_budget_bar: 'Enter "yes" to hide the budget bar display.',
aggregate_statistics: 'Enter "yes" to show aggregate statistics on the budget bar.',
hide_standard_dropdown_values: 'Enter "yes" to hide standard dropdown values. Only custom options (specified on this page) will be used.',
time_zone: "Time zone to use for displaying dates. Default is Eastern. Valid options are Eastern, Central, Mountain, Pacific, Alaska, Hawaii, Arizona, Indiana (East), or Puerto Rico."
time_zone: "Time zone to use for displaying dates. Default is Eastern. Valid options are Eastern, Central, Mountain, Pacific, Alaska, Hawaii, Arizona, Indiana (East), or Puerto Rico.",
days_until_delete: "Number of days (after initial entry) until patients are deleted. Leave blank to never delete old patients.",
}.freeze

enum config_key: {
Expand All @@ -46,7 +47,8 @@ class Config < ApplicationRecord
aggregate_statistics: 17,
hide_standard_dropdown_values: 18,
county: 19,
time_zone: 20
time_zone: 20,
days_until_delete: 21,
}

# which fields are URLs (run special validation only on those)
Expand Down Expand Up @@ -100,6 +102,8 @@ class Config < ApplicationRecord
[:validate_singleton, :validate_patient_archive],
shared_reset:
[:validate_singleton, :validate_shared_reset],
days_until_delete:
[:validate_singleton, :validate_number],

hide_budget_bar:
[:validate_singleton, :validate_yes_or_no],
Expand Down Expand Up @@ -161,6 +165,11 @@ def self.time_zone
ActiveSupport::TimeZone.new(TIME_ZONE[tz])
end

def self.days_until_delete
delete_days = Config.find_or_create_by(config_key: 'days_until_delete').options.try :last
delete_days.blank? ? nil : delete_days.to_i
end

def self.archive_fulfilled_patients
archive_days = Config.find_or_create_by(config_key: 'days_to_keep_fulfilled_patients').options.try :last
# default 3 months
Expand Down Expand Up @@ -242,6 +251,7 @@ def titleize_capitalization
end

# generic validator for numerics
# note: this does NOT validate negative numbers or decimals
def validate_number
options.last =~ /\A\d+\z/
end
Expand Down
4 changes: 3 additions & 1 deletion app/models/patient.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class Patient < ApplicationRecord
include Statusable
include Exportable
include EventLoggable
include Autodeletable

# Callbacks
before_validation :clean_fields
Expand Down Expand Up @@ -214,7 +215,7 @@ def archive_date
initial_call_date + Config.archive_all_patients.days
end
end

def recent_history_tracks
versions.where(updated_at: 6.days.ago..)
end
Expand Down Expand Up @@ -294,4 +295,5 @@ def special_circumstances_length
errors.add(:special_circumstances, 'is invalid') if value && value.length > 50
end
end

end
6 changes: 6 additions & 0 deletions lib/tasks/nightly_cleanup.rake
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ task nightly_cleanup: :environment do

ArchivedPatient.archive_eligible_patients!
puts "#{Time.now} -- archived patients for today for fund #{fund.name}"

Patient.autodelete!
puts "#{Time.now} -- autodeleted old patients for fund #{fund.name}"

ArchivedPatient.autodelete!
puts "#{Time.now} -- autodeleted old archived patients for fund #{fund.name}"
end
end
end
112 changes: 112 additions & 0 deletions test/models/autodeletable_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
require 'test_helper'

class AutodeletableTest < ActiveSupport::TestCase
before do
@line = create :line, name: 'DC'
@pt_a = create :patient,
name: 'Amanda',
primary_phone: '222-222-2222',
line: @line,
initial_call_date: 10.days.ago
@pt_b = create :patient,
name: 'Bethany',
primary_phone: '333-333-3333',
line: @line,
initial_call_date: 0.days.ago
@pt_c = create :patient,
name: 'Charley',
primary_phone: '444-444-4444',
line: @line,
initial_call_date: 400.days.ago
@pt_d = create :archived_patient,
line: @line,
initial_call_date: 400.days.ago

end

# by default, no deletion: config is nil
describe 'without autodeletion' do
it 'should return nil deletion date' do
assert_nil Config.days_until_delete

assert_nil @pt_a.delete_date

assert_no_difference 'Patient.count' do
Patient.autodelete!
end

assert_no_difference 'ArchivedPatient.count' do
ArchivedPatient.autodelete!
end
end
end

describe 'with autodeletion' do
before do
c = Config.find_or_create_by(config_key: 'days_until_delete')
c.config_value = { options: ["100"] }
c.save!
end

it 'should only delete patients outside the window' do
assert_difference 'Patient.count', -1 do
Patient.autodelete!
end

# specifically, that should be pt_c
refute_includes Patient.all, @pt_c

assert_difference 'ArchivedPatient.count', -1 do
ArchivedPatient.autodelete!
end
end
end

describe 'edge cases' do
describe 'config 0 days' do
before do
c = Config.find_or_create_by(config_key: 'days_until_delete')
c.config_value = { options: ["0"] }
c.save!
end

it 'if config is 0, delete all patients now' do
Timecop.freeze(1.hour.ago) do
Patient.autodelete!
# delete doesn't occur because now hasn't happened yet ;)
assert_equal 1, Patient.count
end

Timecop.freeze(1.day.after) do
# later, now has happened
Patient.autodelete!
assert_equal 0, Patient.count
end
end
end

describe 'nonzero config on the edge' do
before do
c = Config.find_or_create_by(config_key: 'days_until_delete')
c.config_value = { options: ["10"] }
c.save!
end

it 'deletes pt_a on the right day' do
Timecop.freeze(1.day.ago) do
Patient.autodelete!
assert_includes Patient.all, @pt_a
end

# present day - doesn't delete because <, not <=
Patient.autodelete!
assert_includes Patient.all, @pt_a

Timecop.freeze(1.day.after) do
Patient.autodelete!
refute_includes Patient.all, @pt_a
end
Comment on lines +92 to +108
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok this is a lil weird, because i'm using < with today, so it does a date-, not time-based comparison. I don't particularly have an issue with this -- I don't think funds will care exactly which day a patient deletes. but let me know and I can make it a bit more intuitive.

end
end
end
end
25 changes: 25 additions & 0 deletions test/models/config_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -322,5 +322,30 @@ class ConfigTest < ActiveSupport::TestCase
end
end

describe 'days_util_default' do
it 'should default to nil' do
assert_nil Config.days_until_delete
end

it 'should validate bounds' do
c = Config.find_or_create_by(config_key: 'days_until_delete')

# non numeric
c.config_value = { options: ["a"] }
refute c.valid?

# negatives don't work
c.config_value = { options: ["-48"] }
refute c.valid?

# non-singleton
c.config_value = { options: ["1", "2"] }
refute c.valid?

# good
c.config_value = { options: ["10"] }
assert c.valid?
end
end
end
end