diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index c7a41e5698..0cf575d826 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -252,6 +252,7 @@ Spree::Product.class_eval do raise end + # Spree creates a permalink already but our implementation fixes an edge case. def sanitize_permalink if permalink.blank? || permalink_changed? requested = permalink.presence || permalink_was.presence || name.presence || 'product' diff --git a/lib/open_food_network/permalink_generator.rb b/lib/open_food_network/permalink_generator.rb index 99d923fd1e..efcd496234 100644 --- a/lib/open_food_network/permalink_generator.rb +++ b/lib/open_food_network/permalink_generator.rb @@ -1,3 +1,10 @@ +# Finds a unique permalink for a new or updated record. +# It considers soft-deleted records which are ignored by Spree. +# Spree's work: +# https://github.com/spree/spree/blob/09b55f7/core/lib/spree/core/permalinks.rb +# +# This may become obsolete with Spree 2.3. +# https://github.com/spree/spree/commits/master/core/lib/spree/core/permalinks.rb module PermalinkGenerator def self.included(base) base.extend(ClassMethods) @@ -16,7 +23,23 @@ module PermalinkGenerator end def create_unique_permalink(requested) - existing = self.class.where('id != ?', id).where("permalink LIKE ?", "#{requested}%").pluck(:permalink) + existing = others.where("permalink LIKE ?", "#{requested}%").pluck(:permalink) self.class.find_available_value(existing, requested) end + + def others + if id.nil? + scope_with_deleted + else + scope_with_deleted.where('id != ?', id) + end + end + + def scope_with_deleted + if self.class.respond_to?(:with_deleted) + self.class.with_deleted + else + self.class.scoped + end + end end diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index b178056706..b84da43c5b 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -35,6 +35,25 @@ module Spree end end + describe "permalink" do + let(:name) { "Banana Permanenta" } + + it "generates a unique permalink" do + product1 = create(:product, name: "Banana Permanenta", permalink: nil) + product2 = create(:product, name: "Banana Permanenta", permalink: nil) + expect(product2.permalink).to_not eq product1.permalink + # "banana-permanenta" != "banana-permanenta-1" # generated by Spree + end + + it "generates a unique permalink considering deleted products" do + product1 = create(:product, name: "Banana Permanenta", permalink: nil) + product1.destroy + product2 = create(:product, name: "Banana Permanenta", permalink: nil) + expect(product2.permalink).to_not eq product1.permalink + # "banana-permanenta" != "banana-permanenta1" # generated by OFN + end + end + describe "tax category" do context "when a tax category is required" do it "is invalid when a tax category is not provided" do