Merge pull request #11230 from macanudo527/fix_products_controller

Add Query Object to Product Model to Fix Metrics/ClassLength

.rubocop_todo.yml

@@ -194,7 +194,6 @@ Metrics/ClassLength:
     - 'app/controllers/admin/resource_controller.rb'
     - 'app/controllers/admin/schedules_controller.rb'
     - 'app/controllers/admin/subscriptions_controller.rb'
-    - 'app/controllers/api/v0/products_controller.rb'
     - 'app/controllers/application_controller.rb'
     - 'app/controllers/payment_gateways/paypal_controller.rb'
     - 'app/controllers/spree/admin/orders_controller.rb'

app/controllers/api/v0/products_controller.rb

@@ -13,7 +13,7 @@ module Api
       skip_authorization_check only: [:show, :bulk_products, :overridable]
 
       def show
-        @product = find_product(params[:id])
+        @product = product_finder.find_product
         render json: @product, serializer: Api::Admin::ProductSerializer
       end
 
@@ -30,7 +30,7 @@ module Api
 
       def update
         authorize! :update, Spree::Product
-        @product = find_product(params[:id])
+        @product = product_finder.find_product
         if @product.update(product_params)
           render json: @product, serializer: Api::Admin::ProductSerializer, status: :ok
         else
@@ -40,36 +40,20 @@ module Api
 
       def destroy
         authorize! :delete, Spree::Product
-        @product = find_product(params[:id])
+        @product = product_finder.find_product
         authorize! :delete, @product
         @product.destroy
         render json: @product, serializer: Api::Admin::ProductSerializer, status: :no_content
       end
 
       def bulk_products
-        product_query = OpenFoodNetwork::Permissions.
-          new(current_api_user).
-          editable_products.
-          merge(product_scope)
-
-        if params[:import_date].present?
-          product_query = product_query.
-            imported_on(params[:import_date]).
-            group_by_products_id
-        end
-
-        @products = product_query.
-          ransack(query_params_with_defaults).
-          result
+        @products = product_finder.bulk_products
 
         render_paged_products @products
       end
 
       def overridable
-        producer_ids = OpenFoodNetwork::Permissions.new(current_api_user).
-          variant_override_producers.by_name.select('enterprises.id')
-
-        @products = paged_products_for_producers producer_ids
+        @products = product_finder.paged_products_for_producers
 
         render_paged_products @products, ::Api::Admin::ProductSimpleSerializer
       end
@@ -78,7 +62,7 @@ module Api
       #
       def clone
         authorize! :create, Spree::Product
-        original_product = find_product(params[:product_id])
+        original_product = product_finder.find_product_to_be_cloned
         authorize! :update, original_product
 
         @product = original_product.duplicate
@@ -88,37 +72,8 @@ module Api
 
       private
 
-      def find_product(id)
-        product_scope.find(id)
-      end
-
-      def product_scope
-        if current_api_user.has_spree_role?("admin") || current_api_user.enterprises.present?
-          scope = Spree::Product
-          if params[:show_deleted]
-            scope = scope.with_deleted
-          end
-        else
-          scope = Spree::Product.active
-        end
-
-        scope.includes(product_query_includes)
-      end
-
-      def product_query_includes
-        [
-          image: { attachment_attachment: :blob },
-          variants: [:default_price, :stock_locations, :stock_items, :variant_overrides]
-        ]
-      end
-
-      def paged_products_for_producers(producer_ids)
-        Spree::Product.where(nil).
-          merge(product_scope).
-          includes(variants: [:product, :default_price, :stock_items]).
-          where(supplier_id: producer_ids).
-          by_producer.by_name.
-          ransack(params[:q]).result
+      def product_finder
+        ProductScopeQuery.new(current_api_user, params)
       end
 
       def render_paged_products(products, product_serializer = ::Api::Admin::ProductSerializer)
@@ -135,10 +90,6 @@ module Api
         }
       end
 
-      def query_params_with_defaults
-        (params[:q] || {}).reverse_merge(s: 'created_at desc')
-      end
-
       def product_params
         @product_params ||=
           params.permit(product: PermittedAttributes::Product.attributes)[:product].to_h

app/queries/product_scope_query.rb

@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+class ProductScopeQuery
+  def initialize(user, params)
+    @user = user
+    @params = params
+  end
+
+  def bulk_products
+    product_query = OpenFoodNetwork::Permissions.
+      new(@user).
+      editable_products.
+      merge(product_scope)
+
+    if @params[:import_date].present?
+      product_query = product_query.
+        imported_on(@params[:import_date]).
+        group_by_products_id
+    end
+
+    product_query.
+      ransack(query_params_with_defaults).
+      result
+  end
+
+  def find_product
+    product_scope.find(@params[:id])
+  end
+
+  def find_product_to_be_cloned
+    product_scope.find(@params[:product_id])
+  end
+
+  def paged_products_for_producers
+    producer_ids = OpenFoodNetwork::Permissions.new(@user).
+      variant_override_producers.by_name.select('enterprises.id')
+
+    Spree::Product.where(nil).
+      merge(product_scope).
+      includes(variants: [:product, :default_price, :stock_items]).
+      where(supplier_id: producer_ids).
+      by_producer.by_name.
+      ransack(@params[:q]).result
+  end
+
+  def product_scope
+    if @user.has_spree_role?("admin") || @user.enterprises.present?
+      scope = Spree::Product
+      if @params[:show_deleted]
+        scope = scope.with_deleted
+      end
+    else
+      scope = Spree::Product.active
+    end
+
+    scope.includes(product_query_includes)
+  end
+
+  def product_query_includes
+    [
+      image: { attachment_attachment: :blob },
+      variants: [:default_price, :stock_locations, :stock_items, :variant_overrides]
+    ]
+  end
+
+  def query_params_with_defaults
+    (@params[:q] || {}).reverse_merge(s: 'created_at desc')
+  end
+end

spec/queries/product_scope_query_spec.rb

@@ -0,0 +1,75 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ProductScopeQuery do
+  let!(:taxon) { create(:taxon) }
+  let(:supplier) { create(:supplier_enterprise) }
+  let(:supplier2) { create(:supplier_enterprise) }
+  let!(:product) { create(:product, supplier:, primary_taxon: taxon) }
+  let!(:product2) { create(:product, supplier: supplier2, primary_taxon: taxon) }
+  let!(:current_api_user) { supplier_enterprise_user(supplier) }
+
+  before { current_api_user.enterprise_roles.create(enterprise: supplier2) }
+
+  describe 'bulk update' do
+    let!(:product3) { create(:product, supplier: supplier2) }
+
+    it "returns a list of products" do
+      expect(ProductScopeQuery.new(current_api_user, {}).bulk_products)
+        .to include(product, product2, product3)
+    end
+
+    it "filters results by supplier" do
+      subject = ProductScopeQuery
+        .new(current_api_user, { q: { supplier_id_eq: supplier.id } }).bulk_products
+
+      expect(subject).to include(product)
+      expect(subject).not_to include(product2, product3)
+    end
+
+    it "filters results by product category" do
+      subject = ProductScopeQuery
+        .new(current_api_user, { q: { primary_taxon_id_eq: taxon.id } }).bulk_products
+
+      expect(subject).to include(product, product2)
+      expect(subject).not_to include(product3)
+    end
+
+    it "filters results by import_date" do
+      product.variants.first.update_attribute :import_date, 1.day.ago
+      product2.variants.first.update_attribute :import_date, 2.days.ago
+      product3.variants.first.update_attribute :import_date, 1.day.ago
+
+      subject = ProductScopeQuery
+        .new(current_api_user, { import_date: 1.day.ago.to_date.to_s }).bulk_products
+
+      expect(subject).to include(product, product3)
+      expect(subject).not_to include(product2)
+    end
+  end
+
+  describe 'finder functions' do
+    describe 'find_product' do
+      subject(:query) { ProductScopeQuery.new(current_api_user, { id: product.id }) }
+      it 'finds product' do
+        expect(query.find_product).to eq(product)
+      end
+    end
+
+    describe 'find_product_to_be_cloned' do
+      subject(:query) { ProductScopeQuery.new(current_api_user, { product_id: product2.id }) }
+      it 'finds product to be cloned' do
+        expect(query.find_product_to_be_cloned).to eq(product2)
+      end
+    end
+  end
+
+  private
+
+  def supplier_enterprise_user(enterprise)
+    user = create(:user)
+    user.enterprise_roles.create(enterprise:)
+    user
+  end
+end