diff --git a/app/models/spree/option_values_line_item.rb b/app/models/spree/option_values_line_item.rb new file mode 100644 index 0000000000..4f1ab992a8 --- /dev/null +++ b/app/models/spree/option_values_line_item.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +module Spree + class OptionValuesLineItem < ActiveRecord::Base + belongs_to :line_item, class_name: 'Spree::LineItem' + belongs_to :option_value, class_name: 'Spree::OptionValue' + end +end diff --git a/config/schedule.rb b/config/schedule.rb index 94fcacc54d..a72cec32ca 100644 --- a/config/schedule.rb +++ b/config/schedule.rb @@ -11,7 +11,7 @@ env "MAILTO", app_config["SCHEDULE_NOTIFICATIONS"] if app_config["SCHEDULE_NOTIF job_type :run_file, "cd :path; :environment_variable=:environment bundle exec script/rails runner :task :output" job_type :enqueue_job, "cd :path; :environment_variable=:environment bundle exec script/enqueue :task :priority :output" -every 1.month do +every 1.month, at: '4:30am' do rake 'ofn:data:remove_transient_data' end diff --git a/db/migrate/20210127174120_add_cascading_deletes.rb b/db/migrate/20210127174120_add_cascading_deletes.rb new file mode 100644 index 0000000000..3babe886b0 --- /dev/null +++ b/db/migrate/20210127174120_add_cascading_deletes.rb @@ -0,0 +1,20 @@ +class AddCascadingDeletes < ActiveRecord::Migration + def change + # Updates foreign key definitions between orders, shipments, and inventory_units + # to allow for cascading deletes at database level. If an order is intentionally + # deleted *without callbacks*, it's shipments and inventory units will be removed + # cleanly without throwing foreign key errors. + + remove_foreign_key :spree_shipments, name: "spree_shipments_order_id_fk" + add_foreign_key :spree_shipments, :spree_orders, column: 'order_id', + name: 'spree_shipments_order_id_fk', on_delete: :cascade + + remove_foreign_key :spree_inventory_units, name: 'spree_inventory_units_shipment_id_fk' + add_foreign_key :spree_inventory_units, :spree_shipments, column: 'shipment_id', + name: 'spree_inventory_units_shipment_id_fk', on_delete: :cascade + + remove_foreign_key :spree_inventory_units, name: 'spree_inventory_units_order_id_fk' + add_foreign_key :spree_inventory_units, :spree_orders, column: 'order_id', + name: 'spree_inventory_units_order_id_fk', on_delete: :cascade + end +end diff --git a/db/schema.rb b/db/schema.rb index 7e26c00970..f3c76493eb 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20210125123000) do +ActiveRecord::Schema.define(version: 20210127174120) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1252,9 +1252,9 @@ ActiveRecord::Schema.define(version: 20210125123000) do add_foreign_key "proxy_orders", "subscriptions", name: "proxy_orders_subscription_id_fk" add_foreign_key "spree_addresses", "spree_countries", column: "country_id", name: "spree_addresses_country_id_fk" add_foreign_key "spree_addresses", "spree_states", column: "state_id", name: "spree_addresses_state_id_fk" - add_foreign_key "spree_inventory_units", "spree_orders", column: "order_id", name: "spree_inventory_units_order_id_fk" + add_foreign_key "spree_inventory_units", "spree_orders", column: "order_id", on_delete: :cascade add_foreign_key "spree_inventory_units", "spree_return_authorizations", column: "return_authorization_id", name: "spree_inventory_units_return_authorization_id_fk" - add_foreign_key "spree_inventory_units", "spree_shipments", column: "shipment_id", name: "spree_inventory_units_shipment_id_fk" + add_foreign_key "spree_inventory_units", "spree_shipments", column: "shipment_id", on_delete: :cascade add_foreign_key "spree_inventory_units", "spree_variants", column: "variant_id", name: "spree_inventory_units_variant_id_fk" add_foreign_key "spree_line_items", "spree_orders", column: "order_id", name: "spree_line_items_order_id_fk" add_foreign_key "spree_line_items", "spree_variants", column: "variant_id", name: "spree_line_items_variant_id_fk" @@ -1290,7 +1290,7 @@ ActiveRecord::Schema.define(version: 20210125123000) do add_foreign_key "spree_roles_users", "spree_roles", column: "role_id", name: "spree_roles_users_role_id_fk" add_foreign_key "spree_roles_users", "spree_users", column: "user_id", name: "spree_roles_users_user_id_fk" add_foreign_key "spree_shipments", "spree_addresses", column: "address_id", name: "spree_shipments_address_id_fk" - add_foreign_key "spree_shipments", "spree_orders", column: "order_id", name: "spree_shipments_order_id_fk" + add_foreign_key "spree_shipments", "spree_orders", column: "order_id", on_delete: :cascade add_foreign_key "spree_state_changes", "spree_users", column: "user_id", name: "spree_state_changes_user_id_fk" add_foreign_key "spree_states", "spree_countries", column: "country_id", name: "spree_states_country_id_fk" add_foreign_key "spree_tax_rates", "spree_tax_categories", column: "tax_category_id", name: "spree_tax_rates_tax_category_id_fk" diff --git a/lib/tasks/data/remove_transient_data.rb b/lib/tasks/data/remove_transient_data.rb index 96eaef027b..a0b8128f04 100644 --- a/lib/tasks/data/remove_transient_data.rb +++ b/lib/tasks/data/remove_transient_data.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true class RemoveTransientData - RETENTION_PERIOD = 6.months.ago.to_date + MEDIUM_RETENTION = 6.months.ago.to_date + SHORT_RETENTION = 3.months.ago.to_date # 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 @@ -12,8 +13,34 @@ class RemoveTransientData def call Rails.logger.info("#{self.class.name}: processing") - Spree::StateChange.where("created_at < ?", RETENTION_PERIOD).delete_all - Spree::LogEntry.where("created_at < ?", RETENTION_PERIOD).delete_all - Session.where("updated_at < ?", RETENTION_PERIOD).delete_all + Spree::StateChange.where("created_at < ?", MEDIUM_RETENTION).delete_all + Spree::LogEntry.where("created_at < ?", MEDIUM_RETENTION).delete_all + Session.where("updated_at < ?", SHORT_RETENTION).delete_all + + clear_old_cart_data! + end + + private + + def clear_old_cart_data! + old_carts = Spree::Order. + where("spree_orders.state = 'cart' AND spree_orders.updated_at < ?", SHORT_RETENTION). + merge(orders_without_payments) + + old_cart_line_items = Spree::LineItem.where(order_id: old_carts) + old_line_item_options = Spree::OptionValuesLineItem.where(line_item_id: old_cart_line_items) + old_cart_adjustments = Spree::Adjustment.where(order_id: old_carts) + + old_cart_adjustments.delete_all + old_line_item_options.delete_all + old_cart_line_items.delete_all + old_carts.delete_all + end + + def orders_without_payments + # Carts with failed payments are ignored, as they contain potentially useful data + Spree::Order. + joins("LEFT OUTER JOIN spree_payments ON spree_orders.id = spree_payments.order_id"). + where("spree_payments.id IS NULL") end end diff --git a/spec/lib/tasks/data/remove_transient_data_spec.rb b/spec/lib/tasks/data/remove_transient_data_spec.rb index 9e61b3f65b..fddf33aef7 100644 --- a/spec/lib/tasks/data/remove_transient_data_spec.rb +++ b/spec/lib/tasks/data/remove_transient_data_spec.rb @@ -5,7 +5,8 @@ require 'tasks/data/remove_transient_data' describe RemoveTransientData do describe '#call' do - let(:retention_period) { RemoveTransientData::RETENTION_PERIOD } + let(:medium_retention) { RemoveTransientData::MEDIUM_RETENTION } + let(:short_retention) { RemoveTransientData::SHORT_RETENTION } before do allow(Spree::StateChange).to receive(:delete_all) @@ -15,25 +16,57 @@ describe RemoveTransientData do end it 'deletes state changes older than rentention_period' do - Spree::StateChange.create(created_at: retention_period - 1.day) + Spree::StateChange.create(created_at: medium_retention - 1.day) RemoveTransientData.new.call expect(Spree::StateChange.all).to be_empty end it 'deletes log entries older than retention_period' do - Spree::LogEntry.create(created_at: retention_period - 1.day) + Spree::LogEntry.create(created_at: medium_retention - 1.day) expect { RemoveTransientData.new.call } .to change(Spree::LogEntry, :count).by(-1) end it 'deletes sessions older than retention_period' do - RemoveTransientData::Session.create(session_id: 1, updated_at: retention_period - 1.day) + RemoveTransientData::Session.create(session_id: 1, updated_at: short_retention - 1.day) RemoveTransientData.new.call expect(RemoveTransientData::Session.all).to be_empty end + + describe "deleting old carts" do + let(:product) { create(:product) } + let(:variant) { product.variants.first } + + let!(:cart) { create(:order, state: 'cart') } + let!(:line_item) { create(:line_item, order: cart, variant: variant) } + let!(:adjustment) { create(:adjustment, order: cart) } + + let!(:old_cart) { create(:order, state: 'cart', updated_at: short_retention - 1.day) } + let!(:old_line_item) { create(:line_item, order: old_cart, variant: variant) } + let!(:old_adjustment) { create(:adjustment, order: old_cart) } + + it 'deletes cart orders and related objects older than retention_period' do + RemoveTransientData.new.call + + expect{ cart.reload }.to_not raise_error + expect{ line_item.reload }.to_not raise_error + expect{ adjustment.reload }.to_not raise_error + + expect{ old_cart.reload }.to raise_error ActiveRecord::RecordNotFound + expect{ old_line_item.reload }.to raise_error ActiveRecord::RecordNotFound + expect{ old_adjustment.reload }.to raise_error ActiveRecord::RecordNotFound + end + + it "removes any defunct line item option value records" do + line_item.delete + + expect{ RemoveTransientData.new.call }. + to change{ Spree::OptionValuesLineItem.count }.by(-1) + end + end end end