From cec3053c4ed13ad2e86fd5aff56df4ca9d95fd39 Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Mon, 17 Feb 2025 19:22:20 +0530 Subject: [PATCH 1/8] Add metadata columns for resources This migration adds customer_metadata and admin_metadata to various tables --- ...9061658_add_metadata_to_spree_resources.rb | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 core/db/migrate/20250129061658_add_metadata_to_spree_resources.rb diff --git a/core/db/migrate/20250129061658_add_metadata_to_spree_resources.rb b/core/db/migrate/20250129061658_add_metadata_to_spree_resources.rb new file mode 100644 index 00000000000..b0cddbf2153 --- /dev/null +++ b/core/db/migrate/20250129061658_add_metadata_to_spree_resources.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class AddMetadataToSpreeResources < ActiveRecord::Migration[7.2] + def change + # List of Resources to add metadata columns to + %i[ + spree_orders + spree_line_items + spree_shipments + spree_payments + spree_refunds + spree_customer_returns + spree_store_credit_events + spree_users + spree_return_authorizations + ].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, :customer_metadata, :jsonb, default: {} + add_column table_name, :admin_metadata, :jsonb, default: {} + else + add_column table_name, :customer_metadata, :json + add_column table_name, :admin_metadata, :json + end + end + end + end +end From 419c96240bf93ffd9ddc19e3bde2e69cbb4397b9 Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Tue, 4 Feb 2025 12:09:31 +0530 Subject: [PATCH 2/8] Add concern and validations for metadata with config options for keys and length Introduced validations and configurable limits for metadata attributes (`customer_metadata` and `admin_metadata`) to ensure consistency and prevent overly large or complex structures. **Implementation:** - Set default values for `customer_metadata` and `admin_metadata` to empty hashes (`{}`) for proper initialization. - Implemented validation to limit the number of keys, and the length of keys and values in metadata. - Added configuration checks to ensure metadata does not exceed the limits defined in `Spree::Config`. - Introduced preferences: `meta_data_max_keys`, `meta_data_max_key_length`, and `meta_data_max_value_length` to control metadata size: - `meta_data_max_keys`: Max keys allowed (default: 6). - `meta_data_max_key_length`: Max key length (default: 16). - `meta_data_max_value_length`: Max value length (default: 256). - Created a shared concern to handle metadata validation logic. - Added tests to ensure default values and validation logic work correctly. - Created a shared example that can be used in other specs to ensure consistent testing of metadata validation across various parts of the system. --- core/app/models/concerns/spree/metadata.rb | 60 ++++++++++++ core/lib/spree/app_configuration.rb | 13 +++ core/spec/lib/spree/app_configuration_spec.rb | 18 ++++ core/spec/support/shared_examples/metadata.rb | 96 +++++++++++++++++++ 4 files changed, 187 insertions(+) create mode 100644 core/app/models/concerns/spree/metadata.rb create mode 100644 core/spec/support/shared_examples/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..6f7c2d7185d --- /dev/null +++ b/core/app/models/concerns/spree/metadata.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module Spree + module Metadata + extend ActiveSupport::Concern + + included do + attribute :customer_metadata, :json, default: {} + attribute :admin_metadata, :json, default: {} + + validate :validate_metadata_limits + end + + class_methods do + def meta_data_columns + %i[customer_metadata admin_metadata] + end + end + + private + + def validate_metadata_limits + self.class.meta_data_columns.each { |column| validate_metadata_column(column) } + end + + def validate_metadata_column(column) + config = Spree::Config + metadata = send(column) + + return if metadata.nil? + + # Check for maximum number of keys + validate_metadata_keys_count(metadata, column, config.meta_data_max_keys) + + # Check for maximum key and value size + metadata.each do |key, value| + validate_metadata_key(key, column, config.meta_data_max_key_length) + validate_metadata_value(key, value, column, config.meta_data_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 8312802683e..182bbec4a3b 100644 --- a/core/lib/spree/app_configuration.rb +++ b/core/lib/spree/app_configuration.rb @@ -216,6 +216,19 @@ class AppConfiguration < Preferences::Configuration # @return [Integer] Orders to show per-page in the admin (default: +15+) preference :orders_per_page, :integer, default: 15 + + # @!attribute [rw] meta_data_max_keys + # @return [Integer] Maximum keys that can be allocated in customer and admin metadata column (default: +6+) + preference :meta_data_max_keys, :integer, default: 6 + + # @!attribute [rw] meta_data_max_key_length + # @return [Integer] Maximum length that key can have in customer and admin metadata column (default: +16+) + preference :meta_data_max_key_length, :integer, default: 16 + + # @!attribute [rw] meta_data_max_value_length + # @return [Integer] Maximum length that value can have in customer and admin metadata column (default: +256+) + preference :meta_data_max_value_length, :integer, default: 256 + # @!attribute [rw] properties_per_page # @return [Integer] Properties to show per-page in the admin (default: +15+) preference :properties_per_page, :integer, default: 15 diff --git a/core/spec/lib/spree/app_configuration_spec.rb b/core/spec/lib/spree/app_configuration_spec.rb index 5eea3ea2b3b..67f5220773c 100644 --- a/core/spec/lib/spree/app_configuration_spec.rb +++ b/core/spec/lib/spree/app_configuration_spec.rb @@ -222,4 +222,22 @@ class DummyClass; end; expect(prefs.admin_vat_location.state_id).to eq(nil) expect(prefs.admin_vat_location.country_id).to eq(nil) end + + describe '@meta_data_max_keys' do + it 'is 6 by default' do + expect(prefs[:meta_data_max_keys]).to eq(6) + end + end + + describe '@meta_data_max_key_length' do + it 'is 16 by default' do + expect(prefs[:meta_data_max_key_length]).to eq(16) + end + end + + describe '@meta_data_max_value_length' do + it 'is 256 by default' do + expect(prefs[:meta_data_max_value_length]).to eq(256) + end + end end diff --git a/core/spec/support/shared_examples/metadata.rb b/core/spec/support/shared_examples/metadata.rb new file mode 100644 index 00000000000..f2922243240 --- /dev/null +++ b/core/spec/support/shared_examples/metadata.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +RSpec.shared_examples "customer and admin metadata fields: storage and validation" do |metadata_class| + let(:object) { build(metadata_class) } + + it "responds to customer_metadata" do + expect(object).to respond_to(:customer_metadata) + end + + it "responds to admin_metadata" do + expect(object).to respond_to(:admin_metadata) + end + + it "can store data in customer_metadata" do + object.customer_metadata = { "order_id" => "OD34236" } + expect(object.customer_metadata["order_id"]).to eq("OD34236") + end + + it "can store data in admin_metadata" do + object.admin_metadata = { "internal_note" => "Generate invoice after payment is received" } + expect(object.admin_metadata["internal_note"]).to eq("Generate invoice after payment is received") + end + + 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(metadata_class) } + + %w[customer_metadata admin_metadata].each do |metadata_type| + describe metadata_type 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 "allows 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 "allows 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 "allows keys shorter than 16 characters" do + subject.send("#{metadata_type}=", valid_key_metadata) + + expect(subject).to be_valid + end + end + end +end From cc5b63cc89f43db99a49df63a6db19c9a3a6ad24 Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Tue, 4 Feb 2025 12:12:07 +0530 Subject: [PATCH 3/8] Include concern to models Call the metadata concern to various models to validate and add admin and customer metadata --- core/app/models/spree/customer_return.rb | 2 ++ core/app/models/spree/legacy_user.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/return_authorization.rb | 2 ++ core/app/models/spree/shipment.rb | 2 ++ core/app/models/spree/store_credit_event.rb | 1 + core/spec/models/spree/customer_return_spec.rb | 2 ++ core/spec/models/spree/line_item_spec.rb | 2 ++ core/spec/models/spree/order_spec.rb | 2 ++ core/spec/models/spree/payment_spec.rb | 2 ++ core/spec/models/spree/refund_spec.rb | 2 ++ core/spec/models/spree/return_authorization_spec.rb | 2 ++ core/spec/models/spree/shipment_spec.rb | 2 ++ core/spec/models/spree/store_credit_event_spec.rb | 2 ++ core/spec/models/spree/user_spec.rb | 2 ++ 18 files changed, 32 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/legacy_user.rb b/core/app/models/spree/legacy_user.rb index e01907c996e..806f702ea7f 100644 --- a/core/app/models/spree/legacy_user.rb +++ b/core/app/models/spree/legacy_user.rb @@ -7,6 +7,7 @@ module Spree # spree_auth_devise) class LegacyUser < Spree::Base include UserMethods + include Metadata self.table_name = 'spree_users' diff --git a/core/app/models/spree/line_item.rb b/core/app/models/spree/line_item.rb index 75845f87f3f..a1b82f6d531 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 0eda7642465..40cae36501c 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/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/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/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/spec/models/spree/customer_return_spec.rb b/core/spec/models/spree/customer_return_spec.rb index da31b8dfcf7..7ec5da90e95 100644 --- a/core/spec/models/spree/customer_return_spec.rb +++ b/core/spec/models/spree/customer_return_spec.rb @@ -303,4 +303,6 @@ end end end + + it_behaves_like "customer and admin metadata fields: storage and validation", :customer_return end diff --git a/core/spec/models/spree/line_item_spec.rb b/core/spec/models/spree/line_item_spec.rb index 6cd9517ca7f..b73c4bef87d 100644 --- a/core/spec/models/spree/line_item_spec.rb +++ b/core/spec/models/spree/line_item_spec.rb @@ -189,4 +189,6 @@ expect(subject.currency).to eq("USD") end end + + it_behaves_like "customer and admin metadata fields: storage and validation", :line_item end diff --git a/core/spec/models/spree/order_spec.rb b/core/spec/models/spree/order_spec.rb index 275436fc887..54b79f08c5f 100644 --- a/core/spec/models/spree/order_spec.rb +++ b/core/spec/models/spree/order_spec.rb @@ -2145,4 +2145,6 @@ def validate(line_item) it { is_expected.to eq(true) } end end + + it_behaves_like "customer and admin metadata fields: storage and validation", :order end diff --git a/core/spec/models/spree/payment_spec.rb b/core/spec/models/spree/payment_spec.rb index c3d12681e5b..0cec1faa314 100644 --- a/core/spec/models/spree/payment_spec.rb +++ b/core/spec/models/spree/payment_spec.rb @@ -1331,4 +1331,6 @@ expect(described_class.valid).to be_empty end end + + it_behaves_like "customer and admin metadata fields: storage and validation", :payment end diff --git a/core/spec/models/spree/refund_spec.rb b/core/spec/models/spree/refund_spec.rb index 3f230326caa..e10755a1eab 100644 --- a/core/spec/models/spree/refund_spec.rb +++ b/core/spec/models/spree/refund_spec.rb @@ -237,4 +237,6 @@ end end end + + it_behaves_like "customer and admin metadata fields: storage and validation", :refund end diff --git a/core/spec/models/spree/return_authorization_spec.rb b/core/spec/models/spree/return_authorization_spec.rb index 39fb9091e2c..7716eb71cad 100644 --- a/core/spec/models/spree/return_authorization_spec.rb +++ b/core/spec/models/spree/return_authorization_spec.rb @@ -223,4 +223,6 @@ it { is_expected.to eq true } end end + + it_behaves_like "customer and admin metadata fields: storage and validation", :return_authorization end diff --git a/core/spec/models/spree/shipment_spec.rb b/core/spec/models/spree/shipment_spec.rb index 384413166d6..45dfad69891 100644 --- a/core/spec/models/spree/shipment_spec.rb +++ b/core/spec/models/spree/shipment_spec.rb @@ -872,4 +872,6 @@ it { is_expected.to include carton } end + + it_behaves_like "customer and admin metadata fields: storage and validation", :shipment 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..0463dd65236 100644 --- a/core/spec/models/spree/store_credit_event_spec.rb +++ b/core/spec/models/spree/store_credit_event_spec.rb @@ -325,4 +325,6 @@ end end end + + it_behaves_like "customer and admin metadata fields: storage and validation", :store_credit_event end diff --git a/core/spec/models/spree/user_spec.rb b/core/spec/models/spree/user_spec.rb index 159f6284dcb..3b8b7dc6496 100644 --- a/core/spec/models/spree/user_spec.rb +++ b/core/spec/models/spree/user_spec.rb @@ -87,6 +87,8 @@ end end end + + it_behaves_like "customer and admin metadata fields: storage and validation", :user end RSpec.describe Spree.user_class, type: :model do From a6a4998593ec36dc881f1449787f350e5157a1e7 Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Tue, 18 Feb 2025 12:12:34 +0530 Subject: [PATCH 4/8] Permit `customer_metadata` attributes for APIs and add support for metadata handling Update the API configuration to permit the `customer_metadata` attribute across various models, such as orders, payments, shipments, and return authorizations. This change allows better handling of customer metadata within API requests, ensuring consistent storage and validation of metadata attributes. Additionally, it updates the `preference` configurations for models to include `customer_metadata`. **Implementation:** - Added `customer_metadata` to the permitted attributes for several API resources. - Updated various `preference` settings, including `order_attributes`, `payment_attributes`, `shipment_attributes`, etc., to include `customer_metadata`. - Ensured that `customer_metadata` is correctly permitted in the API parameters for each relevant model, allowing admins and users to store custom metadata. - This change improves the flexibility of metadata management within the API, enabling dynamic handling and easier customizations. --- api/lib/spree/api_configuration.rb | 16 +++--- .../requests/spree/api/line_items_spec.rb | 41 +++++++++++++- api/spec/requests/spree/api/orders_spec.rb | 53 ++++++++++++++++++- api/spec/requests/spree/api/payments_spec.rb | 23 +++++++- api/spec/requests/spree/api/users_spec.rb | 28 +++++++++- .../controller_helpers/strong_parameters.rb | 1 + core/lib/spree/permitted_attributes.rb | 23 +++++--- 7 files changed, 167 insertions(+), 18 deletions(-) diff --git a/api/lib/spree/api_configuration.rb b/api/lib/spree/api_configuration.rb index 9ce561e23c1..5c1dd75b096 100644 --- a/api/lib/spree/api_configuration.rb +++ b/api/lib/spree/api_configuration.rb @@ -36,22 +36,22 @@ 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, :customer_metadata ] - preference :line_item_attributes, :array, default: [:id, :quantity, :price, :variant_id] + preference :line_item_attributes, :array, default: [:id, :quantity, :price, :variant_id, :customer_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, :customer_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, :customer_metadata] preference :taxonomy_attributes, :array, default: [:id, :name] @@ -81,11 +81,11 @@ 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, :customer_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, :customer_metadata ] preference :creditcard_attributes, :array, default: [ @@ -96,7 +96,7 @@ 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, :customer_metadata] preference :property_attributes, :array, default: [:id, :name, :presentation] @@ -132,7 +132,7 @@ def promotion_attributes=(value) 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, :customer_metadata ] preference :variant_property_attributes, :array, default: [ diff --git a/api/spec/requests/spree/api/line_items_spec.rb b/api/spec/requests/spree/api/line_items_spec.rb index b9f6e03bacb..66e230adcad 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", "customer_metadata"]) required_attributes = json_response["required_attributes"] expect(required_attributes).to include("quantity", "variant_id") end @@ -95,6 +95,29 @@ module Spree::Api expect(response.status).to eq(201) end + it "cannot see admin_metadata" do + post spree.api_order_line_items_path(order), + params: { + line_item: { variant_id: product.master.to_param, quantity: 1 }, + order_token: order.guest_token + } + + expect(response.status).to eq(201) + expect(json_response).not_to have_key('admin_metadata') + end + + it "allows creating line item with customer metadata but not admin metadata" do + post spree.api_order_line_items_path(order), + params: { + line_item: { variant_id: product.master.to_param, + quantity: 1, + customer_metadata: { "Company" => "Sample Company" } } + } + + expect(json_response['customer_metadata']).to eq({ "Company" => "Sample Company" }) + expect(json_response).not_to have_key('admin_metadata') + end + it '#create calls #invalid_resource! if adding a line item fails validation' do allow_any_instance_of(Spree::LineItem).to receive(:valid?).and_return(false) expect_any_instance_of(Spree::Api::BaseController).to receive(:invalid_resource!).once @@ -128,6 +151,22 @@ module Spree::Api expect(json_response["quantity"]).to eq(101) end + it "can update a line item customer metadata on the order" do + line_item = order.line_items.first + + put spree.api_order_line_item_path(order, line_item), + params: { line_item: { quantity: 101, customer_metadata: { "adding_quantity" => "true" } } } + + expect(response.status).to eq(200) + + order.reload + + expect(order.total).to eq(1010) # 10 original due to factory, + 1000 in this test + expect(json_response).to have_attributes(attributes) + expect(json_response["quantity"]).to eq(101) + expect(json_response['customer_metadata']).to eq({ "adding_quantity" => "true" }) + end + it "can update a line item's options on the order" do without_partial_double_verification do expect_any_instance_of(Spree::LineItem).to receive(:some_option=).with("foobar") diff --git a/api/spec/requests/spree/api/orders_spec.rb b/api/spec/requests/spree/api/orders_spec.rb index 1fa53c3dc9b..aff5baef373 100644 --- a/api/spec/requests/spree/api/orders_spec.rb +++ b/api/spec/requests/spree/api/orders_spec.rb @@ -14,7 +14,7 @@ module Spree::Api :user_id, :created_at, :updated_at, :completed_at, :payment_total, :shipment_state, :payment_state, :email, :special_instructions, - :total_quantity, :display_item_total, :currency] + :total_quantity, :display_item_total, :currency, :customer_metadata] } let(:address_params) { { country_id: Country.first.id, state_id: State.first.id } } @@ -79,6 +79,57 @@ module Spree::Api end end + context "when the user is not admin but has ability to create and update orders" do + custom_authorization! do |_| + can [:update, :create], Spree::Order + end + + let(:attributes_with_metadata) { + { email: "foo@foobar.com", + customer_metadata: { 'Note' => 'Do not ring the bell' }, + admin_metadata: { 'Customer_type' => 'Corporate giant' } } + } + + let(:order_update_data_with_admin_metadata){ + { + email: "new_email@update.com", + admin_metadata: { 'Serial_number' => 'Sn98765' } + } + } + + it "allows creating order with customer metadata but not admin metadata" do + post spree.api_orders_path, params: { order: attributes_with_metadata } + + expect(json_response['customer_metadata']).to eq({ 'Note' => 'Do not ring the bell' }) + expect(json_response).not_to have_key('admin_metadata') + + created_order = Spree::Order.last + + expect(created_order.admin_metadata).to eq({}) + end + + it "allows updating order but ignores admin metadata" do + order = create(:order) + + put spree.api_order_path(order), params: { order: order_update_data_with_admin_metadata } + + expect(json_response['email']).to eq( "new_email@update.com" ) + expect(json_response).not_to have_key('admin_metadata') + + order.reload + + expect(order.admin_metadata).to eq({}) + end + + it "cannot view admin_metadata" do + allow_any_instance_of(Spree::Order).to receive_messages user: current_api_user + + get spree.api_order_path(order) + + expect(json_response).not_to have_key('admin_metadata') + end + end + context "when the current user can administrate the order" do custom_authorization! do |_| can [:admin, :create], Spree::Order diff --git a/api/spec/requests/spree/api/payments_spec.rb b/api/spec/requests/spree/api/payments_spec.rb index 4cf4296469a..6f5d0ae3483 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, :customer_metadata] } before do @@ -27,6 +27,11 @@ module Spree::Api expect(json_response["payments"].first).to have_attributes(attributes) end + it "cannot view admin_metadata" do + get spree.api_order_payments_path(order) + expect(json_response).not_to have_key('admin_metadata') + end + it "can learn how to create a new payment" do get spree.new_api_order_payment_path(order) expect(json_response["attributes"]).to eq(attributes.map(&:to_s)) @@ -45,6 +50,22 @@ module Spree::Api expect(json_response).to have_attributes(attributes) end + it "allows creating payment with customer metadata but not admin metadata" do + post spree.api_order_payments_path(order), + params: { + payment: { + customer_metadata: { 'type' => 'credit card' }, + payment_method_id: Spree::PaymentMethod.first.id, + amount: 50, + source_attributes: { gateway_payment_profile_id: 1 } + } + } + + expect(response.status).to eq(201) + expect(json_response['customer_metadata']).to eq({ "type" => "credit card" }) + expect(json_response).not_to have_key('admin_metadata') + end + context "disallowed payment method" do it "does not create a new payment" do Spree::PaymentMethod.first.update!(available_to_users: false) diff --git a/api/spec/requests/spree/api/users_spec.rb b/api/spec/requests/spree/api/users_spec.rb index 7b0d2d9d7b1..c16dbeae6c7 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, :customer_metadata] } context "as a normal user" do it "can get own details" do @@ -15,6 +15,20 @@ module Spree::Api expect(json_response['email']).to eq user.email end + it "can view customer_metadata" do + get spree.api_user_path(user.id), params: { token: user.spree_api_key } + + expect(json_response['email']).to eq user.email + expect(json_response).to have_key('customer_metadata') + end + + it "cannot view admin_metadata" do + get spree.api_user_path(user.id), params: { token: user.spree_api_key } + + expect(json_response['email']).to eq user.email + expect(json_response).not_to have_key('admin_metadata') + end + it "cannot get other users details" do get spree.api_user_path(stranger.id), params: { token: user.spree_api_key } @@ -35,6 +49,18 @@ module Spree::Api expect(json_response['email']).to eq 'new@example.com' end + it "can create a new user with customer_metadata" do + user_params = { + email: 'new@example.com', password: 'spree123', password_confirmation: 'spree123', customer_metadata: { 'username' => 'newuser' } + } + + post spree.api_users_path, params: { user: user_params, token: user.spree_api_key } + + expect(json_response['email']).to eq 'new@example.com' + expect(json_response['customer_metadata']).to eq({ 'username' => 'newuser' }) + expect(json_response).not_to have_key('admin_metadata') + end + it "cannot create a new user with invalid attributes" do allow_any_instance_of(Spree::LegacyUser).to receive(:save).and_return(false) diff --git a/core/app/helpers/spree/core/controller_helpers/strong_parameters.rb b/core/app/helpers/spree/core/controller_helpers/strong_parameters.rb index fa9568c2dec..3514d06ffc3 100644 --- a/core/app/helpers/spree/core/controller_helpers/strong_parameters.rb +++ b/core/app/helpers/spree/core/controller_helpers/strong_parameters.rb @@ -50,6 +50,7 @@ def permitted_order_attributes permitted_checkout_address_attributes + permitted_checkout_delivery_attributes + permitted_checkout_payment_attributes + + permitted_attributes.customer_metadata_attributes + permitted_checkout_confirm_attributes + [ line_items_attributes: permitted_line_item_attributes ] diff --git a/core/lib/spree/permitted_attributes.rb b/core/lib/spree/permitted_attributes.rb index ff783a8bf17..1bb3bc4c8d7 100644 --- a/core/lib/spree/permitted_attributes.rb +++ b/core/lib/spree/permitted_attributes.rb @@ -14,6 +14,7 @@ module PermittedAttributes :checkout_confirm_attributes, :credit_card_update_attributes, :customer_return_attributes, + :customer_metadata_attributes, :image_attributes, :inventory_unit_attributes, :line_item_attributes, @@ -55,21 +56,23 @@ module PermittedAttributes :stock_location_id, return_items_attributes: [ :id, :inventory_unit_id, :return_authorization_id, :returned, :amount, :reception_status_event, :acceptance_status, :exchange_variant_id, - :resellable, :return_reason_id + :resellable, :return_reason_id, customer_metadata: {} ] ] + @@customer_metadata_attributes = [customer_metadata: {}] + @@image_attributes = [:alt, :attachment, :position, :viewable_type, :viewable_id] @@inventory_unit_attributes = [:shipment, :variant_id] - @@line_item_attributes = [:id, :variant_id, :quantity] + @@line_item_attributes = [:id, :variant_id, :quantity, customer_metadata: {}] @@option_value_attributes = [:name, :presentation] @@option_type_attributes = [:name, :presentation, option_values_attributes: option_value_attributes] - @@payment_attributes = [:amount, :payment_method_id, :payment_method] + @@payment_attributes = [:amount, :payment_method_id, :payment_method, customer_metadata: {}] @@product_properties_attributes = [:property_name, :value, :position] @@ -83,11 +86,19 @@ module PermittedAttributes @@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]] + @@return_authorization_attributes = [:memo, :stock_location_id, :return_reason_id, + customer_metadata: {}, + return_items_attributes: [ + :inventory_unit_id, + :exchange_variant_id, + :return_reason_id, + :preferred_reimbursement_type_id + ] + ] @@shipment_attributes = [ :special_instructions, :stock_location_id, :id, :tracking, - :selected_shipping_rate_id + :selected_shipping_rate_id, customer_metadata: {} ] # month / year may be provided by some sources, or others may elect to use one field @@ -126,7 +137,7 @@ module PermittedAttributes # by changing a user with higher priveleges' email to one a lower-priveleged # admin owns. Creating a user with an email is handled separate at the # controller level. - @@user_attributes = [:password, :password_confirmation] + @@user_attributes = [:password, :password_confirmation, customer_metadata: {}] @@variant_attributes = [ :name, :presentation, :cost_price, :lock_version, From 678750d7cc98227415ac02506eddb9fbfc16a041 Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Tue, 4 Feb 2025 12:14:16 +0530 Subject: [PATCH 5/8] Add restriction for metadata update after order is complete User cannot update customer_metadata after the status of order is complete --- api/app/controllers/spree/api/orders_controller.rb | 7 +++++++ api/spec/requests/spree/api/orders_spec.rb | 11 +++++++++++ 2 files changed, 18 insertions(+) diff --git a/api/app/controllers/spree/api/orders_controller.rb b/api/app/controllers/spree/api/orders_controller.rb index 3f8507086da..45f0cf56d52 100644 --- a/api/app/controllers/spree/api/orders_controller.rb +++ b/api/app/controllers/spree/api/orders_controller.rb @@ -112,12 +112,19 @@ def mine def order_params if params[:order] normalize_params + prevent_customer_metadata_update params.require(:order).permit(permitted_order_attributes) else {} end end + def prevent_customer_metadata_update + return unless @order&.completed? && cannot?(:admin, Spree::Order) + + params[:order].delete(:customer_metadata) if params[:order] + end + def normalize_params if params[:order][:payments] payments_params = params[:order].delete(:payments) diff --git a/api/spec/requests/spree/api/orders_spec.rb b/api/spec/requests/spree/api/orders_spec.rb index aff5baef373..a7956a69e8c 100644 --- a/api/spec/requests/spree/api/orders_spec.rb +++ b/api/spec/requests/spree/api/orders_spec.rb @@ -128,6 +128,17 @@ module Spree::Api expect(json_response).not_to have_key('admin_metadata') end + + it "cannot update customer metadata if the order is complete" do + order = create(:order) + order.completed_at = Time.current + order.state = 'complete' + order.save! + + put spree.api_order_path(order), params: { order: attributes_with_metadata } + + expect(json_response['customer_metadata']).to eq({}) + end end context "when the current user can administrate the order" do From 777b6fadeff1437096f4d21b3f63317cbb245392 Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Tue, 18 Feb 2025 12:04:37 +0530 Subject: [PATCH 6/8] Add admin_metadata to attributes for admins Introduce the `admin_metadata` attribute to support dynamic metadata handling for admins. It enables configuration of metadata via API preferences, and ensures that metadata fields are properly permitted for admin users through new helper methods. **Implementation:** - Added `admin_metadata` to resources for admins. - Integrated `metadata_permit_parameters` and `metadata_api_parameters preference` in API configuration. - Implemented `permitted__attributes` and `_attributes` methods for dynamic metadata permissions. - Added `extract_metadata` in `line_items_controller` to handle metadata fields. --- .../controllers/spree/api/base_controller.rb | 19 ++++++++++++- .../spree/api/line_items_controller.rb | 17 +++++++++-- api/app/helpers/spree/api/api_helpers.rb | 10 +++++++ api/lib/spree/api_configuration.rb | 22 +++++++++++++++ .../spree/api/customer_returns_spec.rb | 27 +++++++++++++++++- .../requests/spree/api/line_items_spec.rb | 28 +++++++++++++++++++ api/spec/requests/spree/api/orders_spec.rb | 26 +++++++++++++++++ api/spec/requests/spree/api/payments_spec.rb | 20 +++++++++++++ .../spree/api/return_authorizations_spec.rb | 27 +++++++++++++++++- api/spec/requests/spree/api/shipments_spec.rb | 17 +++++++++++ api/spec/requests/spree/api/users_spec.rb | 20 +++++++++++++ .../app/models/spree/simple_order_contents.rb | 5 +++- 12 files changed, 232 insertions(+), 6 deletions(-) diff --git a/api/app/controllers/spree/api/base_controller.rb b/api/app/controllers/spree/api/base_controller.rb index ec48d666e52..e97677b6fdd 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 :admin_metadata_attributes + self.admin_metadata_attributes = [{ admin_metadata: {} }] + attr_accessor :current_api_user before_action :load_user @@ -35,15 +38,29 @@ class BaseController < ActionController::Base private + Spree::Api::Config.metadata_permit_parameters.each do |resource| + define_method("permitted_#{resource.to_s.underscore}_attributes") do + if can?(:admin, "Spree::#{resource}".constantize) + super() + admin_metadata_attributes + else + super() + end + end + end + # users should be able to set price when importing orders via api def permitted_line_item_attributes if can?(:admin, Spree::LineItem) - super + admin_line_item_attributes + super + admin_line_item_attributes + admin_metadata_attributes else super end end + def permitted_user_attributes + can?(:admin, Spree.user_class) ? super + admin_metadata_attributes : super + 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/controllers/spree/api/line_items_controller.rb b/api/app/controllers/spree/api/line_items_controller.rb index e6b6a43bbe1..48c9cdbfa11 100644 --- a/api/app/controllers/spree/api/line_items_controller.rb +++ b/api/app/controllers/spree/api/line_items_controller.rb @@ -15,8 +15,10 @@ def create @line_item = @order.contents.add( variant, params[:line_item][:quantity] || 1, - options: line_item_params[:options].to_h + options: line_item_params[:options].to_h, + **extract_metadata ) + respond_with(@line_item, status: 201, default_template: :show) rescue ActiveRecord::RecordInvalid => error invalid_resource!(error.record) @@ -56,10 +58,21 @@ def line_items_attributes { line_items_attributes: { id: params[:id], quantity: params[:line_item][:quantity], - options: line_item_params[:options] || {} + options: line_item_params[:options] || {}, + **extract_metadata } } end + def extract_metadata + metadata = { customer_metadata: line_item_params[:customer_metadata] } + + if @current_user_roles&.include?("admin") + metadata[:admin_metadata] = line_item_params[:admin_metadata] + end + + metadata + end + def line_item_params params.require(:line_item).permit(permitted_line_item_attributes) end diff --git a/api/app/helpers/spree/api/api_helpers.rb b/api/app/helpers/spree/api/api_helpers.rb index 8c52db2d772..6e4757a8818 100644 --- a/api/app/helpers/spree/api/api_helpers.rb +++ b/api/app/helpers/spree/api/api_helpers.rb @@ -43,6 +43,16 @@ module ApiHelpers end end + Spree::Api::Config.metadata_api_parameters.each do |method_name, resource| + define_method("#{method_name}_attributes") do + authorized_attributes(resource, "#{method_name}_attributes") + end + end + + def authorized_attributes(resource, config_attribute) + can?(:admin, resource) ? Spree::Api::Config.public_send(config_attribute) + [:admin_metadata] : Spree::Api::Config.public_send(config_attribute) + end + def required_fields_for(model) required_fields = model._validators.select do |_field, validations| validations.any? { |validation| validation.is_a?(ActiveModel::Validations::PresenceValidator) } diff --git a/api/lib/spree/api_configuration.rb b/api/lib/spree/api_configuration.rb index 5c1dd75b096..8fd16fb747c 100644 --- a/api/lib/spree/api_configuration.rb +++ b/api/lib/spree/api_configuration.rb @@ -41,6 +41,28 @@ class ApiConfiguration < Preferences::Configuration preference :line_item_attributes, :array, default: [:id, :quantity, :price, :variant_id, :customer_metadata] + # Spree::Api::Config.metadata_api_parameters contains the models + # to which the admin_metadata attribute is added + preference :metadata_api_parameters, :array, default: [ + [:order, 'Spree::Order'], + [:customer_return, 'Spree::CustomerReturn'], + [:payment, 'Spree::Payment'], + [:return_authorization, 'Spree::ReturnAuthorization'], + [:shipment, 'Spree::Shipment'], + [:user, 'Spree.user_class'], + [:line_item, 'Spree::LineItem'] + ] + + # Spree::Api::Config.metadata_permit_parameters contains the models + # to which the admin_metadata attribute is permitted + preference :metadata_permit_parameters, :array, default: [ + :Order, + :CustomerReturn, + :Payment, + :ReturnAuthorization, + :Shipment + ] + preference :option_type_attributes, :array, default: [:id, :name, :presentation, :position] preference :payment_attributes, :array, default: [ diff --git a/api/spec/requests/spree/api/customer_returns_spec.rb b/api/spec/requests/spree/api/customer_returns_spec.rb index 2da3f9a4c35..a735a999af6 100644 --- a/api/spec/requests/spree/api/customer_returns_spec.rb +++ b/api/spec/requests/spree/api/customer_returns_spec.rb @@ -59,6 +59,14 @@ module Spree::Api expect(json_response).to have_attributes(attributes) end + it "can view admin_metadata" do + customer_return = FactoryBot.create(:customer_return) + + get spree.api_order_customer_return_path(customer_return.order, customer_return.id) + + expect(json_response).to have_key('admin_metadata') + end + it "can get a list of customer returns" do FactoryBot.create(:customer_return, shipped_order: order) FactoryBot.create(:customer_return, shipped_order: order) @@ -97,7 +105,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", "customer_metadata", "admin_metadata"]) end it "can update a customer return" do @@ -112,6 +120,23 @@ module Spree::Api expect(json_response["stock_location_id"]).to eq final_stock_location.id end + it "can update a customer return admin_metadata" do + initial_stock_location = FactoryBot.create(:stock_location) + final_stock_location = FactoryBot.create(:stock_location) + customer_return = FactoryBot.create(:customer_return, stock_location: initial_stock_location) + + put spree.api_order_customer_return_path(customer_return.order, customer_return.id), + params: { + order_id: customer_return.order.number, + customer_return: { stock_location_id: final_stock_location.id, admin_metadata: { 'order_number' => 'PN345678' } } + } + + expect(response.status).to eq(200) + expect(json_response).to have_attributes(attributes) + expect(json_response["stock_location_id"]).to eq final_stock_location.id + expect(json_response["admin_metadata"]).to eq({ 'order_number' => 'PN345678' }) + end + context "when creating new return items" do it "can create a new customer return" do stock_location = FactoryBot.create(:stock_location) diff --git a/api/spec/requests/spree/api/line_items_spec.rb b/api/spec/requests/spree/api/line_items_spec.rb index 66e230adcad..421148c0b55 100644 --- a/api/spec/requests/spree/api/line_items_spec.rb +++ b/api/spec/requests/spree/api/line_items_spec.rb @@ -228,6 +228,34 @@ module Spree::Api end end + context "as an admin" do + sign_in_as_admin! + + it "can see admin_metadata" do + post spree.api_order_line_items_path(order), + params: { + line_item: { variant_id: product.master.to_param, quantity: 1 }, + order_token: order.guest_token + } + + expect(response.status).to eq(201) + expect(json_response).to have_key('admin_metadata') + end + + it "allows creating line item with customer metadata and admin metadata" do + post spree.api_order_line_items_path(order), + params: { + line_item: { variant_id: product.master.to_param, + quantity: 1, + customer_metadata: { "Company" => "Sample Company" }, + admin_metadata: { "discount" => "not_applicable" } } + } + + expect(json_response['customer_metadata']).to eq({ "Company" => "Sample Company" }) + expect(json_response['admin_metadata']).to eq({ "discount" => "not_applicable" }) + end + end + context "as just another user" do before do user = create(:user) diff --git a/api/spec/requests/spree/api/orders_spec.rb b/api/spec/requests/spree/api/orders_spec.rb index a7956a69e8c..c1e6f711c93 100644 --- a/api/spec/requests/spree/api/orders_spec.rb +++ b/api/spec/requests/spree/api/orders_spec.rb @@ -987,6 +987,32 @@ module Spree::Api expect(json_response['error']).to eq(I18n.t(:could_not_transition, scope: "spree.api", resource: 'order')) expect(response.status).to eq(422) end + + it "can update a order admin_metadata" do + put spree.api_order_path(order), params: { order: { admin_metadata: { 'order_number' => 'PN345678' } } } + + expect(response.status).to eq(200) + + expect(json_response["admin_metadata"]).to eq({ 'order_number' => 'PN345678' }) + end + + it "can update customer metadata if the order is complete" do + order = create(:order) + order.completed_at = Time.current + order.state = 'complete' + order.save! + + put spree.api_order_path(order), params: { order: { customer_metadata: { 'Note' => 'Do not ring the bell' }, admin_metadata: { 'order_number' => 'PN345678' } } } + + expect(json_response['customer_metadata']).to eq({ 'Note' => 'Do not ring the bell' }) + expect(json_response["admin_metadata"]).to eq({ 'order_number' => 'PN345678' }) + end + + it "can view admin_metadata" do + get spree.api_order_path(order) + + expect(json_response).to have_key('admin_metadata') + end end context "can cancel an order" do diff --git a/api/spec/requests/spree/api/payments_spec.rb b/api/spec/requests/spree/api/payments_spec.rb index 6f5d0ae3483..cbe42f47415 100644 --- a/api/spec/requests/spree/api/payments_spec.rb +++ b/api/spec/requests/spree/api/payments_spec.rb @@ -187,6 +187,26 @@ module Spree::Api expect(response.status).to eq(403) expect(json_response["error"]).to eq("This payment cannot be updated because it is completed.") end + + it "can update a payment admin_metadata" do + payment.update(state: 'pending') + + put spree.api_order_payment_path(order, payment), + params: { + payment: { + amount: 2.01, + admin_metadata: { 'order_number' => 'PN345678' } + } + } + + expect(response.status).to eq(200) + expect(json_response["admin_metadata"]).to eq({ 'order_number' => 'PN345678' }) + end + + it "can view admin_metadata" do + get spree.api_order_payments_path(order) + expect(json_response["payments"].first).to have_key('admin_metadata') + 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..c755f141c77 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", "customer_metadata", "admin_metadata"]) required_attributes = json_response["required_attributes"] expect(required_attributes).to include("order") end @@ -151,6 +151,31 @@ module Spree::Api expect { return_authorization.reload }.to raise_error(ActiveRecord::RecordNotFound) end + it "can view admin_metadata" do + FactoryBot.create(:return_authorization, order:) + + return_authorization = order.return_authorizations.first + + get spree.api_order_return_authorization_path(order, return_authorization.id) + + expect(response.status).to eq(200) + expect(json_response).to have_attributes(attributes) + expect(json_response).to have_key('admin_metadata') + end + + it "can update a return authorization on the order and metadata" do + FactoryBot.create(:return_authorization, order:) + + return_authorization = order.return_authorizations.first + + put spree.api_order_return_authorization_path(order, return_authorization.id), params: { return_authorization: { memo: "ABC", admin_metadata: { 'return_type' => 'warranty claim' }, customer_metadata: { 'return_reason' => 'product not working' } } } + + expect(response.status).to eq(200) + expect(json_response).to have_attributes(attributes) + expect(json_response["admin_metadata"]).to eq({ 'return_type' => 'warranty claim' }) + expect(json_response["customer_metadata"]).to eq({ 'return_reason' => 'product not working' }) + end + it_behaves_like "a return authorization creator" end diff --git a/api/spec/requests/spree/api/shipments_spec.rb b/api/spec/requests/spree/api/shipments_spec.rb index 4ba4ff7ea7c..4ad42e05b5d 100644 --- a/api/spec/requests/spree/api/shipments_spec.rb +++ b/api/spec/requests/spree/api/shipments_spec.rb @@ -92,6 +92,23 @@ module Spree::Api expect(json_response['stock_location_name']).to eq(stock_location.name) end + it "can update a shipment and it's metadata" do + params = { + shipment: { + stock_location_id: stock_location.to_param, + admin_metadata: { 'delivery_type' => 'express' }, + customer_metadata: { 'timing' => 'standard' } + } + } + + put(spree.api_shipment_path(shipment), params:) + + expect(response.status).to eq(200) + expect(json_response['stock_location_name']).to eq(stock_location.name) + expect(json_response["admin_metadata"]).to eq({ 'delivery_type' => 'express' }) + expect(json_response["customer_metadata"]).to eq({ 'timing' => 'standard' }) + end + it "can make a shipment ready" do allow_any_instance_of(Spree::Order).to receive_messages(paid?: true, complete?: true) put spree.ready_api_shipment_path(shipment) diff --git a/api/spec/requests/spree/api/users_spec.rb b/api/spec/requests/spree/api/users_spec.rb index c16dbeae6c7..e29bbe25ea9 100644 --- a/api/spec/requests/spree/api/users_spec.rb +++ b/api/spec/requests/spree/api/users_spec.rb @@ -188,6 +188,18 @@ module Spree::Api expect(json_response['users'].first['email']).to eq expected_result.email end + it 'can view admin_metadata' do + allow(Spree::LegacyUser).to receive(:find_by).with(hash_including(:spree_api_key)) { current_api_user } + + 2.times { create(:user) } + + get spree.api_users_path + expect(Spree.user_class.count).to eq 2 + expect(json_response['count']).to eq 2 + expect(json_response['users'].size).to eq 2 + expect(json_response['users'].first).to have_key('admin_metadata') + end + it "can create" do post spree.api_users_path, params: { user: { email: "new@example.com", password: 'spree123', password_confirmation: 'spree123' } } expect(json_response).to have_attributes(attributes) @@ -200,6 +212,14 @@ module Spree::Api expect(response.status).to eq(201) end + it "can update admin_metadata" do + post spree.api_users_path, params: { user: { email: "existing@example.com", admin_metadata: { 'user_type' => 'regular' } } } + + expect(json_response).to have_attributes(attributes) + expect(response.status).to eq(201) + expect(json_response["admin_metadata"]).to eq({ 'user_type' => 'regular' }) + end + it "can destroy user without orders" do user.orders.destroy_all delete spree.api_user_path(user) diff --git a/core/app/models/spree/simple_order_contents.rb b/core/app/models/spree/simple_order_contents.rb index 0676544835f..e6735415604 100644 --- a/core/app/models/spree/simple_order_contents.rb +++ b/core/app/models/spree/simple_order_contents.rb @@ -85,8 +85,11 @@ def add_to_line_item(variant, quantity, options = {}) adjustments: [] ) + permitted_attributes = Spree::PermittedAttributes.line_item_attributes.dup + permitted_attributes << { admin_metadata: {} } if options[:admin_metadata].present? + line_item.quantity += quantity.to_i - line_item.options = ActionController::Parameters.new(options).permit(PermittedAttributes.line_item_attributes).to_h + line_item.options = ActionController::Parameters.new(options).permit(permitted_attributes).to_h line_item.target_shipment = options[:shipment] line_item.save! From 0040a2cb04ee2ace5e194f25d822482079755990 Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Mon, 17 Feb 2025 17:02:34 +0530 Subject: [PATCH 7/8] Linting --- Gemfile | 1 - bin/release/extract-pipeline-context | 7 ++++--- core/lib/spree/permitted_attributes.rb | 15 +++++++-------- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/Gemfile b/Gemfile index 117ab9526c4..44bc66f5ea5 100644 --- a/Gemfile +++ b/Gemfile @@ -21,7 +21,6 @@ gem 'pg', '~> 1.0', require: false if dbs.match?(/all|postgres/) gem 'fast_sqlite', require: false if dbs.match?(/all|sqlite/) gem 'sqlite3', '>= 2.1', require: false if dbs.match?(/all|sqlite/) - gem 'database_cleaner', '~> 2.0', require: false gem 'rspec-activemodel-mocks', '~> 1.1', require: false gem 'rspec-rails', '~> 6.0.3', require: false diff --git a/bin/release/extract-pipeline-context b/bin/release/extract-pipeline-context index 19c1e64d32e..09cf2b5815c 100755 --- a/bin/release/extract-pipeline-context +++ b/bin/release/extract-pipeline-context @@ -36,6 +36,7 @@ context_hash = { } warn "~~> Generating context..." -context_hash.each { |k, v| warn "#{k}=#{v}" } - -context_hash.each { |k, v| puts "#{k}=#{v}" } +context_hash.each { |k, v| + warn "#{k}=#{v}" + puts "#{k}=#{v}" +} diff --git a/core/lib/spree/permitted_attributes.rb b/core/lib/spree/permitted_attributes.rb index 1bb3bc4c8d7..cad7f5b1a24 100644 --- a/core/lib/spree/permitted_attributes.rb +++ b/core/lib/spree/permitted_attributes.rb @@ -87,14 +87,13 @@ module PermittedAttributes @@property_attributes = [:name, :presentation] @@return_authorization_attributes = [:memo, :stock_location_id, :return_reason_id, - customer_metadata: {}, - return_items_attributes: [ - :inventory_unit_id, - :exchange_variant_id, - :return_reason_id, - :preferred_reimbursement_type_id - ] - ] + customer_metadata: {}, + return_items_attributes: [ + :inventory_unit_id, + :exchange_variant_id, + :return_reason_id, + :preferred_reimbursement_type_id + ]] @@shipment_attributes = [ :special_instructions, :stock_location_id, :id, :tracking, From b95449178e0094351474c7449b240ce44726e12f Mon Sep 17 00:00:00 2001 From: Mayur Shah Date: Thu, 13 Feb 2025 19:04:13 +0530 Subject: [PATCH 8/8] Introduce Customer and Admin Metadata Restriction Preference This configuration adds the ability to enable or disable metadata restrictions as needed. Following configurations are possible: - `meta_data_validation_enabled` : Enables or disables restrictions globally. --- core/app/models/concerns/spree/metadata.rb | 6 +- core/lib/spree/app_configuration.rb | 8 ++ core/spec/support/shared_examples/metadata.rb | 86 +++++++++++++------ 3 files changed, 74 insertions(+), 26 deletions(-) diff --git a/core/app/models/concerns/spree/metadata.rb b/core/app/models/concerns/spree/metadata.rb index 6f7c2d7185d..09b21c60423 100644 --- a/core/app/models/concerns/spree/metadata.rb +++ b/core/app/models/concerns/spree/metadata.rb @@ -8,7 +8,7 @@ module Metadata attribute :customer_metadata, :json, default: {} attribute :admin_metadata, :json, default: {} - validate :validate_metadata_limits + validate :validate_metadata_limits, if: :validate_metadata_enabled? end class_methods do @@ -19,6 +19,10 @@ def meta_data_columns private + def validate_metadata_enabled? + Spree::Config.meta_data_validation_enabled + end + def validate_metadata_limits self.class.meta_data_columns.each { |column| validate_metadata_column(column) } end diff --git a/core/lib/spree/app_configuration.rb b/core/lib/spree/app_configuration.rb index 182bbec4a3b..c7177454c6a 100644 --- a/core/lib/spree/app_configuration.rb +++ b/core/lib/spree/app_configuration.rb @@ -216,6 +216,14 @@ class AppConfiguration < Preferences::Configuration # @return [Integer] Orders to show per-page in the admin (default: +15+) preference :orders_per_page, :integer, default: 15 + # @!attribute [rw] meta_data_validation_enabled + # @return [Boolean] Indicates whether validation for customer and admin metadata columns is enabled. + # When this is set to true, the following preferences will be used to validate the metadata: + # - The maximum number of keys that can be added to the metadata columns (meta_data_max_keys). + # - The maximum length of each key in the metadata columns (meta_data_max_key_length). + # - The maximum length of each value in the metadata columns (meta_data_max_value_length). + # (default: +true+) + preference :meta_data_validation_enabled, :boolean, default: false # @!attribute [rw] meta_data_max_keys # @return [Integer] Maximum keys that can be allocated in customer and admin metadata column (default: +6+) diff --git a/core/spec/support/shared_examples/metadata.rb b/core/spec/support/shared_examples/metadata.rb index f2922243240..6f930c30d17 100644 --- a/core/spec/support/shared_examples/metadata.rb +++ b/core/spec/support/shared_examples/metadata.rb @@ -53,43 +53,79 @@ %w[customer_metadata admin_metadata].each do |metadata_type| describe metadata_type do - it "does not allow more than 6 keys" do - subject.send("#{metadata_type}=", invalid_metadata_keys) + context "when metadata validation is enabled" do + before do + stub_spree_preferences(meta_data_validation_enabled: true) + end - expect(subject).not_to be_valid - expect(subject.errors[metadata_type.to_sym]).to include("must not have more than 6 keys") - end + it "does not allow more than 6 keys" do + subject.send("#{metadata_type}=", invalid_metadata_keys) - it "allows less than 6 keys" do - subject.send("#{metadata_type}=", valid_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 - expect(subject).to be_valid - end + it "allows less than 6 keys" do + subject.send("#{metadata_type}=", valid_metadata_keys) - it "does not allow values longer than 256 characters" do - subject.send("#{metadata_type}=", oversized_value_metadata) + expect(subject).to be_valid + end - 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 "does not allow values longer than 256 characters" do + subject.send("#{metadata_type}=", oversized_value_metadata) - it "allows values shorter than 256 characters" do - subject.send("#{metadata_type}=", valid_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 - expect(subject).to be_valid - end + it "allows 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 "does not allow keys longer than 16 characters" do - subject.send("#{metadata_type}=", oversized_key_metadata) + it "allows keys shorter than 16 characters" do + subject.send("#{metadata_type}=", valid_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") + expect(subject).to be_valid + end end - it "allows keys shorter than 16 characters" do - subject.send("#{metadata_type}=", valid_key_metadata) + context "when metadata validation is disabled" do + before do + stub_spree_preferences(meta_data_validation_enabled: false) + end + + it "does not validate the metadata" do + subject.send("#{metadata_type}=", invalid_metadata_keys) + + expect(subject).to be_valid + end + + it "allows more than 6 keys" do + subject.send("#{metadata_type}=", invalid_metadata_keys) + + expect(subject).to be_valid + end + + it "allows values longer than 256 characters" do + subject.send("#{metadata_type}=", oversized_value_metadata) + + expect(subject).to be_valid + end + + it "allows keys longer than 16 characters" do + subject.send("#{metadata_type}=", oversized_key_metadata) - expect(subject).to be_valid + expect(subject).to be_valid + end end end end