From 0638a2c09937f54d2ccebd3969d6a6a71dd8ac58 Mon Sep 17 00:00:00 2001 From: Eli Baum Date: Wed, 12 Apr 2023 20:36:55 -0400 Subject: [PATCH 1/6] initial attempt, no tests --- app/models/concerns/autodeletable.rb | 21 +++++++++++++++++++++ app/models/config.rb | 13 +++++++++++-- app/models/patient.rb | 5 +++++ 3 files changed, 37 insertions(+), 2 deletions(-) create mode 100644 app/models/concerns/autodeletable.rb diff --git a/app/models/concerns/autodeletable.rb b/app/models/concerns/autodeletable.rb new file mode 100644 index 000000000..b66c70d50 --- /dev/null +++ b/app/models/concerns/autodeletable.rb @@ -0,0 +1,21 @@ +# Allows models to be deleted after some period of time +module AutoDeletable + extend ActiveSupport::Concern + + def delete_date + if Config.days_until_delete.blank? + initial_call_date + Config.days_until_delete + else + nil + end + end + + def autodelete! + return if delete_date.blank? + + if delete_date < Date.today + destroy! + end + end +end + diff --git a/app/models/config.rb b/app/models/config.rb index f8ba9b85d..e4131a070 100644 --- a/app/models/config.rb +++ b/app/models/config.rb @@ -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: { @@ -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) @@ -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], @@ -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 diff --git a/app/models/patient.rb b/app/models/patient.rb index d807d0d75..4a2769ec8 100644 --- a/app/models/patient.rb +++ b/app/models/patient.rb @@ -215,6 +215,10 @@ def archive_date end end + def delete_date + initial_call_date + Config.days_until_delete + end + def recent_history_tracks versions.where(updated_at: 6.days.ago..) end @@ -294,4 +298,5 @@ def special_circumstances_length errors.add(:special_circumstances, 'is invalid') if value && value.length > 50 end end + end From d3d4956a01e2292950aa0f98572e4b42e7e0efd6 Mon Sep 17 00:00:00 2001 From: Eli Baum Date: Wed, 12 Apr 2023 20:49:20 -0400 Subject: [PATCH 2/6] cleanup, sanity check --- app/models/archived_patient.rb | 1 + app/models/concerns/autodeletable.rb | 7 ++----- app/models/patient.rb | 1 + 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/app/models/archived_patient.rb b/app/models/archived_patient.rb index 5ead4aebb..9d09edd9b 100644 --- a/app/models/archived_patient.rb +++ b/app/models/archived_patient.rb @@ -6,6 +6,7 @@ class ArchivedPatient < ApplicationRecord include PaperTrailable include Exportable include LastMenstrualPeriodMeasureable + include AutoDeletable # Relationships belongs_to :clinic, optional: true diff --git a/app/models/concerns/autodeletable.rb b/app/models/concerns/autodeletable.rb index b66c70d50..ae984b6de 100644 --- a/app/models/concerns/autodeletable.rb +++ b/app/models/concerns/autodeletable.rb @@ -3,11 +3,8 @@ module AutoDeletable extend ActiveSupport::Concern def delete_date - if Config.days_until_delete.blank? - initial_call_date + Config.days_until_delete - else - nil - end + # if config is blank, we return nil here + initial_call_date + Config.days_until_delete if Config.days_until_delete.blank? end def autodelete! diff --git a/app/models/patient.rb b/app/models/patient.rb index 4a2769ec8..dfe319e54 100644 --- a/app/models/patient.rb +++ b/app/models/patient.rb @@ -14,6 +14,7 @@ class Patient < ApplicationRecord include Statusable include Exportable include EventLoggable + include AutoDeletable # Callbacks before_validation :clean_fields From 3626d13eac9e51f87ccec0826dfd19e18e9326df Mon Sep 17 00:00:00 2001 From: Eli Baum Date: Wed, 12 Apr 2023 21:57:41 -0400 Subject: [PATCH 3/6] fix concern, cleanup --- app/models/archived_patient.rb | 2 +- app/models/concerns/autodeletable.rb | 18 ++++++++++++------ app/models/patient.rb | 2 +- lib/tasks/nightly_cleanup.rake | 6 ++++++ 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/app/models/archived_patient.rb b/app/models/archived_patient.rb index 9d09edd9b..0a31000fe 100644 --- a/app/models/archived_patient.rb +++ b/app/models/archived_patient.rb @@ -6,7 +6,7 @@ class ArchivedPatient < ApplicationRecord include PaperTrailable include Exportable include LastMenstrualPeriodMeasureable - include AutoDeletable + include Autodeletable # Relationships belongs_to :clinic, optional: true diff --git a/app/models/concerns/autodeletable.rb b/app/models/concerns/autodeletable.rb index ae984b6de..dee6dc09e 100644 --- a/app/models/concerns/autodeletable.rb +++ b/app/models/concerns/autodeletable.rb @@ -1,17 +1,23 @@ # Allows models to be deleted after some period of time -module AutoDeletable +# 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.blank? end - def autodelete! - return if delete_date.blank? - - if delete_date < Date.today - destroy! + # class method + def self.autodelete! + find_each do |p| + d = p.delete_date + # only delete if a delete date exists and is in the past. + if d.present? && d < Time.zone.today + p.destroy! + end end end end diff --git a/app/models/patient.rb b/app/models/patient.rb index dfe319e54..a8118fd68 100644 --- a/app/models/patient.rb +++ b/app/models/patient.rb @@ -14,7 +14,7 @@ class Patient < ApplicationRecord include Statusable include Exportable include EventLoggable - include AutoDeletable + include Autodeletable # Callbacks before_validation :clean_fields diff --git a/lib/tasks/nightly_cleanup.rake b/lib/tasks/nightly_cleanup.rake index 6348d4dff..904328555 100644 --- a/lib/tasks/nightly_cleanup.rake +++ b/lib/tasks/nightly_cleanup.rake @@ -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 From 331447eb122b542137f466a6bb8efe7a7082b963 Mon Sep 17 00:00:00 2001 From: Eli Baum Date: Wed, 12 Apr 2023 21:58:30 -0400 Subject: [PATCH 4/6] remove delete_date from patient; already moved --- app/models/patient.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/models/patient.rb b/app/models/patient.rb index a8118fd68..b8aadc158 100644 --- a/app/models/patient.rb +++ b/app/models/patient.rb @@ -215,11 +215,7 @@ def archive_date initial_call_date + Config.archive_all_patients.days end end - - def delete_date - initial_call_date + Config.days_until_delete - end - + def recent_history_tracks versions.where(updated_at: 6.days.ago..) end From cb9f89591d926637597a7f1b4ad0f6b6fa11986d Mon Sep 17 00:00:00 2001 From: Eli Baum Date: Thu, 13 Apr 2023 22:19:48 -0400 Subject: [PATCH 5/6] add tests --- app/models/concerns/autodeletable.rb | 24 ++++-- app/models/config.rb | 1 + test/models/autodeletable_test.rb | 112 +++++++++++++++++++++++++++ test/models/config_test.rb | 25 ++++++ 4 files changed, 154 insertions(+), 8 deletions(-) create mode 100644 test/models/autodeletable_test.rb diff --git a/app/models/concerns/autodeletable.rb b/app/models/concerns/autodeletable.rb index dee6dc09e..589bfa485 100644 --- a/app/models/concerns/autodeletable.rb +++ b/app/models/concerns/autodeletable.rb @@ -7,16 +7,24 @@ module Autodeletable # 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.blank? + initial_call_date + Config.days_until_delete if Config.days_until_delete.present? end - # class method - def self.autodelete! - find_each do |p| - d = p.delete_date - # only delete if a delete date exists and is in the past. - if d.present? && d < Time.zone.today - p.destroy! + # 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 + + 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 diff --git a/app/models/config.rb b/app/models/config.rb index e4131a070..316bd992d 100644 --- a/app/models/config.rb +++ b/app/models/config.rb @@ -251,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 diff --git a/test/models/autodeletable_test.rb b/test/models/autodeletable_test.rb new file mode 100644 index 000000000..c6552d118 --- /dev/null +++ b/test/models/autodeletable_test.rb @@ -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 + # 2 hours 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 + end + end + end +end \ No newline at end of file diff --git a/test/models/config_test.rb b/test/models/config_test.rb index 984e02c38..27da39537 100644 --- a/test/models/config_test.rb +++ b/test/models/config_test.rb @@ -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 From 05a3cc5ef3d3a3f2d65dec711cade59557d2a2c1 Mon Sep 17 00:00:00 2001 From: Eli Baum Date: Thu, 13 Apr 2023 22:24:16 -0400 Subject: [PATCH 6/6] oops typo --- test/models/autodeletable_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/models/autodeletable_test.rb b/test/models/autodeletable_test.rb index c6552d118..fc10dfc2a 100644 --- a/test/models/autodeletable_test.rb +++ b/test/models/autodeletable_test.rb @@ -78,7 +78,7 @@ class AutodeletableTest < ActiveSupport::TestCase end Timecop.freeze(1.day.after) do - # 2 hours later, now has happened + # later, now has happened Patient.autodelete! assert_equal 0, Patient.count end