From 4a11a086c381385e065071bbc6374f318b853977 Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Wed, 13 Nov 2024 20:59:03 +0530 Subject: [PATCH 01/14] Added new columns for resources --- ...and_private_metadata_to_spree_resources.rb | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 core/db/migrate/20241112211303_add_public_and_private_metadata_to_spree_resources.rb diff --git a/core/db/migrate/20241112211303_add_public_and_private_metadata_to_spree_resources.rb b/core/db/migrate/20241112211303_add_public_and_private_metadata_to_spree_resources.rb new file mode 100644 index 00000000000..80af0654325 --- /dev/null +++ b/core/db/migrate/20241112211303_add_public_and_private_metadata_to_spree_resources.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class AddPublicAndPrivateMetadataToSpreeResources < ActiveRecord::Migration[7.0] + def change + # List of Resources to add metadata columns to + %i[ + spree_orders + spree_line_items + spree_shipments + spree_payments + spree_stock_movements + spree_addresses + spree_refunds + ].each do |table_name| + change_table table_name do |t| + # Check if the database supports jsonb for efficient querying + if t.respond_to?(:jsonb) + add_column table_name, :public_metadata, :jsonb, default: {} + add_column table_name, :private_metadata, :jsonb, default: {} + else + add_column table_name, :public_metadata, :json, default: {} + add_column table_name, :private_metadata, :json, default: {} + end + end + end + end +end From 6ec8c9b4b97bc7916d2e8694ee2b416aa34b83bb Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Wed, 13 Nov 2024 21:00:32 +0530 Subject: [PATCH 02/14] added model level configurations --- core/app/models/concerns/spree/metadata.rb | 12 ++++++++++++ core/app/models/spree/address.rb | 1 + core/app/models/spree/line_item.rb | 2 ++ core/app/models/spree/order.rb | 1 + core/app/models/spree/payment.rb | 1 + core/app/models/spree/refund.rb | 2 ++ core/app/models/spree/shipment.rb | 2 ++ core/app/models/spree/stock_movement.rb | 2 ++ 8 files changed, 23 insertions(+) create mode 100644 core/app/models/concerns/spree/metadata.rb diff --git a/core/app/models/concerns/spree/metadata.rb b/core/app/models/concerns/spree/metadata.rb new file mode 100644 index 00000000000..2bc12a2fe17 --- /dev/null +++ b/core/app/models/concerns/spree/metadata.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module Spree + module Metadata + extend ActiveSupport::Concern + + included do + store :public_metadata, coder: JSON + store :private_metadata, coder: JSON + end + end +end diff --git a/core/app/models/spree/address.rb b/core/app/models/spree/address.rb index 7e03938d937..0d9ddea83ad 100644 --- a/core/app/models/spree/address.rb +++ b/core/app/models/spree/address.rb @@ -7,6 +7,7 @@ module Spree # class Address < Spree::Base extend ActiveModel::ForbiddenAttributesProtection + include Metadata mattr_accessor :state_validator_class self.state_validator_class = Spree::Address::StateValidator diff --git a/core/app/models/spree/line_item.rb b/core/app/models/spree/line_item.rb index 8130c0ba8a9..0fd56826b81 100644 --- a/core/app/models/spree/line_item.rb +++ b/core/app/models/spree/line_item.rb @@ -10,6 +10,8 @@ module Spree # promotion system. # class LineItem < Spree::Base + include Metadata + belongs_to :order, class_name: "Spree::Order", inverse_of: :line_items, touch: true, optional: true belongs_to :variant, -> { with_discarded }, class_name: "Spree::Variant", inverse_of: :line_items, optional: true belongs_to :tax_category, class_name: "Spree::TaxCategory", optional: true diff --git a/core/app/models/spree/order.rb b/core/app/models/spree/order.rb index 97cee9f75c3..8e8a476f623 100644 --- a/core/app/models/spree/order.rb +++ b/core/app/models/spree/order.rb @@ -26,6 +26,7 @@ class Order < Spree::Base include ::Spree::Config.state_machines.order include Spree::Order::Payments + include Metadata class InsufficientStock < StandardError attr_reader :items diff --git a/core/app/models/spree/payment.rb b/core/app/models/spree/payment.rb index 2e08205caba..9f034f60ac7 100644 --- a/core/app/models/spree/payment.rb +++ b/core/app/models/spree/payment.rb @@ -7,6 +7,7 @@ module Spree # class Payment < Spree::Base include Spree::Payment::Processing + include Metadata IDENTIFIER_CHARS = (('A'..'Z').to_a + ('0'..'9').to_a - %w(0 1 I O)).freeze NON_RISKY_AVS_CODES = ['B', 'D', 'H', 'J', 'M', 'Q', 'T', 'V', 'X', 'Y'].freeze diff --git a/core/app/models/spree/refund.rb b/core/app/models/spree/refund.rb index 87187331af3..2727177c6a7 100644 --- a/core/app/models/spree/refund.rb +++ b/core/app/models/spree/refund.rb @@ -2,6 +2,8 @@ module Spree class Refund < Spree::Base + include Metadata + belongs_to :payment, inverse_of: :refunds, optional: true belongs_to :reason, class_name: 'Spree::RefundReason', foreign_key: :refund_reason_id, optional: true belongs_to :reimbursement, inverse_of: :refunds, optional: true diff --git a/core/app/models/spree/shipment.rb b/core/app/models/spree/shipment.rb index 6808518448d..ce80b85ef51 100644 --- a/core/app/models/spree/shipment.rb +++ b/core/app/models/spree/shipment.rb @@ -4,6 +4,8 @@ module Spree # An order's planned shipments including tracking and cost. # class Shipment < Spree::Base + include Metadata + belongs_to :order, class_name: 'Spree::Order', touch: true, inverse_of: :shipments, optional: true belongs_to :stock_location, class_name: 'Spree::StockLocation', optional: true diff --git a/core/app/models/spree/stock_movement.rb b/core/app/models/spree/stock_movement.rb index c1de4df6eb0..251e0bd81a7 100644 --- a/core/app/models/spree/stock_movement.rb +++ b/core/app/models/spree/stock_movement.rb @@ -2,6 +2,8 @@ module Spree class StockMovement < Spree::Base + include Metadata + belongs_to :stock_item, class_name: 'Spree::StockItem', inverse_of: :stock_movements, optional: true belongs_to :originator, polymorphic: true, optional: true has_one :variant, through: :stock_item From f9b9a162377804739b7f2327961d81839890267a Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Wed, 13 Nov 2024 21:00:51 +0530 Subject: [PATCH 03/14] added test cases for resources --- core/spec/models/spree/address_spec.rb | 22 +++++++++++++++++++ core/spec/models/spree/line_item_spec.rb | 20 +++++++++++++++++ core/spec/models/spree/order_spec.rb | 20 +++++++++++++++++ core/spec/models/spree/payment_spec.rb | 20 +++++++++++++++++ core/spec/models/spree/refund_spec.rb | 20 +++++++++++++++++ core/spec/models/spree/shipment_spec.rb | 20 +++++++++++++++++ core/spec/models/spree/stock_movement_spec.rb | 20 +++++++++++++++++ 7 files changed, 142 insertions(+) diff --git a/core/spec/models/spree/address_spec.rb b/core/spec/models/spree/address_spec.rb index 0f2eea77233..4beb631886d 100644 --- a/core/spec/models/spree/address_spec.rb +++ b/core/spec/models/spree/address_spec.rb @@ -294,4 +294,26 @@ it { is_expected.to be_require_phone } end + + describe "metadata fields" do + subject { described_class.new } + + it "responds to public_metadata" do + expect(subject).to respond_to(:public_metadata) + end + + it "responds to private_metadata" do + expect(subject).to respond_to(:private_metadata) + end + + it "can store data in public_metadata" do + subject.public_metadata = { "address_type" => "work" } + expect(subject.public_metadata["address_type"]).to eq("work") + end + + it "can store data in private_metadata" do + subject.private_metadata = { "preferred_delivery_time" => "Morning" } + expect(subject.private_metadata["preferred_delivery_time"]).to eq("Morning") + end + end end diff --git a/core/spec/models/spree/line_item_spec.rb b/core/spec/models/spree/line_item_spec.rb index 9d225059cbc..a36e9e5e5d9 100644 --- a/core/spec/models/spree/line_item_spec.rb +++ b/core/spec/models/spree/line_item_spec.rb @@ -187,4 +187,24 @@ expect(subject.currency).to eq("USD") end end + + describe "metadata fields" do + it "responds to public_metadata" do + expect(line_item).to respond_to(:public_metadata) + end + + it "responds to private_metadata" do + expect(line_item).to respond_to(:private_metadata) + end + + it "can store data in public_metadata" do + line_item.public_metadata = { "quantity" => "3" } + expect(line_item.public_metadata["quantity"]).to eq("3") + end + + it "can store data in private_metadata" do + line_item.private_metadata = { "supplier_id" => "SUP-34567" } + expect(line_item.private_metadata["supplier_id"]).to eq("SUP-34567") + end + end end diff --git a/core/spec/models/spree/order_spec.rb b/core/spec/models/spree/order_spec.rb index 86c75f11ec3..32f2495da21 100644 --- a/core/spec/models/spree/order_spec.rb +++ b/core/spec/models/spree/order_spec.rb @@ -2107,4 +2107,24 @@ def validate(line_item) it { is_expected.to eq(true) } end end + + describe "metadata fields" do + it "responds to public_metadata" do + expect(order).to respond_to(:public_metadata) + end + + it "responds to private_metadata" do + expect(order).to respond_to(:private_metadata) + end + + it "can store data in public_metadata" do + order.public_metadata = { "order_id" => "OD34236" } + expect(order.public_metadata["order_id"]).to eq("OD34236") + end + + it "can store data in private_metadata" do + order.private_metadata = { "internal_note" => "gift wrap the item" } + expect(order.private_metadata["internal_note"]).to eq("gift wrap the item") + end + end end diff --git a/core/spec/models/spree/payment_spec.rb b/core/spec/models/spree/payment_spec.rb index c3d12681e5b..a99387d1b4e 100644 --- a/core/spec/models/spree/payment_spec.rb +++ b/core/spec/models/spree/payment_spec.rb @@ -1331,4 +1331,24 @@ expect(described_class.valid).to be_empty end end + + describe "metadata fields" do + it "responds to public_metadata" do + expect(payment).to respond_to(:public_metadata) + end + + it "responds to private_metadata" do + expect(payment).to respond_to(:private_metadata) + end + + it "can store data in public_metadata" do + payment.public_metadata = { "transaction_id" => "12345" } + expect(payment.public_metadata["transaction_id"]).to eq("12345") + end + + it "can store data in private_metadata" do + payment.private_metadata = { "internal_note" => "Verified transaction" } + expect(payment.private_metadata["internal_note"]).to eq("Verified transaction") + end + end end diff --git a/core/spec/models/spree/refund_spec.rb b/core/spec/models/spree/refund_spec.rb index 3f230326caa..213b950380e 100644 --- a/core/spec/models/spree/refund_spec.rb +++ b/core/spec/models/spree/refund_spec.rb @@ -236,5 +236,25 @@ expect(amount).to eq 0 end end + + describe "metadata fields" do + it "responds to public_metadata" do + expect(refund).to respond_to(:public_metadata) + end + + it "responds to private_metadata" do + expect(refund).to respond_to(:private_metadata) + end + + it "can store data in public_metadata" do + refund.public_metadata = { "refund_reason" => "price_adjustment" } + expect(refund.public_metadata["refund_reason"]).to eq("price_adjustment") + end + + it "can store data in private_metadata" do + refund.private_metadata = { "internal_notes" => "Refund processed manually" } + expect(refund.private_metadata["internal_notes"]).to eq("Refund processed manually") + end + end end end diff --git a/core/spec/models/spree/shipment_spec.rb b/core/spec/models/spree/shipment_spec.rb index 384413166d6..ccd2d78250f 100644 --- a/core/spec/models/spree/shipment_spec.rb +++ b/core/spec/models/spree/shipment_spec.rb @@ -872,4 +872,24 @@ it { is_expected.to include carton } end + + describe "metadata fields" do + it "responds to public_metadata" do + expect(shipment).to respond_to(:public_metadata) + end + + it "responds to private_metadata" do + expect(shipment).to respond_to(:private_metadata) + end + + it "can store data in public_metadata" do + shipment.public_metadata = { "tracking_info" => "UPS123456789" } + expect(shipment.public_metadata["tracking_info"]).to eq("UPS123456789") + end + + it "can store data in private_metadata" do + shipment.private_metadata = { "internal_note" => "Handle with care" } + expect(shipment.private_metadata["internal_note"]).to eq("Handle with care") + end + end end diff --git a/core/spec/models/spree/stock_movement_spec.rb b/core/spec/models/spree/stock_movement_spec.rb index d3f1e3bed3e..45d344faf51 100644 --- a/core/spec/models/spree/stock_movement_spec.rb +++ b/core/spec/models/spree/stock_movement_spec.rb @@ -59,4 +59,24 @@ end end end + + describe "metadata fields" do + it "responds to public_metadata" do + expect(subject).to respond_to(:public_metadata) + end + + it "responds to private_metadata" do + expect(subject).to respond_to(:private_metadata) + end + + it "can store data in public_metadata" do + subject.public_metadata = { "movement_reason" => "restock" } + expect(subject.public_metadata["movement_reason"]).to eq("restock") + end + + it "can store data in private_metadata" do + subject.private_metadata = { "internal_note" => "Inventory verified" } + expect(subject.private_metadata["internal_note"]).to eq("Inventory verified") + end + end end From e5398a25f0bff378520567f0103c63dad7af454b Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Fri, 15 Nov 2024 02:53:42 +0530 Subject: [PATCH 04/14] Updated migration to add more models --- ...d_public_and_private_metadata_to_spree_resources.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/core/db/migrate/20241112211303_add_public_and_private_metadata_to_spree_resources.rb b/core/db/migrate/20241112211303_add_public_and_private_metadata_to_spree_resources.rb index 80af0654325..d8306b909eb 100644 --- a/core/db/migrate/20241112211303_add_public_and_private_metadata_to_spree_resources.rb +++ b/core/db/migrate/20241112211303_add_public_and_private_metadata_to_spree_resources.rb @@ -11,6 +11,16 @@ def change spree_stock_movements spree_addresses spree_refunds + spree_products + spree_customer_returns + spree_stock_locations + spree_store_credit_events + spree_stores + spree_taxonomies + spree_taxons + spree_variants + spree_users + spree_return_authorizations ].each do |table_name| change_table table_name do |t| # Check if the database supports jsonb for efficient querying From 59203abf1451ea012e9dca580f3dd63487840ca6 Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Fri, 15 Nov 2024 02:55:27 +0530 Subject: [PATCH 05/14] included concern in missing models --- core/app/models/spree/customer_return.rb | 2 ++ core/app/models/spree/product.rb | 1 + core/app/models/spree/return_authorization.rb | 2 ++ core/app/models/spree/stock_location.rb | 1 + core/app/models/spree/store.rb | 2 ++ core/app/models/spree/store_credit_event.rb | 1 + core/app/models/spree/taxon.rb | 2 ++ core/app/models/spree/taxonomy.rb | 2 ++ core/app/models/spree/variant.rb | 1 + 9 files changed, 14 insertions(+) diff --git a/core/app/models/spree/customer_return.rb b/core/app/models/spree/customer_return.rb index ec4b17b750e..e9622f1db3a 100644 --- a/core/app/models/spree/customer_return.rb +++ b/core/app/models/spree/customer_return.rb @@ -2,6 +2,8 @@ module Spree class CustomerReturn < Spree::Base + include Metadata + belongs_to :stock_location, optional: true has_many :return_items, inverse_of: :customer_return diff --git a/core/app/models/spree/product.rb b/core/app/models/spree/product.rb index 1754e939497..5c9aacb8af1 100644 --- a/core/app/models/spree/product.rb +++ b/core/app/models/spree/product.rb @@ -10,6 +10,7 @@ class Product < Spree::Base friendly_id :slug_candidates, use: :history include Spree::SoftDeletable + include Metadata after_discard do variants_including_master.discard_all diff --git a/core/app/models/spree/return_authorization.rb b/core/app/models/spree/return_authorization.rb index e44b64328e5..c37ec82c43c 100644 --- a/core/app/models/spree/return_authorization.rb +++ b/core/app/models/spree/return_authorization.rb @@ -4,6 +4,8 @@ module Spree # Models the return of Inventory Units to a Stock Location for an Order. # class ReturnAuthorization < Spree::Base + include Metadata + belongs_to :order, class_name: 'Spree::Order', inverse_of: :return_authorizations, optional: true has_many :return_items, inverse_of: :return_authorization, dependent: :destroy diff --git a/core/app/models/spree/stock_location.rb b/core/app/models/spree/stock_location.rb index 4ded3100981..0ca347dfbfd 100644 --- a/core/app/models/spree/stock_location.rb +++ b/core/app/models/spree/stock_location.rb @@ -6,6 +6,7 @@ module Spree # class StockLocation < Spree::Base class InvalidMovementError < StandardError; end + include Metadata acts_as_list diff --git a/core/app/models/spree/store.rb b/core/app/models/spree/store.rb index 9350fb0a2b8..dbd605d31c4 100644 --- a/core/app/models/spree/store.rb +++ b/core/app/models/spree/store.rb @@ -9,6 +9,8 @@ module Spree # hosted by a single Solidus implementation can be built. # class Store < Spree::Base + include Metadata + has_many :store_payment_methods, inverse_of: :store has_many :payment_methods, through: :store_payment_methods diff --git a/core/app/models/spree/store_credit_event.rb b/core/app/models/spree/store_credit_event.rb index e20a4a99d6d..d5c6a70686e 100644 --- a/core/app/models/spree/store_credit_event.rb +++ b/core/app/models/spree/store_credit_event.rb @@ -3,6 +3,7 @@ module Spree class StoreCreditEvent < Spree::Base include Spree::SoftDeletable + include Metadata belongs_to :store_credit, optional: true belongs_to :originator, polymorphic: true, optional: true diff --git a/core/app/models/spree/taxon.rb b/core/app/models/spree/taxon.rb index db8c7759b0f..7a89ad4d56f 100644 --- a/core/app/models/spree/taxon.rb +++ b/core/app/models/spree/taxon.rb @@ -6,6 +6,8 @@ module Spree class Taxon < Spree::Base acts_as_nested_set dependent: :destroy + include Metadata + belongs_to :taxonomy, class_name: 'Spree::Taxonomy', inverse_of: :taxons has_many :classifications, -> { order(:position) }, dependent: :delete_all, inverse_of: :taxon has_many :products, through: :classifications diff --git a/core/app/models/spree/taxonomy.rb b/core/app/models/spree/taxonomy.rb index bd8ecd0ef73..6f0e9bfca48 100644 --- a/core/app/models/spree/taxonomy.rb +++ b/core/app/models/spree/taxonomy.rb @@ -4,6 +4,8 @@ module Spree class Taxonomy < Spree::Base acts_as_list + include Metadata + validates :name, presence: true validates :name, uniqueness: true diff --git a/core/app/models/spree/variant.rb b/core/app/models/spree/variant.rb index 40226656db3..416e75b737f 100644 --- a/core/app/models/spree/variant.rb +++ b/core/app/models/spree/variant.rb @@ -19,6 +19,7 @@ class Variant < Spree::Base acts_as_list scope: :product include Spree::SoftDeletable + include Metadata after_discard do stock_items.discard_all From fbbd44c9ee05792270f50ddac795dfec7cc97f98 Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Fri, 15 Nov 2024 02:56:14 +0530 Subject: [PATCH 06/14] Added test cases for new models --- .../spec/models/spree/customer_return_spec.rb | 22 +++++++++++++++++++ core/spec/models/spree/product_spec.rb | 22 +++++++++++++++++++ .../models/spree/return_authorization_spec.rb | 21 ++++++++++++++++++ core/spec/models/spree/stock_location_spec.rb | 22 +++++++++++++++++++ .../models/spree/store_credit_event_spec.rb | 22 +++++++++++++++++++ core/spec/models/spree/store_spec.rb | 22 +++++++++++++++++++ core/spec/models/spree/taxon_spec.rb | 22 +++++++++++++++++++ core/spec/models/spree/taxonomy_spec.rb | 22 +++++++++++++++++++ core/spec/models/spree/variant_spec.rb | 22 +++++++++++++++++++ 9 files changed, 197 insertions(+) diff --git a/core/spec/models/spree/customer_return_spec.rb b/core/spec/models/spree/customer_return_spec.rb index da31b8dfcf7..1b8b2ed8057 100644 --- a/core/spec/models/spree/customer_return_spec.rb +++ b/core/spec/models/spree/customer_return_spec.rb @@ -303,4 +303,26 @@ end end end + + describe "metadata fields" do + subject { described_class.new } + + it "responds to public_metadata" do + expect(subject).to respond_to(:public_metadata) + end + + it "responds to private_metadata" do + expect(subject).to respond_to(:private_metadata) + end + + it "can store data in public_metadata" do + subject.public_metadata = { "return_details" => "canceled" } + expect(subject.public_metadata["return_details"]).to eq("canceled") + end + + it "can store data in private_metadata" do + subject.private_metadata = { "stock_details" => "coffee_beans" } + expect(subject.private_metadata["stock_details"]).to eq("coffee_beans") + end + end end diff --git a/core/spec/models/spree/product_spec.rb b/core/spec/models/spree/product_spec.rb index c5d8132f636..11ecef6ef38 100644 --- a/core/spec/models/spree/product_spec.rb +++ b/core/spec/models/spree/product_spec.rb @@ -709,4 +709,26 @@ class Extension < Spree::Base end end end + + describe "metadata fields" do + subject { described_class.new } + + it "responds to public_metadata" do + expect(subject).to respond_to(:public_metadata) + end + + it "responds to private_metadata" do + expect(subject).to respond_to(:private_metadata) + end + + it "can store data in public_metadata" do + subject.public_metadata = { "delivery_required" => "no" } + expect(subject.public_metadata["delivery_required"]).to eq("no") + end + + it "can store data in private_metadata" do + subject.private_metadata = { "preferred_delivery_time" => "n/a" } + expect(subject.private_metadata["preferred_delivery_time"]).to eq("n/a") + end + end end diff --git a/core/spec/models/spree/return_authorization_spec.rb b/core/spec/models/spree/return_authorization_spec.rb index 39fb9091e2c..ea668bb9ce7 100644 --- a/core/spec/models/spree/return_authorization_spec.rb +++ b/core/spec/models/spree/return_authorization_spec.rb @@ -223,4 +223,25 @@ it { is_expected.to eq true } end end + describe "metadata fields" do + subject { described_class.new } + + it "responds to public_metadata" do + expect(subject).to respond_to(:public_metadata) + end + + it "responds to private_metadata" do + expect(subject).to respond_to(:private_metadata) + end + + it "can store data in public_metadata" do + subject.public_metadata = { "return_details" => "canceled" } + expect(subject.public_metadata["return_details"]).to eq("canceled") + end + + it "can store data in private_metadata" do + subject.private_metadata = { "reason" => "unknown" } + expect(subject.private_metadata["reason"]).to eq("unknown") + end + end end diff --git a/core/spec/models/spree/stock_location_spec.rb b/core/spec/models/spree/stock_location_spec.rb index 739842f78fe..f7af1036d6f 100644 --- a/core/spec/models/spree/stock_location_spec.rb +++ b/core/spec/models/spree/stock_location_spec.rb @@ -285,5 +285,27 @@ def move end end end + + describe "metadata fields" do + subject { described_class.new } + + it "responds to public_metadata" do + expect(subject).to respond_to(:public_metadata) + end + + it "responds to private_metadata" do + expect(subject).to respond_to(:private_metadata) + end + + it "can store data in public_metadata" do + subject.public_metadata = { "location_details" => "classified" } + expect(subject.public_metadata["location_details"]).to eq("classified") + end + + it "can store data in private_metadata" do + subject.private_metadata = { "exchange_details" => "classified" } + expect(subject.private_metadata["exchange_details"]).to eq("classified") + end + end end end diff --git a/core/spec/models/spree/store_credit_event_spec.rb b/core/spec/models/spree/store_credit_event_spec.rb index b0b89554ba8..a02cd6ae1d8 100644 --- a/core/spec/models/spree/store_credit_event_spec.rb +++ b/core/spec/models/spree/store_credit_event_spec.rb @@ -325,4 +325,26 @@ end end end + + describe "metadata fields" do + subject { described_class.new } + + it "responds to public_metadata" do + expect(subject).to respond_to(:public_metadata) + end + + it "responds to private_metadata" do + expect(subject).to respond_to(:private_metadata) + end + + it "can store data in public_metadata" do + subject.public_metadata = { "credits_type" => "gift_cards" } + expect(subject.public_metadata["credits_type"]).to eq("gift_cards") + end + + it "can store data in private_metadata" do + subject.private_metadata = { "preferred_options" => "points" } + expect(subject.private_metadata["preferred_options"]).to eq("points") + end + end end diff --git a/core/spec/models/spree/store_spec.rb b/core/spec/models/spree/store_spec.rb index 8492d4da26b..2c2031f8122 100644 --- a/core/spec/models/spree/store_spec.rb +++ b/core/spec/models/spree/store_spec.rb @@ -106,4 +106,26 @@ end end end + + describe "metadata fields" do + subject { described_class.new } + + it "responds to public_metadata" do + expect(subject).to respond_to(:public_metadata) + end + + it "responds to private_metadata" do + expect(subject).to respond_to(:private_metadata) + end + + it "can store data in public_metadata" do + subject.public_metadata = { "location_preferred" => "remote" } + expect(subject.public_metadata["location_preferred"]).to eq("remote") + end + + it "can store data in private_metadata" do + subject.private_metadata = { "preferred_time" => "Morning" } + expect(subject.private_metadata["preferred_time"]).to eq("Morning") + end + end end diff --git a/core/spec/models/spree/taxon_spec.rb b/core/spec/models/spree/taxon_spec.rb index 2b254a19ba5..deedc526614 100644 --- a/core/spec/models/spree/taxon_spec.rb +++ b/core/spec/models/spree/taxon_spec.rb @@ -269,4 +269,26 @@ end end end + + describe "metadata fields" do + subject { described_class.new } + + it "responds to public_metadata" do + expect(subject).to respond_to(:public_metadata) + end + + it "responds to private_metadata" do + expect(subject).to respond_to(:private_metadata) + end + + it "can store data in public_metadata" do + subject.public_metadata = { "type_preferred" => "custom" } + expect(subject.public_metadata["type_preferred"]).to eq("custom") + end + + it "can store data in private_metadata" do + subject.private_metadata = { "additional_informatio" => "testing" } + expect(subject.private_metadata["additional_informatio"]).to eq("testing") + end + end end diff --git a/core/spec/models/spree/taxonomy_spec.rb b/core/spec/models/spree/taxonomy_spec.rb index 4494653d0a4..3ad9470e73d 100644 --- a/core/spec/models/spree/taxonomy_spec.rb +++ b/core/spec/models/spree/taxonomy_spec.rb @@ -25,4 +25,26 @@ expect(association_options[:dependent]).to eq :destroy end end + + describe "metadata fields" do + subject { described_class.new } + + it "responds to public_metadata" do + expect(subject).to respond_to(:public_metadata) + end + + it "responds to private_metadata" do + expect(subject).to respond_to(:private_metadata) + end + + it "can store data in public_metadata" do + subject.public_metadata = { "options" => "critical" } + expect(subject.public_metadata["options"]).to eq("critical") + end + + it "can store data in private_metadata" do + subject.private_metadata = { "types" => "test" } + expect(subject.private_metadata["types"]).to eq("test") + end + end end diff --git a/core/spec/models/spree/variant_spec.rb b/core/spec/models/spree/variant_spec.rb index 983bd1845d0..422ef539559 100644 --- a/core/spec/models/spree/variant_spec.rb +++ b/core/spec/models/spree/variant_spec.rb @@ -1083,4 +1083,26 @@ expect { variant.option_values << variant.option_values.first } .to raise_error ActiveRecord::RecordNotUnique end + + describe "metadata fields" do + subject { described_class.new } + + it "responds to public_metadata" do + expect(subject).to respond_to(:public_metadata) + end + + it "responds to private_metadata" do + expect(subject).to respond_to(:private_metadata) + end + + it "can store data in public_metadata" do + subject.public_metadata = { "variant_type" => "veg" } + expect(subject.public_metadata["variant_type"]).to eq("veg") + end + + it "can store data in private_metadata" do + subject.private_metadata = { "variant_details" => "veg" } + expect(subject.private_metadata["variant_details"]).to eq("veg") + end + end end From d0977fa2cfea449a15d89b712d1f791bae44f489 Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Fri, 15 Nov 2024 02:57:15 +0530 Subject: [PATCH 07/14] Added new columns to API configuration --- api/lib/spree/api_configuration.rb | 38 +++++++++++++++++------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/api/lib/spree/api_configuration.rb b/api/lib/spree/api_configuration.rb index 2dce13e62d8..cd530cd8228 100644 --- a/api/lib/spree/api_configuration.rb +++ b/api/lib/spree/api_configuration.rb @@ -7,14 +7,14 @@ class ApiConfiguration < Preferences::Configuration preference :product_attributes, :array, default: [ :id, :name, :description, :available_on, :slug, :meta_description, :meta_keywords, :shipping_category_id, - :taxon_ids, :total_on_hand, :meta_title + :taxon_ids, :total_on_hand, :meta_title, :private_metadata, :public_metadata ] preference :product_property_attributes, :array, default: [:id, :product_id, :property_id, :value, :property_name] preference :variant_attributes, :array, default: [ :id, :name, :sku, :weight, :height, :width, :depth, :is_master, - :slug, :description, :track_inventory + :slug, :description, :track_inventory, :private_metadata, :public_metadata ] preference :image_attributes, :array, default: [ @@ -36,34 +36,36 @@ class ApiConfiguration < Preferences::Configuration :covered_by_store_credit, :display_total_applicable_store_credit, :order_total_after_store_credit, :display_order_total_after_store_credit, :total_applicable_store_credit, :display_total_available_store_credit, - :display_store_credit_remaining_after_capture, :canceler_id + :display_store_credit_remaining_after_capture, :canceler_id, :private_metadata, :public_metadata ] - preference :line_item_attributes, :array, default: [:id, :quantity, :price, :variant_id] + preference :line_item_attributes, :array, default: [:id, :quantity, :price, :variant_id, + :private_metadata, :public_metadata] preference :option_type_attributes, :array, default: [:id, :name, :presentation, :position] preference :payment_attributes, :array, default: [ :id, :source_type, :source_id, :amount, :display_amount, :payment_method_id, :state, :avs_response, :created_at, - :updated_at + :updated_at, :private_metadata, :public_metadata ] preference :payment_method_attributes, :array, default: [:id, :name, :description] - preference :shipment_attributes, :array, default: [:id, :tracking, :tracking_url, :number, :cost, :shipped_at, :state] + preference :shipment_attributes, :array, default: [:id, :tracking, :tracking_url, :number, :cost, :shipped_at, :state, + :private_metadata, :public_metadata] - preference :taxonomy_attributes, :array, default: [:id, :name] + preference :taxonomy_attributes, :array, default: [:id, :name, :private_metadata, :public_metadata] preference :taxon_attributes, :array, default: [ :id, :name, :pretty_name, :permalink, :parent_id, - :taxonomy_id + :taxonomy_id, :private_metadata, :public_metadata ] preference :address_attributes, :array, default: [ :id, :name, :address1, :address2, :city, :zipcode, :phone, :company, :alternative_phone, :country_id, :country_iso, :state_id, :state_name, - :state_text + :state_text, :private_metadata, :public_metadata ] preference :country_attributes, :array, default: [:id, :iso_name, :iso, :iso3, :name, :numcode] @@ -81,11 +83,13 @@ class ApiConfiguration < Preferences::Configuration ] preference :customer_return_attributes, :array, default: [ - :id, :number, :stock_location_id, :created_at, :updated_at + :id, :number, :stock_location_id, :created_at, :updated_at, :private_metadata, + :public_metadata ] preference :return_authorization_attributes, :array, default: [ - :id, :number, :state, :order_id, :memo, :created_at, :updated_at + :id, :number, :state, :order_id, :memo, :created_at, :updated_at, + :private_metadata, :public_metadata ] preference :creditcard_attributes, :array, default: [ @@ -96,16 +100,18 @@ class ApiConfiguration < Preferences::Configuration :id, :month, :year, :cc_type, :last_digits, :name ] - preference :user_attributes, :array, default: [:id, :email, :created_at, :updated_at] + preference :user_attributes, :array, default: [:id, :email, :created_at, :updated_at, + :private_metadata, :public_metadata] preference :property_attributes, :array, default: [:id, :name, :presentation] preference :stock_location_attributes, :array, default: [ :id, :name, :address1, :address2, :city, :state_id, :state_name, - :country_id, :zipcode, :phone, :active + :country_id, :zipcode, :phone, :active, :private_metadata, :public_metadata ] - preference :stock_movement_attributes, :array, default: [:id, :quantity, :stock_item_id] + preference :stock_movement_attributes, :array, default: [:id, :quantity, :stock_item_id, + :private_metadata, :public_metadata] preference :stock_item_attributes, :array, default: [ :id, :count_on_hand, :backorderable, :stock_location_id, @@ -127,12 +133,12 @@ def promotion_attributes=(value) preference :store_attributes, :array, default: [ :id, :name, :url, :meta_description, :meta_keywords, :seo_title, :mail_from_address, :default_currency, :code, :default, :available_locales, - :bcc_email + :bcc_email, :private_metadata, :public_metadata ] preference :store_credit_history_attributes, :array, default: [ :display_amount, :display_user_total_amount, :display_action, - :display_event_date, :display_remaining_amount + :display_event_date, :display_remaining_amount, :private_metadata, :public_metadata ] preference :variant_property_attributes, :array, default: [ From 3876f9473efdd4c188f371da464def103a997778 Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Sun, 17 Nov 2024 03:15:29 +0530 Subject: [PATCH 08/14] permitted both metadata for spree product --- api/app/helpers/spree/api/api_helpers.rb | 1 + api/openapi/solidus-api.oas.yml | 4 ++++ core/config/locales/en.yml | 2 ++ .../lib/spree/core/controller_helpers/strong_parameters.rb | 6 +++++- core/lib/spree/permitted_attributes.rb | 7 +++++-- 5 files changed, 17 insertions(+), 3 deletions(-) diff --git a/api/app/helpers/spree/api/api_helpers.rb b/api/app/helpers/spree/api/api_helpers.rb index 8c52db2d772..a87aaebda81 100644 --- a/api/app/helpers/spree/api/api_helpers.rb +++ b/api/app/helpers/spree/api/api_helpers.rb @@ -5,6 +5,7 @@ module Api module ApiHelpers ATTRIBUTES = [ :product_attributes, + :metadata_attributes, :product_property_attributes, :variant_attributes, :image_attributes, diff --git a/api/openapi/solidus-api.oas.yml b/api/openapi/solidus-api.oas.yml index 138f1f7de60..616a9b8a533 100644 --- a/api/openapi/solidus-api.oas.yml +++ b/api/openapi/solidus-api.oas.yml @@ -5655,6 +5655,10 @@ components: $ref: '#/components/schemas/option-type' price: type: string + public_metadata: + type: jsonb + private_metadata: + type: hash product_properties: type: array items: diff --git a/core/config/locales/en.yml b/core/config/locales/en.yml index 5968fab8ad5..a6d84078083 100644 --- a/core/config/locales/en.yml +++ b/core/config/locales/en.yml @@ -179,6 +179,8 @@ en: name: Name on_hand: On Hand price: Master Price + private_metadata: Private Metadata + public_metadata: Public Metadata promotionable: Promotable shipping_category: Shipping Category sku: Master SKU diff --git a/core/lib/spree/core/controller_helpers/strong_parameters.rb b/core/lib/spree/core/controller_helpers/strong_parameters.rb index 01e23d9dcae..25df2772db4 100644 --- a/core/lib/spree/core/controller_helpers/strong_parameters.rb +++ b/core/lib/spree/core/controller_helpers/strong_parameters.rb @@ -56,11 +56,15 @@ def permitted_order_attributes end def permitted_product_attributes - permitted_attributes.product_attributes + [ + permitted_attributes.product_attributes + metadata_attributes + [ product_properties_attributes: permitted_product_properties_attributes ] end + def metadata_attributes + permitted_attributes.metadata_attributes + end + def permitted_user_attributes permitted_attributes.user_attributes + [ bill_address_attributes: permitted_address_attributes, diff --git a/core/lib/spree/permitted_attributes.rb b/core/lib/spree/permitted_attributes.rb index a09a6eb53e6..fcc21f152d6 100644 --- a/core/lib/spree/permitted_attributes.rb +++ b/core/lib/spree/permitted_attributes.rb @@ -33,7 +33,8 @@ module PermittedAttributes :taxon_attributes, :taxonomy_attributes, :user_attributes, - :variant_attributes + :variant_attributes, + :metadata_attributes ] mattr_reader(*ATTRIBUTES) @@ -78,9 +79,11 @@ module PermittedAttributes :meta_keywords, :price, :sku, :deleted_at, :option_values_hash, :weight, :height, :width, :depth, :shipping_category_id, :tax_category_id, - :taxon_ids, :option_type_ids, :cost_currency, :cost_price + :taxon_ids, :option_type_ids, :cost_currency, :cost_price # , :private_metadata, :public_metadata ] + @@metadata_attributes = [private_metadata: {}, public_metadata: {}] + @@property_attributes = [:name, :presentation] @@return_authorization_attributes = [:memo, :stock_location_id, :return_reason_id, return_items_attributes: [:inventory_unit_id, :exchange_variant_id, :return_reason_id, :preferred_reimbursement_type_id]] From 2e02e00db6bbf37519d5cdfaa9922c214bad2fed Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Mon, 18 Nov 2024 15:00:45 +0530 Subject: [PATCH 09/14] remove commented code --- core/lib/spree/permitted_attributes.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/lib/spree/permitted_attributes.rb b/core/lib/spree/permitted_attributes.rb index fcc21f152d6..c6bf7e2cb4d 100644 --- a/core/lib/spree/permitted_attributes.rb +++ b/core/lib/spree/permitted_attributes.rb @@ -79,7 +79,7 @@ module PermittedAttributes :meta_keywords, :price, :sku, :deleted_at, :option_values_hash, :weight, :height, :width, :depth, :shipping_category_id, :tax_category_id, - :taxon_ids, :option_type_ids, :cost_currency, :cost_price # , :private_metadata, :public_metadata + :taxon_ids, :option_type_ids, :cost_currency, :cost_price ] @@metadata_attributes = [private_metadata: {}, public_metadata: {}] From 14a770b59da573b00ad1ef115d05a13f6c9e3615 Mon Sep 17 00:00:00 2001 From: Mayur Date: Tue, 19 Nov 2024 20:39:42 +0530 Subject: [PATCH 10/14] Clean up code by removing extra code --- api/app/helpers/spree/api/api_helpers.rb | 1 - api/openapi/solidus-api.oas.yml | 4 ---- 2 files changed, 5 deletions(-) diff --git a/api/app/helpers/spree/api/api_helpers.rb b/api/app/helpers/spree/api/api_helpers.rb index a87aaebda81..8c52db2d772 100644 --- a/api/app/helpers/spree/api/api_helpers.rb +++ b/api/app/helpers/spree/api/api_helpers.rb @@ -5,7 +5,6 @@ module Api module ApiHelpers ATTRIBUTES = [ :product_attributes, - :metadata_attributes, :product_property_attributes, :variant_attributes, :image_attributes, diff --git a/api/openapi/solidus-api.oas.yml b/api/openapi/solidus-api.oas.yml index 616a9b8a533..138f1f7de60 100644 --- a/api/openapi/solidus-api.oas.yml +++ b/api/openapi/solidus-api.oas.yml @@ -5655,10 +5655,6 @@ components: $ref: '#/components/schemas/option-type' price: type: string - public_metadata: - type: jsonb - private_metadata: - type: hash product_properties: type: array items: From ca64510c7134df76c88ab6133049c0f0a7440e57 Mon Sep 17 00:00:00 2001 From: Mayur Date: Tue, 19 Nov 2024 20:41:05 +0530 Subject: [PATCH 11/14] Added limitation validation for both metadata columns --- core/app/models/concerns/spree/metadata.rb | 30 +++++++++ core/spec/models/spree/product_spec.rb | 75 ++++++++++++++++++++++ 2 files changed, 105 insertions(+) diff --git a/core/app/models/concerns/spree/metadata.rb b/core/app/models/concerns/spree/metadata.rb index 2bc12a2fe17..245b72aa81e 100644 --- a/core/app/models/concerns/spree/metadata.rb +++ b/core/app/models/concerns/spree/metadata.rb @@ -4,9 +4,39 @@ module Spree module Metadata extend ActiveSupport::Concern + MAX_KEYS = 6 + MAX_KEY_LENGTH = 16 + MAX_VALUE_LENGTH = 256 + included do store :public_metadata, coder: JSON store :private_metadata, coder: JSON + + validate :validate_metadata_limits + end + + private + + def validate_metadata_limits + %i[public_metadata private_metadata].each do |column| + metadata = send(column) + + # Check for maximum number of keys + if metadata.keys.count > MAX_KEYS + errors.add(column, "must not have more than #{MAX_KEYS} keys") + end + + # Check for maximum key and value size + metadata.each do |key, value| + if key.to_s.length > MAX_KEY_LENGTH + errors.add(column, "key '#{key}' exceeds #{MAX_KEY_LENGTH} characters") + end + + if value.to_s.length > MAX_VALUE_LENGTH + errors.add(column, "value for key '#{key}' exceeds #{MAX_VALUE_LENGTH} characters") + end + end + end end end end diff --git a/core/spec/models/spree/product_spec.rb b/core/spec/models/spree/product_spec.rb index 11ecef6ef38..e9230c3f876 100644 --- a/core/spec/models/spree/product_spec.rb +++ b/core/spec/models/spree/product_spec.rb @@ -731,4 +731,79 @@ class Extension < Spree::Base expect(subject.private_metadata["preferred_delivery_time"]).to eq("n/a") end end + + describe "metadata validations" do + let(:invalid_metadata_keys) do + { + "company_name" => "demo company", + "warehouse_name" => "warehouse", + "serial_number" => "SN-4567890", + "manufactured_at" => "head office", + "under_warranty" => "true", + "delivered_by" => "FedEx", + "product_type" => "fragile" # Exceeds 6 keys + } + end + + let(:valid_metadata_keys) do + { + "company_name" => "demo company", + "warehouse_name" => "warehouse", + "serial_number" => "SN-4567890", + "manufactured_at" => "head office", + "under_warranty" => "true", + "delivered_by" => "FedEx" + } + end + + let(:oversized_value_metadata) { { "product_details" => "This is an amazing product built to last long" * 10 } } # Exceeds 256 characters + let(:valid_value_metadata) { { "product_details" => "This is an amazing product built to last long" } } + let(:oversized_key_metadata) { { "company_details_for_products" => 'This is made by demo company' } } # Exceeds 16 characters + let(:valid_key_metadata) { { "company_details" => 'This is made by demo company' } } + + subject { create(:product) } + + %w[public_metadata private_metadata].each do |metadata_type| + describe metadata_type.to_s do + it "does not allow more than 6 keys" do + subject.send("#{metadata_type}=", invalid_metadata_keys) + + expect(subject).not_to be_valid + expect(subject.errors[metadata_type.to_sym]).to include("must not have more than 6 keys") + end + + it "allow less than 6 keys" do + subject.send("#{metadata_type}=", valid_metadata_keys) + + expect(subject).to be_valid + end + + it "does not allow values longer than 256 characters" do + subject.send("#{metadata_type}=", oversized_value_metadata) + + expect(subject).not_to be_valid + expect(subject.errors[metadata_type.to_sym]).to include("value for key 'product_details' exceeds 256 characters") + end + + it "allow values shorter than 256 characters" do + subject.send("#{metadata_type}=", valid_value_metadata) + + expect(subject).to be_valid + end + + it "does not allow keys longer than 16 characters" do + subject.send("#{metadata_type}=", oversized_key_metadata) + + expect(subject).not_to be_valid + expect(subject.errors[metadata_type.to_sym]).to include("key 'company_details_for_products' exceeds 16 characters") + end + + it "allow keys shorter than 16 characters" do + subject.send("#{metadata_type}=", valid_key_metadata) + + expect(subject).to be_valid + end + end + end + end end From bc513cd64929cf90f29fc27810a1143da1fa940d Mon Sep 17 00:00:00 2001 From: Mayur Date: Tue, 26 Nov 2024 00:56:25 +0530 Subject: [PATCH 12/14] Added metadata validation keys to config --- core/app/models/concerns/spree/metadata.rb | 54 +++++++++++-------- core/lib/spree/app_configuration.rb | 12 +++++ core/spec/lib/spree/app_configuration_spec.rb | 18 +++++++ 3 files changed, 62 insertions(+), 22 deletions(-) diff --git a/core/app/models/concerns/spree/metadata.rb b/core/app/models/concerns/spree/metadata.rb index 245b72aa81e..44cf7a88f99 100644 --- a/core/app/models/concerns/spree/metadata.rb +++ b/core/app/models/concerns/spree/metadata.rb @@ -4,10 +4,6 @@ module Spree module Metadata extend ActiveSupport::Concern - MAX_KEYS = 6 - MAX_KEY_LENGTH = 16 - MAX_VALUE_LENGTH = 256 - included do store :public_metadata, coder: JSON store :private_metadata, coder: JSON @@ -18,25 +14,39 @@ module Metadata private def validate_metadata_limits - %i[public_metadata private_metadata].each do |column| - metadata = send(column) - - # Check for maximum number of keys - if metadata.keys.count > MAX_KEYS - errors.add(column, "must not have more than #{MAX_KEYS} keys") - end - - # Check for maximum key and value size - metadata.each do |key, value| - if key.to_s.length > MAX_KEY_LENGTH - errors.add(column, "key '#{key}' exceeds #{MAX_KEY_LENGTH} characters") - end - - if value.to_s.length > MAX_VALUE_LENGTH - errors.add(column, "value for key '#{key}' exceeds #{MAX_VALUE_LENGTH} characters") - end - end + %i[public_metadata private_metadata].each { |column| validate_metadata_column(column) } + end + + def validate_metadata_column(column) + config = Spree::Config + metadata = send(column) + + # Check for maximum number of keys + validate_metadata_keys_count(metadata, column, config.max_keys) + + # Check for maximum key and value size + metadata.each do |key, value| + validate_metadata_key(key, column, config.max_key_length) + validate_metadata_value(key, value, column, config.max_value_length) end end + + def validate_metadata_keys_count(metadata, column, max_keys) + return unless metadata.keys.count > max_keys + + errors.add(column, "must not have more than #{max_keys} keys") + end + + def validate_metadata_key(key, column, max_key_length) + return unless key.to_s.length > max_key_length + + errors.add(column, "key '#{key}' exceeds #{max_key_length} characters") + end + + def validate_metadata_value(key, value, column, max_value_length) + return unless value.to_s.length > max_value_length + + errors.add(column, "value for key '#{key}' exceeds #{max_value_length} characters") + end end end diff --git a/core/lib/spree/app_configuration.rb b/core/lib/spree/app_configuration.rb index 8b0223eeb59..da6cc301b3f 100644 --- a/core/lib/spree/app_configuration.rb +++ b/core/lib/spree/app_configuration.rb @@ -217,6 +217,18 @@ class AppConfiguration < Preferences::Configuration # @return [Integer] Products to show per-page in the frontend (default: +12+) preference :products_per_page, :integer, default: 12 + # @!attribute [rw] max_keys + # @return [Integer] Maximum keys that can be allocated in public and private metadata column(default: +6+) + preference :max_keys, :integer, default: 6 + + # @!attribute [rw] max_key_length + # @return [Integer] Maximum length that key can have in public and private metadata column(default: +16+) + preference :max_key_length, :integer, default: 16 + + # @!attribute [rw] max_value_length + # @return [Integer] Maximum length that value can have in public and private metadata column(default: +256+) + preference :max_value_length, :integer, default: 256 + # @!attribute [rw] require_master_price # @return [Boolean] Require a price on the master variant of a product (default: +true+) preference :require_master_price, :boolean, default: true diff --git a/core/spec/lib/spree/app_configuration_spec.rb b/core/spec/lib/spree/app_configuration_spec.rb index 5eea3ea2b3b..5910c12a319 100644 --- a/core/spec/lib/spree/app_configuration_spec.rb +++ b/core/spec/lib/spree/app_configuration_spec.rb @@ -132,6 +132,24 @@ end end + describe '@max_keys' do + it 'is 6 by default' do + expect(prefs[:max_keys]).to eq(6) + end + end + + describe '@max_key_length' do + it 'is 16 by default' do + expect(prefs[:max_key_length]).to eq(16) + end + end + + describe '@max_value_length' do + it 'is 256 by default' do + expect(prefs[:max_value_length]).to eq(256) + end + end + describe '#environment' do class DummyClass; end; From a4b2b150772237e73e3375232d14622ef77e097f Mon Sep 17 00:00:00 2001 From: Mayur Date: Tue, 26 Nov 2024 00:59:22 +0530 Subject: [PATCH 13/14] Restricted private metadata and fixed few test cases --- .../controllers/spree/api/base_controller.rb | 11 +++++++++++ api/app/helpers/spree/api/api_helpers.rb | 8 ++++++++ api/lib/spree/api_configuration.rb | 2 +- .../spree/api/customer_returns_spec.rb | 2 +- .../requests/spree/api/line_items_spec.rb | 2 +- api/spec/requests/spree/api/payments_spec.rb | 2 +- api/spec/requests/spree/api/products_spec.rb | 19 ++++++++++++++++++- .../spree/api/return_authorizations_spec.rb | 2 +- api/spec/requests/spree/api/stores_spec.rb | 6 ++++++ .../requests/spree/api/taxonomies_spec.rb | 2 +- api/spec/requests/spree/api/taxons_spec.rb | 2 +- api/spec/requests/spree/api/users_spec.rb | 2 +- core/config/locales/en.yml | 2 +- .../controller_helpers/strong_parameters.rb | 6 +++--- core/lib/spree/permitted_attributes.rb | 5 ++++- 15 files changed, 59 insertions(+), 14 deletions(-) diff --git a/api/app/controllers/spree/api/base_controller.rb b/api/app/controllers/spree/api/base_controller.rb index ec48d666e52..9352eaf2a2c 100644 --- a/api/app/controllers/spree/api/base_controller.rb +++ b/api/app/controllers/spree/api/base_controller.rb @@ -18,6 +18,9 @@ class BaseController < ActionController::Base class_attribute :admin_line_item_attributes self.admin_line_item_attributes = [:price, :variant_id, :sku] + class_attribute :private_metadata_attributes + self.private_metadata_attributes = [{ private_metadata: {} }] + attr_accessor :current_api_user before_action :load_user @@ -44,6 +47,14 @@ def permitted_line_item_attributes end end + def permitted_product_attributes + if can?(:admin, Spree::Product) + super + private_metadata_attributes + else + super + end + end + def load_user @current_api_user ||= Spree.user_class.find_by(spree_api_key: api_key.to_s) end diff --git a/api/app/helpers/spree/api/api_helpers.rb b/api/app/helpers/spree/api/api_helpers.rb index 8c52db2d772..1c1c3cb6b42 100644 --- a/api/app/helpers/spree/api/api_helpers.rb +++ b/api/app/helpers/spree/api/api_helpers.rb @@ -64,6 +64,14 @@ def variant_attributes end end + def product_attributes + if can?(:admin, Spree::Product) + Spree::Api::Config.product_attributes + [:private_metadata] + else + Spree::Api::Config.product_attributes + end + end + def total_on_hand_for(object) object.total_on_hand.finite? ? object.total_on_hand : nil end diff --git a/api/lib/spree/api_configuration.rb b/api/lib/spree/api_configuration.rb index cd530cd8228..6efac5e7a5c 100644 --- a/api/lib/spree/api_configuration.rb +++ b/api/lib/spree/api_configuration.rb @@ -7,7 +7,7 @@ class ApiConfiguration < Preferences::Configuration preference :product_attributes, :array, default: [ :id, :name, :description, :available_on, :slug, :meta_description, :meta_keywords, :shipping_category_id, - :taxon_ids, :total_on_hand, :meta_title, :private_metadata, :public_metadata + :taxon_ids, :total_on_hand, :meta_title, :public_metadata ] preference :product_property_attributes, :array, default: [:id, :product_id, :property_id, :value, :property_name] diff --git a/api/spec/requests/spree/api/customer_returns_spec.rb b/api/spec/requests/spree/api/customer_returns_spec.rb index 2da3f9a4c35..409ab2820bb 100644 --- a/api/spec/requests/spree/api/customer_returns_spec.rb +++ b/api/spec/requests/spree/api/customer_returns_spec.rb @@ -97,7 +97,7 @@ module Spree::Api it "can learn how to create a new customer return" do get spree.new_api_order_customer_return_path(order) - expect(json_response["attributes"]).to eq(["id", "number", "stock_location_id", "created_at", "updated_at"]) + expect(json_response["attributes"]).to eq(["id", "number", "stock_location_id", "created_at", "updated_at", "private_metadata", "public_metadata"]) end it "can update a customer return" do diff --git a/api/spec/requests/spree/api/line_items_spec.rb b/api/spec/requests/spree/api/line_items_spec.rb index b9f6e03bacb..df8558b8d41 100644 --- a/api/spec/requests/spree/api/line_items_spec.rb +++ b/api/spec/requests/spree/api/line_items_spec.rb @@ -24,7 +24,7 @@ module Spree::Api it "can learn how to create a new line item" do get spree.new_api_order_line_item_path(order) - expect(json_response["attributes"]).to eq(["quantity", "price", "variant_id"]) + expect(json_response["attributes"]).to eq(["quantity", "price", "variant_id", "private_metadata", "public_metadata"]) required_attributes = json_response["required_attributes"] expect(required_attributes).to include("quantity", "variant_id") end diff --git a/api/spec/requests/spree/api/payments_spec.rb b/api/spec/requests/spree/api/payments_spec.rb index 4cf4296469a..c305dd1a98a 100644 --- a/api/spec/requests/spree/api/payments_spec.rb +++ b/api/spec/requests/spree/api/payments_spec.rb @@ -9,7 +9,7 @@ module Spree::Api let!(:attributes) { [:id, :source_type, :source_id, :amount, :display_amount, :payment_method_id, :state, :avs_response, - :created_at, :updated_at] + :created_at, :updated_at, :private_metadata, :public_metadata] } before do diff --git a/api/spec/requests/spree/api/products_spec.rb b/api/spec/requests/spree/api/products_spec.rb index 6554f419f1f..d7c47311304 100644 --- a/api/spec/requests/spree/api/products_spec.rb +++ b/api/spec/requests/spree/api/products_spec.rb @@ -49,6 +49,11 @@ module Spree::Api end end + it "cannot see private_metadata" do + get spree.api_product_path(product) + expect(json_response).not_to have_key('private_metadata') + end + it "retrieves a list of products" do get spree.api_products_path expect(json_response["products"].first).to have_attributes(show_attributes) @@ -242,7 +247,9 @@ module Spree::Api post spree.api_products_path, params: { product: { name: "The Other Product", price: 19.99, - shipping_category_id: create(:shipping_category).id } + shipping_category_id: create(:shipping_category).id, + public_metadata: { 'Company' => 'Sample Company'}, + private_metadata: { 'Serial_number' => 'Sn12345'} } } expect(json_response).to have_attributes(base_attributes) expect(response.status).to eq(201) @@ -351,6 +358,11 @@ module Spree::Api expect(response.status).to eq(200) end + it "can update a product private_metadta" do + put spree.api_product_path(product), params: { product: { private_metadata: {'product_number' => 'PN345678'} } } + expect(response.status).to eq(200) + end + it "can create new option types on a product" do put spree.api_product_path(product), params: { product: { option_types: ['shape', 'color'] } } expect(json_response['option_types'].count).to eq(2) @@ -417,6 +429,11 @@ module Spree::Api expect(response.status).to eq(204) expect(product.reload.deleted_at).not_to be_nil end + + it "can see private_metadata" do + get spree.api_product_path(product) + expect(json_response).to have_key('private_metadata') + end end end end diff --git a/api/spec/requests/spree/api/return_authorizations_spec.rb b/api/spec/requests/spree/api/return_authorizations_spec.rb index 43610d5e2ba..600b7c1bb53 100644 --- a/api/spec/requests/spree/api/return_authorizations_spec.rb +++ b/api/spec/requests/spree/api/return_authorizations_spec.rb @@ -121,7 +121,7 @@ module Spree::Api it "can learn how to create a new return authorization" do get spree.new_api_order_return_authorization_path(order) - expect(json_response["attributes"]).to eq(["id", "number", "state", "order_id", "memo", "created_at", "updated_at"]) + expect(json_response["attributes"]).to eq(["id", "number", "state", "order_id", "memo", "created_at", "updated_at", "private_metadata", "public_metadata"]) required_attributes = json_response["required_attributes"] expect(required_attributes).to include("order") end diff --git a/api/spec/requests/spree/api/stores_spec.rb b/api/spec/requests/spree/api/stores_spec.rb index 55b4f102416..6f6099e1a5e 100644 --- a/api/spec/requests/spree/api/stores_spec.rb +++ b/api/spec/requests/spree/api/stores_spec.rb @@ -35,6 +35,8 @@ module Spree::Api "mail_from_address" => "solidus@example.org", "bcc_email" => nil, "default_currency" => nil, + "private_metadata"=>{}, + "public_metadata"=>{}, "code" => store.code, "default" => true, "available_locales" => ["en"] @@ -49,6 +51,8 @@ module Spree::Api "mail_from_address" => "solidus@example.org", "bcc_email" => nil, "default_currency" => nil, + "private_metadata"=>{}, + "public_metadata"=>{}, "code" => non_default_store.code, "default" => false, "available_locales" => ["en"] @@ -67,6 +71,8 @@ module Spree::Api "seo_title" => nil, "mail_from_address" => "solidus@example.org", "bcc_email" => nil, + "private_metadata"=>{}, + "public_metadata"=>{}, "default_currency" => nil, "code" => store.code, "default" => true, diff --git a/api/spec/requests/spree/api/taxonomies_spec.rb b/api/spec/requests/spree/api/taxonomies_spec.rb index ff927b5b215..c725b6f6fe9 100644 --- a/api/spec/requests/spree/api/taxonomies_spec.rb +++ b/api/spec/requests/spree/api/taxonomies_spec.rb @@ -7,7 +7,7 @@ module Spree::Api let(:taxonomy) { create(:taxonomy) } let(:taxon) { create(:taxon, name: "Ruby", taxonomy:) } let(:taxon2) { create(:taxon, name: "Rails", taxonomy:) } - let(:attributes) { [:id, :name] } + let(:attributes) { [:id, :name, :private_metadata, :public_metadata] } before do stub_authentication! diff --git a/api/spec/requests/spree/api/taxons_spec.rb b/api/spec/requests/spree/api/taxons_spec.rb index d93ff52e549..abc5c44f644 100644 --- a/api/spec/requests/spree/api/taxons_spec.rb +++ b/api/spec/requests/spree/api/taxons_spec.rb @@ -8,7 +8,7 @@ module Spree::Api let!(:taxon) { create(:taxon, name: "Ruby", parent: taxonomy.root, taxonomy:) } let!(:taxon2) { create(:taxon, name: "Rails", parent: taxon, taxonomy:) } let!(:rails_v3_2_2) { create(:taxon, name: "3.2.2", parent: taxon2, taxonomy:) } - let(:attributes) { ["id", "name", "pretty_name", "permalink", "parent_id", "taxonomy_id"] } + let(:attributes) { ["id", "name", "pretty_name", "permalink", "parent_id", "taxonomy_id", "private_metadata", "public_metadata"] } before do stub_authentication! diff --git a/api/spec/requests/spree/api/users_spec.rb b/api/spec/requests/spree/api/users_spec.rb index 7b0d2d9d7b1..fa7303df132 100644 --- a/api/spec/requests/spree/api/users_spec.rb +++ b/api/spec/requests/spree/api/users_spec.rb @@ -6,7 +6,7 @@ module Spree::Api describe 'Users', type: :request do let(:user) { create(:user, spree_api_key: SecureRandom.hex) } let(:stranger) { create(:user, email: 'stranger@example.com') } - let(:attributes) { [:id, :email, :created_at, :updated_at] } + let(:attributes) { [:id, :email, :created_at, :updated_at, :private_metadata, :public_metadata] } context "as a normal user" do it "can get own details" do diff --git a/core/config/locales/en.yml b/core/config/locales/en.yml index a6d84078083..b82691d5241 100644 --- a/core/config/locales/en.yml +++ b/core/config/locales/en.yml @@ -180,8 +180,8 @@ en: on_hand: On Hand price: Master Price private_metadata: Private Metadata - public_metadata: Public Metadata promotionable: Promotable + public_metadata: Public Metadata shipping_category: Shipping Category sku: Master SKU slug: Slug diff --git a/core/lib/spree/core/controller_helpers/strong_parameters.rb b/core/lib/spree/core/controller_helpers/strong_parameters.rb index 25df2772db4..5e6349f59d7 100644 --- a/core/lib/spree/core/controller_helpers/strong_parameters.rb +++ b/core/lib/spree/core/controller_helpers/strong_parameters.rb @@ -56,9 +56,9 @@ def permitted_order_attributes end def permitted_product_attributes - permitted_attributes.product_attributes + metadata_attributes + [ - product_properties_attributes: permitted_product_properties_attributes - ] + permitted_attributes.product_attributes + + permitted_attributes.public_metadata_attributes + + [product_properties_attributes: permitted_product_properties_attributes] end def metadata_attributes diff --git a/core/lib/spree/permitted_attributes.rb b/core/lib/spree/permitted_attributes.rb index c6bf7e2cb4d..7d1685d0a3a 100644 --- a/core/lib/spree/permitted_attributes.rb +++ b/core/lib/spree/permitted_attributes.rb @@ -34,7 +34,8 @@ module PermittedAttributes :taxonomy_attributes, :user_attributes, :variant_attributes, - :metadata_attributes + :metadata_attributes, + :public_metadata_attributes ] mattr_reader(*ATTRIBUTES) @@ -84,6 +85,8 @@ module PermittedAttributes @@metadata_attributes = [private_metadata: {}, public_metadata: {}] + @@public_metadata_attributes = [public_metadata: {}] + @@property_attributes = [:name, :presentation] @@return_authorization_attributes = [:memo, :stock_location_id, :return_reason_id, return_items_attributes: [:inventory_unit_id, :exchange_variant_id, :return_reason_id, :preferred_reimbursement_type_id]] From e023e917a922651ee5a89c8f93de82015bdbdc53 Mon Sep 17 00:00:00 2001 From: Mayur Date: Tue, 26 Nov 2024 12:44:07 +0530 Subject: [PATCH 14/14] Added test case for private metadata restriction --- api/spec/requests/spree/api/products_spec.rb | 52 ++++++++++++++++++-- 1 file changed, 49 insertions(+), 3 deletions(-) diff --git a/api/spec/requests/spree/api/products_spec.rb b/api/spec/requests/spree/api/products_spec.rb index d7c47311304..ba7bfd6de14 100644 --- a/api/spec/requests/spree/api/products_spec.rb +++ b/api/spec/requests/spree/api/products_spec.rb @@ -211,6 +211,52 @@ module Spree::Api it_behaves_like "modifying product actions are restricted" end + context "when the user is not admin but has ability to create and update products" do + # Define custom authorization to grant permissions + custom_authorization! do |_| + can [:create, :update], Spree::Product + end + + let(:product_data_with_private_metadata) do + { + name: "The Other Product", + price: 19.99, + shipping_category_id: create(:shipping_category).id, + public_metadata: { 'Company' => 'Sample Company' }, + private_metadata: { 'Serial_number' => 'Sn12345' } + } + end + + let(:product_update_data_with_private_metadata) do + { + name: "Updated Product", + price: 29.99, + private_metadata: { 'Serial_number' => 'Sn98765' } + } + end + + it "allows creating products with public metadata but not private metadata" do + post spree.api_products_path, params: { product: product_data_with_private_metadata } + + expect(json_response['public_metadata']).to eq({ "Company" => "Sample Company" }) + expect(json_response).not_to have_key('private_metadata') + + created_product = Spree::Product.last + expect(created_product.private_metadata).to eq({}) + end + + it "allows updating products but ignores private metadata" do + product = create(:product) + + put spree.api_product_path(product), params: { product: product_update_data_with_private_metadata } + + expect(json_response['name']).to eq( "Updated Product" ) + expect(json_response).not_to have_key('private_metadata') + product.reload + expect(product.private_metadata).to eq({}) + end + end + context "as an admin" do let(:taxon_1) { create(:taxon) } let(:taxon_2) { create(:taxon) } @@ -248,8 +294,8 @@ module Spree::Api product: { name: "The Other Product", price: 19.99, shipping_category_id: create(:shipping_category).id, - public_metadata: { 'Company' => 'Sample Company'}, - private_metadata: { 'Serial_number' => 'Sn12345'} } + public_metadata: { 'Company' => 'Sample Company' }, + private_metadata: { 'Serial_number' => 'Sn12345' } } } expect(json_response).to have_attributes(base_attributes) expect(response.status).to eq(201) @@ -359,7 +405,7 @@ module Spree::Api end it "can update a product private_metadta" do - put spree.api_product_path(product), params: { product: { private_metadata: {'product_number' => 'PN345678'} } } + put spree.api_product_path(product), params: { product: { private_metadata: { 'product_number' => 'PN345678' } } } expect(response.status).to eq(200) end