From 746feb2395bd9e4bf0774625b606e7aca7a33fa9 Mon Sep 17 00:00:00 2001 From: Sebastian Palma Date: Thu, 21 Oct 2021 16:37:23 -0300 Subject: [PATCH 1/5] Add Webhooks::Subscriber check_uri_path validation. --- api/app/models/spree/webhooks/subscriber.rb | 14 ++++++++++++++ api/spec/models/spree/webhooks/subscriber_spec.rb | 12 ++++++++++-- core/app/validators/spree/url_validator.rb | 1 + core/spec/validators/spree/url_validator_spec.rb | 14 ++------------ 4 files changed, 27 insertions(+), 14 deletions(-) diff --git a/api/app/models/spree/webhooks/subscriber.rb b/api/app/models/spree/webhooks/subscriber.rb index 2f5f728f23c..9c004a2ec2a 100644 --- a/api/app/models/spree/webhooks/subscriber.rb +++ b/api/app/models/spree/webhooks/subscriber.rb @@ -3,7 +3,21 @@ module Webhooks class Subscriber < Spree::Webhooks::Base validates :url, 'spree/url': true, presence: true + validate :check_uri_path + scope :active, -> { where(active: true) } + + private + + def check_uri_path + uri = begin + URI.parse(url) + rescue URI::InvalidURIError + return false + end + + errors.add(:url, 'the URL must have a path') if uri.blank? || uri.path.blank? + end end end end diff --git a/api/spec/models/spree/webhooks/subscriber_spec.rb b/api/spec/models/spree/webhooks/subscriber_spec.rb index 1bf2dfed77b..4ae897297c2 100644 --- a/api/spec/models/spree/webhooks/subscriber_spec.rb +++ b/api/spec/models/spree/webhooks/subscriber_spec.rb @@ -2,16 +2,24 @@ describe Spree::Webhooks::Subscriber do describe 'validations' do - context 'url format' do + context 'url format (UrlValidator)' do it 'is invalid with an invalid url' do endpoint = described_class.new(url: 'google.com') expect(endpoint.valid?).to be(false) end it 'is valid with a valid url' do - endpoint = described_class.new(url: 'http://google.com') + endpoint = described_class.new(url: 'http://google.com/') expect(endpoint.valid?).to be(true) end end + + context 'url path' do + it 'is invalid a url without path' do + endpoint = described_class.new(url: 'http://google.com') + expect(endpoint.valid?).to be(false) + expect(endpoint.errors.messages).to eq(url: ['the URL must have a path']) + end + end end end diff --git a/core/app/validators/spree/url_validator.rb b/core/app/validators/spree/url_validator.rb index 63bc79bc28f..559167973cc 100644 --- a/core/app/validators/spree/url_validator.rb +++ b/core/app/validators/spree/url_validator.rb @@ -1,6 +1,7 @@ module Spree class UrlValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) + value = value[:url] if value.is_a?(Hash) unless url_valid?(value) record.errors.add(attribute, (options[:message] || ERROR_MESSAGE)) end diff --git a/core/spec/validators/spree/url_validator_spec.rb b/core/spec/validators/spree/url_validator_spec.rb index 97789da4924..14f298323fe 100644 --- a/core/spec/validators/spree/url_validator_spec.rb +++ b/core/spec/validators/spree/url_validator_spec.rb @@ -2,8 +2,8 @@ module Spree module Test - class Product < ActiveRecord::Base - self.table_name = 'test_products' + Product = Struct.new(:url) do + include ActiveModel::Validations validates :url, 'spree/url': true end @@ -11,16 +11,6 @@ class Product < ActiveRecord::Base end describe Spree::UrlValidator do - before(:all) do - ActiveRecord::Base.connection.create_table :test_products, force: true do |t| - t.string :url - end - end - - after(:all) do - ActiveRecord::Base.connection.drop_table :test_products, if_exists: true - end - describe 'validating the given URL' do context 'is invalid' do it { expect(Spree::Test::Product.new(url: nil).valid?).to eq(false) } From c116e8e7ea8232540dd46cdb0903a6af5df03864 Mon Sep 17 00:00:00 2001 From: Sebastian Palma Date: Fri, 22 Oct 2021 09:16:33 -0300 Subject: [PATCH 2/5] Add Subscriber#urls_for. --- api/app/models/spree/webhooks/subscriber.rb | 10 +++++++ .../webhooks/subscribers/queue_requests.rb | 18 +------------ .../models/spree/webhooks/subscriber_spec.rb | 26 +++++++++++++++++++ .../subscribers/queue_requests_spec.rb | 24 ++++++++--------- core/app/validators/spree/url_validator.rb | 1 - .../validators/spree/url_validator_spec.rb | 2 +- 6 files changed, 50 insertions(+), 31 deletions(-) diff --git a/api/app/models/spree/webhooks/subscriber.rb b/api/app/models/spree/webhooks/subscriber.rb index 9c004a2ec2a..5636059ea1f 100644 --- a/api/app/models/spree/webhooks/subscriber.rb +++ b/api/app/models/spree/webhooks/subscriber.rb @@ -7,6 +7,16 @@ class Subscriber < Spree::Webhooks::Base scope :active, -> { where(active: true) } + def self.urls_for(event) + where_condition = case ActiveRecord::Base.connection.adapter_name + when 'Mysql2' + ["('*' MEMBER OF(subscriptions) OR ? MEMBER OF(subscriptions))", event] + when 'PostgreSQL' + ["subscriptions @> '[\"*\"]' OR subscriptions @> ?", [event].to_json] + end + active.where(where_condition).pluck(:url) + end + private def check_uri_path diff --git a/api/app/services/spree/webhooks/subscribers/queue_requests.rb b/api/app/services/spree/webhooks/subscribers/queue_requests.rb index c957af06a46..ec8b3ace89e 100644 --- a/api/app/services/spree/webhooks/subscribers/queue_requests.rb +++ b/api/app/services/spree/webhooks/subscribers/queue_requests.rb @@ -5,26 +5,10 @@ class QueueRequests prepend Spree::ServiceModule::Base def call(body:, event:) - subscriberd_urls_for(event).each do |url| + Spree::Webhooks::Subscriber.urls_for(event).each do |url| Spree::Webhooks::Subscribers::MakeRequestJob.perform_later(body, event, url) end end - - private - - def subscriberd_urls_for(event) - Spree::Webhooks::Subscriber.active.where(subscriptions_where_statement(event)).pluck(:url) - end - - # FIXME: this should be written as scope - def subscriptions_where_statement(event) - case ActiveRecord::Base.connection.adapter_name - when 'Mysql2' - ["('*' MEMBER OF(subscriptions) OR ? MEMBER OF(subscriptions))", event] - when 'PostgreSQL' - ["subscriptions @> '[\"*\"]' OR subscriptions @> ?", [event].to_json] - end - end end end end diff --git a/api/spec/models/spree/webhooks/subscriber_spec.rb b/api/spec/models/spree/webhooks/subscriber_spec.rb index 4ae897297c2..779a08a87c3 100644 --- a/api/spec/models/spree/webhooks/subscriber_spec.rb +++ b/api/spec/models/spree/webhooks/subscriber_spec.rb @@ -22,4 +22,30 @@ end end end + + describe '.urls_for' do + subject { described_class.urls_for(event) } + + let(:active) { true } + let(:event) { 'order.complete' } + let(:subscriptions) { ['order.complete'] } + let!(:subscriber) { described_class.create(url: url, subscriptions: subscriptions, active: active) } + let(:url) { 'https://url1.com/' } + + context 'with subscriptions for the given event' do + it { expect(subject).to eq([url]) } + end + + context 'without subscriptions for the given event, but "*"' do + let(:subscriptions) { ['*'] } + + it { expect(subject).to eq([url]) } + end + + context 'without active subscriptions' do + let(:active) { false } + + it { expect(subject).to eq([]) } + end + end end diff --git a/api/spec/services/spree/webhooks/subscribers/queue_requests_spec.rb b/api/spec/services/spree/webhooks/subscribers/queue_requests_spec.rb index 769591395d3..5e8c309c047 100644 --- a/api/spec/services/spree/webhooks/subscribers/queue_requests_spec.rb +++ b/api/spec/services/spree/webhooks/subscribers/queue_requests_spec.rb @@ -16,10 +16,10 @@ end context 'with subscriptions for the given event' do - context 'when endpoint subscriptions includes all events (*)' do - before { stub_request(:post, endpoint.url) } + context 'when subscriber subscriptions includes all events (*)' do + before { stub_request(:post, subscriber.url) } - let(:endpoint) do + let(:subscriber) do Spree::Webhooks::Subscriber.create( url: 'https://url1.com/', subscriptions: ['*'], @@ -29,15 +29,15 @@ it 'queues a job to make a request' do expect { subject }.to( - have_enqueued_job(make_request_job).with(body, event, endpoint.url).on_queue(queue) + have_enqueued_job(make_request_job).with(body, event, subscriber.url).on_queue(queue) ) end end - context 'when endpoint subscriptions includes the specific event being used' do - before { stub_request(:post, endpoint.url) } + context 'when subscriber subscriptions includes the specific event being used' do + before { stub_request(:post, subscriber.url) } - let(:endpoint) do + let(:subscriber) do Spree::Webhooks::Subscriber.create( url: 'https://url2.com/', subscriptions: [event], @@ -47,13 +47,13 @@ it 'queues a job to make a request' do expect { subject }.to( - have_enqueued_job(make_request_job).with(body, event, endpoint.url).on_queue(queue) + have_enqueued_job(make_request_job).with(body, event, subscriber.url).on_queue(queue) ) end end - context 'when endpoint subscriptions are not enabled' do - let(:endpoint) do + context 'when subscriber subscriptions are not enabled' do + let(:subscriber) do Spree::Webhooks::Subscriber.create( url: 'https://url3.com/', subscriptions: [event], @@ -66,8 +66,8 @@ end end - context 'when endpoint subscriptions do not include the event or "*"' do - let(:endpoint) do + context 'when subscriber subscriptions do not include the event or "*"' do + let(:subscriber) do Spree::Webhooks::Subscriber.create( url: 'https://url4.com/', subscriptions: ['order.resume'], diff --git a/core/app/validators/spree/url_validator.rb b/core/app/validators/spree/url_validator.rb index 559167973cc..63bc79bc28f 100644 --- a/core/app/validators/spree/url_validator.rb +++ b/core/app/validators/spree/url_validator.rb @@ -1,7 +1,6 @@ module Spree class UrlValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) - value = value[:url] if value.is_a?(Hash) unless url_valid?(value) record.errors.add(attribute, (options[:message] || ERROR_MESSAGE)) end diff --git a/core/spec/validators/spree/url_validator_spec.rb b/core/spec/validators/spree/url_validator_spec.rb index 14f298323fe..499d7678cef 100644 --- a/core/spec/validators/spree/url_validator_spec.rb +++ b/core/spec/validators/spree/url_validator_spec.rb @@ -2,7 +2,7 @@ module Spree module Test - Product = Struct.new(:url) do + Product = Struct.new(:url, keyword_init: true) do include ActiveModel::Validations validates :url, 'spree/url': true From 274f8c164935422d1ecf51549f2e2358d4f0263d Mon Sep 17 00:00:00 2001 From: Sebastian Palma Date: Fri, 22 Oct 2021 11:48:07 -0300 Subject: [PATCH 3/5] Add missing serializers. --- api/app/models/spree/webhooks/subscriber.rb | 2 +- .../spree/api/v2/platform/asset_serializer.rb | 11 +++++++ .../api/v2/platform/property_serializer.rb | 11 +++++++ .../api/v2/platform/prototype_serializer.rb | 11 +++++++ .../spree/api/v2/platform/role_serializer.rb | 11 +++++++ .../v2/platform/stock_transfer_serializer.rb | 11 +++++++ .../webhooks/subscribers/queue_requests.rb | 2 +- .../models/spree/webhooks/subscriber_spec.rb | 9 +----- .../api/v2/platform/asset_serializer_spec.rb | 32 +++++++++++++++++++ .../v2/platform/property_serializer_spec.rb | 27 ++++++++++++++++ .../v2/platform/prototype_serializer_spec.rb | 24 ++++++++++++++ .../api/v2/platform/role_serializer_spec.rb | 24 ++++++++++++++ .../stock_transfer_serializer_spec.rb | 26 +++++++++++++++ .../subscribers/queue_requests_spec.rb | 8 ++--- .../factories/asset_factory.rb | 15 +++++++++ .../factories/stock_transfer_factory.rb | 9 ++++++ 16 files changed, 219 insertions(+), 14 deletions(-) create mode 100644 api/app/serializers/spree/api/v2/platform/asset_serializer.rb create mode 100644 api/app/serializers/spree/api/v2/platform/property_serializer.rb create mode 100644 api/app/serializers/spree/api/v2/platform/prototype_serializer.rb create mode 100644 api/app/serializers/spree/api/v2/platform/role_serializer.rb create mode 100644 api/app/serializers/spree/api/v2/platform/stock_transfer_serializer.rb create mode 100644 api/spec/serializers/spree/api/v2/platform/asset_serializer_spec.rb create mode 100644 api/spec/serializers/spree/api/v2/platform/property_serializer_spec.rb create mode 100644 api/spec/serializers/spree/api/v2/platform/prototype_serializer_spec.rb create mode 100644 api/spec/serializers/spree/api/v2/platform/role_serializer_spec.rb create mode 100644 api/spec/serializers/spree/api/v2/platform/stock_transfer_serializer_spec.rb create mode 100644 core/lib/spree/testing_support/factories/asset_factory.rb create mode 100644 core/lib/spree/testing_support/factories/stock_transfer_factory.rb diff --git a/api/app/models/spree/webhooks/subscriber.rb b/api/app/models/spree/webhooks/subscriber.rb index 5636059ea1f..ced0423150c 100644 --- a/api/app/models/spree/webhooks/subscriber.rb +++ b/api/app/models/spree/webhooks/subscriber.rb @@ -14,7 +14,7 @@ def self.urls_for(event) when 'PostgreSQL' ["subscriptions @> '[\"*\"]' OR subscriptions @> ?", [event].to_json] end - active.where(where_condition).pluck(:url) + where(where_condition).pluck(:url) end private diff --git a/api/app/serializers/spree/api/v2/platform/asset_serializer.rb b/api/app/serializers/spree/api/v2/platform/asset_serializer.rb new file mode 100644 index 00000000000..f6e7bc05d8a --- /dev/null +++ b/api/app/serializers/spree/api/v2/platform/asset_serializer.rb @@ -0,0 +1,11 @@ +module Spree + module Api + module V2 + module Platform + class AssetSerializer < BaseSerializer + include ResourceSerializerConcern + end + end + end + end +end diff --git a/api/app/serializers/spree/api/v2/platform/property_serializer.rb b/api/app/serializers/spree/api/v2/platform/property_serializer.rb new file mode 100644 index 00000000000..ebf9767a77b --- /dev/null +++ b/api/app/serializers/spree/api/v2/platform/property_serializer.rb @@ -0,0 +1,11 @@ +module Spree + module Api + module V2 + module Platform + class PropertySerializer < BaseSerializer + include ResourceSerializerConcern + end + end + end + end +end diff --git a/api/app/serializers/spree/api/v2/platform/prototype_serializer.rb b/api/app/serializers/spree/api/v2/platform/prototype_serializer.rb new file mode 100644 index 00000000000..abd05c34f15 --- /dev/null +++ b/api/app/serializers/spree/api/v2/platform/prototype_serializer.rb @@ -0,0 +1,11 @@ +module Spree + module Api + module V2 + module Platform + class PrototypeSerializer < BaseSerializer + include ResourceSerializerConcern + end + end + end + end +end diff --git a/api/app/serializers/spree/api/v2/platform/role_serializer.rb b/api/app/serializers/spree/api/v2/platform/role_serializer.rb new file mode 100644 index 00000000000..b747813aa03 --- /dev/null +++ b/api/app/serializers/spree/api/v2/platform/role_serializer.rb @@ -0,0 +1,11 @@ +module Spree + module Api + module V2 + module Platform + class RoleSerializer < BaseSerializer + include ResourceSerializerConcern + end + end + end + end +end diff --git a/api/app/serializers/spree/api/v2/platform/stock_transfer_serializer.rb b/api/app/serializers/spree/api/v2/platform/stock_transfer_serializer.rb new file mode 100644 index 00000000000..0524ba7acb0 --- /dev/null +++ b/api/app/serializers/spree/api/v2/platform/stock_transfer_serializer.rb @@ -0,0 +1,11 @@ +module Spree + module Api + module V2 + module Platform + class StockTransferSerializer < BaseSerializer + include ResourceSerializerConcern + end + end + end + end +end diff --git a/api/app/services/spree/webhooks/subscribers/queue_requests.rb b/api/app/services/spree/webhooks/subscribers/queue_requests.rb index ec8b3ace89e..5aaa94e1806 100644 --- a/api/app/services/spree/webhooks/subscribers/queue_requests.rb +++ b/api/app/services/spree/webhooks/subscribers/queue_requests.rb @@ -5,7 +5,7 @@ class QueueRequests prepend Spree::ServiceModule::Base def call(body:, event:) - Spree::Webhooks::Subscriber.urls_for(event).each do |url| + Spree::Webhooks::Subscriber.active.urls_for(event).each do |url| Spree::Webhooks::Subscribers::MakeRequestJob.perform_later(body, event, url) end end diff --git a/api/spec/models/spree/webhooks/subscriber_spec.rb b/api/spec/models/spree/webhooks/subscriber_spec.rb index 779a08a87c3..36241c4f099 100644 --- a/api/spec/models/spree/webhooks/subscriber_spec.rb +++ b/api/spec/models/spree/webhooks/subscriber_spec.rb @@ -26,10 +26,9 @@ describe '.urls_for' do subject { described_class.urls_for(event) } - let(:active) { true } let(:event) { 'order.complete' } let(:subscriptions) { ['order.complete'] } - let!(:subscriber) { described_class.create(url: url, subscriptions: subscriptions, active: active) } + let!(:subscriber) { described_class.create(url: url, subscriptions: subscriptions, active: true) } let(:url) { 'https://url1.com/' } context 'with subscriptions for the given event' do @@ -41,11 +40,5 @@ it { expect(subject).to eq([url]) } end - - context 'without active subscriptions' do - let(:active) { false } - - it { expect(subject).to eq([]) } - end end end diff --git a/api/spec/serializers/spree/api/v2/platform/asset_serializer_spec.rb b/api/spec/serializers/spree/api/v2/platform/asset_serializer_spec.rb new file mode 100644 index 00000000000..6e46dbfbe8b --- /dev/null +++ b/api/spec/serializers/spree/api/v2/platform/asset_serializer_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Spree::Api::V2::Platform::AssetSerializer do + include_context 'API v2 serializers params' + + subject { described_class.new(asset, params: serializer_params).serializable_hash } + + let(:type) { :asset } + let(:asset) { create(type) } + + it do + expect(subject).to eq( + data: { + id: asset.id.to_s, + type: type, + attributes: { + viewable_type: asset.viewable_type, + attachment_height: asset.attachment_height, + attachment_file_size: asset.attachment_file_size, + position: asset.position, + attachment_content_type: asset.attachment_content_type, + attachment_file_name: asset.attachment_file_name, + type: asset.type, + attachment_updated_at: asset.attachment_updated_at, + alt: asset.alt, + created_at: asset.created_at, + updated_at: asset.updated_at + } + } + ) + end +end diff --git a/api/spec/serializers/spree/api/v2/platform/property_serializer_spec.rb b/api/spec/serializers/spree/api/v2/platform/property_serializer_spec.rb new file mode 100644 index 00000000000..2672c555685 --- /dev/null +++ b/api/spec/serializers/spree/api/v2/platform/property_serializer_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' + +describe Spree::Api::V2::Platform::PropertySerializer do + include_context 'API v2 serializers params' + + subject { described_class.new(resource, params: serializer_params).serializable_hash } + + let(:type) { :property } + let(:resource) { create(type) } + + it do + expect(subject).to eq( + data: { + id: resource.id.to_s, + type: type, + attributes: { + name: resource.name, + presentation: resource.presentation, + created_at: resource.created_at, + updated_at: resource.updated_at, + filterable: resource.filterable, + filter_param: resource.filter_param + } + } + ) + end +end diff --git a/api/spec/serializers/spree/api/v2/platform/prototype_serializer_spec.rb b/api/spec/serializers/spree/api/v2/platform/prototype_serializer_spec.rb new file mode 100644 index 00000000000..b594447a791 --- /dev/null +++ b/api/spec/serializers/spree/api/v2/platform/prototype_serializer_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe Spree::Api::V2::Platform::PrototypeSerializer do + include_context 'API v2 serializers params' + + subject { described_class.new(resource, params: serializer_params).serializable_hash } + + let(:type) { :prototype } + let(:resource) { create(type) } + + it do + expect(subject).to eq( + data: { + id: resource.id.to_s, + type: type, + attributes: { + name: resource.name, + created_at: resource.created_at, + updated_at: resource.updated_at + } + } + ) + end +end diff --git a/api/spec/serializers/spree/api/v2/platform/role_serializer_spec.rb b/api/spec/serializers/spree/api/v2/platform/role_serializer_spec.rb new file mode 100644 index 00000000000..531b625276f --- /dev/null +++ b/api/spec/serializers/spree/api/v2/platform/role_serializer_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe Spree::Api::V2::Platform::RoleSerializer do + include_context 'API v2 serializers params' + + subject { described_class.new(resource, params: serializer_params).serializable_hash } + + let(:type) { :role } + let(:resource) { create(type) } + + it do + expect(subject).to eq( + data: { + id: resource.id.to_s, + type: type, + attributes: { + name: resource.name, + created_at: resource.created_at, + updated_at: resource.updated_at + } + } + ) + end +end diff --git a/api/spec/serializers/spree/api/v2/platform/stock_transfer_serializer_spec.rb b/api/spec/serializers/spree/api/v2/platform/stock_transfer_serializer_spec.rb new file mode 100644 index 00000000000..62a404e7752 --- /dev/null +++ b/api/spec/serializers/spree/api/v2/platform/stock_transfer_serializer_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe Spree::Api::V2::Platform::StockTransferSerializer do + include_context 'API v2 serializers params' + + subject { described_class.new(resource, params: serializer_params).serializable_hash } + + let(:type) { :stock_transfer } + let(:resource) { create(type) } + + it do + expect(subject).to eq( + data: { + id: resource.id.to_s, + type: type, + attributes: { + type: resource.type, + reference: resource.reference, + created_at: resource.created_at, + updated_at: resource.updated_at, + number: resource.number + } + } + ) + end +end diff --git a/api/spec/services/spree/webhooks/subscribers/queue_requests_spec.rb b/api/spec/services/spree/webhooks/subscribers/queue_requests_spec.rb index 5e8c309c047..99bf120ef1f 100644 --- a/api/spec/services/spree/webhooks/subscribers/queue_requests_spec.rb +++ b/api/spec/services/spree/webhooks/subscribers/queue_requests_spec.rb @@ -52,12 +52,12 @@ end end - context 'when subscriber subscriptions are not enabled' do - let(:subscriber) do + context 'when subscriber subscriptions are not active' do + let!(:subscriber) do Spree::Webhooks::Subscriber.create( url: 'https://url3.com/', subscriptions: [event], - enabled: false + active: false ) end @@ -67,7 +67,7 @@ end context 'when subscriber subscriptions do not include the event or "*"' do - let(:subscriber) do + let!(:subscriber) do Spree::Webhooks::Subscriber.create( url: 'https://url4.com/', subscriptions: ['order.resume'], diff --git a/core/lib/spree/testing_support/factories/asset_factory.rb b/core/lib/spree/testing_support/factories/asset_factory.rb new file mode 100644 index 00000000000..e08b6142db3 --- /dev/null +++ b/core/lib/spree/testing_support/factories/asset_factory.rb @@ -0,0 +1,15 @@ +FactoryBot.define do + factory :asset, class: Spree::Asset do + viewable_type {} + viewable_id {} + attachment_width { 340 } + attachment_height { 280 } + attachment_file_size { 128 } + position { 1 } + attachment_content_type { '.jpg' } + attachment_file_name { 'attachment.jpg' } + type {} + attachment_updated_at {} + alt {} + end +end diff --git a/core/lib/spree/testing_support/factories/stock_transfer_factory.rb b/core/lib/spree/testing_support/factories/stock_transfer_factory.rb new file mode 100644 index 00000000000..68c85e3772d --- /dev/null +++ b/core/lib/spree/testing_support/factories/stock_transfer_factory.rb @@ -0,0 +1,9 @@ +FactoryBot.define do + factory :stock_transfer, class: Spree::StockTransfer do + destination_location {} + number {} + reference {} + source_location {} + type {} + end +end From c11d0b4acab2501b2fde1333901f9f2a6287238e Mon Sep 17 00:00:00 2001 From: Sebastian Palma Date: Fri, 22 Oct 2021 13:12:18 -0300 Subject: [PATCH 4/5] Add serializers relationships. Update examples. --- api/app/models/spree/webhooks/subscriber.rb | 8 ++-- .../spree/api/v2/platform/asset_serializer.rb | 2 + .../api/v2/platform/prototype_serializer.rb | 4 ++ .../v2/platform/stock_transfer_serializer.rb | 4 ++ .../api/v2/platform/asset_serializer_spec.rb | 41 +++++++++++-------- .../v2/platform/prototype_serializer_spec.rb | 29 +++++++++++-- .../stock_transfer_serializer_spec.rb | 32 +++++++++++++-- 7 files changed, 94 insertions(+), 26 deletions(-) diff --git a/api/app/models/spree/webhooks/subscriber.rb b/api/app/models/spree/webhooks/subscriber.rb index ced0423150c..31c10731256 100644 --- a/api/app/models/spree/webhooks/subscriber.rb +++ b/api/app/models/spree/webhooks/subscriber.rb @@ -21,10 +21,10 @@ def self.urls_for(event) def check_uri_path uri = begin - URI.parse(url) - rescue URI::InvalidURIError - return false - end + URI.parse(url) + rescue URI::InvalidURIError + return false + end errors.add(:url, 'the URL must have a path') if uri.blank? || uri.path.blank? end diff --git a/api/app/serializers/spree/api/v2/platform/asset_serializer.rb b/api/app/serializers/spree/api/v2/platform/asset_serializer.rb index f6e7bc05d8a..e1f9b931ee5 100644 --- a/api/app/serializers/spree/api/v2/platform/asset_serializer.rb +++ b/api/app/serializers/spree/api/v2/platform/asset_serializer.rb @@ -4,6 +4,8 @@ module V2 module Platform class AssetSerializer < BaseSerializer include ResourceSerializerConcern + + belongs_to :viewable, polymorphic: true end end end diff --git a/api/app/serializers/spree/api/v2/platform/prototype_serializer.rb b/api/app/serializers/spree/api/v2/platform/prototype_serializer.rb index abd05c34f15..3fef1158d07 100644 --- a/api/app/serializers/spree/api/v2/platform/prototype_serializer.rb +++ b/api/app/serializers/spree/api/v2/platform/prototype_serializer.rb @@ -4,6 +4,10 @@ module V2 module Platform class PrototypeSerializer < BaseSerializer include ResourceSerializerConcern + + has_many :properties + has_many :option_types + has_many :taxons end end end diff --git a/api/app/serializers/spree/api/v2/platform/stock_transfer_serializer.rb b/api/app/serializers/spree/api/v2/platform/stock_transfer_serializer.rb index 0524ba7acb0..9c2235edfb5 100644 --- a/api/app/serializers/spree/api/v2/platform/stock_transfer_serializer.rb +++ b/api/app/serializers/spree/api/v2/platform/stock_transfer_serializer.rb @@ -4,6 +4,10 @@ module V2 module Platform class StockTransferSerializer < BaseSerializer include ResourceSerializerConcern + + belongs_to :destination_location, serializer: :stock_location, record_type: :destination_location + belongs_to :source_location, serializer: :stock_location, record_type: :source_location + has_many :stock_movements end end end diff --git a/api/spec/serializers/spree/api/v2/platform/asset_serializer_spec.rb b/api/spec/serializers/spree/api/v2/platform/asset_serializer_spec.rb index 6e46dbfbe8b..174a333ba69 100644 --- a/api/spec/serializers/spree/api/v2/platform/asset_serializer_spec.rb +++ b/api/spec/serializers/spree/api/v2/platform/asset_serializer_spec.rb @@ -3,29 +3,38 @@ describe Spree::Api::V2::Platform::AssetSerializer do include_context 'API v2 serializers params' - subject { described_class.new(asset, params: serializer_params).serializable_hash } + subject { described_class.new(resource, params: serializer_params).serializable_hash } + let(:viewable) { create(:variant) } let(:type) { :asset } - let(:asset) { create(type) } + let(:resource) { create(type, viewable: viewable) } it do expect(subject).to eq( data: { - id: asset.id.to_s, - type: type, + id: resource.id.to_s, attributes: { - viewable_type: asset.viewable_type, - attachment_height: asset.attachment_height, - attachment_file_size: asset.attachment_file_size, - position: asset.position, - attachment_content_type: asset.attachment_content_type, - attachment_file_name: asset.attachment_file_name, - type: asset.type, - attachment_updated_at: asset.attachment_updated_at, - alt: asset.alt, - created_at: asset.created_at, - updated_at: asset.updated_at - } + viewable_type: resource.viewable_type, + attachment_height: resource.attachment_height, + attachment_file_size: resource.attachment_file_size, + position: resource.position, + attachment_content_type: resource.attachment_content_type, + attachment_file_name: resource.attachment_file_name, + type: resource.type, + attachment_updated_at: resource.attachment_updated_at, + alt: resource.alt, + created_at: resource.created_at, + updated_at: resource.updated_at + }, + relationships: { + viewable: { + data: { + id: viewable.id.to_s, + type: :variant + } + } + }, + type: type } ) end diff --git a/api/spec/serializers/spree/api/v2/platform/prototype_serializer_spec.rb b/api/spec/serializers/spree/api/v2/platform/prototype_serializer_spec.rb index b594447a791..16261718491 100644 --- a/api/spec/serializers/spree/api/v2/platform/prototype_serializer_spec.rb +++ b/api/spec/serializers/spree/api/v2/platform/prototype_serializer_spec.rb @@ -5,19 +5,42 @@ subject { described_class.new(resource, params: serializer_params).serializable_hash } + let(:property) { create(:property) } + let(:option_type) { create(:option_type) } + let(:resource) { create(type, properties: [property], option_types: [option_type], taxons: [taxon]) } + let(:taxon) { create(:taxon) } let(:type) { :prototype } - let(:resource) { create(type) } it do expect(subject).to eq( data: { id: resource.id.to_s, - type: type, attributes: { name: resource.name, created_at: resource.created_at, updated_at: resource.updated_at - } + }, + relationships: { + properties: { + data: [{ + id: property.id.to_s, + type: :property + }] + }, + option_types: { + data: [{ + id: option_type.id.to_s, + type: :option_type + }] + }, + taxons: { + data: [{ + id: taxon.id.to_s, + type: :taxon + }] + } + }, + type: type } ) end diff --git a/api/spec/serializers/spree/api/v2/platform/stock_transfer_serializer_spec.rb b/api/spec/serializers/spree/api/v2/platform/stock_transfer_serializer_spec.rb index 62a404e7752..dd406851c70 100644 --- a/api/spec/serializers/spree/api/v2/platform/stock_transfer_serializer_spec.rb +++ b/api/spec/serializers/spree/api/v2/platform/stock_transfer_serializer_spec.rb @@ -5,21 +5,47 @@ subject { described_class.new(resource, params: serializer_params).serializable_hash } + let(:destination_location) { create(:stock_location) } + let(:resource) { create(type, destination_location: destination_location, source_location: source_location) } + let(:source_location) { create(:stock_location) } + let(:stock_item) { stock_location.stock_items.order(:id).first } + let(:stock_location) { create(:stock_location_with_items) } let(:type) { :stock_transfer } - let(:resource) { create(type) } + + let!(:stock_movement) { create(:stock_movement, stock_item: stock_item, originator: resource) } it do expect(subject).to eq( data: { id: resource.id.to_s, - type: type, attributes: { type: resource.type, reference: resource.reference, created_at: resource.created_at, updated_at: resource.updated_at, number: resource.number - } + }, + relationships: { + stock_movements: { + data: [{ + id: stock_movement.id.to_s, + type: :stock_movement + }] + }, + source_location: { + data: { + id: source_location.id.to_s, + type: :source_location + } + }, + destination_location: { + data: { + id: destination_location.id.to_s, + type: :destination_location + } + } + }, + type: type, } ) end From eb2ff76505accd3af2979716963e9e4ed2626451 Mon Sep 17 00:00:00 2001 From: Sebastian Palma Date: Fri, 22 Oct 2021 15:46:39 -0300 Subject: [PATCH 5/5] Add missing serializers examples. --- .../reimbursement_credit_serializer.rb | 10 +++ .../api/v2/platform/stock_item_serializer.rb | 2 - .../v2/platform/stock_transfer_serializer.rb | 4 +- .../platform/refund_reason_serializer_spec.rb | 26 ++++++++ .../reimbursement_credit_serializer_spec.rb | 19 ++++++ .../platform/reimbursement_serializer_spec.rb | 62 +++++++++++++++++++ .../v2/platform/stock_item_serializer_spec.rb | 42 +++++++++++++ .../stock_location_serializer_spec.rb | 53 ++++++++++++++++ .../stock_movement_serializer_spec.rb | 28 +++++++++ .../stock_transfer_serializer_spec.rb | 8 +-- .../factories/reimbursement_credit_factory.rb | 7 +++ 11 files changed, 253 insertions(+), 8 deletions(-) create mode 100644 api/app/serializers/spree/api/v2/platform/reimbursement_credit_serializer.rb create mode 100644 api/spec/serializers/spree/api/v2/platform/refund_reason_serializer_spec.rb create mode 100644 api/spec/serializers/spree/api/v2/platform/reimbursement_credit_serializer_spec.rb create mode 100644 api/spec/serializers/spree/api/v2/platform/reimbursement_serializer_spec.rb create mode 100644 api/spec/serializers/spree/api/v2/platform/stock_item_serializer_spec.rb create mode 100644 api/spec/serializers/spree/api/v2/platform/stock_location_serializer_spec.rb create mode 100644 api/spec/serializers/spree/api/v2/platform/stock_movement_serializer_spec.rb create mode 100644 core/lib/spree/testing_support/factories/reimbursement_credit_factory.rb diff --git a/api/app/serializers/spree/api/v2/platform/reimbursement_credit_serializer.rb b/api/app/serializers/spree/api/v2/platform/reimbursement_credit_serializer.rb new file mode 100644 index 00000000000..8665a342444 --- /dev/null +++ b/api/app/serializers/spree/api/v2/platform/reimbursement_credit_serializer.rb @@ -0,0 +1,10 @@ +module Spree + module Api + module V2 + module Platform + class ReimbursementCreditSerializer < BaseSerializer + end + end + end + end +end diff --git a/api/app/serializers/spree/api/v2/platform/stock_item_serializer.rb b/api/app/serializers/spree/api/v2/platform/stock_item_serializer.rb index ff7fb00d1c1..ded1fab01c1 100644 --- a/api/app/serializers/spree/api/v2/platform/stock_item_serializer.rb +++ b/api/app/serializers/spree/api/v2/platform/stock_item_serializer.rb @@ -5,8 +5,6 @@ module Platform class StockItemSerializer < BaseSerializer include ResourceSerializerConcern - set_type :stock_item - attribute :is_available do |stock_item| stock_item.available? end diff --git a/api/app/serializers/spree/api/v2/platform/stock_transfer_serializer.rb b/api/app/serializers/spree/api/v2/platform/stock_transfer_serializer.rb index 9c2235edfb5..c5049be4cb9 100644 --- a/api/app/serializers/spree/api/v2/platform/stock_transfer_serializer.rb +++ b/api/app/serializers/spree/api/v2/platform/stock_transfer_serializer.rb @@ -5,8 +5,8 @@ module Platform class StockTransferSerializer < BaseSerializer include ResourceSerializerConcern - belongs_to :destination_location, serializer: :stock_location, record_type: :destination_location - belongs_to :source_location, serializer: :stock_location, record_type: :source_location + belongs_to :destination_location, serializer: :stock_location + belongs_to :source_location, serializer: :stock_location has_many :stock_movements end end diff --git a/api/spec/serializers/spree/api/v2/platform/refund_reason_serializer_spec.rb b/api/spec/serializers/spree/api/v2/platform/refund_reason_serializer_spec.rb new file mode 100644 index 00000000000..807a0f29c10 --- /dev/null +++ b/api/spec/serializers/spree/api/v2/platform/refund_reason_serializer_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe Spree::Api::V2::Platform::RefundReasonSerializer do + include_context 'API v2 serializers params' + + subject { described_class.new(resource, params: serializer_params).serializable_hash } + + let(:type) { :refund_reason } + let(:resource) { create(type) } + + it do + expect(subject).to eq( + data: { + id: resource.id.to_s, + attributes: { + name: resource.name, + active: resource.active, + mutable: resource.mutable, + created_at: resource.created_at, + updated_at: resource.updated_at + }, + type: type + } + ) + end +end diff --git a/api/spec/serializers/spree/api/v2/platform/reimbursement_credit_serializer_spec.rb b/api/spec/serializers/spree/api/v2/platform/reimbursement_credit_serializer_spec.rb new file mode 100644 index 00000000000..c9361e73976 --- /dev/null +++ b/api/spec/serializers/spree/api/v2/platform/reimbursement_credit_serializer_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +describe Spree::Api::V2::Platform::ReimbursementCreditSerializer do + include_context 'API v2 serializers params' + + subject { described_class.new(resource, params: serializer_params).serializable_hash } + + let(:type) { :reimbursement_credit } + let(:resource) { create(type, creditable: create(:store_credit)) } + + it do + expect(subject).to eq( + data: { + id: resource.id.to_s, + type: type + } + ) + end +end diff --git a/api/spec/serializers/spree/api/v2/platform/reimbursement_serializer_spec.rb b/api/spec/serializers/spree/api/v2/platform/reimbursement_serializer_spec.rb new file mode 100644 index 00000000000..e17ef9d0141 --- /dev/null +++ b/api/spec/serializers/spree/api/v2/platform/reimbursement_serializer_spec.rb @@ -0,0 +1,62 @@ +require 'spec_helper' + +describe Spree::Api::V2::Platform::ReimbursementSerializer do + include_context 'API v2 serializers params' + + subject { described_class.new(resource, params: serializer_params).serializable_hash } + + let(:reimbursement_credit) { create(:reimbursement_credit, creditable: create(:store_credit)) } + let(:payment) { create(:payment, state: 'completed') } + let(:type) { :reimbursement } + let(:refund) { create(:refund, amount: payment.credit_allowed - 1) } + let(:resource) { create(type, refunds: [refund], credits: [reimbursement_credit]) } + + it do + expect(subject).to eq( + data: { + id: resource.id.to_s, + attributes: { + number: resource.number, + reimbursement_status: resource.reimbursement_status, + total: resource.total, + created_at: resource.created_at, + updated_at: resource.updated_at, + display_total: resource.display_total, + }, + relationships: { + order: { + data: { + id: resource.order.id.to_s, + type: :order + } + }, + customer_return: { + data: { + id: resource.customer_return.id.to_s, + type: :customer_return + } + }, + refunds: { + data: [{ + id: refund.id.to_s, + type: :refund + }] + }, + reimbursement_credits: { + data: [{ + id: reimbursement_credit.id.to_s, + type: :reimbursement_credit + }] + }, + return_items: { + data: [{ + id: resource.return_items.first.id.to_s, + type: :return_item + }] + } + }, + type: type + } + ) + end +end diff --git a/api/spec/serializers/spree/api/v2/platform/stock_item_serializer_spec.rb b/api/spec/serializers/spree/api/v2/platform/stock_item_serializer_spec.rb new file mode 100644 index 00000000000..b317caf0c12 --- /dev/null +++ b/api/spec/serializers/spree/api/v2/platform/stock_item_serializer_spec.rb @@ -0,0 +1,42 @@ +require 'spec_helper' + +describe Spree::Api::V2::Platform::StockItemSerializer do + include_context 'API v2 serializers params' + + subject { described_class.new(resource, params: serializer_params).serializable_hash } + + let(:resource) { stock_location.stock_items.order(:id).first } + let(:stock_location) { create(:stock_location_with_items) } + let(:type) { :stock_item } + + it do + expect(subject).to eq( + data: { + id: resource.id.to_s, + attributes: { + backorderable: resource.backorderable, + count_on_hand: resource.count_on_hand, + created_at: resource.created_at, + deleted_at: resource.deleted_at, + is_available: resource.available?, + updated_at: resource.updated_at + }, + relationships: { + stock_location: { + data: { + id: stock_location.id.to_s, + type: :stock_location + } + }, + variant: { + data: { + id: resource.variant.id.to_s, + type: :variant + } + } + }, + type: type + } + ) + end +end diff --git a/api/spec/serializers/spree/api/v2/platform/stock_location_serializer_spec.rb b/api/spec/serializers/spree/api/v2/platform/stock_location_serializer_spec.rb new file mode 100644 index 00000000000..fad1e40eb73 --- /dev/null +++ b/api/spec/serializers/spree/api/v2/platform/stock_location_serializer_spec.rb @@ -0,0 +1,53 @@ +require 'spec_helper' + +describe Spree::Api::V2::Platform::StockLocationSerializer do + include_context 'API v2 serializers params' + + subject { described_class.new(resource, params: serializer_params).serializable_hash } + + let(:resource) { create(:stock_location_with_items, shipments: [shipment]) } + let(:type) { :stock_location } + let(:shipment) { create(:shipment) } + + it do + expect(subject).to eq( + data: { + id: resource.id.to_s, + type: type, + attributes: { + active: resource.active, + address1: resource.address1, + address2: resource.address2, + admin_name: resource.admin_name, + backorderable_default: resource.backorderable_default, + city: resource.city, + created_at: resource.created_at, + default: resource.default, + name: resource.name, + phone: resource.phone, + propagate_all_variants: resource.propagate_all_variants, + state_name: resource.state_name, + updated_at: resource.updated_at, + zipcode: resource.zipcode + }, + relationships: { + shipments: { + data: [{ + id: shipment.id.to_s, + type: :shipment + }] + }, + stock_items: { + data: [{ + id: resource.stock_items[0].id.to_s, + type: :stock_item + }, { + id: resource.stock_items[1].id.to_s, + type: :stock_item + }] + } + } + } + ) + end +end diff --git a/api/spec/serializers/spree/api/v2/platform/stock_movement_serializer_spec.rb b/api/spec/serializers/spree/api/v2/platform/stock_movement_serializer_spec.rb new file mode 100644 index 00000000000..33fd0066d6f --- /dev/null +++ b/api/spec/serializers/spree/api/v2/platform/stock_movement_serializer_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe Spree::Api::V2::Platform::StockMovementSerializer do + include_context 'API v2 serializers params' + + subject { described_class.new(resource, params: serializer_params).serializable_hash } + + let(:type) { :stock_movement } + let(:stock_location) { create(:stock_location_with_items) } + let(:stock_item) { stock_location.stock_items.order(:id).first } + let(:resource) { create(type, stock_item: stock_item) } + + it do + expect(subject).to eq( + data: { + id: resource.id.to_s, + attributes: { + quantity: resource.quantity, + action: resource.action, + originator_type: resource.originator_type, + created_at: resource.created_at, + updated_at: resource.updated_at + }, + type: type + } + ) + end +end diff --git a/api/spec/serializers/spree/api/v2/platform/stock_transfer_serializer_spec.rb b/api/spec/serializers/spree/api/v2/platform/stock_transfer_serializer_spec.rb index dd406851c70..745a88370e9 100644 --- a/api/spec/serializers/spree/api/v2/platform/stock_transfer_serializer_spec.rb +++ b/api/spec/serializers/spree/api/v2/platform/stock_transfer_serializer_spec.rb @@ -1,10 +1,10 @@ require 'spec_helper' describe Spree::Api::V2::Platform::StockTransferSerializer do - include_context 'API v2 serializers params' - subject { described_class.new(resource, params: serializer_params).serializable_hash } + include_context 'API v2 serializers params' + let(:destination_location) { create(:stock_location) } let(:resource) { create(type, destination_location: destination_location, source_location: source_location) } let(:source_location) { create(:stock_location) } @@ -35,13 +35,13 @@ source_location: { data: { id: source_location.id.to_s, - type: :source_location + type: :stock_location } }, destination_location: { data: { id: destination_location.id.to_s, - type: :destination_location + type: :stock_location } } }, diff --git a/core/lib/spree/testing_support/factories/reimbursement_credit_factory.rb b/core/lib/spree/testing_support/factories/reimbursement_credit_factory.rb new file mode 100644 index 00000000000..8a03944383b --- /dev/null +++ b/core/lib/spree/testing_support/factories/reimbursement_credit_factory.rb @@ -0,0 +1,7 @@ +FactoryBot.define do + factory :reimbursement_credit, class: Spree::Reimbursement::Credit do + reimbursement + + amount { 100.00 } + end +end