diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 2a88db9bfe..b874419ffa 100644 --- a/.rubocop_todo.yml +++ b/.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' diff --git a/app/controllers/api/v0/products_controller.rb b/app/controllers/api/v0/products_controller.rb index 6b6baa311a..eead37ca49 100644 --- a/app/controllers/api/v0/products_controller.rb +++ b/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 diff --git a/app/queries/product_scope_query.rb b/app/queries/product_scope_query.rb new file mode 100644 index 0000000000..bc8b6953b2 --- /dev/null +++ b/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 diff --git a/spec/queries/product_scope_query_spec.rb b/spec/queries/product_scope_query_spec.rb new file mode 100755 index 0000000000..f8bde2c8da --- /dev/null +++ b/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