Merge pull request #7800 from pacodelaluna/improve-dfc-logic

Improve DFC Provider logic to use existing business logic

engines/dfc_provider/README.md

@@ -8,3 +8,5 @@ Basically, it allows an OFN user linked to an enterprise:
 * to be authenticated thanks to an Access Token from DFC Authorization server (using an OIDC implementation)
 
 The API endpoint for the catalog is `/api/dfc_provider/enterprise/prodcuts.json` and you need to pass the token inside an authentication header (`Authentication: Bearer 123mytoken456`).
+
+This feature is still under active development.

engines/dfc_provider/app/controllers/dfc_provider/api/base_controller.rb

@@ -7,8 +7,7 @@ module DfcProvider
       rescue_from ActiveRecord::RecordNotFound, with: :not_found
 
       before_action :check_authorization,
-                    :check_user,
-                    :check_enterprise
+                    :check_user
 
       respond_to :json
 
@@ -27,12 +26,15 @@ module DfcProvider
       end
 
       def check_enterprise
-        current_enterprise
+        return if current_enterprise.present?
+
+        not_found
       end
 
       def current_enterprise
         @current_enterprise ||=
-          if params[enterprise_id_param_name] == 'default'
+          case params[enterprise_id_param_name]
+          when 'default'
             current_user.enterprises.first!
           else
             current_user.enterprises.find(params[enterprise_id_param_name])

engines/dfc_provider/app/controllers/dfc_provider/api/catalog_items_controller.rb

@@ -1,10 +1,17 @@
 # frozen_string_literal: true
 
 # Controller used to provide the API products for the DFC application
+# CatalogItems are items that are being sold by the entreprise.
 module DfcProvider
   module Api
     class CatalogItemsController < DfcProvider::Api::BaseController
+      before_action :check_enterprise
+
       def index
+        # CatalogItem is nested into an entreprise which is also nested into
+        # an user on the DFC specifications, as defined here:
+        # https://datafoodconsortium.gitbook.io/dfc-standard-documentation
+        #  /technical-specification/api-examples
         render json: current_user, serializer: DfcProvider::PersonSerializer
       end
 
@@ -16,10 +23,7 @@ module DfcProvider
 
       def variant
         @variant ||=
-          Spree::Variant.
-            joins(product: :supplier).
-            where('enterprises.id' => current_enterprise.id).
-            find(params[:id])
+          DfcProvider::VariantFetcher.new(current_enterprise).scope.find(params[:id])
       end
     end
   end

engines/dfc_provider/app/controllers/dfc_provider/api/enterprises_controller.rb

@@ -4,6 +4,8 @@
 module DfcProvider
   module Api
     class EnterprisesController < DfcProvider::Api::BaseController
+      before_action :check_enterprise
+
       def show
         render json: current_enterprise, serializer: DfcProvider::EnterpriseSerializer
       end

engines/dfc_provider/app/controllers/dfc_provider/api/persons_controller.rb

@@ -4,8 +4,6 @@
 module DfcProvider
   module Api
     class PersonsController < DfcProvider::Api::BaseController
-      skip_before_action :check_enterprise
-
       before_action :check_user_accessibility
 
       def show

engines/dfc_provider/app/controllers/dfc_provider/api/supplied_products_controller.rb

@@ -1,9 +1,12 @@
 # frozen_string_literal: true
 
 # Controller used to provide the SuppliedProducts API for the DFC application
+# SuppliedProducts are products that are managed by an enterprise.
 module DfcProvider
   module Api
     class SuppliedProductsController < DfcProvider::Api::BaseController
+      before_action :check_enterprise
+
       def show
         render json: variant, serializer: DfcProvider::SuppliedProductSerializer
       end
@@ -12,10 +15,7 @@ module DfcProvider
 
       def variant
         @variant ||=
-          Spree::Variant.
-            joins(product: :supplier).
-            where('enterprises.id' => current_enterprise.id).
-            find(params[:id])
+          DfcProvider::VariantFetcher.new(current_enterprise).scope.find(params[:id])
       end
     end
   end

engines/dfc_provider/app/serializers/dfc_provider/address_serializer.rb

@@ -3,7 +3,7 @@
 # Serializer used to render the DFC Address from an OFN User
 # into JSON-LD format based on DFC ontology
 module DfcProvider
-  class AddressSerializer < ActiveModel::Serializer
+  class AddressSerializer < BaseSerializer
     attribute :type, key: '@type'
     attribute :city, key: 'dfc:city'
     attribute :country, key: 'dfc:country'

engines/dfc_provider/app/serializers/dfc_provider/base_serializer.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+# Serializer used to render the DFC Address from an OFN User
+# into JSON-LD format based on DFC ontology
+module DfcProvider
+  class BaseSerializer < ActiveModel::Serializer
+    private
+
+    def host
+      Rails.application.routes.default_url_options[:host]
+    end
+
+    def dfc_provider_routes
+      DfcProvider::Engine.routes.url_helpers
+    end
+  end
+end

engines/dfc_provider/app/serializers/dfc_provider/catalog_item_serializer.rb

@@ -3,7 +3,7 @@
 # Serializer used to render a DFC CatalogItem from an OFN Product
 # into JSON-LD format based on DFC ontology
 module DfcProvider
-  class CatalogItemSerializer < ActiveModel::Serializer
+  class CatalogItemSerializer < BaseSerializer
     attribute :id, key: '@id'
     attribute :type, key: '@type'
     attribute :references, key: 'dfc:references'
@@ -17,7 +17,7 @@ module DfcProvider
       dfc_provider_routes.api_dfc_provider_enterprise_catalog_item_url(
         enterprise_id: object.product.supplier_id,
         id: object.id,
-        host: root_url
+        host: host
       )
     end
 
@@ -28,7 +28,7 @@ module DfcProvider
     def references
       {
         '@type' => '@id',
-        '@id' => "/supplied_products/#{object.product_id}"
+        '@id' => reference_id
       }
     end
 
@@ -43,13 +43,9 @@ module DfcProvider
     def reference_id
       dfc_provider_routes.api_dfc_provider_enterprise_supplied_product_url(
         enterprise_id: object.product.supplier_id,
-        id: object.product_id,
-        host: root_url
+        id: object.id,
+        host: host
       )
     end
-
-    def dfc_provider_routes
-      DfcProvider::Engine.routes.url_helpers
-    end
   end
 end

engines/dfc_provider/app/serializers/dfc_provider/enterprise_serializer.rb

@@ -3,7 +3,7 @@
 # Serializer used to render a DFC Enterprise from an OFN Enterprise
 # into JSON-LD format based on DFC ontology
 module DfcProvider
-  class EnterpriseSerializer < ActiveModel::Serializer
+  class EnterpriseSerializer < BaseSerializer
     attribute :id, key: '@id'
     attribute :type, key: '@type'
     attribute :vat_number, key: 'dfc:VATnumber'
@@ -18,7 +18,7 @@ module DfcProvider
     def id
       dfc_provider_routes.api_dfc_provider_enterprise_url(
         id: object.id,
-        host: root_url
+        host: host
       )
     end
 
@@ -33,21 +33,11 @@ module DfcProvider
     end
 
     def supplies
-      Spree::Variant.
-        joins(product: :supplier).
-        where('enterprises.id' => object.id)
+      DfcProvider::VariantFetcher.new(object).scope
     end
 
     def manages
-      Spree::Variant.
-        joins(product: :supplier).
-        where('enterprises.id' => object.id)
-    end
-
-    private
-
-    def dfc_provider_routes
-      DfcProvider::Engine.routes.url_helpers
+      DfcProvider::VariantFetcher.new(object).scope
     end
   end
 end

engines/dfc_provider/app/serializers/dfc_provider/offer_serializer.rb

@@ -3,10 +3,10 @@
 # Serializer used to render the DFC Offer from an OFN Product
 # into JSON-LD format based on DFC ontology
 module DfcProvider
-  class OfferSerializer < ActiveModel::Serializer
+  class OfferSerializer < BaseSerializer
     attribute :id, key: '@id'
     attribute :type, key: '@type'
-    attribute :offeres_to, key: 'dfc:offeres_to'
+    attribute :offers_to, key: 'dfc:offers_to'
     attribute :price, key: 'dfc:price'
     attribute :stock_limitation, key: 'dfc:stockLimitation'
 
@@ -18,7 +18,7 @@ module DfcProvider
       'dfc:Offer'
     end
 
-    def offeres_to
+    def offers_to
       {
         '@type' => '@id',
         '@id' => nil

engines/dfc_provider/app/serializers/dfc_provider/person_serializer.rb

@@ -3,7 +3,7 @@
 # Serializer used to render the DFC Person from an OFN User
 # into JSON-LD format based on DFC ontology
 module DfcProvider
-  class PersonSerializer < ActiveModel::Serializer
+  class PersonSerializer < BaseSerializer
     attribute :context, key: '@context'
     attribute :id, key: '@id'
     attribute :type, key: '@type'
@@ -28,7 +28,7 @@ module DfcProvider
     def id
       dfc_provider_routes.api_dfc_provider_person_url(
         id: object.id,
-        host: root_url
+        host: host
       )
     end
 
@@ -45,11 +45,5 @@ module DfcProvider
     def affiliates
       object.enterprises
     end
-
-    private
-
-    def dfc_provider_routes
-      DfcProvider::Engine.routes.url_helpers
-    end
   end
 end

engines/dfc_provider/app/serializers/dfc_provider/supplied_product_serializer.rb

@@ -3,7 +3,7 @@
 # Serializer used to render a DFC SuppliedProduct from an OFN Variant
 # into JSON-LD format based on DFC ontology
 module DfcProvider
-  class SuppliedProductSerializer < ActiveModel::Serializer
+  class SuppliedProductSerializer < BaseSerializer
     attribute :id, key: '@id'
     attribute :type, key: '@type'
     attribute :unit, key: 'dfc:hasUnit'
@@ -20,7 +20,7 @@ module DfcProvider
       dfc_provider_routes.api_dfc_provider_enterprise_supplied_product_url(
         enterprise_id: object.product.supplier_id,
         id: object.id,
-        host: root_url
+        host: host
       )
     end
 
@@ -64,9 +64,5 @@ module DfcProvider
     def unit_name
       object.unit_description.presence || 'piece'
     end
-
-    def dfc_provider_routes
-      DfcProvider::Engine.routes.url_helpers
-    end
   end
 end

engines/dfc_provider/app/services/dfc_provider/variant_fetcher.rb

@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+# Service used to fetch variants related to an entreprise.
+# It improves maintenance as it is the central point requesting
+# Spree::Varaint inside the DfcProvider engine.
+module DfcProvider
+  class VariantFetcher
+    def initialize(enterprise)
+      @enterprise = enterprise
+    end
+
+    def scope
+      Spree::Variant.
+        joins(product: :supplier).
+        where('enterprises.id' => @enterprise.id)
+    end
+  end
+end

engines/dfc_provider/spec/controllers/dfc_provider/api/catalog_items_controller_spec.rb

@@ -10,7 +10,7 @@ describe DfcProvider::Api::CatalogItemsController, type: :controller do
   let!(:product) { create(:simple_product, supplier: enterprise ) }
   let!(:variant) { product.variants.first }
 
-  describe('.index') do
+  describe '.index' do
     context 'with authorization token' do
       before do
         request.headers['Authorization'] = 'Bearer 123456.abcdef.123456'
@@ -29,7 +29,7 @@ describe DfcProvider::Api::CatalogItemsController, type: :controller do
               before { api_get :index, enterprise_id: 'default' }
 
               it 'is successful' do
-                expect(response.status).to eq 200
+                expect(response).to be_successful
               end
 
               it 'renders the required content' do
@@ -47,7 +47,7 @@ describe DfcProvider::Api::CatalogItemsController, type: :controller do
 
               it 'returns not_found head' do
                 api_get :index, enterprise_id: enterprise.id
-                expect(response.status).to eq 404
+                expect(response).to be_not_found
               end
             end
           end
@@ -75,7 +75,7 @@ describe DfcProvider::Api::CatalogItemsController, type: :controller do
 
           it 'is not found' do
             api_get :index, enterprise_id: 'default'
-            expect(response.status).to eq 404
+            expect(response).to be_not_found
           end
         end
       end
@@ -87,7 +87,7 @@ describe DfcProvider::Api::CatalogItemsController, type: :controller do
             .and_return(nil)
 
           api_get :index, enterprise_id: 'default'
-          expect(response.status).to eq 401
+          expect(response.response_code).to eq(401)
         end
       end
     end
@@ -95,12 +95,12 @@ describe DfcProvider::Api::CatalogItemsController, type: :controller do
     context 'without an authorization token' do
       it 'returns unprocessable_entity head' do
         api_get :index, enterprise_id: enterprise.id
-        expect(response.status).to eq 422
+        expect(response).to be_unprocessable
       end
     end
   end
 
-  describe('.show') do
+  describe '.show' do
     context 'with authorization token' do
       before do
         request.headers['Authorization'] = 'Bearer 123456.abcdef.123456'
@@ -116,20 +116,16 @@ describe DfcProvider::Api::CatalogItemsController, type: :controller do
         context 'with an enterprise' do
           context 'given with an id' do
             before do
-              api_get :show,
-                      enterprise_id: enterprise.id,
-                      id: variant.id
+              api_get :show, enterprise_id: enterprise.id, id: variant.id
             end
 
             it 'is successful' do
-              expect(response.status).to eq 200
+              expect(response).to be_successful
             end
 
             it 'renders the required content' do
-              expect(response.body)
-                .to include('dfc:CatalogItem')
-              expect(response.body)
-                .to include("offers/#{variant.id}")
+              expect(response.body).to include('dfc:CatalogItem')
+              expect(response.body).to include("offers/#{variant.id}")
             end
           end
 
@@ -141,7 +137,7 @@ describe DfcProvider::Api::CatalogItemsController, type: :controller do
             end
 
             it 'is not found' do
-              expect(response.status).to eq 404
+              expect(response).to be_not_found
             end
           end
         end

engines/dfc_provider/spec/controllers/dfc_provider/api/enterprises_spec.rb

@@ -9,7 +9,7 @@ describe DfcProvider::Api::EnterprisesController, type: :controller do
   let!(:enterprise) { create(:distributor_enterprise, owner: user) }
   let!(:product) { create(:simple_product, supplier: enterprise ) }
 
-  describe('.show') do
+  describe '.show' do
     context 'with authorization token' do
       before do
         request.headers['Authorization'] = 'Bearer 123456.abcdef.123456'
@@ -27,7 +27,7 @@ describe DfcProvider::Api::EnterprisesController, type: :controller do
             before { api_get :show, id: 'default' }
 
             it 'is successful' do
-              expect(response.status).to eq 200
+              expect(response).to be_successful
             end
 
             it 'renders the required content' do
@@ -44,7 +44,7 @@ describe DfcProvider::Api::EnterprisesController, type: :controller do
             before { api_get :show, id: 999 }
 
             it 'is not found' do
-              expect(response.status).to eq 404
+              expect(response).to be_not_found
             end
           end
         end

engines/dfc_provider/spec/controllers/dfc_provider/api/persons_controller_spec.rb

@@ -7,7 +7,7 @@ describe DfcProvider::Api::PersonsController, type: :controller do
 
   let!(:user) { create(:user) }
 
-  describe('.show') do
+  describe '.show' do
     context 'with authorization token' do
       before do
         request.headers['Authorization'] = 'Bearer 123456.abcdef.123456'
@@ -21,13 +21,10 @@ describe DfcProvider::Api::PersonsController, type: :controller do
         end
 
         context 'given with an accessible id' do
-          before do
-            api_get :show,
-                    id: user.id
-          end
+          before { api_get :show, id: user.id }
 
           it 'is successful' do
-            expect(response.status).to eq 200
+            expect(response).to be_successful
           end
 
           it 'renders the required content' do
@@ -39,7 +36,7 @@ describe DfcProvider::Api::PersonsController, type: :controller do
           before { api_get :show, id: create(:user).id }
 
           it 'is not found' do
-            expect(response.status).to eq 404
+            expect(response).to be_not_found
           end
         end
       end

engines/dfc_provider/spec/controllers/dfc_provider/api/supplied_products_controller_spec.rb

@@ -10,7 +10,7 @@ describe DfcProvider::Api::SuppliedProductsController, type: :controller do
   let!(:product) { create(:simple_product, supplier: enterprise ) }
   let!(:variant) { product.variants.first }
 
-  describe('.show') do
+  describe '.show' do
     context 'with authorization token' do
       before do
         request.headers['Authorization'] = 'Bearer 123456.abcdef.123456'
@@ -26,26 +26,23 @@ describe DfcProvider::Api::SuppliedProductsController, type: :controller do
         context 'with an enterprise' do
           context 'given with an id' do
             before do
-              api_get :show,
-                      enterprise_id: 'default',
-                      id: variant.id
+              api_get :show, enterprise_id: 'default', id: variant.id
             end
 
             it 'is successful' do
-              expect(response.status).to eq 200
+              expect(response).to be_successful
             end
 
             it 'renders the required content' do
-              expect(response.body)
-                .to include(variant.name)
+              expect(response.body).to include(variant.name)
             end
           end
 
           context 'given with a wrong id' do
-            before { api_get :show, id: 999 }
+            before { api_get :show, enterprise_id: 'default', id: 999 }
 
             it 'is not found' do
-              expect(response.status).to eq 404
+              expect(response).to be_not_found
             end
           end
         end

engines/dfc_provider/spec/serializers/dfc_provider/catalog_item_serializer_spec.rb

@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DfcProvider::CatalogItemSerializer do
+  let!(:product) { create(:simple_product ) }
+  let!(:variant) { product.variants.first }
+
+  subject { described_class.new(variant) }
+
+  describe '#id' do
+    let(:catalog_item_id) {
+      [
+        'http://test.host/api/dfc_provider',
+        'enterprises',
+        product.supplier_id,
+        'catalog_items',
+        variant.id
+      ].join('/')
+    }
+
+    it 'returns the expected value' do
+      expect(subject.id).to eq(catalog_item_id)
+    end
+  end
+
+  describe '#references' do
+    let(:supplied_product_id) {
+      [
+        'http://test.host/api/dfc_provider',
+        'enterprises',
+        product.supplier_id,
+        'supplied_products',
+        variant.id
+      ].join('/')
+    }
+
+    it 'returns the expected value' do
+      expect(subject.references).to eq(
+        "@id" => supplied_product_id,
+        "@type" => "@id"
+      )
+    end
+  end
+end

engines/dfc_provider/spec/serializers/dfc_provider/supplied_product_serializer_spec.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DfcProvider::SuppliedProductSerializer do
+  let!(:product) { create(:simple_product ) }
+  let!(:variant) { product.variants.first }
+
+  subject { described_class.new(variant) }
+
+  describe '#id' do
+    let(:supplied_product_id) {
+      [
+        'http://test.host/api/dfc_provider',
+        'enterprises',
+        product.supplier_id,
+        'supplied_products',
+        variant.id
+      ].join('/')
+    }
+
+    it 'returns the expected value' do
+      expect(subject.id).to eq(supplied_product_id)
+    end
+  end
+
+  describe '#unit' do
+    it 'returns the rdfs label value' do
+      expect(subject.unit).to eq(
+        '@id' => '/unit/piece',
+        'rdfs:label' => 'piece'
+      )
+    end
+  end
+end