Merge pull request #11418 from spree/feature/subscriber-validations

Add Webhooks::Subscriber check_uri_path validation

api/app/models/spree/webhooks/subscriber.rb

@@ -3,7 +3,31 @@ module Webhooks
     class Subscriber < Spree::Webhooks::Base
       validates :url, 'spree/url': true, presence: true
 
+      validate :check_uri_path
+
       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
+        where(where_condition).pluck(:url)
+      end
+
+      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

api/app/serializers/spree/api/v2/platform/asset_serializer.rb

@@ -0,0 +1,13 @@
+module Spree
+  module Api
+    module V2
+      module Platform
+        class AssetSerializer < BaseSerializer
+          include ResourceSerializerConcern
+
+          belongs_to :viewable, polymorphic: true
+        end
+      end
+    end
+  end
+end

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

api/app/serializers/spree/api/v2/platform/prototype_serializer.rb

@@ -0,0 +1,15 @@
+module Spree
+  module Api
+    module V2
+      module Platform
+        class PrototypeSerializer < BaseSerializer
+          include ResourceSerializerConcern
+
+          has_many :properties
+          has_many :option_types
+          has_many :taxons
+        end
+      end
+    end
+  end
+end

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

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

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

api/app/serializers/spree/api/v2/platform/stock_transfer_serializer.rb

@@ -0,0 +1,15 @@
+module Spree
+  module Api
+    module V2
+      module Platform
+        class StockTransferSerializer < BaseSerializer
+          include ResourceSerializerConcern
+
+          belongs_to :destination_location, serializer: :stock_location
+          belongs_to :source_location, serializer: :stock_location
+          has_many :stock_movements
+        end
+      end
+    end
+  end
+end

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.active.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

api/spec/models/spree/webhooks/subscriber_spec.rb

@@ -2,16 +2,43 @@
 
 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
+
+  describe '.urls_for' do
+    subject { described_class.urls_for(event) }
+
+    let(:event) { 'order.complete' }
+    let(:subscriptions) { ['order.complete'] }
+    let!(:subscriber) { described_class.create(url: url, subscriptions: subscriptions, active: true) }
+    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
   end
 end

api/spec/serializers/spree/api/v2/platform/asset_serializer_spec.rb

@@ -0,0 +1,41 @@
+require 'spec_helper'
+
+describe Spree::Api::V2::Platform::AssetSerializer do
+  include_context 'API v2 serializers params'
+
+  subject { described_class.new(resource, params: serializer_params).serializable_hash }
+
+  let(:viewable) { create(:variant) }
+  let(:type) { :asset }
+  let(:resource) { create(type, viewable: viewable) }
+
+  it do
+    expect(subject).to eq(
+      data: {
+        id: resource.id.to_s,
+        attributes: {
+          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
+end

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

api/spec/serializers/spree/api/v2/platform/prototype_serializer_spec.rb

@@ -0,0 +1,47 @@
+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(: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 }
+
+  it do
+    expect(subject).to eq(
+      data: {
+        id: resource.id.to_s,
+        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
+end

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

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