From 78fd785f0c5e1b1a8c9bb99d78a5612cff85203d Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Fri, 6 Mar 2020 17:26:49 +0100 Subject: [PATCH 01/18] Extract TruncateData out of :truncate_data task --- lib/tasks/data/truncate_data.rb | 77 +++++++++++++++++++++++ spec/lib/tasks/data/truncate_data_spec.rb | 56 +++++++++++++++++ 2 files changed, 133 insertions(+) create mode 100644 lib/tasks/data/truncate_data.rb create mode 100644 spec/lib/tasks/data/truncate_data_spec.rb diff --git a/lib/tasks/data/truncate_data.rb b/lib/tasks/data/truncate_data.rb new file mode 100644 index 0000000000..2add9f6111 --- /dev/null +++ b/lib/tasks/data/truncate_data.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +class TruncateData + def initialize(months_to_keep: nil) + @date = (months_to_keep || 3).months.ago + end + + def call + sql_delete_from " + spree_inventory_units #{where_order_id_in_orders_to_delete}" + sql_delete_from " + spree_inventory_units + where shipment_id in (select id from spree_shipments #{where_order_id_in_orders_to_delete})" + + truncate_adjustments + + sql_delete_from "spree_line_items #{where_order_id_in_orders_to_delete}" + sql_delete_from "spree_payments #{where_order_id_in_orders_to_delete}" + sql_delete_from "spree_shipments #{where_order_id_in_orders_to_delete}" + Spree::ReturnAuthorization.delete_all + + truncate_order_cycle_data + + sql_delete_from "proxy_orders #{where_oc_id_in_ocs_to_delete}" + + sql_delete_from "spree_orders #{where_oc_id_in_ocs_to_delete}" + sql_delete_from "order_cycle_schedules #{where_oc_id_in_ocs_to_delete}" + sql_delete_from "order_cycles #{where_ocs_to_delete}" + + Spree::TokenizedPermission.where("created_at < '#{date}'").delete_all + Spree::StateChange.delete_all + Spree::LogEntry.delete_all + sql_delete_from "sessions" + end + + private + + attr_reader :date + + def sql_delete_from(sql) + ActiveRecord::Base.connection.execute("delete from #{sql}") + end + + def where_order_id_in_orders_to_delete + "where order_id in (select id from spree_orders #{where_oc_id_in_ocs_to_delete})" + end + + def where_oc_id_in_ocs_to_delete + "where order_cycle_id in (select id from order_cycles #{where_ocs_to_delete} )" + end + + def where_ocs_to_delete + "where orders_close_at < '#{date}'" + end + + def truncate_adjustments + sql_delete_from "spree_adjustments where source_type = 'Spree::Order' + and source_id in (select id from spree_orders #{where_oc_id_in_ocs_to_delete})" + sql_delete_from "spree_adjustments where source_type = 'Spree::Shipment' + and source_id in (select id from spree_shipments #{where_order_id_in_orders_to_delete})" + sql_delete_from "spree_adjustments where source_type = 'Spree::Payment' + and source_id in (select id from spree_payments #{where_order_id_in_orders_to_delete})" + sql_delete_from "spree_adjustments where source_type = 'Spree::LineItem' + and source_id in (select id from spree_line_items #{where_order_id_in_orders_to_delete})" + end + + def truncate_order_cycle_data + sql_delete_from "coordinator_fees #{where_oc_id_in_ocs_to_delete}" + sql_delete_from " + exchange_variants where exchange_id + in (select id from exchanges #{where_oc_id_in_ocs_to_delete})" + sql_delete_from " + exchange_fees where exchange_id + in (select id from exchanges #{where_oc_id_in_ocs_to_delete})" + sql_delete_from "exchanges #{where_oc_id_in_ocs_to_delete}" + end +end diff --git a/spec/lib/tasks/data/truncate_data_spec.rb b/spec/lib/tasks/data/truncate_data_spec.rb new file mode 100644 index 0000000000..9ed8075c72 --- /dev/null +++ b/spec/lib/tasks/data/truncate_data_spec.rb @@ -0,0 +1,56 @@ +require 'spec_helper' +require 'tasks/data/truncate_data' + +describe TruncateData do + describe '#call' do + before do + allow(Spree::ReturnAuthorization).to receive(:delete_all) + allow(Spree::StateChange).to receive(:delete_all) + allow(Spree::LogEntry).to receive(:delete_all) + end + + context 'when months_to_keep is not specified' do + it 'truncates order cycles closed earlier than 3 months ago' do + order_cycle = create( + :order_cycle, orders_open_at: 4.months.ago, orders_close_at: 4.months.ago + 1.day + ) + create(:order, order_cycle: order_cycle) + + TruncateData.new.call + + expect(OrderCycle.all).to be_empty + end + end + + context 'when months_to_keep is nil' do + it 'truncates order cycles closed earlier than 3 months ago' do + order_cycle = create( + :order_cycle, orders_open_at: 4.months.ago, orders_close_at: 4.months.ago + 1.day + ) + create(:order, order_cycle: order_cycle) + + TruncateData.new(months_to_keep: nil).call + + expect(OrderCycle.all).to be_empty + end + end + + context 'when months_to_keep is specified' do + it 'truncates order cycles closed earlier than months_to_keep months ago' do + old_order_cycle = create( + :order_cycle, orders_open_at: 7.months.ago, orders_close_at: 7.months.ago + 1.day + ) + create(:order, order_cycle: old_order_cycle) + recent_order_cycle = create( + :order_cycle, orders_open_at: 1.months.ago, orders_close_at: 1.months.ago + 1.day + ) + create(:order, order_cycle: recent_order_cycle) + + TruncateData.new(months_to_keep: 6).call + + expect(OrderCycle.all).to contain_exactly(recent_order_cycle) + end + end + end +end + From 60d29d619ffe696343faa03eadbb595d295d2d4f Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Fri, 6 Mar 2020 17:34:45 +0100 Subject: [PATCH 02/18] Replace :truncate_data definition with new class And cover it with a test. --- lib/tasks/data/truncate_data.rake | 80 ++++--------------- .../lib/tasks/data/truncate_data_rake_spec.rb | 34 ++++++++ 2 files changed, 48 insertions(+), 66 deletions(-) create mode 100644 spec/lib/tasks/data/truncate_data_rake_spec.rb diff --git a/lib/tasks/data/truncate_data.rake b/lib/tasks/data/truncate_data.rake index a815f6ea1f..69f529cc67 100644 --- a/lib/tasks/data/truncate_data.rake +++ b/lib/tasks/data/truncate_data.rake @@ -1,3 +1,8 @@ +# frozen_string_literal: true + +require 'highline' +require 'tasks/data/truncate_data' + # This task can be used to significantly reduce the size of a database # This is used for example when loading live data into a staging server # This way the staging server is not overloaded with too much data @@ -7,75 +12,18 @@ namespace :ofn do task truncate: :environment do guard_and_warn - sql_delete_from " - spree_inventory_units #{where_order_id_in_orders_to_delete}" - sql_delete_from " - spree_inventory_units - where shipment_id in (select id from spree_shipments #{where_order_id_in_orders_to_delete})" - - truncate_adjustments - - sql_delete_from "spree_line_items #{where_order_id_in_orders_to_delete}" - sql_delete_from "spree_payments #{where_order_id_in_orders_to_delete}" - sql_delete_from "spree_shipments #{where_order_id_in_orders_to_delete}" - Spree::ReturnAuthorization.delete_all - - truncate_order_cycle_data - - sql_delete_from "proxy_orders #{where_oc_id_in_ocs_to_delete}" - - sql_delete_from "spree_orders #{where_oc_id_in_ocs_to_delete}" - sql_delete_from "order_cycle_schedules #{where_oc_id_in_ocs_to_delete}" - sql_delete_from "order_cycles #{where_ocs_to_delete}" - - Spree::TokenizedPermission.where("created_at < '#{date}'").delete_all - Spree::StateChange.delete_all - Spree::LogEntry.delete_all - sql_delete_from "sessions" + TruncateData.new.call end - def sql_delete_from(sql) - ActiveRecord::Base.connection.execute("delete from #{sql}") - end + def guard_and_warn + if Rails.env.production? + Rails.logger.info("This task cannot be executed in production") + exit + end - private - - def date - 3.months.ago - end - - def where_ocs_to_delete - "where orders_close_at < '#{date}'" - end - - def where_oc_id_in_ocs_to_delete - "where order_cycle_id in (select id from order_cycles #{where_ocs_to_delete} )" - end - - def where_order_id_in_orders_to_delete - "where order_id in (select id from spree_orders #{where_oc_id_in_ocs_to_delete})" - end - - def truncate_adjustments - sql_delete_from "spree_adjustments where source_type = 'Spree::Order' - and source_id in (select id from spree_orders #{where_oc_id_in_ocs_to_delete})" - sql_delete_from "spree_adjustments where source_type = 'Spree::Shipment' - and source_id in (select id from spree_shipments #{where_order_id_in_orders_to_delete})" - sql_delete_from "spree_adjustments where source_type = 'Spree::Payment' - and source_id in (select id from spree_payments #{where_order_id_in_orders_to_delete})" - sql_delete_from "spree_adjustments where source_type = 'Spree::LineItem' - and source_id in (select id from spree_line_items #{where_order_id_in_orders_to_delete})" - end - - def truncate_order_cycle_data - sql_delete_from "coordinator_fees #{where_oc_id_in_ocs_to_delete}" - sql_delete_from " - exchange_variants where exchange_id - in (select id from exchanges #{where_oc_id_in_ocs_to_delete})" - sql_delete_from " - exchange_fees where exchange_id - in (select id from exchanges #{where_oc_id_in_ocs_to_delete})" - sql_delete_from "exchanges #{where_oc_id_in_ocs_to_delete}" + message = "\n <%= color('This will permanently change DB contents', :yellow) %>, + are you sure you want to proceed? (y/N)" + exit unless HighLine.new.agree(message) { |q| q.default = "n" } end end end diff --git a/spec/lib/tasks/data/truncate_data_rake_spec.rb b/spec/lib/tasks/data/truncate_data_rake_spec.rb new file mode 100644 index 0000000000..fff40d5b1f --- /dev/null +++ b/spec/lib/tasks/data/truncate_data_rake_spec.rb @@ -0,0 +1,34 @@ +require 'spec_helper' +require 'rake' + +describe 'truncate_data.rake' do + describe ':truncate' do + context 'when months_to_keep is specified' do + it 'truncates order cycles closed earlier than months_to_keep months ago' do + Rake.application.rake_require 'tasks/data/truncate_data' + Rake::Task.define_task(:environment) + + highline = instance_double(HighLine, agree: true) + allow(HighLine).to receive(:new).and_return(highline) + + old_order_cycle = create( + :order_cycle, + orders_open_at: 7.months.ago, + orders_close_at: 7.months.ago + 1.day, + ) + create(:order, order_cycle: old_order_cycle) + recent_order_cycle = create( + :order_cycle, + orders_open_at: 1.months.ago, + orders_close_at: 1.months.ago + 1.day, + ) + create(:order, order_cycle: recent_order_cycle) + + months_to_keep = 6 + Rake.application.invoke_task "ofn:data:truncate[#{months_to_keep}]" + + expect(OrderCycle.all).to contain_exactly(recent_order_cycle) + end + end + end +end From be123b2a720d1e5657e59a564d57c081932d5569 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Fri, 6 Mar 2020 17:40:11 +0100 Subject: [PATCH 03/18] Specify how much data to remove in :truncate_data --- lib/tasks/data/truncate_data.rake | 5 +++-- lib/tasks/data/truncate_data.rb | 2 +- spec/lib/tasks/data/truncate_data_spec.rb | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/tasks/data/truncate_data.rake b/lib/tasks/data/truncate_data.rake index 69f529cc67..12c7866926 100644 --- a/lib/tasks/data/truncate_data.rake +++ b/lib/tasks/data/truncate_data.rake @@ -9,10 +9,11 @@ require 'tasks/data/truncate_data' namespace :ofn do namespace :data do desc 'Truncate data' - task truncate: :environment do + task :truncate, [:months_to_keep] => :environment do |_task, args| guard_and_warn - TruncateData.new.call + months_to_keep = args.months_to_keep.to_i + TruncateData.new(months_to_keep).call end def guard_and_warn diff --git a/lib/tasks/data/truncate_data.rb b/lib/tasks/data/truncate_data.rb index 2add9f6111..5534ec4dfc 100644 --- a/lib/tasks/data/truncate_data.rb +++ b/lib/tasks/data/truncate_data.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class TruncateData - def initialize(months_to_keep: nil) + def initialize(months_to_keep = nil) @date = (months_to_keep || 3).months.ago end diff --git a/spec/lib/tasks/data/truncate_data_spec.rb b/spec/lib/tasks/data/truncate_data_spec.rb index 9ed8075c72..6d26ae9a29 100644 --- a/spec/lib/tasks/data/truncate_data_spec.rb +++ b/spec/lib/tasks/data/truncate_data_spec.rb @@ -29,7 +29,7 @@ describe TruncateData do ) create(:order, order_cycle: order_cycle) - TruncateData.new(months_to_keep: nil).call + TruncateData.new(nil).call expect(OrderCycle.all).to be_empty end @@ -46,7 +46,7 @@ describe TruncateData do ) create(:order, order_cycle: recent_order_cycle) - TruncateData.new(months_to_keep: 6).call + TruncateData.new(6).call expect(OrderCycle.all).to contain_exactly(recent_order_cycle) end From f199cb1bea6a22566056db1d58cd7edc23806bcb Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Fri, 6 Mar 2020 17:45:03 +0100 Subject: [PATCH 04/18] Warn but allow executing :truncate_data in prod --- lib/tasks/data/truncate_data.rake | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/tasks/data/truncate_data.rake b/lib/tasks/data/truncate_data.rake index 12c7866926..c1c94b7310 100644 --- a/lib/tasks/data/truncate_data.rake +++ b/lib/tasks/data/truncate_data.rake @@ -10,20 +10,18 @@ namespace :ofn do namespace :data do desc 'Truncate data' task :truncate, [:months_to_keep] => :environment do |_task, args| - guard_and_warn + warn_with_confirmation months_to_keep = args.months_to_keep.to_i TruncateData.new(months_to_keep).call end - def guard_and_warn - if Rails.env.production? - Rails.logger.info("This task cannot be executed in production") - exit - end - - message = "\n <%= color('This will permanently change DB contents', :yellow) %>, - are you sure you want to proceed? (y/N)" + def warn_with_confirmation + message = <<-MSG.strip_heredoc + \n + <%= color('This will permanently change DB contents. Please, make a backup first.', :yellow) %> + Are you sure you want to proceed? (y/N) + MSG exit unless HighLine.new.agree(message) { |q| q.default = "n" } end end From b6d3c3039a58a52cc74295fcb95d566cdda803df Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Fri, 6 Mar 2020 17:01:00 +0100 Subject: [PATCH 05/18] Fix "Method has too many lines" violation --- lib/tasks/data/truncate_data.rb | 51 +++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/lib/tasks/data/truncate_data.rb b/lib/tasks/data/truncate_data.rb index 5534ec4dfc..3e0b7390c4 100644 --- a/lib/tasks/data/truncate_data.rb +++ b/lib/tasks/data/truncate_data.rb @@ -6,37 +6,52 @@ class TruncateData end def call - sql_delete_from " - spree_inventory_units #{where_order_id_in_orders_to_delete}" - sql_delete_from " - spree_inventory_units - where shipment_id in (select id from spree_shipments #{where_order_id_in_orders_to_delete})" - + truncate_inventory truncate_adjustments - - sql_delete_from "spree_line_items #{where_order_id_in_orders_to_delete}" - sql_delete_from "spree_payments #{where_order_id_in_orders_to_delete}" - sql_delete_from "spree_shipments #{where_order_id_in_orders_to_delete}" - Spree::ReturnAuthorization.delete_all - + truncate_order_associations truncate_order_cycle_data - sql_delete_from "proxy_orders #{where_oc_id_in_ocs_to_delete}" - sql_delete_from "spree_orders #{where_oc_id_in_ocs_to_delete}" - sql_delete_from "order_cycle_schedules #{where_oc_id_in_ocs_to_delete}" + + truncate_subscriptions + sql_delete_from "order_cycles #{where_ocs_to_delete}" Spree::TokenizedPermission.where("created_at < '#{date}'").delete_all - Spree::StateChange.delete_all - Spree::LogEntry.delete_all - sql_delete_from "sessions" + + remove_transient_data end private attr_reader :date + def truncate_order_associations + sql_delete_from "spree_line_items #{where_order_id_in_orders_to_delete}" + sql_delete_from "spree_payments #{where_order_id_in_orders_to_delete}" + sql_delete_from "spree_shipments #{where_order_id_in_orders_to_delete}" + end + + def remove_transient_data + Spree::ReturnAuthorization.delete_all + Spree::StateChange.delete_all + Spree::LogEntry.delete_all + sql_delete_from "sessions" + end + + def truncate_subscriptions + sql_delete_from "order_cycle_schedules #{where_oc_id_in_ocs_to_delete}" + sql_delete_from "proxy_orders #{where_oc_id_in_ocs_to_delete}" + end + + def truncate_inventory + sql_delete_from " + spree_inventory_units #{where_order_id_in_orders_to_delete}" + sql_delete_from " + spree_inventory_units + where shipment_id in (select id from spree_shipments #{where_order_id_in_orders_to_delete})" + end + def sql_delete_from(sql) ActiveRecord::Base.connection.execute("delete from #{sql}") end From a4372e4d313157a154a6e44797e25bc2142649a6 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Fri, 6 Mar 2020 17:49:38 +0100 Subject: [PATCH 06/18] Fix long lines --- lib/tasks/data/truncate_data.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/tasks/data/truncate_data.rb b/lib/tasks/data/truncate_data.rb index 3e0b7390c4..68f08977ac 100644 --- a/lib/tasks/data/truncate_data.rb +++ b/lib/tasks/data/truncate_data.rb @@ -70,13 +70,16 @@ class TruncateData def truncate_adjustments sql_delete_from "spree_adjustments where source_type = 'Spree::Order' - and source_id in (select id from spree_orders #{where_oc_id_in_ocs_to_delete})" + and source_id in (select id from spree_orders #{where_oc_id_in_ocs_to_delete})" + sql_delete_from "spree_adjustments where source_type = 'Spree::Shipment' - and source_id in (select id from spree_shipments #{where_order_id_in_orders_to_delete})" + and source_id in (select id from spree_shipments #{where_order_id_in_orders_to_delete})" + sql_delete_from "spree_adjustments where source_type = 'Spree::Payment' - and source_id in (select id from spree_payments #{where_order_id_in_orders_to_delete})" + and source_id in (select id from spree_payments #{where_order_id_in_orders_to_delete})" + sql_delete_from "spree_adjustments where source_type = 'Spree::LineItem' - and source_id in (select id from spree_line_items #{where_order_id_in_orders_to_delete})" + and source_id in (select id from spree_line_items #{where_order_id_in_orders_to_delete})" end def truncate_order_cycle_data From e1a80edb7e7b5292e6d14c22f03569ee22272694 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Fri, 6 Mar 2020 18:09:42 +0100 Subject: [PATCH 07/18] Carefully doc how to archive data from an instance --- lib/tasks/data/truncate_data.rake | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/lib/tasks/data/truncate_data.rake b/lib/tasks/data/truncate_data.rake index c1c94b7310..c5e9985b3d 100644 --- a/lib/tasks/data/truncate_data.rake +++ b/lib/tasks/data/truncate_data.rake @@ -6,6 +6,22 @@ require 'tasks/data/truncate_data' # This task can be used to significantly reduce the size of a database # This is used for example when loading live data into a staging server # This way the staging server is not overloaded with too much data +# +# This is also aimed at implementing data archiving. We assume data older than +# 2 years can be safely removed and restored from a backup. This gives room for +# hubs to do their tax declaration. +# +# It's a must to perform a backup right before executing this. Then, to allow +# for a later data recovery we need to keep track of the exact moment this rake +# task was executed. +# +# Execute this in production only when the instance users are sleeping to avoid any trouble. +# +# Example: +# +# $ bundle exec rake "ofn:data:truncate[24]" +# +# This will remove data older than 2 years (24 months). namespace :ofn do namespace :data do desc 'Truncate data' From d215c76bc98aaf9d466805e1e6da87c2cae2eaaa Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Tue, 10 Mar 2020 12:23:14 +0100 Subject: [PATCH 08/18] Make it even more explicit the action is dangerous We're yelling at the person to make a backup before proceeding. --- lib/tasks/data/truncate_data.rake | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/tasks/data/truncate_data.rake b/lib/tasks/data/truncate_data.rake index c5e9985b3d..ef01918205 100644 --- a/lib/tasks/data/truncate_data.rake +++ b/lib/tasks/data/truncate_data.rake @@ -35,10 +35,12 @@ namespace :ofn do def warn_with_confirmation message = <<-MSG.strip_heredoc \n - <%= color('This will permanently change DB contents. Please, make a backup first.', :yellow) %> + <% highlighted_message = "This will permanently change DB contents. Please, make a backup first." %> + <%= color(highlighted_message, :blink, :on_red) %> Are you sure you want to proceed? (y/N) MSG - exit unless HighLine.new.agree(message) { |q| q.default = "n" } + + exit unless HighLine.new.agree(message) { |q| q.default = "N" } end end end From 5f84c51c1343c8701f0d9aba15b05ba43e4e514f Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Tue, 10 Mar 2020 13:07:34 +0100 Subject: [PATCH 09/18] Delete StateChanges older than a month They are useful for troubleshooting but a month data seems enough. --- lib/tasks/data/truncate_data.rb | 2 +- spec/lib/tasks/data/truncate_data_spec.rb | 24 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/tasks/data/truncate_data.rb b/lib/tasks/data/truncate_data.rb index 68f08977ac..d01c914ea7 100644 --- a/lib/tasks/data/truncate_data.rb +++ b/lib/tasks/data/truncate_data.rb @@ -34,7 +34,7 @@ class TruncateData def remove_transient_data Spree::ReturnAuthorization.delete_all - Spree::StateChange.delete_all + Spree::StateChange.delete_all("created_at < '#{1.month.ago.to_date}'") Spree::LogEntry.delete_all sql_delete_from "sessions" end diff --git a/spec/lib/tasks/data/truncate_data_spec.rb b/spec/lib/tasks/data/truncate_data_spec.rb index 6d26ae9a29..faf2e6268d 100644 --- a/spec/lib/tasks/data/truncate_data_spec.rb +++ b/spec/lib/tasks/data/truncate_data_spec.rb @@ -20,6 +20,14 @@ describe TruncateData do expect(OrderCycle.all).to be_empty end + + it 'deletes state changes older than a month' do + TruncateData.new.call + + expect(Spree::StateChange) + .to have_received(:delete_all) + .with("created_at < '#{1.month.ago.to_date}'") + end end context 'when months_to_keep is nil' do @@ -33,6 +41,14 @@ describe TruncateData do expect(OrderCycle.all).to be_empty end + + it 'deletes state changes older than a month' do + TruncateData.new.call + + expect(Spree::StateChange) + .to have_received(:delete_all) + .with("created_at < '#{1.month.ago.to_date}'") + end end context 'when months_to_keep is specified' do @@ -50,6 +66,14 @@ describe TruncateData do expect(OrderCycle.all).to contain_exactly(recent_order_cycle) end + + it 'deletes state changes older than a month' do + TruncateData.new.call + + expect(Spree::StateChange) + .to have_received(:delete_all) + .with("created_at < '#{1.month.ago.to_date}'") + end end end end From 0f1d57db73e0c352f4c06f419e317aef2a32f5c3 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Tue, 10 Mar 2020 13:10:52 +0100 Subject: [PATCH 10/18] Delete LogEntries older than a month They are useful for troubleshooting but a month data seems enough. --- lib/tasks/data/truncate_data.rb | 2 +- spec/lib/tasks/data/truncate_data_spec.rb | 24 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/tasks/data/truncate_data.rb b/lib/tasks/data/truncate_data.rb index d01c914ea7..fbf658973c 100644 --- a/lib/tasks/data/truncate_data.rb +++ b/lib/tasks/data/truncate_data.rb @@ -35,7 +35,7 @@ class TruncateData def remove_transient_data Spree::ReturnAuthorization.delete_all Spree::StateChange.delete_all("created_at < '#{1.month.ago.to_date}'") - Spree::LogEntry.delete_all + Spree::LogEntry.delete_all("created_at < '#{1.month.ago.to_date}'") sql_delete_from "sessions" end diff --git a/spec/lib/tasks/data/truncate_data_spec.rb b/spec/lib/tasks/data/truncate_data_spec.rb index faf2e6268d..bc79463f48 100644 --- a/spec/lib/tasks/data/truncate_data_spec.rb +++ b/spec/lib/tasks/data/truncate_data_spec.rb @@ -28,6 +28,14 @@ describe TruncateData do .to have_received(:delete_all) .with("created_at < '#{1.month.ago.to_date}'") end + + it 'deletes log entries older than a month' do + TruncateData.new.call + + expect(Spree::LogEntry) + .to have_received(:delete_all) + .with("created_at < '#{1.month.ago.to_date}'") + end end context 'when months_to_keep is nil' do @@ -49,6 +57,14 @@ describe TruncateData do .to have_received(:delete_all) .with("created_at < '#{1.month.ago.to_date}'") end + + it 'deletes log entries older than a month' do + TruncateData.new.call + + expect(Spree::LogEntry) + .to have_received(:delete_all) + .with("created_at < '#{1.month.ago.to_date}'") + end end context 'when months_to_keep is specified' do @@ -74,6 +90,14 @@ describe TruncateData do .to have_received(:delete_all) .with("created_at < '#{1.month.ago.to_date}'") end + + it 'deletes log entries older than a month' do + TruncateData.new.call + + expect(Spree::LogEntry) + .to have_received(:delete_all) + .with("created_at < '#{1.month.ago.to_date}'") + end end end end From 4f015320a3facfaa12fffc34180483870bce7590 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Tue, 10 Mar 2020 13:15:54 +0100 Subject: [PATCH 11/18] Upper case DELETE statement This keeps it consistent with the rest of the log. --- lib/tasks/data/truncate_data.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tasks/data/truncate_data.rb b/lib/tasks/data/truncate_data.rb index fbf658973c..6e789128c8 100644 --- a/lib/tasks/data/truncate_data.rb +++ b/lib/tasks/data/truncate_data.rb @@ -53,7 +53,7 @@ class TruncateData end def sql_delete_from(sql) - ActiveRecord::Base.connection.execute("delete from #{sql}") + ActiveRecord::Base.connection.execute("DELETE FROM #{sql}") end def where_order_id_in_orders_to_delete From a3b8638faf6874df7a1319d76c099e0321edef6d Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Tue, 10 Mar 2020 13:40:39 +0100 Subject: [PATCH 12/18] Delete sessions older than two weeks This affects users that are actively purchasing, so 2 weeks data is more than enough. Others can afford to log in again. --- lib/tasks/data/truncate_data.rb | 8 +++++++- spec/lib/tasks/data/truncate_data_spec.rb | 25 +++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/lib/tasks/data/truncate_data.rb b/lib/tasks/data/truncate_data.rb index 6e789128c8..68b10c0a63 100644 --- a/lib/tasks/data/truncate_data.rb +++ b/lib/tasks/data/truncate_data.rb @@ -1,6 +1,12 @@ # frozen_string_literal: true class TruncateData + # This model lets us operate on the sessions DB table using ActiveRecord's + # methods within the scope of this service. This relies on the AR's + # convention where a Session model maps to a sessions table. + class Session < ActiveRecord::Base + end + def initialize(months_to_keep = nil) @date = (months_to_keep || 3).months.ago end @@ -36,7 +42,7 @@ class TruncateData Spree::ReturnAuthorization.delete_all Spree::StateChange.delete_all("created_at < '#{1.month.ago.to_date}'") Spree::LogEntry.delete_all("created_at < '#{1.month.ago.to_date}'") - sql_delete_from "sessions" + Session.delete_all("created_at < '#{2.weeks.ago.to_date}'") end def truncate_subscriptions diff --git a/spec/lib/tasks/data/truncate_data_spec.rb b/spec/lib/tasks/data/truncate_data_spec.rb index bc79463f48..d5972fa2ea 100644 --- a/spec/lib/tasks/data/truncate_data_spec.rb +++ b/spec/lib/tasks/data/truncate_data_spec.rb @@ -7,6 +7,7 @@ describe TruncateData do allow(Spree::ReturnAuthorization).to receive(:delete_all) allow(Spree::StateChange).to receive(:delete_all) allow(Spree::LogEntry).to receive(:delete_all) + allow(TruncateData::Session).to receive(:delete_all) end context 'when months_to_keep is not specified' do @@ -36,6 +37,14 @@ describe TruncateData do .to have_received(:delete_all) .with("created_at < '#{1.month.ago.to_date}'") end + + it 'deletes sessions older than two weeks' do + TruncateData.new.call + + expect(TruncateData::Session) + .to have_received(:delete_all) + .with("created_at < '#{2.weeks.ago.to_date}'") + end end context 'when months_to_keep is nil' do @@ -65,6 +74,14 @@ describe TruncateData do .to have_received(:delete_all) .with("created_at < '#{1.month.ago.to_date}'") end + + it 'deletes sessions older than two weeks' do + TruncateData.new.call + + expect(TruncateData::Session) + .to have_received(:delete_all) + .with("created_at < '#{2.weeks.ago.to_date}'") + end end context 'when months_to_keep is specified' do @@ -98,6 +115,14 @@ describe TruncateData do .to have_received(:delete_all) .with("created_at < '#{1.month.ago.to_date}'") end + + it 'deletes sessions older than two weeks' do + TruncateData.new.call + + expect(TruncateData::Session) + .to have_received(:delete_all) + .with("created_at < '#{2.weeks.ago.to_date}'") + end end end end From e2a3dd0c6faa4f81b426f928d9a920a8eca07289 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Tue, 10 Mar 2020 13:50:13 +0100 Subject: [PATCH 13/18] Delete only return auths. of the deleted orders They are associated to order and as such we can't remove them all blindly. --- lib/tasks/data/truncate_data.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tasks/data/truncate_data.rb b/lib/tasks/data/truncate_data.rb index 68b10c0a63..0c5e86c538 100644 --- a/lib/tasks/data/truncate_data.rb +++ b/lib/tasks/data/truncate_data.rb @@ -36,10 +36,10 @@ class TruncateData sql_delete_from "spree_line_items #{where_order_id_in_orders_to_delete}" sql_delete_from "spree_payments #{where_order_id_in_orders_to_delete}" sql_delete_from "spree_shipments #{where_order_id_in_orders_to_delete}" + sql_delete_from "spree_return_authorizations #{where_order_id_in_orders_to_delete}" end def remove_transient_data - Spree::ReturnAuthorization.delete_all Spree::StateChange.delete_all("created_at < '#{1.month.ago.to_date}'") Spree::LogEntry.delete_all("created_at < '#{1.month.ago.to_date}'") Session.delete_all("created_at < '#{2.weeks.ago.to_date}'") From 703706ee0b588d33d0cd3568f8ec97fb65b2e8d9 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Mon, 16 Mar 2020 18:37:55 +0100 Subject: [PATCH 14/18] Replace one-letter variable with full word --- lib/tasks/data/truncate_data.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tasks/data/truncate_data.rake b/lib/tasks/data/truncate_data.rake index ef01918205..dce1476e69 100644 --- a/lib/tasks/data/truncate_data.rake +++ b/lib/tasks/data/truncate_data.rake @@ -40,7 +40,7 @@ namespace :ofn do Are you sure you want to proceed? (y/N) MSG - exit unless HighLine.new.agree(message) { |q| q.default = "N" } + exit unless HighLine.new.agree(message) { |question| question.default = "N" } end end end From 6ceeda7d9e4e3c79586f3d511e2caeae26544e0c Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Mon, 16 Mar 2020 18:57:25 +0100 Subject: [PATCH 15/18] Instrument TruncateData logging start and end --- lib/tasks/data/truncate_data.rb | 26 +++++++++++++++-------- spec/lib/tasks/data/truncate_data_spec.rb | 1 + 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/lib/tasks/data/truncate_data.rb b/lib/tasks/data/truncate_data.rb index 0c5e86c538..2e9a703535 100644 --- a/lib/tasks/data/truncate_data.rb +++ b/lib/tasks/data/truncate_data.rb @@ -12,26 +12,34 @@ class TruncateData end def call - truncate_inventory - truncate_adjustments - truncate_order_associations - truncate_order_cycle_data + logging do + truncate_inventory + truncate_adjustments + truncate_order_associations + truncate_order_cycle_data - sql_delete_from "spree_orders #{where_oc_id_in_ocs_to_delete}" + sql_delete_from "spree_orders #{where_oc_id_in_ocs_to_delete}" - truncate_subscriptions + truncate_subscriptions - sql_delete_from "order_cycles #{where_ocs_to_delete}" + sql_delete_from "order_cycles #{where_ocs_to_delete}" - Spree::TokenizedPermission.where("created_at < '#{date}'").delete_all + Spree::TokenizedPermission.where("created_at < '#{date}'").delete_all - remove_transient_data + remove_transient_data + end end private attr_reader :date + def logging + Rails.logger.info("TruncateData started with truncation date #{date}") + yield + Rails.logger.info("TruncateData finished") + end + def truncate_order_associations sql_delete_from "spree_line_items #{where_order_id_in_orders_to_delete}" sql_delete_from "spree_payments #{where_order_id_in_orders_to_delete}" diff --git a/spec/lib/tasks/data/truncate_data_spec.rb b/spec/lib/tasks/data/truncate_data_spec.rb index d5972fa2ea..3362450c48 100644 --- a/spec/lib/tasks/data/truncate_data_spec.rb +++ b/spec/lib/tasks/data/truncate_data_spec.rb @@ -8,6 +8,7 @@ describe TruncateData do allow(Spree::StateChange).to receive(:delete_all) allow(Spree::LogEntry).to receive(:delete_all) allow(TruncateData::Session).to receive(:delete_all) + allow(Rails.logger).to receive(:info) end context 'when months_to_keep is not specified' do From 38ea95ea856dcadbb0eb728b95d6a46ce1d2b090 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Tue, 17 Mar 2020 19:25:11 +0100 Subject: [PATCH 16/18] Prevent nil input to turn into 0 --- lib/tasks/data/truncate_data.rake | 3 +-- lib/tasks/data/truncate_data.rb | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/tasks/data/truncate_data.rake b/lib/tasks/data/truncate_data.rake index dce1476e69..da4a63674a 100644 --- a/lib/tasks/data/truncate_data.rake +++ b/lib/tasks/data/truncate_data.rake @@ -28,8 +28,7 @@ namespace :ofn do task :truncate, [:months_to_keep] => :environment do |_task, args| warn_with_confirmation - months_to_keep = args.months_to_keep.to_i - TruncateData.new(months_to_keep).call + TruncateData.new(args.months_to_keep).call end def warn_with_confirmation diff --git a/lib/tasks/data/truncate_data.rb b/lib/tasks/data/truncate_data.rb index 2e9a703535..56645eea3e 100644 --- a/lib/tasks/data/truncate_data.rb +++ b/lib/tasks/data/truncate_data.rb @@ -8,7 +8,7 @@ class TruncateData end def initialize(months_to_keep = nil) - @date = (months_to_keep || 3).months.ago + @date = (months_to_keep || 3).to_i.months.ago end def call From e0228f66af27cb9e3601372684f599d5a9b0ac4c Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Tue, 17 Mar 2020 19:29:32 +0100 Subject: [PATCH 17/18] Default to archiving data older than 2 years. This is safer than the current 3 months. --- lib/tasks/data/truncate_data.rb | 2 +- spec/lib/tasks/data/truncate_data_spec.rb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/tasks/data/truncate_data.rb b/lib/tasks/data/truncate_data.rb index 56645eea3e..34300ca1dc 100644 --- a/lib/tasks/data/truncate_data.rb +++ b/lib/tasks/data/truncate_data.rb @@ -8,7 +8,7 @@ class TruncateData end def initialize(months_to_keep = nil) - @date = (months_to_keep || 3).to_i.months.ago + @date = (months_to_keep || 24).to_i.months.ago end def call diff --git a/spec/lib/tasks/data/truncate_data_spec.rb b/spec/lib/tasks/data/truncate_data_spec.rb index 3362450c48..850b9bb9a8 100644 --- a/spec/lib/tasks/data/truncate_data_spec.rb +++ b/spec/lib/tasks/data/truncate_data_spec.rb @@ -12,9 +12,9 @@ describe TruncateData do end context 'when months_to_keep is not specified' do - it 'truncates order cycles closed earlier than 3 months ago' do + it 'truncates order cycles closed earlier than 2 years ago' do order_cycle = create( - :order_cycle, orders_open_at: 4.months.ago, orders_close_at: 4.months.ago + 1.day + :order_cycle, orders_open_at: 25.months.ago, orders_close_at: 25.months.ago + 1.day ) create(:order, order_cycle: order_cycle) @@ -49,9 +49,9 @@ describe TruncateData do end context 'when months_to_keep is nil' do - it 'truncates order cycles closed earlier than 3 months ago' do + it 'truncates order cycles closed earlier than 2 years ago' do order_cycle = create( - :order_cycle, orders_open_at: 4.months.ago, orders_close_at: 4.months.ago + 1.day + :order_cycle, orders_open_at: 25.months.ago, orders_close_at: 25.months.ago + 1.day ) create(:order, order_cycle: order_cycle) From 647a7bdddf1e0f13f3460086688d538183aa461c Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Wed, 29 Apr 2020 17:00:53 +0200 Subject: [PATCH 18/18] Clarify we won't use truncate_date in prod yet We initially aimed at implementing data archiving in production reusing this rake task but priorities have changed. It'll be just a refactor for now. --- lib/tasks/data/truncate_data.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tasks/data/truncate_data.rake b/lib/tasks/data/truncate_data.rake index da4a63674a..65075a3ffb 100644 --- a/lib/tasks/data/truncate_data.rake +++ b/lib/tasks/data/truncate_data.rake @@ -34,7 +34,7 @@ namespace :ofn do def warn_with_confirmation message = <<-MSG.strip_heredoc \n - <% highlighted_message = "This will permanently change DB contents. Please, make a backup first." %> + <% highlighted_message = "This will permanently change DB contents. This is not meant to be run in production as it needs more thorough testing." %> <%= color(highlighted_message, :blink, :on_red) %> Are you sure you want to proceed? (y/N) MSG