raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-02-17 00:07:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

Remove :permalink attribute from Product

Browse Source
This commit is contained in:
Matt-Yorkley
2023-06-28 14:30:48 +01:00
parent b175793b91
commit fefa9288a4
24 changed files with 50 additions and 204 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
app/assets/javascripts/admin/bulk_product_update.js.coffee
View File

@@ -113,7 +113,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout
(DirtyProducts.count() > 0 and confirm(t("unsaved_changes_confirmation"))) or (DirtyProducts.count() == 0)
editProductUrl = (product, variant) ->
"/admin/products/" + product.permalink_live + ((if variant then "/variants/" + variant.id else "")) + "/edit"
"/admin/products/" + product.id + ((if variant then "/variants/" + variant.id else "")) + "/edit"
$scope.editWarn = (product, variant) ->
if confirm_unsaved_changes()
@@ -162,7 +162,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout
if confirm(t("are_you_sure"))
$http(
method: "DELETE"
url: "/api/v0/products/" + product.permalink_live + "/variants/" + variant.id
url: "/api/v0/products/" + product.id + "/variants/" + variant.id
).then (response) ->
$scope.removeVariant(product, variant)
else

15
app/controllers/api/v0/products_controller.rb
View File

@@ -23,15 +23,10 @@ module Api
authorize! :create, Spree::Product
@product = Spree::Product.new(product_params)
begin
if @product.save
render json: @product, serializer: Api::Admin::ProductSerializer, status: :created
else
invalid_resource!(@product)
end
rescue ActiveRecord::RecordNotUnique
@product.permalink = nil
retry
if @product.save
render json: @product, serializer: Api::Admin::ProductSerializer, status: :created
else
invalid_resource!(@product)
end
end
@@ -96,8 +91,6 @@ module Api
private
def find_product(id)
product_scope.find_by!(permalink: id.to_s)
rescue ActiveRecord::RecordNotFound
product_scope.find(id)
end

2
app/controllers/api/v0/variants_controller.rb
View File

@@ -50,7 +50,7 @@ module Api
private
def product
@product ||= Spree::Product.find_by(permalink: params[:product_id]) if params[:product_id]
@product ||= Spree::Product.find(params[:product_id]) if params[:product_id]
end
def scope

4
app/controllers/spree/admin/images_controller.rb
View File

@@ -6,7 +6,7 @@ module Spree
# This will make resource controller redirect correctly after deleting product images.
# This can be removed after upgrading to Spree 2.1.
# See here https://github.com/spree/spree/commit/334a011d2b8e16355e4ae77ae07cd93f7cbc8fd1
belongs_to 'spree/product', find_by: :permalink
belongs_to 'spree/product'
before_action :load_data
@@ -80,7 +80,7 @@ module Spree
end
def load_data
@product = Product.find_by(permalink: params[:product_id])
@product = Product.find(params[:product_id])
end
def set_viewable

2
app/controllers/spree/admin/product_properties_controller.rb
View File

@@ -3,7 +3,7 @@
module Spree
module Admin
class ProductPropertiesController < ::Admin::ResourceController
belongs_to 'spree/product', find_by: :permalink
belongs_to 'spree/product'
before_action :find_properties
before_action :setup_property, only: [:index]

2
app/controllers/spree/admin/products_controller.rb
View File

@@ -103,7 +103,7 @@ module Spree
protected
def find_resource
Product.find_by!(permalink: params[:id])
Product.find(params[:id])
end
def location_after_save

2
app/controllers/spree/admin/variants_controller.rb
View File

@@ -5,7 +5,7 @@ require 'open_food_network/scope_variants_for_search'
module Spree
module Admin
class VariantsController < ::Admin::ResourceController
belongs_to 'spree/product', find_by: :permalink
belongs_to 'spree/product'
def index
@url_filters = ::ProductFilters.new.extract(request.query_parameters)

24
app/models/spree/product.rb
View File

@@ -23,7 +23,6 @@ require 'concerns/product_stock'
#
module Spree
class Product < ApplicationRecord
include PermalinkGenerator
include ProductStock
acts_as_paranoid
@@ -56,7 +55,6 @@ module Spree
through: :variants
validates :name, presence: true
validates :permalink, presence: true
validates :shipping_category, presence: true
validates :supplier, presence: true
@@ -83,14 +81,9 @@ module Spree
# these values are persisted on the product's variant
attr_accessor :price, :display_as, :unit_value, :unit_description
make_permalink order: :name
after_initialize :set_available_on_to_now, if: :new_record?
before_validation :sanitize_permalink
before_save :add_primary_taxon_to_taxons
before_destroy :punch_permalink
after_save :remove_previous_primary_taxon_from_taxons
after_create :ensure_standard_variant
@@ -216,10 +209,6 @@ module Spree
group(column_names.map { |col_name| "#{table_name}.#{col_name}" })
end
def to_param
permalink.present? ? permalink : (permalink_was || UrlGenerator.to_url(name))
end
def tax_category
if self[:tax_category_id].nil?
TaxCategory.find_by(is_default: true)
@@ -306,11 +295,6 @@ module Spree
private
def punch_permalink
# Punch permalink with date prefix
update_attribute :permalink, "#{Time.now.to_i}_#{permalink}"
end
def set_available_on_to_now
self.available_on ||= Time.zone.now
end
@@ -347,14 +331,6 @@ module Spree
variants << variant
end
# Spree creates a permalink already but our implementation fixes an edge case.
def sanitize_permalink
return unless permalink.blank? || saved_change_to_permalink? || permalink_changed?
requested = permalink.presence || permalink_was.presence || name.presence || 'product'
self.permalink = create_unique_permalink(requested.parameterize)
end
def validate_image
return if image.blank? || image.valid?

2
app/models/spree/variant.rb
View File

@@ -23,7 +23,7 @@ module Spree
belongs_to :product, -> { with_deleted }, touch: true, class_name: 'Spree::Product'
delegate_belongs_to :product, :name, :description, :permalink, :available_on,
delegate_belongs_to :product, :name, :description, :available_on,
:tax_category_id, :shipping_category_id,
:meta_keywords, :tax_category, :shipping_category

6
app/serializers/api/admin/product_serializer.rb
View File

@@ -4,7 +4,7 @@ module Api
module Admin
class ProductSerializer < ActiveModel::Serializer
attributes :id, :name, :sku, :variant_unit, :variant_unit_scale, :variant_unit_name,
:inherits_properties, :on_hand, :price, :available_on, :permalink_live,
:inherits_properties, :on_hand, :price, :available_on,
:tax_category_id, :import_date, :image_url, :thumb_url, :variants
has_one :supplier, key: :producer_id, embed: :id
@@ -40,10 +40,6 @@ module Api
def available_on
object.available_on.blank? ? "" : object.available_on.strftime("%F %T")
end
def permalink_live
object.permalink
end
end
end
end

2
app/serializers/api/product_serializer.rb
View File

@@ -3,7 +3,7 @@
require "open_food_network/scope_variant_to_hub"
class Api::ProductSerializer < ActiveModel::Serializer
attributes :id, :name, :permalink, :meta_keywords
attributes :id, :name, :meta_keywords
attributes :group_buy, :notes, :description, :description_html
attributes :properties_with_values

2
app/services/permitted_attributes/product.rb
View File

@@ -4,7 +4,7 @@ module PermittedAttributes
class Product
def self.attributes
[
:id, :name, :description, :supplier_id, :price, :permalink,
:id, :name, :description, :supplier_id, :price,
:variant_unit, :variant_unit_scale, :unit_value, :unit_description, :variant_unit_name,
:display_as, :sku, :available_on, :group_buy, :group_buy_unit_size,
:taxon_ids, :primary_taxon_id, :tax_category_id, :shipping_category_id,

5
app/views/spree/admin/products/_form.html.haml
View File

@@ -5,11 +5,6 @@
= f.text_field :name, :class => 'fullwidth title'
= f.error_message_on :name
= f.field_container :permalink do
= f.label :permalink, raw(t(:permalink) + content_tag(:span, ' *', :class => "required"))
= f.text_field :permalink, :class => 'fullwidth title'
= f.error_message_on :permalink
= f.field_container :description do
= f.label :description, t(:description)
%text-angular{'id' => 'product_description', 'name' => 'product[description]', 'class' => 'text-angular', 'textangular-unsafe-sanitizer' => true, "textangular-links-target-blank" => true, 'ta-toolbar' => "[['bold','italics','underline','clear'],['insertLink']]"}

5
db/migrate/20230628131123_remove_product_permalink.rb Normal file
View File

@@ -0,0 +1,5 @@
class RemoveProductPermalink < ActiveRecord::Migration[7.0]
def change
remove_column :spree_products, :permalink
end
end

5
db/schema.rb
View File

@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema[7.0].define(version: 2023_06_05_133804) do
ActiveRecord::Schema[7.0].define(version: 2023_06_28_131123) do
# These are extensions that must be enabled in order to support this database
enable_extension "pg_stat_statements"
enable_extension "plpgsql"
@@ -743,7 +743,6 @@ ActiveRecord::Schema[7.0].define(version: 2023_06_05_133804) do
t.text "description"
t.datetime "available_on", precision: nil
t.datetime "deleted_at", precision: nil
t.string "permalink", limit: 255
t.string "meta_keywords", limit: 255
t.integer "tax_category_id"
t.integer "shipping_category_id"
@@ -762,8 +761,6 @@ ActiveRecord::Schema[7.0].define(version: 2023_06_05_133804) do
t.index ["available_on"], name: "index_products_on_available_on"
t.index ["deleted_at"], name: "index_products_on_deleted_at"
t.index ["name"], name: "index_products_on_name"
t.index ["permalink"], name: "index_products_on_permalink"
t.index ["permalink"], name: "permalink_idx_unique", unique: true
t.index ["primary_taxon_id"], name: "index_spree_products_on_primary_taxon_id"
t.index ["supplier_id"], name: "index_spree_products_on_supplier_id"
end

20
spec/controllers/api/v0/products_controller_spec.rb
View File

@@ -45,26 +45,6 @@ describe Api::V0::ProductsController, type: :controller do
} ).to eq(true)
end
context "finds a product by permalink first then by id" do
let!(:other_product) {
create(:product, permalink: "these-are-not-the-droids-you-are-looking-for")
}
before do
product.update_attribute(:permalink, "#{other_product.id}-and-1-ways")
end
specify do
api_get :show, id: product.to_param
expect(json_response["permalink_live"]).to match(/and-1-ways/)
product.reload.destroy
api_get :show, id: other_product.id
expect(json_response["permalink_live"]).to match(/droids/)
end
end
it "cannot see inactive products" do
api_get :show, id: inactive_product.to_param

4
spec/controllers/api/v0/variants_controller_spec.rb
View File

@@ -135,7 +135,7 @@ describe Api::V0::VariantsController, type: :controller do
end
it "are visible by admin" do
api_get :index, show_deleted: 1, product_id: variant.product.to_param
api_get :index, show_deleted: 1, product_id: variant.product.id
expect(json_response.count).to eq(2)
end
@@ -145,7 +145,7 @@ describe Api::V0::VariantsController, type: :controller do
original_number_of_variants = variant.product.variants.count
api_post :create, variant: { sku: "12345", unit_value: "1",
unit_description: "L", price: "1" },
product_id: variant.product.to_param
product_id: variant.product.id
expect(attributes.all?{ |attr| json_response.include? attr.to_s }).to eq(true)
expect(response.status).to eq(201)

15
spec/controllers/spree/admin/variants_controller_spec.rb
View File

@@ -17,12 +17,12 @@ module Spree
end
it "lists only non-deleted variants with params[:deleted] == off" do
spree_get :index, product_id: product.permalink, deleted: "off"
spree_get :index, product_id: product.id, deleted: "off"
expect(assigns(:variants)).to eq(product.variants)
end
it "lists only deleted variants with params[:deleted] == on" do
spree_get :index, product_id: product.permalink, deleted: "on"
spree_get :index, product_id: product.id, deleted: "on"
expect(assigns(:variants)).to eq([deleted_variant])
end
end
@@ -69,29 +69,28 @@ module Spree
end
it 'deletes the variant' do
spree_delete :destroy, id: variant.id, product_id: variant.product.permalink,
spree_delete :destroy, id: variant.id, product_id: variant.product.id,
format: 'html'
expect(variant).to have_received(:destroy)
end
it 'shows a success flash message' do
spree_delete :destroy, id: variant.id, product_id: variant.product.permalink,
spree_delete :destroy, id: variant.id, product_id: variant.product.id,
format: 'html'
expect(flash[:success]).to be
end
it 'redirects to admin_product_variants_url' do
spree_delete :destroy, id: variant.id, product_id: variant.product.permalink,
spree_delete :destroy, id: variant.id, product_id: variant.product.id,
format: 'html'
expect(response).to redirect_to spree
.admin_product_variants_url(variant.product.permalink)
expect(response).to redirect_to spree.admin_product_variants_url(variant.product.id)
end
it 'destroys all its exchanges' do
exchange = create(:exchange)
variant.exchanges << exchange
spree_delete :destroy, id: variant.id, product_id: variant.product.permalink,
spree_delete :destroy, id: variant.id, product_id: variant.product.id,
format: 'html'
expect(variant.exchanges.reload).to be_empty
end

24
spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee
View File

@@ -191,7 +191,6 @@ describe "filtering products for submission to database", ->
description: ""
available_on: available_on
deleted_at: null
permalink: null
meta_keywords: null
tax_category_id: null
shipping_category_id: null
@@ -845,11 +844,9 @@ describe "AdminProductEditCtrl", ->
$scope.products = [
{
id: 9
permalink_live: "apples"
}
{
id: 13
permalink_live: "oranges"
}
]
$scope.dirtyProducts = {}
@@ -863,11 +860,9 @@ describe "AdminProductEditCtrl", ->
$scope.products = [
{
id: 9
permalink_live: "apples"
}
{
id: 13
permalink_live: "oranges"
}
]
DirtyProducts.addProductProperty 9, "someProperty", "something"
@@ -878,7 +873,6 @@ describe "AdminProductEditCtrl", ->
$httpBackend.flush()
expect($scope.products).toEqual [
id: 9
permalink_live: "apples"
]
expect(DirtyProducts.all()).toEqual 9:
id: 9
@@ -913,12 +907,11 @@ describe "AdminProductEditCtrl", ->
describe "when the variant has been saved", ->
it "deletes variants with a http delete request to /api/products/product_permalink/variants/(variant_id)", ->
it "deletes variants with a http delete request to /api/products/(id)/variants/(variant_id)", ->
spyOn(window, "confirm").and.returnValue true
$scope.products = [
{
id: 9
permalink_live: "apples"
variants: [{
id: 3
price: 12
@@ -931,11 +924,10 @@ describe "AdminProductEditCtrl", ->
}
{
id: 13
permalink_live: "oranges"
}
]
$scope.dirtyProducts = {}
$httpBackend.expectDELETE("/api/v0/products/apples/variants/3").respond 200, "data"
$httpBackend.expectDELETE("/api/v0/products/9/variants/3").respond 200, "data"
$scope.deleteVariant $scope.products[0], $scope.products[0].variants[0]
$httpBackend.flush()
@@ -944,7 +936,6 @@ describe "AdminProductEditCtrl", ->
$scope.products = [
{
id: 9
permalink_live: "apples"
variants: [
{
id: 3
@@ -958,14 +949,13 @@ describe "AdminProductEditCtrl", ->
}
{
id: 13
permalink_live: "oranges"
}
]
DirtyProducts.addVariantProperty 9, 3, "price", 12.0
DirtyProducts.addVariantProperty 9, 4, "price", 6.0
DirtyProducts.addProductProperty 13, "name", "P1"
$httpBackend.expectDELETE("/api/v0/products/apples/variants/3").respond 200, "data"
$httpBackend.expectDELETE("/api/v0/products/9/variants/3").respond 200, "data"
$scope.deleteVariant $scope.products[0], $scope.products[0].variants[0]
$httpBackend.flush()
expect($scope.products[0].variants).toEqual [
@@ -995,8 +985,6 @@ describe "AdminProductEditCtrl", ->
description: ""
available_on: available_on
deleted_at: null
permalink: 'test-product'
permalink_live: 'test-product'
meta_keywords: null
tax_category_id: null
shipping_category_id: null
@@ -1021,7 +1009,7 @@ describe "AdminProductEditCtrl", ->
$scope.editWarn(testProduct, testVariant)
expect(windowStub.location.href).toBe(
"/admin/products/#{testProduct.permalink_live}/variants/#{testVariant.id}/edit"
"/admin/products/#{testProduct.id}/variants/#{testVariant.id}/edit"
)
describe 'product has no variant', ->
@@ -1036,7 +1024,7 @@ describe "AdminProductEditCtrl", ->
$scope.editWarn(testProduct, null)
expect(windowStub.location.href).toBe(
"/admin/products/#{testProduct.permalink_live}/edit"
"/admin/products/#{testProduct.id}/edit"
)
it 'should load edit product page including the selected filters', inject ($httpParamSerializer) ->
@@ -1051,7 +1039,7 @@ describe "AdminProductEditCtrl", ->
$scope.editWarn(testProduct, null)
expect(windowStub.location.href).toBe(
"/admin/products/#{testProduct.permalink_live}/edit?#{expectedFilter}"
"/admin/products/#{testProduct.id}/edit?#{expectedFilter}"
)
describe "filtering products", ->

83
spec/models/spree/product_spec.rb
View File

@@ -44,69 +44,6 @@ module Spree
end
end
context "permalink" do
context "build product with similar name" do
let!(:other) { create(:product, name: 'foo bar') }
let(:product) { build(:product, name: 'foo') }
before { product.valid? }
it "increments name" do
expect(product.permalink).to eq 'foo-1'
end
end
context "build permalink with quotes" do
it "does not save quotes" do
product = create(:product, name: "Joe's", permalink: "joe's")
expect(product.permalink).to eq "joe-s"
end
end
context "permalinks must be unique" do
before do
@product1 = create(:product, name: 'foo')
end
it "cannot create another product with the same permalink" do
pending '[Spree build] Failing spec'
@product2 = create(:product, name: 'foo')
expect do
@product2.update(permalink: @product1.permalink)
end.to raise_error(ActiveRecord::RecordNotUnique)
end
end
it "supports Chinese" do
expect(create(:product, name: "你好").permalink).to eq "ni-hao"
end
context "manual permalink override" do
let(:product) { create(:product, name: "foo") }
it "calling save_permalink with a parameter" do
product.name = "foobar"
product.save
expect(product.permalink).to eq "foo"
product.save_permalink(product.name)
expect(product.permalink).to eq "foobar"
end
end
context "override permalink of deleted product" do
let(:product) { create(:product, name: "foo") }
it "should create product with same permalink from name like deleted product" do
expect(product.permalink).to eq "foo"
product.destroy
new_product = create(:product, name: "foo")
expect(new_product.permalink).to eq "foo"
end
end
end
context "properties" do
let(:product) { create(:product) }
@@ -264,26 +201,6 @@ 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 = build_stubbed(:product, name: "Banana Permanenta", permalink: nil)
expect(product2).to be_valid
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

2
spec/serializers/api/product_serializer_spec.rb
View File

@@ -27,7 +27,7 @@ describe Api::ProductSerializer do
it "serializes various attributes" do
expect(serializer.serializable_hash.keys).to eq [
:id, :name, :permalink, :meta_keywords, :group_buy, :notes, :description, :description_html,
:id, :name, :meta_keywords, :group_buy, :notes, :description, :description_html,
:properties_with_values, :variants, :primary_taxon, :taxons, :image, :supplier
]
end

4
spec/services/sets/product_set_spec.rb
View File

@@ -136,13 +136,13 @@ describe Sets::ProductSet do
context 'and when product attributes are also passed' do
it 'updates product and variant attributes' do
collection_hash[0][:permalink] = "test_permalink"
collection_hash[0][:sku] = "test_sku"
product_set.save
expect(product.reload.variants.first[:sku]).to eq variants_attributes.first[:sku]
expect(product.reload.attributes).to include(
'permalink' => "test_permalink"
'sku' => "test_sku"
)
end
end

8
spec/system/admin/bulk_product_update_spec.rb
View File

@@ -618,7 +618,7 @@ describe '
find("a.edit-product").click
end
expect(URI.parse(current_url).path).to eq spree.edit_admin_product_path(v1.product.permalink)
expect(URI.parse(current_url).path).to eq spree.edit_admin_product_path(v1.product.id)
end
it "shows an edit button for products, which takes the user to the standard edit page for that product, url includes selected filter" do
@@ -634,7 +634,7 @@ describe '
uri = URI.parse(current_url)
expect("#{uri.path}?#{uri.query}").to eq spree.edit_admin_product_path(
v1.product.permalink, producerFilter: p1.supplier.id
v1.product.id, producerFilter: p1.supplier.id
)
end
@@ -650,7 +650,7 @@ describe '
uri = URI.parse(current_url)
expect(URI.parse(current_url).path).to eq spree.edit_admin_product_variant_path(
v1.product.permalink, v1.id
v1.product.id, v1.id
)
end
@@ -670,7 +670,7 @@ describe '
uri = URI.parse(current_url)
expect("#{uri.path}?#{uri.query}").to eq spree.edit_admin_product_variant_path(
v1.product.permalink, v1.id, producerFilter: p1.supplier.id
v1.product.id, v1.id, producerFilter: p1.supplier.id
)
end
end

12
spec/system/admin/products_spec.rb
View File

@@ -324,37 +324,37 @@ describe '
expect(page).to have_link('Cancel', href: expected_admin_product_url)
expected_product_url = Regexp.new(Regexp.escape(spree.edit_admin_product_path(
product.permalink, filter
product.id, filter
)))
expect(page).to have_link('Product Details',
href: expected_product_url)
expected_product_image_url = Regexp.new(Regexp.escape(spree.admin_product_images_path(
product.permalink, filter
product.id, filter
)))
expect(page).to have_link('Images',
href: expected_product_image_url)
expected_product_variant_url = Regexp.new(Regexp.escape(spree.admin_product_variants_path(
product.permalink, filter
product.id, filter
)))
expect(page).to have_link('Variants',
href: expected_product_variant_url)
expected_product_properties_url =
Regexp.new(Regexp.escape(spree.admin_product_product_properties_path(
product.permalink, filter)))
product.id, filter)))
expect(page).to have_link('Product Properties',
href: expected_product_properties_url)
expected_product_group_buy_option_url =
Regexp.new(Regexp.escape(spree.group_buy_options_admin_product_path(
product.permalink, filter)))
product.id, filter)))
expect(page).to have_link('Group Buy Options',
href: expected_product_group_buy_option_url)
expected_product_seo_url = Regexp.new(Regexp.escape(spree.seo_admin_product_path(
product.permalink, filter
product.id, filter
)))
expect(page).to have_link('Search', href: expected_product_seo_url)
end