From 5fe5f65291a2a30673278e972e6fd02109dc5ec6 Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Wed, 24 Jan 2024 11:53:09 +0100 Subject: [PATCH 1/8] Add Promotion#discarded_at Let's use the `discard` gem for soft-deleting promotions. --- .../migrate/20240124104855_add_deleted_at_to_promotions.rb | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 friendly_promotions/db/migrate/20240124104855_add_deleted_at_to_promotions.rb diff --git a/friendly_promotions/db/migrate/20240124104855_add_deleted_at_to_promotions.rb b/friendly_promotions/db/migrate/20240124104855_add_deleted_at_to_promotions.rb new file mode 100644 index 00000000..92cc06da --- /dev/null +++ b/friendly_promotions/db/migrate/20240124104855_add_deleted_at_to_promotions.rb @@ -0,0 +1,6 @@ +class AddDeletedAtToPromotions < ActiveRecord::Migration[7.0] + def change + add_column :friendly_promotions, :deleted_at, :datetime + add_index :friendly_promotions, :deleted_at + end +end From 83752cb6c533a8f28afcc9b65a027726d4386eba Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Wed, 24 Jan 2024 11:56:56 +0100 Subject: [PATCH 2/8] Add Spree::SoftDeletable to promotions --- .../solidus_friendly_promotions/promotion.rb | 2 ++ .../load_promotions_spec.rb | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/friendly_promotions/app/models/solidus_friendly_promotions/promotion.rb b/friendly_promotions/app/models/solidus_friendly_promotions/promotion.rb index 2583f17a..1854766b 100644 --- a/friendly_promotions/app/models/solidus_friendly_promotions/promotion.rb +++ b/friendly_promotions/app/models/solidus_friendly_promotions/promotion.rb @@ -2,6 +2,8 @@ module SolidusFriendlyPromotions class Promotion < Spree::Base + include Spree::SoftDeletable + belongs_to :category, class_name: "SolidusFriendlyPromotions::PromotionCategory", foreign_key: :promotion_category_id, optional: true belongs_to :original_promotion, class_name: "Spree::Promotion", optional: true diff --git a/friendly_promotions/spec/models/solidus_friendly_promotions/friendly_promotion_adjuster/load_promotions_spec.rb b/friendly_promotions/spec/models/solidus_friendly_promotions/friendly_promotion_adjuster/load_promotions_spec.rb index be873cc2..f8135e66 100644 --- a/friendly_promotions/spec/models/solidus_friendly_promotions/friendly_promotion_adjuster/load_promotions_spec.rb +++ b/friendly_promotions/spec/models/solidus_friendly_promotions/friendly_promotion_adjuster/load_promotions_spec.rb @@ -43,6 +43,25 @@ expect(subject).to eq([active_promotion]) end end + + context "discarded promotions" do + let!(:discarded_promotion) { create(:friendly_promotion, :with_adjustable_action, deleted_at: 1.hour.ago, apply_automatically: true) } + + it "does not check discarded promotions" do + expect(subject).not_to include(discarded_promotion) + end + + context "a discarded promo is connected to the order" do + before do + order.friendly_promotions << discarded_promotion + end + + it "does not check connected discarded promotions" do + expect(subject).not_to include(discarded_promotion) + expect(subject).to eq([active_promotion]) + end + end + end end context "promotions in the past" do From e2ea0d856828ff7efb41c10c79efd206f0ab3e18 Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Wed, 24 Jan 2024 13:53:42 +0100 Subject: [PATCH 3/8] Disable the new admin temporarily There are still a few bugs with the new admin, so let's disable that for the time being. --- friendly_promotions/bin/sandbox | 1 + 1 file changed, 1 insertion(+) diff --git a/friendly_promotions/bin/sandbox b/friendly_promotions/bin/sandbox index 9266d651..9d520ee4 100755 --- a/friendly_promotions/bin/sandbox +++ b/friendly_promotions/bin/sandbox @@ -65,6 +65,7 @@ unbundled bundle exec rake db:drop db:create unbundled bin/rails generate solidus:install \ --auto-accept \ + --admin_preview=false \ $@ unbundled bundle exec rails generate solidus:auth:install --auto-run-migrations From 44b3dca9deadf87e3399d7b19bb9d1ccbd16c93a Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Wed, 24 Jan 2024 16:56:59 +0100 Subject: [PATCH 4/8] UI work for soft-deleted promotions We don't want to allow people to edit or re-create deleted promotions, so we just indicate they're there for now. Let's see if we find a need to show them in some kind of uneditable mode. --- .../app/models/solidus_friendly_promotions/promotion.rb | 2 +- .../admin/promotions/_table.html.erb | 6 +++--- .../admin/promotions/_table_filter.html.erb | 9 +++++++++ 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/friendly_promotions/app/models/solidus_friendly_promotions/promotion.rb b/friendly_promotions/app/models/solidus_friendly_promotions/promotion.rb index 1854766b..28d92455 100644 --- a/friendly_promotions/app/models/solidus_friendly_promotions/promotion.rb +++ b/friendly_promotions/app/models/solidus_friendly_promotions/promotion.rb @@ -51,7 +51,7 @@ def self.ordered_lanes self.allowed_ransackable_associations = ["codes"] self.allowed_ransackable_attributes = %w[name customer_label path promotion_category_id lane updated_at] - self.allowed_ransackable_scopes = %i[active] + self.allowed_ransackable_scopes = %i[active with_discarded] # All orders that have been discounted using this promotion def discounted_orders diff --git a/friendly_promotions/app/views/solidus_friendly_promotions/admin/promotions/_table.html.erb b/friendly_promotions/app/views/solidus_friendly_promotions/admin/promotions/_table.html.erb index 36eb4812..1606df7b 100644 --- a/friendly_promotions/app/views/solidus_friendly_promotions/admin/promotions/_table.html.erb +++ b/friendly_promotions/app/views/solidus_friendly_promotions/admin/promotions/_table.html.erb @@ -14,7 +14,7 @@ <% promotions.each do |promotion| %> - + <%= promotion.name %> <%= (promotion.codes.size == 1) ? promotion.codes.pluck(:value).first : t('solidus_friendly_promotions.number_of_codes', count: promotion.codes.size) %> @@ -43,10 +43,10 @@ <%= l(promotion.updated_at, format: :short) %> - <% if can?(:edit, promotion) %> + <% if can?(:edit, promotion) && !promotion.discarded? %> <%= link_to_edit promotion, no_text: true %> <% end %> - <% if can?(:destroy, promotion) %> + <% if can?(:destroy, promotion) && !promotion.discarded? %> <%= link_to_delete promotion, no_text: true %> <% end %> diff --git a/friendly_promotions/app/views/solidus_friendly_promotions/admin/promotions/_table_filter.html.erb b/friendly_promotions/app/views/solidus_friendly_promotions/admin/promotions/_table_filter.html.erb index 44e592d4..df04c672 100644 --- a/friendly_promotions/app/views/solidus_friendly_promotions/admin/promotions/_table_filter.html.erb +++ b/friendly_promotions/app/views/solidus_friendly_promotions/admin/promotions/_table_filter.html.erb @@ -43,6 +43,15 @@ +
+
+ +
+
+
<%= label_tag :q_promotion_category_id_eq, SolidusFriendlyPromotions::PromotionCategory.model_name.human %>
From b1f80a6cd9e7a7c4662c9f4e241e23578cb1509c Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Thu, 25 Jan 2024 10:56:36 +0100 Subject: [PATCH 5/8] Disallow destroying promotion actions for completed orders When a promotion has been applied to an order, we need to make sure no adjustments remain that have that promotion's actions as their source. For incomplete orders, we can simply delete the adjustments, but for complete orders, we must stop the promotion action from being deleted. --- .../promotion_action.rb | 11 +++++ friendly_promotions/config/locales/en.yml | 4 ++ .../promotion_action_spec.rb | 40 +++++++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/friendly_promotions/app/models/solidus_friendly_promotions/promotion_action.rb b/friendly_promotions/app/models/solidus_friendly_promotions/promotion_action.rb index bfd10391..4183cb29 100644 --- a/friendly_promotions/app/models/solidus_friendly_promotions/promotion_action.rb +++ b/friendly_promotions/app/models/solidus_friendly_promotions/promotion_action.rb @@ -12,6 +12,8 @@ class PromotionAction < Spree::Base include Spree::SoftDeletable include Spree::CalculatedAdjustments include Spree::AdjustmentSource + before_destroy :remove_adjustments_from_incomplete_orders + before_destroy :raise_for_adjustments_for_completed_orders belongs_to :promotion, inverse_of: :actions belongs_to :original_promotion_action, class_name: "Spree::PromotionAction", optional: true @@ -69,5 +71,14 @@ def relevant_rules def available_calculators SolidusFriendlyPromotions.config.promotion_calculators[self.class] || (raise NotImplementedError) end + + private + + def raise_for_adjustments_for_completed_orders + if adjustments.joins(:order).merge(Spree::Order.complete).any? + errors.add(:base, :cannot_destroy_if_order_completed) + throw(:abort) + end + end end end diff --git a/friendly_promotions/config/locales/en.yml b/friendly_promotions/config/locales/en.yml index 8c85ab8c..8aa5639b 100644 --- a/friendly_promotions/config/locales/en.yml +++ b/friendly_promotions/config/locales/en.yml @@ -314,6 +314,10 @@ en: errors: models: + solidus_friendly_promotions/promotion_action: + attributes: + base: + cannot_destroy_if_order_completed: Action has been applied to complete orders. It cannot be destroyed. solidus_friendly_promotions/promotion_code: attributes: base: diff --git a/friendly_promotions/spec/models/solidus_friendly_promotions/promotion_action_spec.rb b/friendly_promotions/spec/models/solidus_friendly_promotions/promotion_action_spec.rb index 4f962c3b..8743d3db 100644 --- a/friendly_promotions/spec/models/solidus_friendly_promotions/promotion_action_spec.rb +++ b/friendly_promotions/spec/models/solidus_friendly_promotions/promotion_action_spec.rb @@ -17,6 +17,46 @@ end end + describe "#destroy" do + subject { action.destroy } + let(:action) { promotion.actions.first } + let!(:promotion) { create(:friendly_promotion, :with_adjustable_action, apply_automatically: true) } + + it "destroys the action" do + expect { subject }.to change { SolidusFriendlyPromotions::PromotionAction.count }.by(-1) + end + + context "when the action has adjustments on an incomplete order" do + let(:order) { create(:order_with_line_items) } + + before do + order.recalculate + end + + it "destroys the action" do + expect { subject }.to change { SolidusFriendlyPromotions::PromotionAction.count }.by(-1) + end + + it "destroys the adjustments" do + expect { subject }.to change { Spree::Adjustment.count }.by(-1) + end + + context "when the action has adjustments on a complete order" do + let(:order) { create(:order_ready_to_complete) } + + before do + order.recalculate + order.complete! + end + + it "raises an error" do + expect { subject }.not_to change { SolidusFriendlyPromotions::PromotionAction.count } + expect(action.errors.full_messages).to include("Action has been applied to complete orders. It cannot be destroyed.") + end + end + end + end + describe "#discount" do subject { action.discount(discountable) } From 45708ae5a9bfce38bb1da0adc54c596b50bf85b0 Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Thu, 25 Jan 2024 11:07:55 +0100 Subject: [PATCH 6/8] Actually destroy promotion actions We now protect against destroying promotion actions that have been applied to a complete order, so we can stop the dependent: :nullify thing. --- .../solidus_friendly_promotions/promotion.rb | 2 +- .../promotion_spec.rb | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/friendly_promotions/app/models/solidus_friendly_promotions/promotion.rb b/friendly_promotions/app/models/solidus_friendly_promotions/promotion.rb index 28d92455..5b653f39 100644 --- a/friendly_promotions/app/models/solidus_friendly_promotions/promotion.rb +++ b/friendly_promotions/app/models/solidus_friendly_promotions/promotion.rb @@ -8,7 +8,7 @@ class Promotion < Spree::Base foreign_key: :promotion_category_id, optional: true belongs_to :original_promotion, class_name: "Spree::Promotion", optional: true has_many :rules, class_name: "SolidusFriendlyPromotions::PromotionRule", dependent: :destroy - has_many :actions, class_name: "SolidusFriendlyPromotions::PromotionAction", dependent: :nullify + has_many :actions, class_name: "SolidusFriendlyPromotions::PromotionAction", dependent: :destroy has_many :codes, class_name: "SolidusFriendlyPromotions::PromotionCode", dependent: :destroy has_many :code_batches, class_name: "SolidusFriendlyPromotions::PromotionCodeBatch", dependent: :destroy has_many :order_promotions, class_name: "SolidusFriendlyPromotions::OrderPromotion", dependent: :destroy diff --git a/friendly_promotions/spec/models/solidus_friendly_promotions/promotion_spec.rb b/friendly_promotions/spec/models/solidus_friendly_promotions/promotion_spec.rb index 7aa8fd47..3c87533b 100644 --- a/friendly_promotions/spec/models/solidus_friendly_promotions/promotion_spec.rb +++ b/friendly_promotions/spec/models/solidus_friendly_promotions/promotion_spec.rb @@ -20,14 +20,26 @@ end describe "#destroy" do - let!(:promotion) { create(:friendly_promotion, :with_adjustable_action) } + let!(:promotion) { create(:friendly_promotion, :with_adjustable_action, apply_automatically: true) } subject { promotion.destroy! } it "destroys the promotion and nullifies the action" do expect { subject }.to change { SolidusFriendlyPromotions::Promotion.count }.by(-1) - expect(SolidusFriendlyPromotions::PromotionAction.count).to eq(1) - expect(SolidusFriendlyPromotions::PromotionAction.first.promotion_id).to be nil + expect(SolidusFriendlyPromotions::PromotionAction.count).to be_zero + end + + context "when the promotion has been applied to a complete order" do + let(:order) { create(:order_ready_to_complete) } + + before do + order.recalculate + order.complete! + end + + it "raises an error" do + expect { subject }.to raise_exception(ActiveRecord::RecordNotDestroyed) + end end end From 88a750ae817574e269dc0d7d4c9ad15890ec9fa5 Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Thu, 25 Jan 2024 11:19:47 +0100 Subject: [PATCH 7/8] Actually destroy promotion actions There's no need for soft-delete on promotion actions, now that we have soft-delete on promotions. --- .../admin/promotion_actions_controller.rb | 2 +- .../app/models/solidus_friendly_promotions/promotion_action.rb | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/friendly_promotions/app/controllers/solidus_friendly_promotions/admin/promotion_actions_controller.rb b/friendly_promotions/app/controllers/solidus_friendly_promotions/admin/promotion_actions_controller.rb index 6d379ee3..394f33c5 100644 --- a/friendly_promotions/app/controllers/solidus_friendly_promotions/admin/promotion_actions_controller.rb +++ b/friendly_promotions/app/controllers/solidus_friendly_promotions/admin/promotion_actions_controller.rb @@ -53,7 +53,7 @@ def update def destroy @promotion_action = @promotion.actions.find(params[:id]) - if @promotion_action.discard + if @promotion_action.destroy flash[:success] = t("spree.successfully_removed", resource: SolidusFriendlyPromotions::PromotionAction.model_name.human) end diff --git a/friendly_promotions/app/models/solidus_friendly_promotions/promotion_action.rb b/friendly_promotions/app/models/solidus_friendly_promotions/promotion_action.rb index 4183cb29..642379fe 100644 --- a/friendly_promotions/app/models/solidus_friendly_promotions/promotion_action.rb +++ b/friendly_promotions/app/models/solidus_friendly_promotions/promotion_action.rb @@ -9,7 +9,6 @@ module SolidusFriendlyPromotions # by an event and determined to be eligible. class PromotionAction < Spree::Base include Spree::Preferences::Persistable - include Spree::SoftDeletable include Spree::CalculatedAdjustments include Spree::AdjustmentSource before_destroy :remove_adjustments_from_incomplete_orders From a547c4d10ddacdbc40ccdf0a7cfef6aa209872e0 Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Thu, 25 Jan 2024 11:22:59 +0100 Subject: [PATCH 8/8] Drop deleted_at from promotion actions We don't need soft-delete on promotion actions when we have soft-delete on promotions. --- .../20240125102050_drop_deleted_at_from_promotion_actions.rb | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 friendly_promotions/db/migrate/20240125102050_drop_deleted_at_from_promotion_actions.rb diff --git a/friendly_promotions/db/migrate/20240125102050_drop_deleted_at_from_promotion_actions.rb b/friendly_promotions/db/migrate/20240125102050_drop_deleted_at_from_promotion_actions.rb new file mode 100644 index 00000000..8b068249 --- /dev/null +++ b/friendly_promotions/db/migrate/20240125102050_drop_deleted_at_from_promotion_actions.rb @@ -0,0 +1,5 @@ +class DropDeletedAtFromPromotionActions < ActiveRecord::Migration[7.0] + def change + remove_column :friendly_promotion_actions, :deleted_at, :datetime, null: true + end +end