Consider deleted products when creating permalinks

https://github.com/openfoodfoundation/openfoodnetwork/issues/3844

Spree's implementation and our implementation to create a unique
permalink failed to notice conflicts with soft-deleted products. This
patch looks at deleted products as well.
This commit is contained in:
Maikel Linke
2019-06-11 18:03:42 +10:00
parent 568e3003ba
commit a10bb5acbd
3 changed files with 44 additions and 1 deletions

View File

@@ -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'

View File

@@ -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

View File

@@ -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