From d26a0b6b73269c69c0f36914c2cf9f7fc593f732 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 18 Jul 2019 17:14:04 +0100 Subject: [PATCH 01/36] Bring from spree_api the api controllers that are overriden in OFN so that we can merge the original and the override afterwards --- .../spree/api/products_controller.rb | 61 ++++++++++++ .../spree/api/shipments_controller.rb | 93 +++++++++++++++++++ .../spree/api/variants_controller.rb | 75 +++++++++++++++ 3 files changed, 229 insertions(+) create mode 100644 app/controllers/spree/api/products_controller.rb create mode 100644 app/controllers/spree/api/shipments_controller.rb create mode 100644 app/controllers/spree/api/variants_controller.rb diff --git a/app/controllers/spree/api/products_controller.rb b/app/controllers/spree/api/products_controller.rb new file mode 100644 index 0000000000..2e2574091c --- /dev/null +++ b/app/controllers/spree/api/products_controller.rb @@ -0,0 +1,61 @@ +module Spree + module Api + class ProductsController < Spree::Api::BaseController + respond_to :json + + def index + if params[:ids] + @products = product_scope.where(:id => params[:ids]) + else + @products = product_scope.ransack(params[:q]).result + end + + @products = @products.page(params[:page]).per(params[:per_page]) + + respond_with(@products) + end + + def show + @product = find_product(params[:id]) + respond_with(@product) + end + + def new + end + + def create + authorize! :create, Product + params[:product][:available_on] ||= Time.now + @product = Product.new(params[:product]) + begin + if @product.save + respond_with(@product, :status => 201, :default_template => :show) + else + invalid_resource!(@product) + end + rescue ActiveRecord::RecordNotUnique + @product.permalink = nil + retry + end + end + + def update + authorize! :update, Product + @product = find_product(params[:id]) + if @product.update_attributes(params[:product]) + respond_with(@product, :status => 200, :default_template => :show) + else + invalid_resource!(@product) + end + end + + def destroy + authorize! :delete, Product + @product = find_product(params[:id]) + @product.update_attribute(:deleted_at, Time.now) + @product.variants_including_master.update_all(:deleted_at => Time.now) + respond_with(@product, :status => 204) + end + end + end +end diff --git a/app/controllers/spree/api/shipments_controller.rb b/app/controllers/spree/api/shipments_controller.rb new file mode 100644 index 0000000000..ad0b71755e --- /dev/null +++ b/app/controllers/spree/api/shipments_controller.rb @@ -0,0 +1,93 @@ +module Spree + module Api + class ShipmentsController < Spree::Api::BaseController + respond_to :json + + before_filter :find_order + before_filter :find_and_update_shipment, :only => [:ship, :ready, :add, :remove] + + def create + variant = Spree::Variant.find(params[:variant_id]) + quantity = params[:quantity].to_i + @shipment = @order.shipments.create(:stock_location_id => params[:stock_location_id]) + @order.contents.add(variant, quantity, nil, @shipment) + + @shipment.refresh_rates + @shipment.save! + + respond_with(@shipment.reload, :default_template => :show) + end + + def update + authorize! :read, Shipment + @shipment = @order.shipments.find_by_number!(params[:id]) + params[:shipment] ||= [] + unlock = params[:shipment].delete(:unlock) + + if unlock == 'yes' + @shipment.adjustment.open + end + + @shipment.update_attributes(params[:shipment]) + + if unlock == 'yes' + @shipment.adjustment.close + end + + @shipment.reload + respond_with(@shipment, :default_template => :show) + end + + def ready + authorize! :read, Shipment + unless @shipment.ready? + if @shipment.can_ready? + @shipment.ready! + else + render "spree/api/shipments/cannot_ready_shipment", :status => 422 and return + end + end + respond_with(@shipment, :default_template => :show) + end + + def ship + authorize! :read, Shipment + unless @shipment.shipped? + @shipment.ship! + end + respond_with(@shipment, :default_template => :show) + end + + def add + variant = Spree::Variant.find(params[:variant_id]) + quantity = params[:quantity].to_i + + @order.contents.add(variant, quantity, nil, @shipment) + + respond_with(@shipment, :default_template => :show) + end + + def remove + variant = Spree::Variant.find(params[:variant_id]) + quantity = params[:quantity].to_i + + @order.contents.remove(variant, quantity, @shipment) + @shipment.reload if @shipment.persisted? + respond_with(@shipment, :default_template => :show) + end + + private + + def find_order + @order = Spree::Order.find_by_number!(params[:order_id]) + authorize! :read, @order + end + + def find_and_update_shipment + @shipment = @order.shipments.find_by_number!(params[:id]) + @shipment.update_attributes(params[:shipment]) + @shipment.reload + end + end + end +end diff --git a/app/controllers/spree/api/variants_controller.rb b/app/controllers/spree/api/variants_controller.rb new file mode 100644 index 0000000000..7086dd9740 --- /dev/null +++ b/app/controllers/spree/api/variants_controller.rb @@ -0,0 +1,75 @@ +module Spree + module Api + class VariantsController < Spree::Api::BaseController + respond_to :json + + before_filter :product + + def index + @variants = scope.includes(:option_values).ransack(params[:q]).result. + page(params[:page]).per(params[:per_page]) + respond_with(@variants) + end + + def show + @variant = scope.includes(:option_values).find(params[:id]) + respond_with(@variant) + end + + def new + end + + def create + authorize! :create, Variant + @variant = scope.new(params[:variant]) + if @variant.save + respond_with(@variant, :status => 201, :default_template => :show) + else + invalid_resource!(@variant) + end + end + + def update + authorize! :update, Variant + @variant = scope.find(params[:id]) + if @variant.update_attributes(params[:variant]) + respond_with(@variant, :status => 200, :default_template => :show) + else + invalid_resource!(@product) + end + end + + def destroy + authorize! :delete, Variant + @variant = scope.find(params[:id]) + @variant.destroy + respond_with(@variant, :status => 204) + end + + private + def product + @product ||= Spree::Product.find_by_permalink(params[:product_id]) if params[:product_id] + end + + def scope + if @product + unless current_api_user.has_spree_role?("admin") || params[:show_deleted] + variants = @product.variants_including_master + else + variants = @product.variants_including_master.with_deleted + end + else + variants = Variant.scoped + if current_api_user.has_spree_role?("admin") + unless params[:show_deleted] + variants = Variant.active + end + else + variants = variants.active + end + end + variants + end + end + end +end From c5bcef6ae4fa593064ada94cf8f96fea9a772dd5 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sat, 20 Jul 2019 10:49:33 +0100 Subject: [PATCH 02/36] Delete unused spree/api/line_items_controller_decorator.rb --- .../api/line_items_controller_decorator.rb | 13 ------- .../spree/api/line_items_controller_spec.rb | 35 ------------------- 2 files changed, 48 deletions(-) delete mode 100644 app/controllers/spree/api/line_items_controller_decorator.rb delete mode 100644 spec/controllers/spree/api/line_items_controller_spec.rb diff --git a/app/controllers/spree/api/line_items_controller_decorator.rb b/app/controllers/spree/api/line_items_controller_decorator.rb deleted file mode 100644 index 93e4099d2a..0000000000 --- a/app/controllers/spree/api/line_items_controller_decorator.rb +++ /dev/null @@ -1,13 +0,0 @@ -Spree::Api::LineItemsController.class_eval do - around_filter :apply_enterprise_fees_with_lock, only: :update - - private - - def apply_enterprise_fees_with_lock - authorize! :read, order - order.with_lock do - yield - order.update_distribution_charge! - end - end -end diff --git a/spec/controllers/spree/api/line_items_controller_spec.rb b/spec/controllers/spree/api/line_items_controller_spec.rb deleted file mode 100644 index cda1a5a81e..0000000000 --- a/spec/controllers/spree/api/line_items_controller_spec.rb +++ /dev/null @@ -1,35 +0,0 @@ -require 'spec_helper' - -module Spree - describe Spree::Api::LineItemsController, type: :controller do - render_views - - before do - allow(controller).to receive(:spree_current_user) { current_api_user } - end - - # test that when a line item is updated, an order's fees are updated too - context "as an admin user" do - sign_in_as_admin! - - let(:order) { FactoryBot.create(:order, state: 'complete', completed_at: Time.zone.now) } - let(:line_item) { FactoryBot.create(:line_item_with_shipment, order: order, final_weight_volume: 500) } - - context "as a line item is updated" do - before { allow(controller).to receive(:order) { order } } - - it "update distribution charge on the order" do - line_item_params = { - order_id: order.number, - id: line_item.id, - line_item: { id: line_item.id, final_weight_volume: 520 }, - format: :json - } - - expect(order).to receive(:update_distribution_charge!) - spree_post :update, line_item_params - end - end - end - end -end From 660ce92c27f68370fba638b72da08633a7d418c3 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 18 Jul 2019 17:20:40 +0100 Subject: [PATCH 03/36] Merge spree api controllers and its decorators --- .../spree/api/products_controller.rb | 85 ++++++++++++++++++ .../api/products_controller_decorator.rb | 86 ------------------- .../spree/api/shipments_controller.rb | 28 ++++-- .../api/shipments_controller_decorator.rb | 47 ---------- .../spree/api/variants_controller.rb | 8 ++ .../api/variants_controller_decorator.rb | 9 -- 6 files changed, 114 insertions(+), 149 deletions(-) delete mode 100644 app/controllers/spree/api/products_controller_decorator.rb delete mode 100644 app/controllers/spree/api/shipments_controller_decorator.rb delete mode 100644 app/controllers/spree/api/variants_controller_decorator.rb diff --git a/app/controllers/spree/api/products_controller.rb b/app/controllers/spree/api/products_controller.rb index 2e2574091c..fb9b76d654 100644 --- a/app/controllers/spree/api/products_controller.rb +++ b/app/controllers/spree/api/products_controller.rb @@ -1,3 +1,5 @@ +require 'open_food_network/permissions' + module Spree module Api class ProductsController < Spree::Api::BaseController @@ -56,6 +58,89 @@ module Spree @product.variants_including_master.update_all(:deleted_at => Time.now) respond_with(@product, :status => 204) end + + def managed + authorize! :admin, Spree::Product + authorize! :read, Spree::Product + + @products = product_scope.ransack(params[:q]).result.managed_by(current_api_user).page(params[:page]).per(params[:per_page]) + respond_with(@products, default_template: :index) + end + + # TODO: This should be named 'managed'. Is the action above used? Maybe we should remove it. + def bulk_products + @products = OpenFoodNetwork::Permissions.new(current_api_user).editable_products. + merge(product_scope). + order('created_at DESC'). + ransack(params[:q]).result. + page(params[:page]).per(params[:per_page]) + + render_paged_products @products + end + + def overridable + producers = OpenFoodNetwork::Permissions.new(current_api_user). + variant_override_producers.by_name + + @products = paged_products_for_producers producers + + render_paged_products @products + end + + def soft_delete + authorize! :delete, Spree::Product + @product = find_product(params[:product_id]) + authorize! :delete, @product + @product.destroy + respond_with(@product, status: 204) + end + + # POST /api/products/:product_id/clone + # + def clone + authorize! :create, Spree::Product + original_product = find_product(params[:product_id]) + authorize! :update, original_product + + @product = original_product.duplicate + + respond_with(@product, status: 201, default_template: :show) + end + + private + + # Copied and modified from Spree::Api::BaseController to allow + # enterprise users to access inactive products + def product_scope + if current_api_user.has_spree_role?("admin") || current_api_user.enterprises.present? # This line modified + scope = Spree::Product + if params[:show_deleted] + scope = scope.with_deleted + end + else + scope = Spree::Product.active + end + + scope.includes(:master) + end + + def paged_products_for_producers(producers) + Spree::Product.scoped. + merge(product_scope). + where(supplier_id: producers). + by_producer.by_name. + ransack(params[:q]).result. + page(params[:page]).per(params[:per_page]) + end + + def render_paged_products(products) + serializer = ActiveModel::ArraySerializer.new( + products, + each_serializer: Api::Admin::ProductSerializer + ) + + render text: { products: serializer, pages: products.num_pages }.to_json + end end end end diff --git a/app/controllers/spree/api/products_controller_decorator.rb b/app/controllers/spree/api/products_controller_decorator.rb deleted file mode 100644 index 906cbb3d0a..0000000000 --- a/app/controllers/spree/api/products_controller_decorator.rb +++ /dev/null @@ -1,86 +0,0 @@ -require 'open_food_network/permissions' - -Spree::Api::ProductsController.class_eval do - def managed - authorize! :admin, Spree::Product - authorize! :read, Spree::Product - - @products = product_scope.ransack(params[:q]).result.managed_by(current_api_user).page(params[:page]).per(params[:per_page]) - respond_with(@products, default_template: :index) - end - - # TODO: This should be named 'managed'. Is the action above used? Maybe we should remove it. - def bulk_products - @products = OpenFoodNetwork::Permissions.new(current_api_user).editable_products. - merge(product_scope). - order('created_at DESC'). - ransack(params[:q]).result. - page(params[:page]).per(params[:per_page]) - - render_paged_products @products - end - - def overridable - producers = OpenFoodNetwork::Permissions.new(current_api_user). - variant_override_producers.by_name - - @products = paged_products_for_producers producers - - render_paged_products @products - end - - def soft_delete - authorize! :delete, Spree::Product - @product = find_product(params[:product_id]) - authorize! :delete, @product - @product.destroy - respond_with(@product, status: 204) - end - - # POST /api/products/:product_id/clone - # - def clone - authorize! :create, Spree::Product - original_product = find_product(params[:product_id]) - authorize! :update, original_product - - @product = original_product.duplicate - - respond_with(@product, status: 201, default_template: :show) - end - - private - - # Copied and modified from Spree::Api::BaseController to allow - # enterprise users to access inactive products - def product_scope - if current_api_user.has_spree_role?("admin") || current_api_user.enterprises.present? # This line modified - scope = Spree::Product - if params[:show_deleted] - scope = scope.with_deleted - end - else - scope = Spree::Product.active - end - - scope.includes(:master) - end - - def paged_products_for_producers(producers) - Spree::Product.scoped. - merge(product_scope). - where(supplier_id: producers). - by_producer.by_name. - ransack(params[:q]).result. - page(params[:page]).per(params[:per_page]) - end - - def render_paged_products(products) - serializer = ActiveModel::ArraySerializer.new( - products, - each_serializer: Api::Admin::ProductSerializer - ) - - render text: { products: serializer, pages: products.num_pages }.to_json - end -end diff --git a/app/controllers/spree/api/shipments_controller.rb b/app/controllers/spree/api/shipments_controller.rb index ad0b71755e..07f2cf3f7b 100644 --- a/app/controllers/spree/api/shipments_controller.rb +++ b/app/controllers/spree/api/shipments_controller.rb @@ -1,3 +1,5 @@ +require 'open_food_network/scope_variant_to_hub' + module Spree module Api class ShipmentsController < Spree::Api::BaseController @@ -7,15 +9,16 @@ module Spree before_filter :find_and_update_shipment, :only => [:ship, :ready, :add, :remove] def create - variant = Spree::Variant.find(params[:variant_id]) + variant = scoped_variant(params[:variant_id]) quantity = params[:quantity].to_i - @shipment = @order.shipments.create(:stock_location_id => params[:stock_location_id]) + @shipment = get_or_create_shipment(params[:stock_location_id]) + @order.contents.add(variant, quantity, nil, @shipment) @shipment.refresh_rates @shipment.save! - respond_with(@shipment.reload, :default_template => :show) + respond_with(@shipment.reload, default_template: :show) end def update @@ -59,21 +62,22 @@ module Spree end def add - variant = Spree::Variant.find(params[:variant_id]) + variant = scoped_variant(params[:variant_id]) quantity = params[:quantity].to_i @order.contents.add(variant, quantity, nil, @shipment) - respond_with(@shipment, :default_template => :show) + respond_with(@shipment, default_template: :show) end def remove - variant = Spree::Variant.find(params[:variant_id]) + variant = scoped_variant(params[:variant_id]) quantity = params[:quantity].to_i @order.contents.remove(variant, quantity, @shipment) @shipment.reload if @shipment.persisted? - respond_with(@shipment, :default_template => :show) + + respond_with(@shipment, default_template: :show) end private @@ -88,6 +92,16 @@ module Spree @shipment.update_attributes(params[:shipment]) @shipment.reload end + + def scoped_variant(variant_id) + variant = Spree::Variant.find(variant_id) + OpenFoodNetwork::ScopeVariantToHub.new(@order.distributor).scope(variant) + variant + end + + def get_or_create_shipment(stock_location_id) + @order.shipment || @order.shipments.create(stock_location_id: stock_location_id) + end end end end diff --git a/app/controllers/spree/api/shipments_controller_decorator.rb b/app/controllers/spree/api/shipments_controller_decorator.rb deleted file mode 100644 index 6fb5e242ec..0000000000 --- a/app/controllers/spree/api/shipments_controller_decorator.rb +++ /dev/null @@ -1,47 +0,0 @@ -require 'open_food_network/scope_variant_to_hub' - -Spree::Api::ShipmentsController.class_eval do - def create - variant = scoped_variant(params[:variant_id]) - quantity = params[:quantity].to_i - @shipment = get_or_create_shipment(params[:stock_location_id]) - - @order.contents.add(variant, quantity, nil, @shipment) - - @shipment.refresh_rates - @shipment.save! - - respond_with(@shipment.reload, default_template: :show) - end - - def add - variant = scoped_variant(params[:variant_id]) - quantity = params[:quantity].to_i - - @order.contents.add(variant, quantity, nil, @shipment) - - respond_with(@shipment, default_template: :show) - end - - def remove - variant = scoped_variant(params[:variant_id]) - quantity = params[:quantity].to_i - - @order.contents.remove(variant, quantity, @shipment) - @shipment.reload if @shipment.persisted? - - respond_with(@shipment, default_template: :show) - end - - private - - def scoped_variant(variant_id) - variant = Spree::Variant.find(variant_id) - OpenFoodNetwork::ScopeVariantToHub.new(@order.distributor).scope(variant) - variant - end - - def get_or_create_shipment(stock_location_id) - @order.shipment || @order.shipments.create(stock_location_id: stock_location_id) - end -end diff --git a/app/controllers/spree/api/variants_controller.rb b/app/controllers/spree/api/variants_controller.rb index 7086dd9740..2e7a55c37d 100644 --- a/app/controllers/spree/api/variants_controller.rb +++ b/app/controllers/spree/api/variants_controller.rb @@ -39,6 +39,14 @@ module Spree end end + def soft_delete + @variant = scope.find(params[:variant_id]) + authorize! :delete, @variant + + VariantDeleter.new.delete(@variant) + respond_with @variant, status: 204 + end + def destroy authorize! :delete, Variant @variant = scope.find(params[:id]) diff --git a/app/controllers/spree/api/variants_controller_decorator.rb b/app/controllers/spree/api/variants_controller_decorator.rb deleted file mode 100644 index 0c5a1dd2a6..0000000000 --- a/app/controllers/spree/api/variants_controller_decorator.rb +++ /dev/null @@ -1,9 +0,0 @@ -Spree::Api::VariantsController.class_eval do - def soft_delete - @variant = scope.find(params[:variant_id]) - authorize! :delete, @variant - - VariantDeleter.new.delete(@variant) - respond_with @variant, status: 204 - end -end From 6abbdecb9797453e3aad4a7a0a8d53aa2e995a37 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 18 Jul 2019 17:30:16 +0100 Subject: [PATCH 04/36] Fix the easy rubocop issues in the new spree api controllers --- .../spree/api/products_controller.rb | 23 +++++----- .../spree/api/shipments_controller.rb | 11 +++-- .../spree/api/variants_controller.rb | 46 +++++++++---------- 3 files changed, 41 insertions(+), 39 deletions(-) diff --git a/app/controllers/spree/api/products_controller.rb b/app/controllers/spree/api/products_controller.rb index fb9b76d654..fbad83daf7 100644 --- a/app/controllers/spree/api/products_controller.rb +++ b/app/controllers/spree/api/products_controller.rb @@ -7,7 +7,7 @@ module Spree def index if params[:ids] - @products = product_scope.where(:id => params[:ids]) + @products = product_scope.where(id: params[:ids]) else @products = product_scope.ransack(params[:q]).result end @@ -22,16 +22,15 @@ module Spree respond_with(@product) end - def new - end + def new; end def create authorize! :create, Product - params[:product][:available_on] ||= Time.now + params[:product][:available_on] ||= Time.zone.now @product = Product.new(params[:product]) begin if @product.save - respond_with(@product, :status => 201, :default_template => :show) + respond_with(@product, status: 201, default_template: :show) else invalid_resource!(@product) end @@ -45,7 +44,7 @@ module Spree authorize! :update, Product @product = find_product(params[:id]) if @product.update_attributes(params[:product]) - respond_with(@product, :status => 200, :default_template => :show) + respond_with(@product, status: 200, default_template: :show) else invalid_resource!(@product) end @@ -54,16 +53,17 @@ module Spree def destroy authorize! :delete, Product @product = find_product(params[:id]) - @product.update_attribute(:deleted_at, Time.now) - @product.variants_including_master.update_all(:deleted_at => Time.now) - respond_with(@product, :status => 204) + @product.update_attribute(:deleted_at, Time.zone.now) + @product.variants_including_master.update_all(deleted_at: Time.zone.now) + respond_with(@product, status: 204) end def managed authorize! :admin, Spree::Product authorize! :read, Spree::Product - @products = product_scope.ransack(params[:q]).result.managed_by(current_api_user).page(params[:page]).per(params[:per_page]) + @products = product_scope. + ransack(params[:q]).result.managed_by(current_api_user).page(params[:page]).per(params[:per_page]) respond_with(@products, default_template: :index) end @@ -112,7 +112,8 @@ module Spree # Copied and modified from Spree::Api::BaseController to allow # enterprise users to access inactive products def product_scope - if current_api_user.has_spree_role?("admin") || current_api_user.enterprises.present? # This line modified + # This line modified + if current_api_user.has_spree_role?("admin") || current_api_user.enterprises.present? scope = Spree::Product if params[:show_deleted] scope = scope.with_deleted diff --git a/app/controllers/spree/api/shipments_controller.rb b/app/controllers/spree/api/shipments_controller.rb index 07f2cf3f7b..4963c204a4 100644 --- a/app/controllers/spree/api/shipments_controller.rb +++ b/app/controllers/spree/api/shipments_controller.rb @@ -6,7 +6,7 @@ module Spree respond_to :json before_filter :find_order - before_filter :find_and_update_shipment, :only => [:ship, :ready, :add, :remove] + before_filter :find_and_update_shipment, only: [:ship, :ready, :add, :remove] def create variant = scoped_variant(params[:variant_id]) @@ -38,7 +38,7 @@ module Spree end @shipment.reload - respond_with(@shipment, :default_template => :show) + respond_with(@shipment, default_template: :show) end def ready @@ -47,10 +47,11 @@ module Spree if @shipment.can_ready? @shipment.ready! else - render "spree/api/shipments/cannot_ready_shipment", :status => 422 and return + render "spree/api/shipments/cannot_ready_shipment", status: :unprocessable_entity + return end end - respond_with(@shipment, :default_template => :show) + respond_with(@shipment, default_template: :show) end def ship @@ -58,7 +59,7 @@ module Spree unless @shipment.shipped? @shipment.ship! end - respond_with(@shipment, :default_template => :show) + respond_with(@shipment, default_template: :show) end def add diff --git a/app/controllers/spree/api/variants_controller.rb b/app/controllers/spree/api/variants_controller.rb index 2e7a55c37d..fe0fa1d793 100644 --- a/app/controllers/spree/api/variants_controller.rb +++ b/app/controllers/spree/api/variants_controller.rb @@ -16,14 +16,13 @@ module Spree respond_with(@variant) end - def new - end + def new; end def create authorize! :create, Variant @variant = scope.new(params[:variant]) if @variant.save - respond_with(@variant, :status => 201, :default_template => :show) + respond_with(@variant, status: 201, default_template: :show) else invalid_resource!(@variant) end @@ -33,7 +32,7 @@ module Spree authorize! :update, Variant @variant = scope.find(params[:id]) if @variant.update_attributes(params[:variant]) - respond_with(@variant, :status => 200, :default_template => :show) + respond_with(@variant, status: 200, default_template: :show) else invalid_resource!(@product) end @@ -51,33 +50,34 @@ module Spree authorize! :delete, Variant @variant = scope.find(params[:id]) @variant.destroy - respond_with(@variant, :status => 204) + respond_with(@variant, status: 204) end private - def product - @product ||= Spree::Product.find_by_permalink(params[:product_id]) if params[:product_id] - end - def scope - if @product - unless current_api_user.has_spree_role?("admin") || params[:show_deleted] - variants = @product.variants_including_master - else - variants = @product.variants_including_master.with_deleted + def product + @product ||= Spree::Product.find_by_permalink(params[:product_id]) if params[:product_id] + end + + def scope + if @product + unless current_api_user.has_spree_role?("admin") || params[:show_deleted] + variants = @product.variants_including_master + else + variants = @product.variants_including_master.with_deleted + end + else + variants = Variant.scoped + if current_api_user.has_spree_role?("admin") + unless params[:show_deleted] + variants = Variant.active end else - variants = Variant.scoped - if current_api_user.has_spree_role?("admin") - unless params[:show_deleted] - variants = Variant.active - end - else - variants = variants.active - end + variants = variants.active end - variants end + variants + end end end end From 9d40ee49e6e3df13ccf4d7166f9eca7657869805 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 18 Jul 2019 17:33:35 +0100 Subject: [PATCH 05/36] Bring spree/api/base_controller from spree_api --- app/controllers/spree/api/base_controller.rb | 133 +++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 app/controllers/spree/api/base_controller.rb diff --git a/app/controllers/spree/api/base_controller.rb b/app/controllers/spree/api/base_controller.rb new file mode 100644 index 0000000000..f447beef20 --- /dev/null +++ b/app/controllers/spree/api/base_controller.rb @@ -0,0 +1,133 @@ +require_dependency 'spree/api/controller_setup' + +module Spree + module Api + class BaseController < ActionController::Metal + include Spree::Api::ControllerSetup + include Spree::Core::ControllerHelpers::SSL + include ::ActionController::Head + + self.responder = Spree::Api::Responders::AppResponder + + respond_to :json + + attr_accessor :current_api_user + + before_filter :set_content_type + before_filter :check_for_user_or_api_key, :if => :requires_authentication? + before_filter :authenticate_user + after_filter :set_jsonp_format + + rescue_from Exception, :with => :error_during_processing + rescue_from CanCan::AccessDenied, :with => :unauthorized + rescue_from ActiveRecord::RecordNotFound, :with => :not_found + + helper Spree::Api::ApiHelpers + + ssl_allowed + + def set_jsonp_format + if params[:callback] && request.get? + self.response_body = "#{params[:callback]}(#{self.response_body})" + headers["Content-Type"] = 'application/javascript' + end + end + + def map_nested_attributes_keys(klass, attributes) + nested_keys = klass.nested_attributes_options.keys + attributes.inject({}) do |h, (k,v)| + key = nested_keys.include?(k.to_sym) ? "#{k}_attributes" : k + h[key] = v + h + end.with_indifferent_access + end + + private + + def set_content_type + content_type = case params[:format] + when "json" + "application/json" + when "xml" + "text/xml" + end + headers["Content-Type"] = content_type + end + + def check_for_user_or_api_key + # User is already authenticated with Spree, make request this way instead. + return true if @current_api_user = try_spree_current_user || !Spree::Api::Config[:requires_authentication] + + if api_key.blank? + render "spree/api/errors/must_specify_api_key", :status => 401 and return + end + end + + def authenticate_user + unless @current_api_user + if requires_authentication? || api_key.present? + unless @current_api_user = Spree.user_class.find_by_spree_api_key(api_key.to_s) + render "spree/api/errors/invalid_api_key", :status => 401 and return + end + else + # An anonymous user + @current_api_user = Spree.user_class.new + end + end + end + + def unauthorized + render "spree/api/errors/unauthorized", :status => 401 and return + end + + def error_during_processing(exception) + render :text => { :exception => exception.message }.to_json, + :status => 422 and return + end + + def requires_authentication? + Spree::Api::Config[:requires_authentication] + end + + def not_found + render "spree/api/errors/not_found", :status => 404 and return + end + + def current_ability + Spree::Ability.new(current_api_user) + end + + def invalid_resource!(resource) + @resource = resource + render "spree/api/errors/invalid_resource", :status => 422 + end + + def api_key + request.headers["X-Spree-Token"] || params[:token] + end + helper_method :api_key + + def find_product(id) + begin + product_scope.find_by_permalink!(id.to_s) + rescue ActiveRecord::RecordNotFound + product_scope.find(id) + end + end + + def product_scope + if current_api_user.has_spree_role?("admin") + scope = Product + if params[:show_deleted] + scope = scope.with_deleted + end + else + scope = Product.active + end + + scope.includes(:master) + end + + end + end +end From a94128098211bb12ee43dc500d4d005114f67ac5 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 18 Jul 2019 17:41:12 +0100 Subject: [PATCH 06/36] Fix easy rubocop issues in spree/api/base_controller --- app/controllers/spree/api/base_controller.rb | 57 ++++++++++---------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/app/controllers/spree/api/base_controller.rb b/app/controllers/spree/api/base_controller.rb index f447beef20..c64223b493 100644 --- a/app/controllers/spree/api/base_controller.rb +++ b/app/controllers/spree/api/base_controller.rb @@ -28,14 +28,14 @@ module Spree def set_jsonp_format if params[:callback] && request.get? - self.response_body = "#{params[:callback]}(#{self.response_body})" + self.response_body = "#{params[:callback]}(#{response_body})" headers["Content-Type"] = 'application/javascript' end end def map_nested_attributes_keys(klass, attributes) nested_keys = klass.nested_attributes_options.keys - attributes.inject({}) do |h, (k,v)| + attributes.inject({}) do |h, (k, v)| key = nested_keys.include?(k.to_sym) ? "#{k}_attributes" : k h[key] = v h @@ -46,43 +46,43 @@ module Spree def set_content_type content_type = case params[:format] - when "json" - "application/json" - when "xml" - "text/xml" - end + when "json" + "application/json" + when "xml" + "text/xml" + end headers["Content-Type"] = content_type end def check_for_user_or_api_key # User is already authenticated with Spree, make request this way instead. - return true if @current_api_user = try_spree_current_user || !Spree::Api::Config[:requires_authentication] + return true if @current_api_user = try_spree_current_user || + !Spree::Api::Config[:requires_authentication] - if api_key.blank? - render "spree/api/errors/must_specify_api_key", :status => 401 and return - end + return if api_key.present? + render "spree/api/errors/must_specify_api_key", status: :unauthorized && return end def authenticate_user - unless @current_api_user - if requires_authentication? || api_key.present? - unless @current_api_user = Spree.user_class.find_by_spree_api_key(api_key.to_s) - render "spree/api/errors/invalid_api_key", :status => 401 and return - end - else - # An anonymous user - @current_api_user = Spree.user_class.new + return if @current_api_user + + if requires_authentication? || api_key.present? + unless @current_api_user = Spree.user_class.find_by_spree_api_key(api_key.to_s) + render "spree/api/errors/invalid_api_key", status: :unauthorized && return end + else + # An anonymous user + @current_api_user = Spree.user_class.new end end def unauthorized - render "spree/api/errors/unauthorized", :status => 401 and return + render "spree/api/errors/unauthorized", status: :unauthorized && return end def error_during_processing(exception) - render :text => { :exception => exception.message }.to_json, - :status => 422 and return + render text: { exception: exception.message }.to_json, + status: :unprocessable_entity && return end def requires_authentication? @@ -90,7 +90,7 @@ module Spree end def not_found - render "spree/api/errors/not_found", :status => 404 and return + render "spree/api/errors/not_found", status: :not_foun && return end def current_ability @@ -99,7 +99,7 @@ module Spree def invalid_resource!(resource) @resource = resource - render "spree/api/errors/invalid_resource", :status => 422 + render "spree/api/errors/invalid_resource", status: :unprocessable_entity end def api_key @@ -108,11 +108,9 @@ module Spree helper_method :api_key def find_product(id) - begin - product_scope.find_by_permalink!(id.to_s) - rescue ActiveRecord::RecordNotFound - product_scope.find(id) - end + product_scope.find_by_permalink!(id.to_s) + rescue ActiveRecord::RecordNotFound + product_scope.find(id) end def product_scope @@ -127,7 +125,6 @@ module Spree scope.includes(:master) end - end end end From f77beb50ff8cf5e0f93dac7a26f697f5475b509b Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 18 Jul 2019 17:58:08 +0100 Subject: [PATCH 07/36] Fix class scope in spree/api/products_controller, should not use Spree namespace here Also, add missing dependency to spree/admin/products_controller_decorator --- app/controllers/spree/admin/products_controller_decorator.rb | 1 + app/controllers/spree/api/products_controller.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/spree/admin/products_controller_decorator.rb b/app/controllers/spree/admin/products_controller_decorator.rb index 016883b840..5ecf0ecce0 100644 --- a/app/controllers/spree/admin/products_controller_decorator.rb +++ b/app/controllers/spree/admin/products_controller_decorator.rb @@ -1,5 +1,6 @@ require 'open_food_network/spree_api_key_loader' require 'open_food_network/referer_parser' +require 'open_food_network/permissions' Spree::Admin::ProductsController.class_eval do include OpenFoodNetwork::SpreeApiKeyLoader diff --git a/app/controllers/spree/api/products_controller.rb b/app/controllers/spree/api/products_controller.rb index fbad83daf7..491abe6f4a 100644 --- a/app/controllers/spree/api/products_controller.rb +++ b/app/controllers/spree/api/products_controller.rb @@ -137,7 +137,7 @@ module Spree def render_paged_products(products) serializer = ActiveModel::ArraySerializer.new( products, - each_serializer: Api::Admin::ProductSerializer + each_serializer: ::Api::Admin::ProductSerializer ) render text: { products: serializer, pages: products.num_pages }.to_json From b70cfa596879db9f0ea4573c371b929bbb330841 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 18 Jul 2019 18:18:15 +0100 Subject: [PATCH 08/36] Bring spree/api/taxons controller from spree_api as it is needed in OFN admin --- .../spree/api/taxons_controller.rb | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 app/controllers/spree/api/taxons_controller.rb diff --git a/app/controllers/spree/api/taxons_controller.rb b/app/controllers/spree/api/taxons_controller.rb new file mode 100644 index 0000000000..fe31cebeb2 --- /dev/null +++ b/app/controllers/spree/api/taxons_controller.rb @@ -0,0 +1,77 @@ +module Spree + module Api + class TaxonsController < Spree::Api::BaseController + respond_to :json + + def index + if taxonomy + @taxons = taxonomy.root.children + else + if params[:ids] + @taxons = Taxon.where(:id => params[:ids].split(",")) + else + @taxons = Taxon.ransack(params[:q]).result + end + end + respond_with(@taxons) + end + + def show + @taxon = taxon + respond_with(@taxon) + end + + def jstree + show + end + + def create + authorize! :create, Taxon + @taxon = Taxon.new(params[:taxon]) + @taxon.taxonomy_id = params[:taxonomy_id] + taxonomy = Taxonomy.find_by_id(params[:taxonomy_id]) + + if taxonomy.nil? + @taxon.errors[:taxonomy_id] = I18n.t(:invalid_taxonomy_id, :scope => 'spree.api') + invalid_resource!(@taxon) and return + end + + @taxon.parent_id = taxonomy.root.id unless params[:taxon][:parent_id] + + if @taxon.save + respond_with(@taxon, :status => 201, :default_template => :show) + else + invalid_resource!(@taxon) + end + end + + def update + authorize! :update, Taxon + if taxon.update_attributes(params[:taxon]) + respond_with(taxon, :status => 200, :default_template => :show) + else + invalid_resource!(taxon) + end + end + + def destroy + authorize! :delete, Taxon + taxon.destroy + respond_with(taxon, :status => 204) + end + + private + + def taxonomy + if params[:taxonomy_id].present? + @taxonomy ||= Taxonomy.find(params[:taxonomy_id]) + end + end + + def taxon + @taxon ||= taxonomy.taxons.find(params[:id]) + end + + end + end +end From cf0f716534478cd99c3dabe8d36f229de732b5a1 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 18 Jul 2019 18:20:27 +0100 Subject: [PATCH 09/36] Fix easy rubocop issues in spree/api/taxons_controller --- app/controllers/spree/api/taxons_controller.rb | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/app/controllers/spree/api/taxons_controller.rb b/app/controllers/spree/api/taxons_controller.rb index fe31cebeb2..a6fe754184 100644 --- a/app/controllers/spree/api/taxons_controller.rb +++ b/app/controllers/spree/api/taxons_controller.rb @@ -8,7 +8,7 @@ module Spree @taxons = taxonomy.root.children else if params[:ids] - @taxons = Taxon.where(:id => params[:ids].split(",")) + @taxons = Taxon.where(id: params[:ids].split(",")) else @taxons = Taxon.ransack(params[:q]).result end @@ -32,14 +32,14 @@ module Spree taxonomy = Taxonomy.find_by_id(params[:taxonomy_id]) if taxonomy.nil? - @taxon.errors[:taxonomy_id] = I18n.t(:invalid_taxonomy_id, :scope => 'spree.api') - invalid_resource!(@taxon) and return + @taxon.errors[:taxonomy_id] = I18n.t(:invalid_taxonomy_id, scope: 'spree.api') + invalid_resource!(@taxon) && return end @taxon.parent_id = taxonomy.root.id unless params[:taxon][:parent_id] if @taxon.save - respond_with(@taxon, :status => 201, :default_template => :show) + respond_with(@taxon, status: 201, default_template: :show) else invalid_resource!(@taxon) end @@ -48,7 +48,7 @@ module Spree def update authorize! :update, Taxon if taxon.update_attributes(params[:taxon]) - respond_with(taxon, :status => 200, :default_template => :show) + respond_with(taxon, status: 200, default_template: :show) else invalid_resource!(taxon) end @@ -57,21 +57,19 @@ module Spree def destroy authorize! :delete, Taxon taxon.destroy - respond_with(taxon, :status => 204) + respond_with(taxon, status: 204) end private def taxonomy - if params[:taxonomy_id].present? - @taxonomy ||= Taxonomy.find(params[:taxonomy_id]) - end + return if params[:taxonomy_id].blank? + @taxonomy ||= Taxonomy.find(params[:taxonomy_id]) end def taxon @taxon ||= taxonomy.taxons.find(params[:id]) end - end end end From 0e4fe08ac4124f4f4139c23dbbe7ab2dae66f1f1 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 18 Jul 2019 19:06:39 +0100 Subject: [PATCH 10/36] Fix logical problem in spree/api/base_controller and in spree/checkout_controller See this stack overflow post for more info: https://stackoverflow.com/questions/39629976/ruby-return-vs-and-return --- app/controllers/spree/api/base_controller.rb | 12 ++++++------ app/controllers/spree/checkout_controller.rb | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/controllers/spree/api/base_controller.rb b/app/controllers/spree/api/base_controller.rb index c64223b493..7dc2867eb3 100644 --- a/app/controllers/spree/api/base_controller.rb +++ b/app/controllers/spree/api/base_controller.rb @@ -60,7 +60,7 @@ module Spree !Spree::Api::Config[:requires_authentication] return if api_key.present? - render "spree/api/errors/must_specify_api_key", status: :unauthorized && return + render("spree/api/errors/must_specify_api_key", status: :unauthorized) && return end def authenticate_user @@ -68,7 +68,7 @@ module Spree if requires_authentication? || api_key.present? unless @current_api_user = Spree.user_class.find_by_spree_api_key(api_key.to_s) - render "spree/api/errors/invalid_api_key", status: :unauthorized && return + render("spree/api/errors/invalid_api_key", status: :unauthorized) && return end else # An anonymous user @@ -77,12 +77,12 @@ module Spree end def unauthorized - render "spree/api/errors/unauthorized", status: :unauthorized && return + render("spree/api/errors/unauthorized", status: :unauthorized) && return end def error_during_processing(exception) - render text: { exception: exception.message }.to_json, - status: :unprocessable_entity && return + render(text: { exception: exception.message }.to_json, + status: :unprocessable_entity) && return end def requires_authentication? @@ -90,7 +90,7 @@ module Spree end def not_found - render "spree/api/errors/not_found", status: :not_foun && return + render("spree/api/errors/not_found", status: :not_found) && return end def current_ability diff --git a/app/controllers/spree/checkout_controller.rb b/app/controllers/spree/checkout_controller.rb index a01750072f..e05ad99535 100644 --- a/app/controllers/spree/checkout_controller.rb +++ b/app/controllers/spree/checkout_controller.rb @@ -29,7 +29,7 @@ module Spree def load_order @order = current_order - redirect_to main_app.cart_path && return unless @order + redirect_to(main_app.cart_path) && return unless @order if params[:state] redirect_to checkout_state_path(@order.state) if @order.can_go_to_state?(params[:state]) From 20a46a791c7537041d9e2495a690f7a4ea2886b4 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 18 Jul 2019 20:00:11 +0100 Subject: [PATCH 11/36] Bring and adapt spree/api/base_controller_spec from spree_api --- .../spree/api/base_controller_spec.rb | 64 +++++++++++++++++++ spec/support/controller_hacks.rb | 28 ++++++++ 2 files changed, 92 insertions(+) create mode 100644 spec/controllers/spree/api/base_controller_spec.rb create mode 100644 spec/support/controller_hacks.rb diff --git a/spec/controllers/spree/api/base_controller_spec.rb b/spec/controllers/spree/api/base_controller_spec.rb new file mode 100644 index 0000000000..73121c989e --- /dev/null +++ b/spec/controllers/spree/api/base_controller_spec.rb @@ -0,0 +1,64 @@ +require 'spec_helper' + +describe Spree::Api::BaseController do + render_views + controller(Spree::Api::BaseController) do + def index + render :text => { "products" => [] }.to_json + end + + def spree_current_user; end + end + + context "signed in as a user using an authentication extension" do + before do + controller.stub :try_spree_current_user => double(:email => "spree@example.com") + Spree::Api::Config[:requires_authentication] = true + end + + it "can make a request" do + api_get :index + json_response.should == { "products" => [] } + response.status.should == 200 + end + end + + context "cannot make a request to the API" do + it "without an API key" do + api_get :index + json_response.should == { "error" => "You must specify an API key." } + response.status.should == 401 + end + + it "with an invalid API key" do + request.env["X-Spree-Token"] = "fake_key" + get :index, {} + json_response.should == { "error" => "Invalid API key (fake_key) specified." } + response.status.should == 401 + end + + it "using an invalid token param" do + get :index, :token => "fake_key" + json_response.should == { "error" => "Invalid API key (fake_key) specified." } + end + end + + it 'handles exceptions' do + subject.should_receive(:authenticate_user).and_return(true) + subject.should_receive(:index).and_raise(Exception.new("no joy")) + get :index, :token => "fake_key" + json_response.should == { "exception" => "no joy" } + end + + it "maps symantec keys to nested_attributes keys" do + klass = double(:nested_attributes_options => { :line_items => {}, + :bill_address => {} }) + attributes = { 'line_items' => { :id => 1 }, + 'bill_address' => { :id => 2 }, + 'name' => 'test order' } + + mapped = subject.map_nested_attributes_keys(klass, attributes) + mapped.has_key?('line_items_attributes').should be_truthy + mapped.has_key?('name').should be_truthy + end +end diff --git a/spec/support/controller_hacks.rb b/spec/support/controller_hacks.rb new file mode 100644 index 0000000000..d57123c5bf --- /dev/null +++ b/spec/support/controller_hacks.rb @@ -0,0 +1,28 @@ +require 'active_support/all' + +module ControllerHacks + def api_get(action, params={}, session=nil, flash=nil) + api_process(action, params, session, flash, "GET") + end + + def api_post(action, params={}, session=nil, flash=nil) + api_process(action, params, session, flash, "POST") + end + + def api_put(action, params={}, session=nil, flash=nil) + api_process(action, params, session, flash, "PUT") + end + + def api_delete(action, params={}, session=nil, flash=nil) + api_process(action, params, session, flash, "DELETE") + end + + def api_process(action, params={}, session=nil, flash=nil, method="get") + scoping = respond_to?(:resource_scoping) ? resource_scoping : {} + process(action, params.merge(scoping).reverse_merge!(:use_route => :spree, :format => :json), session, flash, method) + end +end + +RSpec.configure do |config| + config.include ControllerHacks, :type => :controller +end From 2490cbfccb51db5413f91d54e2b3d4710eb2c676 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 18 Jul 2019 20:04:00 +0100 Subject: [PATCH 12/36] Transpec and fix rubocop issues in spree/api/base_controller_spec --- .../spree/api/base_controller_spec.rb | 41 ++++++++++--------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/spec/controllers/spree/api/base_controller_spec.rb b/spec/controllers/spree/api/base_controller_spec.rb index 73121c989e..b8ad0580a1 100644 --- a/spec/controllers/spree/api/base_controller_spec.rb +++ b/spec/controllers/spree/api/base_controller_spec.rb @@ -4,7 +4,7 @@ describe Spree::Api::BaseController do render_views controller(Spree::Api::BaseController) do def index - render :text => { "products" => [] }.to_json + render text: { "products" => [] }.to_json end def spree_current_user; end @@ -12,53 +12,54 @@ describe Spree::Api::BaseController do context "signed in as a user using an authentication extension" do before do - controller.stub :try_spree_current_user => double(:email => "spree@example.com") + allow(controller).to receive_messages try_spree_current_user: + double(email: "spree@example.com") Spree::Api::Config[:requires_authentication] = true end it "can make a request" do api_get :index - json_response.should == { "products" => [] } - response.status.should == 200 + expect(json_response).to eq( "products" => [] ) + expect(response.status).to eq(200) end end context "cannot make a request to the API" do it "without an API key" do api_get :index - json_response.should == { "error" => "You must specify an API key." } - response.status.should == 401 + expect(json_response).to eq( "error" => "You must specify an API key." ) + expect(response.status).to eq(401) end it "with an invalid API key" do request.env["X-Spree-Token"] = "fake_key" get :index, {} - json_response.should == { "error" => "Invalid API key (fake_key) specified." } - response.status.should == 401 + expect(json_response).to eq( "error" => "Invalid API key (fake_key) specified." ) + expect(response.status).to eq(401) end it "using an invalid token param" do - get :index, :token => "fake_key" - json_response.should == { "error" => "Invalid API key (fake_key) specified." } + get :index, token: "fake_key" + expect(json_response).to eq( "error" => "Invalid API key (fake_key) specified." ) end end it 'handles exceptions' do - subject.should_receive(:authenticate_user).and_return(true) - subject.should_receive(:index).and_raise(Exception.new("no joy")) - get :index, :token => "fake_key" - json_response.should == { "exception" => "no joy" } + expect(subject).to receive(:authenticate_user).and_return(true) + expect(subject).to receive(:index).and_raise(Exception.new("no joy")) + get :index, token: "fake_key" + expect(json_response).to eq( "exception" => "no joy" ) end it "maps symantec keys to nested_attributes keys" do - klass = double(:nested_attributes_options => { :line_items => {}, - :bill_address => {} }) - attributes = { 'line_items' => { :id => 1 }, - 'bill_address' => { :id => 2 }, + klass = double(nested_attributes_options: { line_items: {}, + bill_address: {} }) + attributes = { 'line_items' => { id: 1 }, + 'bill_address' => { id: 2 }, 'name' => 'test order' } mapped = subject.map_nested_attributes_keys(klass, attributes) - mapped.has_key?('line_items_attributes').should be_truthy - mapped.has_key?('name').should be_truthy + expect(mapped.key?('line_items_attributes')).to be_truthy + expect(mapped.key?('name')).to be_truthy end end From c668677b8a4e2e878ee85669d9c43378c7dff128 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 19 Jul 2019 10:16:32 +0100 Subject: [PATCH 13/36] Bring spree/api/taxons_controller_spec from spree_api, adapt it, transpec it and fix rubocop issues --- .../spree/api/taxons_controller.rb | 4 - .../spree/api/taxons_controller_spec.rb | 132 ++++++++++++++++++ 2 files changed, 132 insertions(+), 4 deletions(-) create mode 100644 spec/controllers/spree/api/taxons_controller_spec.rb diff --git a/app/controllers/spree/api/taxons_controller.rb b/app/controllers/spree/api/taxons_controller.rb index a6fe754184..e2450839cf 100644 --- a/app/controllers/spree/api/taxons_controller.rb +++ b/app/controllers/spree/api/taxons_controller.rb @@ -21,10 +21,6 @@ module Spree respond_with(@taxon) end - def jstree - show - end - def create authorize! :create, Taxon @taxon = Taxon.new(params[:taxon]) diff --git a/spec/controllers/spree/api/taxons_controller_spec.rb b/spec/controllers/spree/api/taxons_controller_spec.rb new file mode 100644 index 0000000000..652aaecb2e --- /dev/null +++ b/spec/controllers/spree/api/taxons_controller_spec.rb @@ -0,0 +1,132 @@ +require 'spec_helper' + +module Spree + describe Api::TaxonsController do + render_views + + let(:taxonomy) { create(:taxonomy) } + let(:taxon) { create(:taxon, name: "Ruby", taxonomy: taxonomy) } + let(:taxon2) { create(:taxon, name: "Rails", taxonomy: taxonomy) } + let(:attributes) { + ["id", "name", "pretty_name", "permalink", "position", "parent_id", "taxonomy_id"] + } + + before do + taxon2.children << create(:taxon, name: "3.2.2", taxonomy: taxonomy) + taxon.children << taxon2 + taxonomy.root.children << taxon + end + + context "as a normal user" do + controller(Spree::Api::TaxonsController) do + def spree_current_user + FactoryBot.create(:user) + end + end + + it "gets all taxons for a taxonomy" do + api_get :index, taxonomy_id: taxonomy.id + + expect(json_response.first['name']).to eq taxon.name + children = json_response.first['taxons'] + expect(children.count).to eq 1 + expect(children.first['name']).to eq taxon2.name + expect(children.first['taxons'].count).to eq 1 + end + + it "gets all taxons" do + api_get :index + + expect(json_response.first['name']).to eq taxonomy.root.name + children = json_response.first['taxons'] + expect(children.count).to eq 1 + expect(children.first['name']).to eq taxon.name + expect(children.first['taxons'].count).to eq 1 + end + + it "can search for a single taxon" do + api_get :index, q: { name_cont: "Ruby" } + + expect(json_response.count).to eq(1) + expect(json_response.first['name']).to eq "Ruby" + end + + it "gets a single taxon" do + api_get :show, id: taxon.id, taxonomy_id: taxonomy.id + + expect(json_response['name']).to eq taxon.name + expect(json_response['taxons'].count).to eq 1 + end + + it "can learn how to create a new taxon" do + api_get :new, taxonomy_id: taxonomy.id + expect(json_response["attributes"]).to eq(attributes.map(&:to_s)) + required_attributes = json_response["required_attributes"] + expect(required_attributes).to include("name") + end + + it "cannot create a new taxon if not an admin" do + api_post :create, taxonomy_id: taxonomy.id, taxon: { name: "Location" } + assert_unauthorized! + end + + it "cannot update a taxon" do + api_put :update, taxonomy_id: taxonomy.id, + id: taxon.id, + taxon: { name: "I hacked your store!" } + assert_unauthorized! + end + + it "cannot delete a taxon" do + api_delete :destroy, taxonomy_id: taxonomy.id, id: taxon.id + assert_unauthorized! + end + end + + context "as an admin" do + controller(Spree::Api::TaxonsController) do + def spree_current_user + FactoryBot.create(:admin_user) + end + end + + it "can create" do + api_post :create, taxonomy_id: taxonomy.id, taxon: { name: "Colors" } + + expect(attributes.all? { |a| json_response.include? a }).to be true + expect(response.status).to eq(201) + + expect(taxonomy.reload.root.children.count).to eq 2 + + expect(Spree::Taxon.last.parent_id).to eq taxonomy.root.id + expect(Spree::Taxon.last.taxonomy_id).to eq taxonomy.id + end + + it "cannot create a new taxon with invalid attributes" do + api_post :create, taxonomy_id: taxonomy.id, taxon: {} + expect(response.status).to eq(422) + expect(json_response["error"]).to eq("Invalid resource. Please fix errors and try again.") + errors = json_response["errors"] + + expect(taxonomy.reload.root.children.count).to eq 1 + end + + it "cannot create a new taxon with invalid taxonomy_id" do + api_post :create, taxonomy_id: 1000, taxon: { name: "Colors" } + expect(response.status).to eq(422) + expect(json_response["error"]).to eq("Invalid resource. Please fix errors and try again.") + + errors = json_response["errors"] + expect(errors["taxonomy_id"]).not_to be_nil + expect(errors["taxonomy_id"].first).to eq "Invalid taxonomy id." + + expect(taxonomy.reload.root.children.count).to eq 1 + end + + it "can destroy" do + api_delete :destroy, taxonomy_id: taxonomy.id, id: taxon2.id + expect(response.status).to eq(204) + end + end + end +end From 84a2886003550ce9872ea95846692982e35710ba Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 19 Jul 2019 11:59:28 +0100 Subject: [PATCH 14/36] Improve auth code in spree/api/taxons_controller_spec --- .../spree/api/taxons_controller_spec.rb | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/spec/controllers/spree/api/taxons_controller_spec.rb b/spec/controllers/spree/api/taxons_controller_spec.rb index 652aaecb2e..efd5ec1880 100644 --- a/spec/controllers/spree/api/taxons_controller_spec.rb +++ b/spec/controllers/spree/api/taxons_controller_spec.rb @@ -12,17 +12,15 @@ module Spree } before do + allow(controller).to receive(:spree_current_user) { current_api_user } + taxon2.children << create(:taxon, name: "3.2.2", taxonomy: taxonomy) taxon.children << taxon2 taxonomy.root.children << taxon end context "as a normal user" do - controller(Spree::Api::TaxonsController) do - def spree_current_user - FactoryBot.create(:user) - end - end + let(:current_api_user) { build(:user) } it "gets all taxons for a taxonomy" do api_get :index, taxonomy_id: taxonomy.id @@ -84,11 +82,7 @@ module Spree end context "as an admin" do - controller(Spree::Api::TaxonsController) do - def spree_current_user - FactoryBot.create(:admin_user) - end - end + let(:current_api_user) { build(:admin_user) } it "can create" do api_post :create, taxonomy_id: taxonomy.id, taxon: { name: "Colors" } From e746a0db7d67546c24bf19f358b6a369ad630c5e Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 19 Jul 2019 13:18:49 +0100 Subject: [PATCH 15/36] Bring tests from spree/api/products_controller_spec and add them to existing ones on the ofn side Adapt these tests to have a green build --- .../spree/api/products_controller_spec.rb | 237 ++++++++++++++++-- spec/support/products_helper.rb | 17 ++ 2 files changed, 227 insertions(+), 27 deletions(-) diff --git a/spec/controllers/spree/api/products_controller_spec.rb b/spec/controllers/spree/api/products_controller_spec.rb index 89fb08c40c..feca064899 100644 --- a/spec/controllers/spree/api/products_controller_spec.rb +++ b/spec/controllers/spree/api/products_controller_spec.rb @@ -6,10 +6,12 @@ module Spree let(:supplier) { create(:supplier_enterprise) } let(:supplier2) { create(:supplier_enterprise) } - let!(:product1) { create(:product, supplier: supplier) } + let!(:product) { create(:product, supplier: supplier) } + let!(:inactive_product) { create(:product, :available_on => Time.now.tomorrow, :name => "inactive") } let(:product_other_supplier) { create(:product, supplier: supplier2) } let(:product_with_image) { create(:product_with_image, supplier: supplier) } - let(:attributes) { [:id, :name, :supplier, :price, :on_hand, :available_on, :permalink_live] } + let(:attributes) { ["id", "name", "supplier", "price", "on_hand", "available_on", "permalink_live"] } + let(:all_attributes) { ["id", "name", "description", "price", "available_on", "permalink", "meta_description", "meta_keywords", "shipping_category_id", "taxon_ids", "variants", "option_types", "product_properties"] } let(:current_api_user) { build(:user) } @@ -27,6 +29,121 @@ module Spree spree_get :managed, template: 'bulk_index', format: :json assert_unauthorized! end + + it "retrieves a list of products" do + api_get :index + json_response["products"].first.should have_attributes({keys: all_attributes}) + + + json_response["count"].should == 1 + json_response["current_page"].should == 1 + json_response["pages"].should == 1 + end + + it "retrieves a list of products by id" do + api_get :index, :ids => [product.id] + json_response["products"].first.should have_attributes({keys: all_attributes}) + json_response["count"].should == 1 + json_response["current_page"].should == 1 + json_response["pages"].should == 1 + end + + it "does not return inactive products when queried by ids" do + api_get :index, :ids => [inactive_product.id] + json_response["count"].should == 0 + end + + it "does not list unavailable products" do + api_get :index + json_response["products"].first["name"].should_not eq("inactive") + end + + context "pagination" do + it "can select the next page of products" do + second_product = create(:product) + api_get :index, :page => 2, :per_page => 1 + json_response["products"].first.should have_attributes({keys: all_attributes}) + json_response["total_count"].should == 2 + json_response["current_page"].should == 2 + json_response["pages"].should == 2 + end + + it 'can control the page size through a parameter' do + create(:product) + api_get :index, :per_page => 1 + json_response['count'].should == 1 + json_response['total_count'].should == 2 + json_response['current_page'].should == 1 + json_response['pages'].should == 2 + end + end + + context "jsonp" do + it "retrieves a list of products of jsonp" do + api_get :index, {:callback => 'callback'} + response.body.should =~ /^callback\(.*\)$/ + response.header['Content-Type'].should include('application/javascript') + end + end + + it "can search for products" do + create(:product, :name => "The best product in the world") + api_get :index, :q => { :name_cont => "best" } + json_response["products"].first.should have_attributes({keys: all_attributes}) + json_response["count"].should == 1 + end + + it "gets a single product" do + product.master.images.create!(:attachment => image("thinking-cat.jpg")) + product.variants.create!({ unit_value: "1", unit_description: "thing"}) + product.variants.first.images.create!(:attachment => image("thinking-cat.jpg")) + product.set_property("spree", "rocks") + api_get :show, :id => product.to_param + json_response.should have_attributes({keys: all_attributes}) + json_response['variants'].first.should have_attributes({keys: ["id", "name", "sku", "price", "weight", "height", "width", "depth", "is_master", "cost_price", "permalink", "option_values", "images"]}) + json_response['variants'].first['images'].first.should have_attributes({keys: ["id", "position", "attachment_content_type", "attachment_file_name", "type", "attachment_updated_at", "attachment_width", "attachment_height", "alt", "viewable_type", "viewable_id", "attachment_url"]}) + json_response["product_properties"].first.should have_attributes({keys: ["id", "product_id", "property_id", "value", "property_name"]}) + end + + context "finds a product by permalink first then by id" do + let!(:other_product) { create(:product, :permalink => "these-are-not-the-droids-you-are-looking-for") } + + before do + product.update_attribute(:permalink, "#{other_product.id}-and-1-ways") + end + + specify do + api_get :show, :id => product.to_param + json_response["permalink"].should =~ /and-1-ways/ + product.destroy + + api_get :show, :id => other_product.id + json_response["permalink"].should =~ /droids/ + end + end + + it "cannot see inactive products" do + api_get :show, :id => inactive_product.to_param + json_response["error"].should == "The resource you were looking for could not be found." + response.status.should == 404 + end + + it "returns a 404 error when it cannot find a product" do + api_get :show, :id => "non-existant" + json_response["error"].should == "The resource you were looking for could not be found." + response.status.should == 404 + end + + it "can learn how to create a new product" do + api_get :new + json_response["attributes"].should == ["id", "name", "description", "price", "available_on", "permalink", "meta_description", "meta_keywords", "shipping_category_id", "taxon_ids"] + required_attributes = json_response["required_attributes"] + required_attributes.should include("name") + required_attributes.should include("price") + required_attributes.should include("shipping_category_id") + end + + include_examples "modifying product actions are restricted" end context "as an enterprise user" do @@ -38,15 +155,15 @@ module Spree it "retrieves a list of managed products" do spree_get :managed, template: 'bulk_index', format: :json - keys = json_response.first.keys.map(&:to_sym) - expect(attributes.all?{ |attr| keys.include? attr }).to eq(true) + response_keys = json_response.first.keys + expect(attributes.all?{ |attr| response_keys.include? attr }).to eq(true) end it "soft deletes my products" do - spree_delete :soft_delete, product_id: product1.to_param, format: :json + spree_delete :soft_delete, product_id: product.to_param, format: :json expect(response.status).to eq(204) - expect { product1.reload }.not_to raise_error - expect(product1.deleted_at).not_to be_nil + expect { product.reload }.not_to raise_error + expect(product.deleted_at).not_to be_nil end it "is denied access to soft deleting another enterprises' product" do @@ -65,14 +182,14 @@ module Spree it "retrieves a list of managed products" do spree_get :managed, template: 'bulk_index', format: :json - keys = json_response.first.keys.map(&:to_sym) - expect(attributes.all?{ |attr| keys.include? attr }).to eq(true) + response_keys = json_response.first.keys + expect(attributes.all?{ |attr| response_keys.include? attr }).to eq(true) end it "retrieves a list of products with appropriate attributes" do spree_get :index, template: 'bulk_index', format: :json - keys = json_response.first.keys.map(&:to_sym) - expect(attributes.all?{ |attr| keys.include? attr }).to eq(true) + response_keys = json_response.first.keys + expect(attributes.all?{ |attr| response_keys.include? attr }).to eq(true) end it "sorts products in ascending id order" do @@ -93,26 +210,92 @@ module Spree it "returns permalink as permalink_live" do spree_get :index, template: 'bulk_index', format: :json - expect(json_response.detect{ |product| product['id'] == product1.id }['permalink_live']).to eq(product1.permalink) + expect(json_response.detect{ |product_in_response| product_in_response['id'] == product.id }['permalink_live']).to eq(product.permalink) end it "should allow available_on to be nil" do spree_get :index, template: 'bulk_index', format: :json - expect(json_response.size).to eq(1) + expect(json_response.size).to eq(2) - product5 = FactoryBot.create(:product) - product5.available_on = nil - product5.save! + another_product = FactoryBot.create(:product) + another_product.available_on = nil + another_product.save! spree_get :index, template: 'bulk_index', format: :json - expect(json_response.size).to eq(2) + expect(json_response.size).to eq(3) end it "soft deletes a product" do - spree_delete :soft_delete, product_id: product1.to_param, format: :json + spree_delete :soft_delete, product_id: product.to_param, format: :json expect(response.status).to eq(204) - expect { product1.reload }.not_to raise_error - expect(product1.deleted_at).not_to be_nil + expect { product.reload }.not_to raise_error + expect(product.deleted_at).not_to be_nil + end + + it "can see all products" do + api_get :index + json_response["products"].count.should == 2 + json_response["count"].should == 2 + json_response["current_page"].should == 1 + json_response["pages"].should == 1 + end + + # Regression test for #1626 + context "deleted products" do + before do + create(:product, :deleted_at => 1.day.ago) + end + + it "does not include deleted products" do + api_get :index + json_response["products"].count.should == 2 + end + + it "can include deleted products" do + api_get :index, :show_deleted => 1 + json_response["products"].count.should == 3 + end + end + + it "can create a new product" do + api_post :create, :product => { :name => "The Other Product", + :price => 19.99, + :shipping_category_id => create(:shipping_category).id, + :supplier_id => supplier.id, + :primary_taxon_id => FactoryBot.create(:taxon).id, + :variant_unit => "items", + :variant_unit_name => "things", + :unit_description => "things" + } + json_response.should have_attributes({keys: all_attributes}) + response.status.should == 201 + end + + it "cannot create a new product with invalid attributes" do + api_post :create, :product => {} + response.status.should == 422 + json_response["error"].should == "Invalid resource. Please fix errors and try again." + errors = json_response["errors"] + errors.keys.should =~ ["name", "price", "primary_taxon", "shipping_category_id", "supplier", "variant_unit"] + end + + it "can update a product" do + api_put :update, :id => product.to_param, :product => { :name => "New and Improved Product!" } + response.status.should == 200 + end + + it "cannot update a product with an invalid attribute" do + api_put :update, :id => product.to_param, :product => { :name => "" } + response.status.should == 422 + json_response["error"].should == "Invalid resource. Please fix errors and try again." + json_response["errors"]["name"].should == ["can't be blank"] + end + + it "can delete a product" do + product.deleted_at.should be_nil + api_delete :destroy, :id => product.to_param + response.status.should == 204 + product.reload.deleted_at.should_not be_nil end end @@ -124,7 +307,7 @@ module Spree end it 'denies access' do - spree_post :clone, product_id: product1.id, format: :json + spree_post :clone, product_id: product.id, format: :json assert_unauthorized! end end @@ -137,13 +320,13 @@ module Spree end it 'responds with a successful response' do - spree_post :clone, product_id: product1.id, format: :json + spree_post :clone, product_id: product.id, format: :json expect(response.status).to eq(201) end it 'clones the product' do - spree_post :clone, product_id: product1.id, format: :json - expect(json_response['name']).to eq("COPY OF #{product1.name}") + spree_post :clone, product_id: product.id, format: :json + expect(json_response['name']).to eq("COPY OF #{product.name}") end it 'clones a product with image' do @@ -160,13 +343,13 @@ module Spree end it 'responds with a successful response' do - spree_post :clone, product_id: product1.id, format: :json + spree_post :clone, product_id: product.id, format: :json expect(response.status).to eq(201) end it 'clones the product' do - spree_post :clone, product_id: product1.id, format: :json - expect(json_response['name']).to eq("COPY OF #{product1.name}") + spree_post :clone, product_id: product.id, format: :json + expect(json_response['name']).to eq("COPY OF #{product.name}") end it 'clones a product with image' do diff --git a/spec/support/products_helper.rb b/spec/support/products_helper.rb index 8e9ebc1c98..24ef6bfcac 100644 --- a/spec/support/products_helper.rb +++ b/spec/support/products_helper.rb @@ -8,5 +8,22 @@ module OpenFoodNetwork ensure Spree::Config.products_require_tax_category = original_value end + + shared_examples "modifying product actions are restricted" do + it "cannot create a new product if not an admin" do + api_post :create, :product => { :name => "Brand new product!" } + assert_unauthorized! + end + + it "cannot update a product" do + api_put :update, :id => product.to_param, :product => { :name => "I hacked your store!" } + assert_unauthorized! + end + + it "cannot delete a product" do + api_delete :destroy, :id => product.to_param + assert_unauthorized! + end + end end end From 2912c1b87dc7e30fb8a3e624497c6451ae8a347c Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 19 Jul 2019 13:23:11 +0100 Subject: [PATCH 16/36] Transpec and fix rubocop issues in spree/api/product_controller_spec --- .../spree/api/products_controller_spec.rb | 172 +++++++++--------- 1 file changed, 85 insertions(+), 87 deletions(-) diff --git a/spec/controllers/spree/api/products_controller_spec.rb b/spec/controllers/spree/api/products_controller_spec.rb index feca064899..c726487052 100644 --- a/spec/controllers/spree/api/products_controller_spec.rb +++ b/spec/controllers/spree/api/products_controller_spec.rb @@ -7,7 +7,7 @@ module Spree let(:supplier) { create(:supplier_enterprise) } let(:supplier2) { create(:supplier_enterprise) } let!(:product) { create(:product, supplier: supplier) } - let!(:inactive_product) { create(:product, :available_on => Time.now.tomorrow, :name => "inactive") } + let!(:inactive_product) { create(:product, available_on: Time.zone.now.tomorrow, name: "inactive") } let(:product_other_supplier) { create(:product, supplier: supplier2) } let(:product_with_image) { create(:product_with_image, supplier: supplier) } let(:attributes) { ["id", "name", "supplier", "price", "on_hand", "available_on", "permalink_live"] } @@ -32,115 +32,114 @@ module Spree it "retrieves a list of products" do api_get :index - json_response["products"].first.should have_attributes({keys: all_attributes}) + expect(json_response["products"].first).to have_attributes(keys: all_attributes) - - json_response["count"].should == 1 - json_response["current_page"].should == 1 - json_response["pages"].should == 1 + expect(json_response["count"]).to eq(1) + expect(json_response["current_page"]).to eq(1) + expect(json_response["pages"]).to eq(1) end it "retrieves a list of products by id" do - api_get :index, :ids => [product.id] - json_response["products"].first.should have_attributes({keys: all_attributes}) - json_response["count"].should == 1 - json_response["current_page"].should == 1 - json_response["pages"].should == 1 + api_get :index, ids: [product.id] + expect(json_response["products"].first).to have_attributes(keys: all_attributes) + expect(json_response["count"]).to eq(1) + expect(json_response["current_page"]).to eq(1) + expect(json_response["pages"]).to eq(1) end it "does not return inactive products when queried by ids" do - api_get :index, :ids => [inactive_product.id] - json_response["count"].should == 0 + api_get :index, ids: [inactive_product.id] + expect(json_response["count"]).to eq(0) end it "does not list unavailable products" do api_get :index - json_response["products"].first["name"].should_not eq("inactive") + expect(json_response["products"].first["name"]).not_to eq("inactive") end context "pagination" do it "can select the next page of products" do second_product = create(:product) - api_get :index, :page => 2, :per_page => 1 - json_response["products"].first.should have_attributes({keys: all_attributes}) - json_response["total_count"].should == 2 - json_response["current_page"].should == 2 - json_response["pages"].should == 2 + api_get :index, page: 2, per_page: 1 + expect(json_response["products"].first).to have_attributes(keys: all_attributes) + expect(json_response["total_count"]).to eq(2) + expect(json_response["current_page"]).to eq(2) + expect(json_response["pages"]).to eq(2) end it 'can control the page size through a parameter' do create(:product) - api_get :index, :per_page => 1 - json_response['count'].should == 1 - json_response['total_count'].should == 2 - json_response['current_page'].should == 1 - json_response['pages'].should == 2 + api_get :index, per_page: 1 + expect(json_response['count']).to eq(1) + expect(json_response['total_count']).to eq(2) + expect(json_response['current_page']).to eq(1) + expect(json_response['pages']).to eq(2) end end context "jsonp" do it "retrieves a list of products of jsonp" do - api_get :index, {:callback => 'callback'} - response.body.should =~ /^callback\(.*\)$/ - response.header['Content-Type'].should include('application/javascript') + api_get :index, callback: 'callback' + expect(response.body).to match(/^callback\(.*\)$/) + expect(response.header['Content-Type']).to include('application/javascript') end end it "can search for products" do - create(:product, :name => "The best product in the world") - api_get :index, :q => { :name_cont => "best" } - json_response["products"].first.should have_attributes({keys: all_attributes}) - json_response["count"].should == 1 + create(:product, name: "The best product in the world") + api_get :index, q: { name_cont: "best" } + expect(json_response["products"].first).to have_attributes(keys: all_attributes) + expect(json_response["count"]).to eq(1) end it "gets a single product" do - product.master.images.create!(:attachment => image("thinking-cat.jpg")) - product.variants.create!({ unit_value: "1", unit_description: "thing"}) - product.variants.first.images.create!(:attachment => image("thinking-cat.jpg")) + product.master.images.create!(attachment: image("thinking-cat.jpg")) + product.variants.create!(unit_value: "1", unit_description: "thing") + product.variants.first.images.create!(attachment: image("thinking-cat.jpg")) product.set_property("spree", "rocks") - api_get :show, :id => product.to_param - json_response.should have_attributes({keys: all_attributes}) - json_response['variants'].first.should have_attributes({keys: ["id", "name", "sku", "price", "weight", "height", "width", "depth", "is_master", "cost_price", "permalink", "option_values", "images"]}) - json_response['variants'].first['images'].first.should have_attributes({keys: ["id", "position", "attachment_content_type", "attachment_file_name", "type", "attachment_updated_at", "attachment_width", "attachment_height", "alt", "viewable_type", "viewable_id", "attachment_url"]}) - json_response["product_properties"].first.should have_attributes({keys: ["id", "product_id", "property_id", "value", "property_name"]}) + api_get :show, id: product.to_param + expect(json_response).to have_attributes(keys: all_attributes) + expect(json_response['variants'].first).to have_attributes(keys: ["id", "name", "sku", "price", "weight", "height", "width", "depth", "is_master", "cost_price", "permalink", "option_values", "images"]) + expect(json_response['variants'].first['images'].first).to have_attributes(keys: ["id", "position", "attachment_content_type", "attachment_file_name", "type", "attachment_updated_at", "attachment_width", "attachment_height", "alt", "viewable_type", "viewable_id", "attachment_url"]) + expect(json_response["product_properties"].first).to have_attributes(keys: ["id", "product_id", "property_id", "value", "property_name"]) end context "finds a product by permalink first then by id" do - let!(:other_product) { create(:product, :permalink => "these-are-not-the-droids-you-are-looking-for") } + let!(:other_product) { create(:product, permalink: "these-are-not-the-droids-you-are-looking-for") } before do product.update_attribute(:permalink, "#{other_product.id}-and-1-ways") end specify do - api_get :show, :id => product.to_param - json_response["permalink"].should =~ /and-1-ways/ + api_get :show, id: product.to_param + expect(json_response["permalink"]).to match(/and-1-ways/) product.destroy - api_get :show, :id => other_product.id - json_response["permalink"].should =~ /droids/ + api_get :show, id: other_product.id + expect(json_response["permalink"]).to match(/droids/) end end it "cannot see inactive products" do - api_get :show, :id => inactive_product.to_param - json_response["error"].should == "The resource you were looking for could not be found." - response.status.should == 404 + api_get :show, id: inactive_product.to_param + expect(json_response["error"]).to eq("The resource you were looking for could not be found.") + expect(response.status).to eq(404) end it "returns a 404 error when it cannot find a product" do - api_get :show, :id => "non-existant" - json_response["error"].should == "The resource you were looking for could not be found." - response.status.should == 404 + api_get :show, id: "non-existant" + expect(json_response["error"]).to eq("The resource you were looking for could not be found.") + expect(response.status).to eq(404) end it "can learn how to create a new product" do api_get :new - json_response["attributes"].should == ["id", "name", "description", "price", "available_on", "permalink", "meta_description", "meta_keywords", "shipping_category_id", "taxon_ids"] + expect(json_response["attributes"]).to eq(["id", "name", "description", "price", "available_on", "permalink", "meta_description", "meta_keywords", "shipping_category_id", "taxon_ids"]) required_attributes = json_response["required_attributes"] - required_attributes.should include("name") - required_attributes.should include("price") - required_attributes.should include("shipping_category_id") + expect(required_attributes).to include("name") + expect(required_attributes).to include("price") + expect(required_attributes).to include("shipping_category_id") end include_examples "modifying product actions are restricted" @@ -234,68 +233,67 @@ module Spree it "can see all products" do api_get :index - json_response["products"].count.should == 2 - json_response["count"].should == 2 - json_response["current_page"].should == 1 - json_response["pages"].should == 1 + expect(json_response["products"].count).to eq(2) + expect(json_response["count"]).to eq(2) + expect(json_response["current_page"]).to eq(1) + expect(json_response["pages"]).to eq(1) end # Regression test for #1626 context "deleted products" do before do - create(:product, :deleted_at => 1.day.ago) + create(:product, deleted_at: 1.day.ago) end it "does not include deleted products" do api_get :index - json_response["products"].count.should == 2 + expect(json_response["products"].count).to eq(2) end it "can include deleted products" do - api_get :index, :show_deleted => 1 - json_response["products"].count.should == 3 + api_get :index, show_deleted: 1 + expect(json_response["products"].count).to eq(3) end end it "can create a new product" do - api_post :create, :product => { :name => "The Other Product", - :price => 19.99, - :shipping_category_id => create(:shipping_category).id, - :supplier_id => supplier.id, - :primary_taxon_id => FactoryBot.create(:taxon).id, - :variant_unit => "items", - :variant_unit_name => "things", - :unit_description => "things" - } - json_response.should have_attributes({keys: all_attributes}) - response.status.should == 201 + api_post :create, product: { name: "The Other Product", + price: 19.99, + shipping_category_id: create(:shipping_category).id, + supplier_id: supplier.id, + primary_taxon_id: FactoryBot.create(:taxon).id, + variant_unit: "items", + variant_unit_name: "things", + unit_description: "things" } + expect(json_response).to have_attributes(keys: all_attributes) + expect(response.status).to eq(201) end it "cannot create a new product with invalid attributes" do - api_post :create, :product => {} - response.status.should == 422 - json_response["error"].should == "Invalid resource. Please fix errors and try again." + api_post :create, product: {} + expect(response.status).to eq(422) + expect(json_response["error"]).to eq("Invalid resource. Please fix errors and try again.") errors = json_response["errors"] - errors.keys.should =~ ["name", "price", "primary_taxon", "shipping_category_id", "supplier", "variant_unit"] + expect(errors.keys).to match_array(["name", "price", "primary_taxon", "shipping_category_id", "supplier", "variant_unit"]) end it "can update a product" do - api_put :update, :id => product.to_param, :product => { :name => "New and Improved Product!" } - response.status.should == 200 + api_put :update, id: product.to_param, product: { name: "New and Improved Product!" } + expect(response.status).to eq(200) end it "cannot update a product with an invalid attribute" do - api_put :update, :id => product.to_param, :product => { :name => "" } - response.status.should == 422 - json_response["error"].should == "Invalid resource. Please fix errors and try again." - json_response["errors"]["name"].should == ["can't be blank"] + api_put :update, id: product.to_param, product: { name: "" } + expect(response.status).to eq(422) + expect(json_response["error"]).to eq("Invalid resource. Please fix errors and try again.") + expect(json_response["errors"]["name"]).to eq(["can't be blank"]) end it "can delete a product" do - product.deleted_at.should be_nil - api_delete :destroy, :id => product.to_param - response.status.should == 204 - product.reload.deleted_at.should_not be_nil + expect(product.deleted_at).to be_nil + api_delete :destroy, id: product.to_param + expect(response.status).to eq(204) + expect(product.reload.deleted_at).not_to be_nil end end From 1417b924d2f78f3b7dcf181305bad8f6649f31c3 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 19 Jul 2019 14:03:32 +0100 Subject: [PATCH 17/36] Bring and adapt tests from spree/api/shipments_controller_spec and mix them with exiting tests in OFN --- .../spree/api/shipments_controller_spec.rb | 73 ++++++++++++++++++- 1 file changed, 71 insertions(+), 2 deletions(-) diff --git a/spec/controllers/spree/api/shipments_controller_spec.rb b/spec/controllers/spree/api/shipments_controller_spec.rb index 3c71618c50..9f5dd63d5f 100644 --- a/spec/controllers/spree/api/shipments_controller_spec.rb +++ b/spec/controllers/spree/api/shipments_controller_spec.rb @@ -5,12 +5,27 @@ describe Spree::Api::ShipmentsController, type: :controller do let!(:shipment) { create(:shipment) } let!(:attributes) { [:id, :tracking, :number, :cost, :shipped_at, :stock_location_name, :order_id, :shipping_rates, :shipping_method, :inventory_units] } + let!(:resource_scoping) { { :order_id => shipment.order.to_param, :id => shipment.to_param } } + let(:current_api_user) { build(:user) } before do allow(controller).to receive(:spree_current_user) { current_api_user } end + context "as a non-admin" do + it "cannot make a shipment ready" do + api_put :ready + assert_unauthorized! + end + + it "cannot make a shipment shipped" do + api_put :ship + assert_unauthorized! + end + end + context "as an admin" do + let(:current_api_user) { build(:admin_user) } let!(:order) { shipment.order } let(:order_ship_address) { create(:address) } let!(:stock_location) { create(:stock_location_with_items) } @@ -30,8 +45,6 @@ describe Spree::Api::ShipmentsController, type: :controller do shipment.shipping_method.distributors << variant.product.supplier end - sign_in_as_admin! - context '#create' do it 'creates a shipment if order does not have a shipment' do order.shipment.destroy @@ -77,6 +90,62 @@ describe Spree::Api::ShipmentsController, type: :controller do end end + it "can make a shipment ready" do + Spree::Order.any_instance.stub(:paid? => true, :complete? => true) + api_put :ready + + expect(attributes.all?{ |attr| json_response.key? attr.to_s }).to be_truthy + json_response["state"].should == "ready" + shipment.reload.state.should == "ready" + end + + it "cannot make a shipment ready if the order is unpaid" do + Spree::Order.any_instance.stub(:paid? => false) + api_put :ready + + json_response["error"].should == "Cannot ready shipment." + response.status.should == 422 + end + + context 'for completed shipments' do + let(:order) { create :completed_order_with_totals } + let!(:resource_scoping) { { :order_id => order.to_param, :id => order.shipments.first.to_param } } + + it 'adds a variant to a shipment' do + api_put :add, { variant_id: variant.to_param, quantity: 2 } + response.status.should == 200 + json_response['inventory_units'].select { |h| h['variant_id'] == variant.id }.size.should == 2 + end + + it 'removes a variant from a shipment' do + order.contents.add(variant, 2) + + api_put :remove, { variant_id: variant.to_param, quantity: 1 } + response.status.should == 200 + json_response['inventory_units'].select { |h| h['variant_id'] == variant.id }.size.should == 1 + end + end + + context "can transition a shipment from ready to ship" do + before do + Spree::Order.any_instance.stub(:paid? => true, :complete? => true) + # For the shipment notification email + Spree::Config[:mails_from] = "spree@example.com" + + shipment.update!(shipment.order) + shipment.state.should == "ready" + Spree::ShippingRate.any_instance.stub(:cost => 5) + end + + it "can transition a shipment from ready to ship" do + pending "[Spree build] Failing spec (with postgres)" + shipment.reload + api_put :ship, :order_id => shipment.order.to_param, :id => shipment.to_param, :shipment => { :tracking => "123123" } + json_response.should have_attributes(attributes) + json_response["state"].should == "shipped" + end + end + context 'for a completed order with shipment' do let(:order) { create :completed_order_with_totals } From fd21d35aeebcd7375a2d6fb591282c0e38057adc Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 19 Jul 2019 14:27:11 +0100 Subject: [PATCH 18/36] Transpec and fix rubocop issues in spree/api/shipments_controller_spec --- .../spree/api/shipments_controller_spec.rb | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/spec/controllers/spree/api/shipments_controller_spec.rb b/spec/controllers/spree/api/shipments_controller_spec.rb index 9f5dd63d5f..2b02eb8534 100644 --- a/spec/controllers/spree/api/shipments_controller_spec.rb +++ b/spec/controllers/spree/api/shipments_controller_spec.rb @@ -5,7 +5,7 @@ describe Spree::Api::ShipmentsController, type: :controller do let!(:shipment) { create(:shipment) } let!(:attributes) { [:id, :tracking, :number, :cost, :shipped_at, :stock_location_name, :order_id, :shipping_rates, :shipping_method, :inventory_units] } - let!(:resource_scoping) { { :order_id => shipment.order.to_param, :id => shipment.to_param } } + let!(:resource_scoping) { { order_id: shipment.order.to_param, id: shipment.to_param } } let(:current_api_user) { build(:user) } before do @@ -91,58 +91,58 @@ describe Spree::Api::ShipmentsController, type: :controller do end it "can make a shipment ready" do - Spree::Order.any_instance.stub(:paid? => true, :complete? => true) + allow_any_instance_of(Spree::Order).to receive_messages(paid?: true, complete?: true) api_put :ready expect(attributes.all?{ |attr| json_response.key? attr.to_s }).to be_truthy - json_response["state"].should == "ready" - shipment.reload.state.should == "ready" + expect(json_response["state"]).to eq("ready") + expect(shipment.reload.state).to eq("ready") end it "cannot make a shipment ready if the order is unpaid" do - Spree::Order.any_instance.stub(:paid? => false) + allow_any_instance_of(Spree::Order).to receive_messages(paid?: false) api_put :ready - json_response["error"].should == "Cannot ready shipment." - response.status.should == 422 + expect(json_response["error"]).to eq("Cannot ready shipment.") + expect(response.status).to eq(422) end context 'for completed shipments' do let(:order) { create :completed_order_with_totals } - let!(:resource_scoping) { { :order_id => order.to_param, :id => order.shipments.first.to_param } } + let!(:resource_scoping) { { order_id: order.to_param, id: order.shipments.first.to_param } } it 'adds a variant to a shipment' do - api_put :add, { variant_id: variant.to_param, quantity: 2 } - response.status.should == 200 - json_response['inventory_units'].select { |h| h['variant_id'] == variant.id }.size.should == 2 + api_put :add, variant_id: variant.to_param, quantity: 2 + expect(response.status).to eq(200) + expect(json_response['inventory_units'].select { |h| h['variant_id'] == variant.id }.size).to eq(2) end it 'removes a variant from a shipment' do order.contents.add(variant, 2) - api_put :remove, { variant_id: variant.to_param, quantity: 1 } - response.status.should == 200 - json_response['inventory_units'].select { |h| h['variant_id'] == variant.id }.size.should == 1 + api_put :remove, variant_id: variant.to_param, quantity: 1 + expect(response.status).to eq(200) + expect(json_response['inventory_units'].select { |h| h['variant_id'] == variant.id }.size).to eq(1) end end context "can transition a shipment from ready to ship" do before do - Spree::Order.any_instance.stub(:paid? => true, :complete? => true) + allow_any_instance_of(Spree::Order).to receive_messages(paid?: true, complete?: true) # For the shipment notification email Spree::Config[:mails_from] = "spree@example.com" shipment.update!(shipment.order) - shipment.state.should == "ready" - Spree::ShippingRate.any_instance.stub(:cost => 5) + expect(shipment.state).to eq("ready") + allow_any_instance_of(Spree::ShippingRate).to receive_messages(cost: 5) end it "can transition a shipment from ready to ship" do - pending "[Spree build] Failing spec (with postgres)" shipment.reload - api_put :ship, :order_id => shipment.order.to_param, :id => shipment.to_param, :shipment => { :tracking => "123123" } - json_response.should have_attributes(attributes) - json_response["state"].should == "shipped" + api_put :ship, order_id: shipment.order.to_param, id: shipment.to_param, shipment: { tracking: "123123" } + + expect(attributes.all?{ |attr| json_response.key? attr.to_s }).to be_truthy + expect(json_response["state"]).to eq("shipped") end end From 3771e26ebae743c88da37737be69d301090d9303 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 19 Jul 2019 15:22:43 +0100 Subject: [PATCH 19/36] Bring tests from spree/api/variants_controller_spec from spree_api --- .../spree/api/variants_controller_spec.rb | 156 ++++++++++++++++++ 1 file changed, 156 insertions(+) diff --git a/spec/controllers/spree/api/variants_controller_spec.rb b/spec/controllers/spree/api/variants_controller_spec.rb index 19f4bccc95..da1f6f36a2 100644 --- a/spec/controllers/spree/api/variants_controller_spec.rb +++ b/spec/controllers/spree/api/variants_controller_spec.rb @@ -32,6 +32,129 @@ module Spree expect { variant.reload }.not_to raise_error expect(variant.deleted_at).to be_nil end + + let!(:product) { create(:product) } + let!(:variant) do + variant = product.master + variant.option_values << create(:option_value) + variant + end + let!(:attributes) { [:id, :name, :sku, :price, :weight, :height, + :width, :depth, :is_master, :cost_price, + :permalink] } + + it "can see a paginated list of variants" do + api_get :index + json_response["variants"].first.should have_attributes(attributes) + json_response["count"].should == 1 + json_response["current_page"].should == 1 + json_response["pages"].should == 1 + end + + it 'can control the page size through a parameter' do + create(:variant) + api_get :index, :per_page => 1 + json_response['count'].should == 1 + json_response['current_page'].should == 1 + json_response['pages'].should == 3 + end + + it 'can query the results through a paramter' do + expected_result = create(:variant, :sku => 'FOOBAR') + api_get :index, :q => { :sku_cont => 'FOO' } + json_response['count'].should == 1 + json_response['variants'].first['sku'].should eq expected_result.sku + end + + it "variants returned contain option values data" do + api_get :index + option_values = json_response["variants"].last["option_values"] + option_values.first.should have_attributes([:name, + :presentation, + :option_type_name, + :option_type_id]) + end + + it "variants returned contain images data" do + variant.images.create!(:attachment => image("thinking-cat.jpg")) + + api_get :index + + json_response["variants"].last.should have_attributes([:images]) + end + + # Regression test for #2141 + context "a deleted variant" do + before do + variant.update_column(:deleted_at, Time.now) + end + + it "is not returned in the results" do + api_get :index + json_response["variants"].count.should == 0 + end + + it "is not returned even when show_deleted is passed" do + api_get :index, :show_deleted => true + json_response["variants"].count.should == 0 + end + end + + context "pagination" do + it "can select the next page of variants" do + second_variant = create(:variant) + api_get :index, :page => 2, :per_page => 1 + json_response["variants"].first.should have_attributes(attributes) + json_response["total_count"].should == 3 + json_response["current_page"].should == 2 + json_response["pages"].should == 3 + end + end + + it "can see a single variant" do + api_get :show, :id => variant.to_param + json_response.should have_attributes(attributes) + option_values = json_response["option_values"] + option_values.first.should have_attributes([:name, + :presentation, + :option_type_name, + :option_type_id]) + end + + it "can see a single variant with images" do + variant.images.create!(:attachment => image("thinking-cat.jpg")) + + api_get :show, :id => variant.to_param + + json_response.should have_attributes(attributes + [:images]) + option_values = json_response["option_values"] + option_values.first.should have_attributes([:name, + :presentation, + :option_type_name, + :option_type_id]) + end + + it "can learn how to create a new variant" do + api_get :new + json_response["attributes"].should == attributes.map(&:to_s) + json_response["required_attributes"].should be_empty + end + + it "cannot create a new variant if not an admin" do + api_post :create, :variant => { :sku => "12345" } + assert_unauthorized! + end + + it "cannot update a variant" do + api_put :update, :id => variant.to_param, :variant => { :sku => "12345" } + assert_unauthorized! + end + + it "cannot delete a variant" do + api_delete :destroy, :id => variant.to_param + assert_unauthorized! + lambda { variant.reload }.should_not raise_error + end end context "as an enterprise user" do @@ -71,6 +194,7 @@ module Spree let(:product) { create(:product) } let(:variant) { product.master } + let(:resource_scoping) { { :product_id => variant.product.to_param } } it "soft deletes a variant" do spree_delete :soft_delete, variant_id: variant.to_param, product_id: product.to_param, format: :json @@ -97,6 +221,38 @@ module Spree spree_delete :soft_delete, variant_id: variant.id, product_id: variant.product.permalink, format: :json end end + + # Test for #2141 + context "deleted variants" do + before do + variant.update_column(:deleted_at, Time.now) + end + + it "are visible by admin" do + api_get :index, :show_deleted => 1 + json_response["variants"].count.should == 1 + end + end + + it "can create a new variant" do + api_post :create, :variant => { :sku => "12345" } + json_response.should have_attributes(attributes) + response.status.should == 201 + json_response["sku"].should == "12345" + + variant.product.variants.count.should == 1 + end + + it "can update a variant" do + api_put :update, :id => variant.to_param, :variant => { :sku => "12345" } + response.status.should == 200 + end + + it "can delete a variant" do + api_delete :destroy, :id => variant.to_param + response.status.should == 204 + lambda { Spree::Variant.find(variant.id) }.should raise_error(ActiveRecord::RecordNotFound) + end end end end From 6dfc927730b9c701e3fe78294a052509bd72a9c0 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 19 Jul 2019 16:14:12 +0100 Subject: [PATCH 20/36] Make spree/api/variant_controllers_spec pass --- .../spree/api/variants_controller_spec.rb | 104 ++++++++++-------- 1 file changed, 59 insertions(+), 45 deletions(-) diff --git a/spec/controllers/spree/api/variants_controller_spec.rb b/spec/controllers/spree/api/variants_controller_spec.rb index da1f6f36a2..f4ee2d8d4b 100644 --- a/spec/controllers/spree/api/variants_controller_spec.rb +++ b/spec/controllers/spree/api/variants_controller_spec.rb @@ -9,6 +9,8 @@ module Spree let!(:variant2) { FactoryBot.create(:variant) } let!(:variant3) { FactoryBot.create(:variant) } let(:attributes) { [:id, :options_text, :price, :on_hand, :unit_value, :unit_description, :on_demand, :display_as, :display_name] } + let!(:standard_attributes) { [:id, :name, :sku, :price, :weight, :height, + :width, :depth, :is_master, :cost_price, :permalink] } before do allow(controller).to receive(:spree_current_user) { current_api_user } @@ -17,8 +19,16 @@ module Spree context "as a normal user" do sign_in_as_user! + let!(:product) { create(:product) } + let!(:variant) do + variant = product.master + variant.option_values << create(:option_value) + variant + end + it "retrieves a list of variants with appropriate attributes" do spree_get :index, template: 'bulk_index', format: :json + keys = json_response.first.keys.map(&:to_sym) expect(attributes.all?{ |attr| keys.include? attr }).to eq(true) end @@ -26,27 +36,19 @@ module Spree it "is denied access when trying to delete a variant" do product = create(:product) variant = product.master - spree_delete :soft_delete, variant_id: variant.to_param, product_id: product.to_param, format: :json + assert_unauthorized! expect { variant.reload }.not_to raise_error expect(variant.deleted_at).to be_nil end - let!(:product) { create(:product) } - let!(:variant) do - variant = product.master - variant.option_values << create(:option_value) - variant - end - let!(:attributes) { [:id, :name, :sku, :price, :weight, :height, - :width, :depth, :is_master, :cost_price, - :permalink] } - it "can see a paginated list of variants" do api_get :index - json_response["variants"].first.should have_attributes(attributes) - json_response["count"].should == 1 + + keys = json_response["variants"].first.keys.map(&:to_sym) + expect(standard_attributes.all?{ |attr| keys.include? attr }).to eq(true) + json_response["count"].should == 11 json_response["current_page"].should == 1 json_response["pages"].should == 1 end @@ -54,25 +56,29 @@ module Spree it 'can control the page size through a parameter' do create(:variant) api_get :index, :per_page => 1 + json_response['count'].should == 1 json_response['current_page'].should == 1 - json_response['pages'].should == 3 + json_response['pages'].should == 14 end it 'can query the results through a paramter' do expected_result = create(:variant, :sku => 'FOOBAR') api_get :index, :q => { :sku_cont => 'FOO' } + json_response['count'].should == 1 json_response['variants'].first['sku'].should eq expected_result.sku end it "variants returned contain option values data" do api_get :index + option_values = json_response["variants"].last["option_values"] - option_values.first.should have_attributes([:name, - :presentation, - :option_type_name, - :option_type_id]) + option_values.first.should have_attributes(keys: ["id", + "name", + "presentation", + "option_type_name", + "option_type_id"]) end it "variants returned contain images data" do @@ -80,10 +86,10 @@ module Spree api_get :index - json_response["variants"].last.should have_attributes([:images]) + json_response["variants"].last["images"].should_not be_nil end - # Regression test for #2141 + # Regression test for spree#2141 context "a deleted variant" do before do variant.update_column(:deleted_at, Time.now) @@ -91,12 +97,12 @@ module Spree it "is not returned in the results" do api_get :index - json_response["variants"].count.should == 0 + json_response["variants"].count.should == 10 # there are 11 variants end it "is not returned even when show_deleted is passed" do api_get :index, :show_deleted => true - json_response["variants"].count.should == 0 + json_response["variants"].count.should == 10 # there are 11 variants end end @@ -104,54 +110,57 @@ module Spree it "can select the next page of variants" do second_variant = create(:variant) api_get :index, :page => 2, :per_page => 1 - json_response["variants"].first.should have_attributes(attributes) - json_response["total_count"].should == 3 + + keys = json_response["variants"].first.keys.map(&:to_sym) + expect(standard_attributes.all?{ |attr| keys.include? attr }).to eq(true) + json_response["total_count"].should == 14 json_response["current_page"].should == 2 - json_response["pages"].should == 3 + json_response["pages"].should == 14 end end it "can see a single variant" do api_get :show, :id => variant.to_param - json_response.should have_attributes(attributes) + + keys = json_response.keys.map(&:to_sym) + expect((standard_attributes + [:options_text, :option_values, :images]).all?{ |attr| keys.include? attr }).to eq(true) option_values = json_response["option_values"] - option_values.first.should have_attributes([:name, - :presentation, - :option_type_name, - :option_type_id]) + option_values.first.should have_attributes(keys: ["id", "name", "presentation", + "option_type_name", "option_type_id"]) end it "can see a single variant with images" do variant.images.create!(:attachment => image("thinking-cat.jpg")) - api_get :show, :id => variant.to_param - json_response.should have_attributes(attributes + [:images]) - option_values = json_response["option_values"] - option_values.first.should have_attributes([:name, - :presentation, - :option_type_name, - :option_type_id]) + keys = json_response.keys.map(&:to_sym) + expect((standard_attributes + [:images]).all?{ |attr| keys.include? attr }).to eq(true) + option_values_keys = json_response["option_values"].first.keys.map(&:to_sym) + expect([:name,:presentation,:option_type_name,:option_type_id].all?{ |attr| option_values_keys.include? attr }).to eq(true) end it "can learn how to create a new variant" do api_get :new - json_response["attributes"].should == attributes.map(&:to_s) + + json_response["attributes"].should == standard_attributes.map(&:to_s) json_response["required_attributes"].should be_empty end it "cannot create a new variant if not an admin" do api_post :create, :variant => { :sku => "12345" } + assert_unauthorized! end it "cannot update a variant" do api_put :update, :id => variant.to_param, :variant => { :sku => "12345" } + assert_unauthorized! end it "cannot delete a variant" do api_delete :destroy, :id => variant.to_param + assert_unauthorized! lambda { variant.reload }.should_not raise_error end @@ -167,6 +176,7 @@ module Spree it "soft deletes a variant" do spree_delete :soft_delete, variant_id: variant.to_param, product_id: product.to_param, format: :json + expect(response.status).to eq(204) expect { variant.reload }.not_to raise_error expect(variant.deleted_at).to be_present @@ -174,6 +184,7 @@ module Spree it "is denied access to soft deleting another enterprises' variant" do spree_delete :soft_delete, variant_id: variant_other.to_param, product_id: product_other.to_param, format: :json + assert_unauthorized! expect { variant.reload }.not_to raise_error expect(variant.deleted_at).to be_nil @@ -198,6 +209,7 @@ module Spree it "soft deletes a variant" do spree_delete :soft_delete, variant_id: variant.to_param, product_id: product.to_param, format: :json + expect(response.status).to eq(204) expect { variant.reload }.not_to raise_error expect(variant.deleted_at).not_to be_nil @@ -206,7 +218,6 @@ module Spree it "doesn't delete the only variant of the product" do product = create(:product) variant = product.variants.first - spree_delete :soft_delete, variant_id: variant.to_param, product_id: product.to_param, format: :json expect(variant.reload).to_not be_deleted @@ -222,7 +233,6 @@ module Spree end end - # Test for #2141 context "deleted variants" do before do variant.update_column(:deleted_at, Time.now) @@ -230,26 +240,30 @@ module Spree it "are visible by admin" do api_get :index, :show_deleted => 1 - json_response["variants"].count.should == 1 + + json_response["variants"].count.should == 2 end end it "can create a new variant" do - api_post :create, :variant => { :sku => "12345" } - json_response.should have_attributes(attributes) + original_number_of_variants = variant.product.variants.count + api_post :create, :variant => { :sku => "12345", :unit_value => "weight", :unit_description => "L" } + + expect(standard_attributes.all?{ |attr| json_response.include? attr.to_s }).to eq(true) response.status.should == 201 json_response["sku"].should == "12345" - - variant.product.variants.count.should == 1 + variant.product.variants.count.should == original_number_of_variants + 1 end it "can update a variant" do api_put :update, :id => variant.to_param, :variant => { :sku => "12345" } + response.status.should == 200 end it "can delete a variant" do api_delete :destroy, :id => variant.to_param + response.status.should == 204 lambda { Spree::Variant.find(variant.id) }.should raise_error(ActiveRecord::RecordNotFound) end From 8bc9985edb90aee0bfad5331633c492308ae4631 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 19 Jul 2019 16:17:37 +0100 Subject: [PATCH 21/36] Transpec and fix rubocop issues in spree/api/variants_controller_spec --- .../spree/api/variants_controller_spec.rb | 109 +++++++++--------- 1 file changed, 55 insertions(+), 54 deletions(-) diff --git a/spec/controllers/spree/api/variants_controller_spec.rb b/spec/controllers/spree/api/variants_controller_spec.rb index f4ee2d8d4b..e32f34809a 100644 --- a/spec/controllers/spree/api/variants_controller_spec.rb +++ b/spec/controllers/spree/api/variants_controller_spec.rb @@ -9,8 +9,10 @@ module Spree let!(:variant2) { FactoryBot.create(:variant) } let!(:variant3) { FactoryBot.create(:variant) } let(:attributes) { [:id, :options_text, :price, :on_hand, :unit_value, :unit_description, :on_demand, :display_as, :display_name] } - let!(:standard_attributes) { [:id, :name, :sku, :price, :weight, :height, - :width, :depth, :is_master, :cost_price, :permalink] } + let!(:standard_attributes) { + [:id, :name, :sku, :price, :weight, :height, + :width, :depth, :is_master, :cost_price, :permalink] + } before do allow(controller).to receive(:spree_current_user) { current_api_user } @@ -48,121 +50,120 @@ module Spree keys = json_response["variants"].first.keys.map(&:to_sym) expect(standard_attributes.all?{ |attr| keys.include? attr }).to eq(true) - json_response["count"].should == 11 - json_response["current_page"].should == 1 - json_response["pages"].should == 1 + expect(json_response["count"]).to eq(11) + expect(json_response["current_page"]).to eq(1) + expect(json_response["pages"]).to eq(1) end it 'can control the page size through a parameter' do create(:variant) - api_get :index, :per_page => 1 + api_get :index, per_page: 1 - json_response['count'].should == 1 - json_response['current_page'].should == 1 - json_response['pages'].should == 14 + expect(json_response['count']).to eq(1) + expect(json_response['current_page']).to eq(1) + expect(json_response['pages']).to eq(14) end it 'can query the results through a paramter' do - expected_result = create(:variant, :sku => 'FOOBAR') - api_get :index, :q => { :sku_cont => 'FOO' } + expected_result = create(:variant, sku: 'FOOBAR') + api_get :index, q: { sku_cont: 'FOO' } - json_response['count'].should == 1 - json_response['variants'].first['sku'].should eq expected_result.sku + expect(json_response['count']).to eq(1) + expect(json_response['variants'].first['sku']).to eq expected_result.sku end it "variants returned contain option values data" do api_get :index option_values = json_response["variants"].last["option_values"] - option_values.first.should have_attributes(keys: ["id", - "name", - "presentation", - "option_type_name", - "option_type_id"]) + expect(option_values.first).to have_attributes(keys: ["id", + "name", + "presentation", + "option_type_name", + "option_type_id"]) end it "variants returned contain images data" do - variant.images.create!(:attachment => image("thinking-cat.jpg")) + variant.images.create!(attachment: image("thinking-cat.jpg")) api_get :index - json_response["variants"].last["images"].should_not be_nil + expect(json_response["variants"].last["images"]).not_to be_nil end # Regression test for spree#2141 context "a deleted variant" do before do - variant.update_column(:deleted_at, Time.now) + variant.update_column(:deleted_at, Time.zone.now) end it "is not returned in the results" do api_get :index - json_response["variants"].count.should == 10 # there are 11 variants + expect(json_response["variants"].count).to eq(10) # there are 11 variants end it "is not returned even when show_deleted is passed" do - api_get :index, :show_deleted => true - json_response["variants"].count.should == 10 # there are 11 variants + api_get :index, show_deleted: true + expect(json_response["variants"].count).to eq(10) # there are 11 variants end end context "pagination" do it "can select the next page of variants" do second_variant = create(:variant) - api_get :index, :page => 2, :per_page => 1 + api_get :index, page: 2, per_page: 1 keys = json_response["variants"].first.keys.map(&:to_sym) expect(standard_attributes.all?{ |attr| keys.include? attr }).to eq(true) - json_response["total_count"].should == 14 - json_response["current_page"].should == 2 - json_response["pages"].should == 14 + expect(json_response["total_count"]).to eq(14) + expect(json_response["current_page"]).to eq(2) + expect(json_response["pages"]).to eq(14) end end it "can see a single variant" do - api_get :show, :id => variant.to_param + api_get :show, id: variant.to_param keys = json_response.keys.map(&:to_sym) expect((standard_attributes + [:options_text, :option_values, :images]).all?{ |attr| keys.include? attr }).to eq(true) option_values = json_response["option_values"] - option_values.first.should have_attributes(keys: ["id", "name", "presentation", - "option_type_name", "option_type_id"]) + expect(option_values.first).to have_attributes(keys: ["id", "name", "presentation", "option_type_name", "option_type_id"]) end it "can see a single variant with images" do - variant.images.create!(:attachment => image("thinking-cat.jpg")) - api_get :show, :id => variant.to_param + variant.images.create!(attachment: image("thinking-cat.jpg")) + api_get :show, id: variant.to_param keys = json_response.keys.map(&:to_sym) expect((standard_attributes + [:images]).all?{ |attr| keys.include? attr }).to eq(true) option_values_keys = json_response["option_values"].first.keys.map(&:to_sym) - expect([:name,:presentation,:option_type_name,:option_type_id].all?{ |attr| option_values_keys.include? attr }).to eq(true) + expect([:name, :presentation, :option_type_id].all?{ |attr| option_values_keys.include? attr }).to eq(true) end it "can learn how to create a new variant" do api_get :new - json_response["attributes"].should == standard_attributes.map(&:to_s) - json_response["required_attributes"].should be_empty + expect(json_response["attributes"]).to eq(standard_attributes.map(&:to_s)) + expect(json_response["required_attributes"]).to be_empty end it "cannot create a new variant if not an admin" do - api_post :create, :variant => { :sku => "12345" } + api_post :create, variant: { sku: "12345" } assert_unauthorized! end it "cannot update a variant" do - api_put :update, :id => variant.to_param, :variant => { :sku => "12345" } + api_put :update, id: variant.to_param, variant: { sku: "12345" } assert_unauthorized! end it "cannot delete a variant" do - api_delete :destroy, :id => variant.to_param + api_delete :destroy, id: variant.to_param assert_unauthorized! - lambda { variant.reload }.should_not raise_error + expect { variant.reload }.not_to raise_error end end @@ -205,7 +206,7 @@ module Spree let(:product) { create(:product) } let(:variant) { product.master } - let(:resource_scoping) { { :product_id => variant.product.to_param } } + let(:resource_scoping) { { product_id: variant.product.to_param } } it "soft deletes a variant" do spree_delete :soft_delete, variant_id: variant.to_param, product_id: product.to_param, format: :json @@ -235,38 +236,38 @@ module Spree context "deleted variants" do before do - variant.update_column(:deleted_at, Time.now) + variant.update_column(:deleted_at, Time.zone.now) end it "are visible by admin" do - api_get :index, :show_deleted => 1 + api_get :index, show_deleted: 1 - json_response["variants"].count.should == 2 + expect(json_response["variants"].count).to eq(2) end end it "can create a new variant" do original_number_of_variants = variant.product.variants.count - api_post :create, :variant => { :sku => "12345", :unit_value => "weight", :unit_description => "L" } + api_post :create, variant: { sku: "12345", unit_value: "weight", unit_description: "L" } expect(standard_attributes.all?{ |attr| json_response.include? attr.to_s }).to eq(true) - response.status.should == 201 - json_response["sku"].should == "12345" - variant.product.variants.count.should == original_number_of_variants + 1 + expect(response.status).to eq(201) + expect(json_response["sku"]).to eq("12345") + expect(variant.product.variants.count).to eq(original_number_of_variants + 1) end it "can update a variant" do - api_put :update, :id => variant.to_param, :variant => { :sku => "12345" } + api_put :update, id: variant.to_param, variant: { sku: "12345" } - response.status.should == 200 + expect(response.status).to eq(200) end it "can delete a variant" do - api_delete :destroy, :id => variant.to_param + api_delete :destroy, id: variant.to_param - response.status.should == 204 - lambda { Spree::Variant.find(variant.id) }.should raise_error(ActiveRecord::RecordNotFound) - end + expect(response.status).to eq(204) + expect { Spree::Variant.find(variant.id) }.to raise_error(ActiveRecord::RecordNotFound) + end end end end From 104bd31f9b202ec0df87b20c13f74a0f54428d20 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 19 Jul 2019 17:15:51 +0100 Subject: [PATCH 22/36] Add necessary spree api routes: taxons, variants and shipments --- config/routes/spree.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/config/routes/spree.rb b/config/routes/spree.rb index 5613ae9c11..0f9bb97617 100644 --- a/config/routes/spree.rb +++ b/config/routes/spree.rb @@ -76,9 +76,22 @@ Spree::Core::Engine.routes.prepend do end end + resources :variants, :only => [:index] + resources :orders do get :managed, on: :collection + + resources :shipments, :only => [:create, :update] do + member do + put :ready + put :ship + put :add + put :remove + end + end end + + resources :taxons, :only => [:index] end namespace :admin do From a267848394b39098aa6cda88122ef55832940ed4 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 19 Jul 2019 17:34:50 +0100 Subject: [PATCH 23/36] Remove unused api routes from views/spree/admin/shared/routes view --- app/views/spree/admin/shared/_routes.html.erb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/app/views/spree/admin/shared/_routes.html.erb b/app/views/spree/admin/shared/_routes.html.erb index 863052a4ab..056a49cf0c 100644 --- a/app/views/spree/admin/shared/_routes.html.erb +++ b/app/views/spree/admin/shared/_routes.html.erb @@ -3,11 +3,6 @@ :variants_search => spree.admin_search_variants_path(:format => 'json'), :taxons_search => spree.api_taxons_path(:format => 'json'), :user_search => spree.admin_search_users_path(:format => 'json'), - :product_search => spree.api_products_path(:format => 'json'), - :option_type_search => spree.api_option_types_path(:format => 'json'), - :states_search => spree.api_states_path(:format => 'json'), - :orders_api => spree.api_orders_path(:format => 'json'), - :stock_locations_api => spree.api_stock_locations_path(:format => 'json'), - :variants_api => spree.api_variants_path(:format => 'json') + :orders_api => spree.api_orders_path(:format => 'json') }.to_json %>; From 5182286218f02a37c8bf2b1f65cdb160fda179a1 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 19 Jul 2019 18:03:25 +0100 Subject: [PATCH 24/36] Add necessary spree api routes related to api keys for users and bring respective implementations from spree_api --- app/models/spree/user.rb | 10 ++++++++++ config/routes/spree.rb | 7 +++++++ 2 files changed, 17 insertions(+) diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index cad09a6073..9e33717c34 100644 --- a/app/models/spree/user.rb +++ b/app/models/spree/user.rb @@ -136,6 +136,16 @@ module Spree has_spree_role?('admin') end + def generate_spree_api_key! + self.spree_api_key = SecureRandom.hex(24) + save! + end + + def clear_spree_api_key! + self.spree_api_key = nil + save! + end + protected def password_required? diff --git a/config/routes/spree.rb b/config/routes/spree.rb index 0f9bb97617..05a709f0ff 100644 --- a/config/routes/spree.rb +++ b/config/routes/spree.rb @@ -118,6 +118,13 @@ Spree::Core::Engine.routes.prepend do end end end + + resources :users do + member do + put :generate_api_key + put :clear_api_key + end + end end resources :orders do From 7346a4998267fd5cfed4f2e58869bff6a64f839b Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 19 Jul 2019 16:39:14 +0100 Subject: [PATCH 25/36] Move routes in ofn api namespace to separate routes file --- config/application.rb | 1 + config/routes.rb | 32 -------------------------------- config/routes/api.rb | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 32 deletions(-) create mode 100644 config/routes/api.rb diff --git a/config/application.rb b/config/application.rb index b86316c960..cb4160c647 100644 --- a/config/application.rb +++ b/config/application.rb @@ -113,6 +113,7 @@ module Openfoodnetwork ) config.paths["config/routes"] = %w( + config/routes/api.rb config/routes.rb config/routes/admin.rb config/routes/spree.rb diff --git a/config/routes.rb b/config/routes.rb index ac889a2d52..6e7df6ea64 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -88,38 +88,6 @@ Openfoodnetwork::Application.routes.draw do get '/:id/shop', to: 'enterprises#shop', as: 'enterprise_shop' get "/enterprises/:permalink", to: redirect("/") # Legacy enterprise URL - namespace :api do - resources :enterprises do - post :update_image, on: :member - get :managed, on: :collection - get :accessible, on: :collection - - resource :logo, only: [:destroy] - resource :promo_image, only: [:destroy] - - member do - get :shopfront - end - end - - resources :order_cycles do - get :managed, on: :collection - get :accessible, on: :collection - end - - resources :orders, only: [:index] - - resource :status do - get :job_queue - end - - resources :customers, only: [:index, :update] - - resources :enterprise_fees, only: [:destroy] - - post '/product_images/:product_id', to: 'product_images#update_product_image' - end - get 'sitemap.xml', to: 'sitemap#index', defaults: { format: 'xml' } # Mount engine routes diff --git a/config/routes/api.rb b/config/routes/api.rb new file mode 100644 index 0000000000..99c7f76eda --- /dev/null +++ b/config/routes/api.rb @@ -0,0 +1,33 @@ +Openfoodnetwork::Application.routes.draw do + namespace :api do + resources :enterprises do + post :update_image, on: :member + get :managed, on: :collection + get :accessible, on: :collection + + resource :logo, only: [:destroy] + resource :promo_image, only: [:destroy] + + member do + get :shopfront + end + end + + resources :order_cycles do + get :managed, on: :collection + get :accessible, on: :collection + end + + resources :orders, only: [:index] + + resource :status do + get :job_queue + end + + resources :customers, only: [:index, :update] + + resources :enterprise_fees, only: [:destroy] + + post '/product_images/:product_id', to: 'product_images#update_product_image' + end +end From 314ed50e0f9ede36277136d8d3be2fb7ef1c13d5 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 19 Jul 2019 21:07:28 +0100 Subject: [PATCH 26/36] Fix a rubocop issue in spree/api/products_controller --- app/controllers/spree/api/products_controller.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/controllers/spree/api/products_controller.rb b/app/controllers/spree/api/products_controller.rb index 491abe6f4a..615596383e 100644 --- a/app/controllers/spree/api/products_controller.rb +++ b/app/controllers/spree/api/products_controller.rb @@ -63,7 +63,9 @@ module Spree authorize! :read, Spree::Product @products = product_scope. - ransack(params[:q]).result.managed_by(current_api_user).page(params[:page]).per(params[:per_page]) + ransack(params[:q]).result. + managed_by(current_api_user). + page(params[:page]).per(params[:per_page]) respond_with(@products, default_template: :index) end From 18aa16650d8030c28591a7a7e0234cfd5b4ebc9c Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 19 Jul 2019 23:12:19 +0100 Subject: [PATCH 27/36] Remove dependency to Spree::ApiConfiguration, overall requires_authentication? is true, exceptions will be endpoint specific --- app/controllers/spree/api/base_controller.rb | 4 ++-- spec/controllers/spree/api/base_controller_spec.rb | 1 - spec/spec_helper.rb | 2 -- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/app/controllers/spree/api/base_controller.rb b/app/controllers/spree/api/base_controller.rb index 7dc2867eb3..3f28596fec 100644 --- a/app/controllers/spree/api/base_controller.rb +++ b/app/controllers/spree/api/base_controller.rb @@ -57,7 +57,7 @@ module Spree def check_for_user_or_api_key # User is already authenticated with Spree, make request this way instead. return true if @current_api_user = try_spree_current_user || - !Spree::Api::Config[:requires_authentication] + !requires_authentication? return if api_key.present? render("spree/api/errors/must_specify_api_key", status: :unauthorized) && return @@ -86,7 +86,7 @@ module Spree end def requires_authentication? - Spree::Api::Config[:requires_authentication] + true end def not_found diff --git a/spec/controllers/spree/api/base_controller_spec.rb b/spec/controllers/spree/api/base_controller_spec.rb index b8ad0580a1..cab638c74e 100644 --- a/spec/controllers/spree/api/base_controller_spec.rb +++ b/spec/controllers/spree/api/base_controller_spec.rb @@ -14,7 +14,6 @@ describe Spree::Api::BaseController do before do allow(controller).to receive_messages try_spree_current_user: double(email: "spree@example.com") - Spree::Api::Config[:requires_authentication] = true end it "can make a request" do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 499a810302..59354e2701 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -127,8 +127,6 @@ RSpec.configure do |config| spree_config.shipping_instructions = true spree_config.auto_capture = true end - - Spree::Api::Config[:requires_authentication] = true end # Helpers From 2ae75ce13e7584025b5d548d384f043e90b1d37d Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 19 Jul 2019 23:33:28 +0100 Subject: [PATCH 28/36] Add ControllerSetup from spree_api as it is used in spree/api/base_controller --- lib/spree/api/controller_setup.rb | 33 +++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 lib/spree/api/controller_setup.rb diff --git a/lib/spree/api/controller_setup.rb b/lib/spree/api/controller_setup.rb new file mode 100644 index 0000000000..e26c300342 --- /dev/null +++ b/lib/spree/api/controller_setup.rb @@ -0,0 +1,33 @@ +require 'spree/api/responders' + +module Spree + module Api + module ControllerSetup + def self.included(klass) + klass.class_eval do + include AbstractController::Rendering + include AbstractController::ViewPaths + include AbstractController::Callbacks + include AbstractController::Helpers + + include ActiveSupport::Rescuable + + include ActionController::Rendering + include ActionController::ImplicitRender + include ActionController::Rescue + include ActionController::MimeResponds + include ActionController::Head + + include CanCan::ControllerAdditions + include Spree::Core::ControllerHelpers::Auth + + prepend_view_path Rails.root + "app/views" + append_view_path File.expand_path("../../../app/views", File.dirname(__FILE__)) + + self.responder = Spree::Api::Responders::AppResponder + respond_to :json + end + end + end + end +end From 50765563f8a07163287b86a00f81eb5a399d124a Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 19 Jul 2019 23:44:50 +0100 Subject: [PATCH 29/36] Bring spree/api_helpers from spree_api --- app/helpers/spree/api/api_helpers.rb | 112 +++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 app/helpers/spree/api/api_helpers.rb diff --git a/app/helpers/spree/api/api_helpers.rb b/app/helpers/spree/api/api_helpers.rb new file mode 100644 index 0000000000..5341d92d2f --- /dev/null +++ b/app/helpers/spree/api/api_helpers.rb @@ -0,0 +1,112 @@ +module Spree + module Api + module ApiHelpers + def required_fields_for(model) + required_fields = model._validators.select do |field, validations| + validations.any? { |v| v.is_a?(ActiveModel::Validations::PresenceValidator) } + end.map(&:first) # get fields that are invalid + # Permalinks presence is validated, but are really automatically generated + # Therefore we shouldn't tell API clients that they MUST send one through + required_fields.map!(&:to_s).delete("permalink") + required_fields + end + + def product_attributes + [:id, :name, :description, :price, :available_on, :permalink, :meta_description, :meta_keywords, :shipping_category_id, :taxon_ids] + end + + def product_property_attributes + [:id, :product_id, :property_id, :value, :property_name] + end + + def variant_attributes + [:id, :name, :sku, :price, :weight, :height, :width, :depth, :is_master, :cost_price, :permalink] + end + + def image_attributes + [:id, :position, :attachment_content_type, :attachment_file_name, :type, :attachment_updated_at, :attachment_width, :attachment_height, :alt] + end + + def option_value_attributes + [:id, :name, :presentation, :option_type_name, :option_type_id] + end + + def order_attributes + [:id, :number, :item_total, :total, :state, :adjustment_total, :user_id, :created_at, :updated_at, :completed_at, :payment_total, :shipment_state, :payment_state, :email, :special_instructions, :token] + end + + def line_item_attributes + [:id, :quantity, :price, :variant_id] + end + + def option_type_attributes + [:id, :name, :presentation, :position] + end + + def payment_attributes + [:id, :source_type, :source_id, :amount, :payment_method_id, :response_code, :state, :avs_response, :created_at, :updated_at] + end + + def payment_method_attributes + [:id, :name, :description] + end + + def shipment_attributes + [:id, :tracking, :number, :cost, :shipped_at, :state] + end + + def taxonomy_attributes + [:id, :name] + end + + def taxon_attributes + [:id, :name, :pretty_name, :permalink, :position, :parent_id, :taxonomy_id] + end + + def inventory_unit_attributes + [:id, :lock_version, :state, :variant_id, :shipment_id, :return_authorization_id] + end + + def return_authorization_attributes + [:id, :number, :state, :amount, :order_id, :reason, :created_at, :updated_at] + end + + def country_attributes + [:id, :iso_name, :iso, :iso3, :name, :numcode] + end + + def state_attributes + [:id, :name, :abbr, :country_id] + end + + def adjustment_attributes + [:id, :source_type, :source_id, :adjustable_type, :adjustable_id, :originator_type, :originator_id, :amount, :label, :mandatory, :locked, :eligible, :created_at, :updated_at] + end + + def creditcard_attributes + [:id, :month, :year, :cc_type, :last_digits, :first_name, :last_name, :gateway_customer_profile_id, :gateway_payment_profile_id] + end + + def user_attributes + [:id, :email, :created_at, :updated_at] + end + + def property_attributes + [:id, :name, :presentation] + end + + def stock_location_attributes + [:id, :name, :address1, :address2, :city, :state_id, :state_name, :country_id, :zipcode, :phone, :active] + end + + def stock_movement_attributes + [:id, :quantity, :stock_item_id] + end + + def stock_item_attributes + [:id, :count_on_hand, :backorderable, :lock_version, :stock_location_id, :variant_id] + end + end + end +end + From 25451eed6bca00bd33aed6f512f0e3ad67b38d6c Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sat, 20 Jul 2019 00:35:36 +0100 Subject: [PATCH 30/36] Bring api spec helpers from spree_api into ofn/api_helper --- lib/spree/api/testing_support/helpers_decorator.rb | 7 ------- spec/controllers/api/customers_controller_spec.rb | 1 - spec/controllers/api/orders_controller_spec.rb | 1 - spec/controllers/api/product_images_controller_spec.rb | 1 - spec/controllers/spree/checkout_controller_spec.rb | 1 - spec/controllers/spree/users_controller_spec.rb | 1 - spec/controllers/user_passwords_controller_spec.rb | 1 - spec/controllers/user_registrations_controller_spec.rb | 1 - spec/spec_helper.rb | 5 ++--- spec/support/api_helper.rb | 9 +++++++++ 10 files changed, 11 insertions(+), 17 deletions(-) delete mode 100644 lib/spree/api/testing_support/helpers_decorator.rb diff --git a/lib/spree/api/testing_support/helpers_decorator.rb b/lib/spree/api/testing_support/helpers_decorator.rb deleted file mode 100644 index eb8e1f107d..0000000000 --- a/lib/spree/api/testing_support/helpers_decorator.rb +++ /dev/null @@ -1,7 +0,0 @@ -require 'spree/api/testing_support/helpers' - -Spree::Api::TestingSupport::Helpers.class_eval do - def current_api_user - @current_api_user ||= Spree::LegacyUser.new(email: "spree@example.com", enterprises: []) - end -end diff --git a/spec/controllers/api/customers_controller_spec.rb b/spec/controllers/api/customers_controller_spec.rb index f6c8f4e0a5..cab5b2e1e1 100644 --- a/spec/controllers/api/customers_controller_spec.rb +++ b/spec/controllers/api/customers_controller_spec.rb @@ -3,7 +3,6 @@ require 'spec_helper' module Api describe CustomersController, type: :controller do include AuthenticationWorkflow - include OpenFoodNetwork::ApiHelper render_views let(:user) { create(:user) } diff --git a/spec/controllers/api/orders_controller_spec.rb b/spec/controllers/api/orders_controller_spec.rb index 9c11b8bd0b..0819733e82 100644 --- a/spec/controllers/api/orders_controller_spec.rb +++ b/spec/controllers/api/orders_controller_spec.rb @@ -1,5 +1,4 @@ require 'spec_helper' -require 'spree/api/testing_support/helpers' module Api describe OrdersController, type: :controller do diff --git a/spec/controllers/api/product_images_controller_spec.rb b/spec/controllers/api/product_images_controller_spec.rb index e0d70c2fac..5015d41b00 100644 --- a/spec/controllers/api/product_images_controller_spec.rb +++ b/spec/controllers/api/product_images_controller_spec.rb @@ -1,5 +1,4 @@ require 'spec_helper' -require 'spree/api/testing_support/helpers' module Api describe ProductImagesController, type: :controller do diff --git a/spec/controllers/spree/checkout_controller_spec.rb b/spec/controllers/spree/checkout_controller_spec.rb index aa7229367a..00f385a810 100644 --- a/spec/controllers/spree/checkout_controller_spec.rb +++ b/spec/controllers/spree/checkout_controller_spec.rb @@ -1,5 +1,4 @@ require 'spec_helper' -require 'spree/api/testing_support/helpers' require 'support/request/authentication_workflow' describe Spree::CheckoutController, type: :controller do diff --git a/spec/controllers/spree/users_controller_spec.rb b/spec/controllers/spree/users_controller_spec.rb index 8e68b73720..50ac3ebf6f 100644 --- a/spec/controllers/spree/users_controller_spec.rb +++ b/spec/controllers/spree/users_controller_spec.rb @@ -1,5 +1,4 @@ require 'spec_helper' -require 'spree/api/testing_support/helpers' describe Spree::UsersController, type: :controller do include AuthenticationWorkflow diff --git a/spec/controllers/user_passwords_controller_spec.rb b/spec/controllers/user_passwords_controller_spec.rb index 90234ffd0d..a546fddb96 100644 --- a/spec/controllers/user_passwords_controller_spec.rb +++ b/spec/controllers/user_passwords_controller_spec.rb @@ -1,5 +1,4 @@ require 'spec_helper' -require 'spree/api/testing_support/helpers' describe UserPasswordsController, type: :controller do include OpenFoodNetwork::EmailHelper diff --git a/spec/controllers/user_registrations_controller_spec.rb b/spec/controllers/user_registrations_controller_spec.rb index a90906da51..a1c75d83e1 100644 --- a/spec/controllers/user_registrations_controller_spec.rb +++ b/spec/controllers/user_registrations_controller_spec.rb @@ -1,5 +1,4 @@ require 'spec_helper' -require 'spree/api/testing_support/helpers' describe UserRegistrationsController, type: :controller do include OpenFoodNetwork::EmailHelper diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 59354e2701..ef9ae66667 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -39,10 +39,9 @@ Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f } require 'spree/testing_support/controller_requests' require 'spree/testing_support/capybara_ext' require 'spree/api/testing_support/setup' -require 'spree/api/testing_support/helpers' -require 'spree/api/testing_support/helpers_decorator' require 'spree/testing_support/authorization_helpers' require 'spree/testing_support/preferences' +require 'support/api_helper' # Capybara config require 'selenium-webdriver' @@ -138,7 +137,7 @@ RSpec.configure do |config| config.include Spree::TestingSupport::Preferences config.include Devise::TestHelpers, type: :controller config.extend Spree::Api::TestingSupport::Setup, type: :controller - config.include Spree::Api::TestingSupport::Helpers, type: :controller + config.include OpenFoodNetwork::ApiHelper, type: :controller config.include OpenFoodNetwork::ControllerHelper, type: :controller config.include Features::DatepickerHelper, type: :feature config.include OpenFoodNetwork::FeatureToggleHelper diff --git a/spec/support/api_helper.rb b/spec/support/api_helper.rb index 08918fb761..bdd15e57d4 100644 --- a/spec/support/api_helper.rb +++ b/spec/support/api_helper.rb @@ -11,5 +11,14 @@ module OpenFoodNetwork json_response end end + + def current_api_user + @current_api_user ||= Spree::LegacyUser.new(email: "spree@example.com", enterprises: []) + end + + def assert_unauthorized! + json_response.should == { "error" => "You are not authorized to perform that action." } + response.status.should == 401 + end end end From a57504ba1fef7c828c5d286a5fb9797240ea297e Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Sat, 20 Jul 2019 10:20:34 +0100 Subject: [PATCH 31/36] Bring api_helper.image from spree_api to support spree/api/products_controller_spec --- spec/support/api_helper.rb | 4 ++++ spec/support/fixtures/thinking-cat.jpg | Bin 0 -> 18090 bytes 2 files changed, 4 insertions(+) create mode 100644 spec/support/fixtures/thinking-cat.jpg diff --git a/spec/support/api_helper.rb b/spec/support/api_helper.rb index bdd15e57d4..582f7ea341 100644 --- a/spec/support/api_helper.rb +++ b/spec/support/api_helper.rb @@ -20,5 +20,9 @@ module OpenFoodNetwork json_response.should == { "error" => "You are not authorized to perform that action." } response.status.should == 401 end + + def image(filename) + File.open(Rails.root + "spec/support/fixtures" + filename) + end end end diff --git a/spec/support/fixtures/thinking-cat.jpg b/spec/support/fixtures/thinking-cat.jpg new file mode 100644 index 0000000000000000000000000000000000000000..7e8524d367bb9b358fd351985893ab2eba304b92 GIT binary patch literal 18090 zcmb5UWl)?=w6;5FaCdiif3e^Mml@pMJ-7sS4LT59hu{Pm+zAe02oAvsPH;bb``hRI zKdb+(yPmG@u3ArB_v(IIdiw*wQC3h=0KmZk0C4XE@U{w&1)v}!e?Ue;`S9TbDk=&Z z1_34pIywd^9zHe!B^fmpB^d<;4FeZ54IKwP1qF*RD+dp+fPers^M4W|eBxaE0(}2x z5;#;;R19u`=0J$op0^S_+{rg;c*6W+KJ{(jp z|GsU3dGqgZp#T9)0DNl3-mm4)VTd4@|Jp1fuMk+Xp9eYD9ssbUcr@ZlMI%r&N)gM$ z4dsFR)dwH}Z zTIPKKv_534UtG8VDUq>ZDb<&62dDr%cZx?7cCs;0Iit!7??*Y zGS{5PGrP4i58Y&)!hps9vWe}Gy`;v#InIXfuaA{R!DJ3Cjhb`|!_tdxfvYFu^?5T` z-UIAV%Be86m`i$7t)A_2@W)D9olc4lAn+;MtGnOeyY97Urq@3m5rfX$%DN==6QO_H zG;Ozg8ihk)sPGQIc>vjpFsuYgpsSxmUKKzTHIPgZBJkeAg=?FB%P^ z6r6B9WPR@ndVB3QETI|6IyFC+r&wDe>JY=_?bS5%(W+RlaKBdG^|C@Qhz>eJApqVX2?f>uwzDQBxnj@yyY^|Xgy_*t$Oh5q=t26wvw|2-*@b_8 zp~q5T2#DMLtEXcAj(@6_`O$mJN0%jidfet(4ok-@mhE!8560$VyfO!+5$7F5Vi5&d zeP6=H$B#DEC4H0}=jVDSef~4i(Chq|cVJLI^(aZrUhC4Wu7}1EZi)O2Cntv9tnTE> zVWcCdK-XaX_pXm9MeR4yr`KlT4=EZNNvUnKW2X;5t{u{azFNWJ!jvd{CbMig$8{F_T45MUPj9no7t-h2f+PCAQ93iJrlfut_0>Mx0*>zPLlclW!VFw@g2l?zED1c{hxcq z(<9~PQ9*Dt=6h{R0pIj}Kl|9HDbi<4Y{d6`h&Y$l*gc!5bPww{g?4L?dtR+6Qr&h_8k$i6DQMh_h!th%CR46UW+4wBP zd96@_?&(RjOM5p8w^vonxtX4%KbVa~gCWQ=2|X=iloBIZlx^pwfv z!Mq|?`?SC~FAlksMbK}> zgmi;iYn(94sIOP^{Pns^09R|DyML*?cnVkB@>kNUh3w3+t>5J+We5sI&Z$C5?Z|1vIE<=JQ{cP0Htd2iZ(DU=;+O(H zXLA}^c&V}f-m4_JP3W}4dcC^C>!9j7*L`Q*)EV|?qJL{1mjML_fn-E!xf%9oFusIGxzNbo z7pLXVQ6vNHa_X!!T7?xyH}ncY#?xQ86LznPQ31KB{fTL;j@MLe!I|+L)lrVPl@z})NgZGbdRAWMap7t3k$GG9BtklWAGfl3 z<_p+HFs!?01=tMTbwli?(o@p0<99)^;r6Hz&J0S+b2l4ps^6TO*hmi(GGu;k*W0Dt zc25wQ+R`WOiQnMDp}?y}xU{jl-i9n)KL7RCzE$l9`&3IkNq zIJ=AkH%}!@$I67!T=HmXN;yf-j&4qslih?fHR8AJq-X&ZOUty=+J7Hs%sO1PRDI}( zlyRacszr%>d~iyc#ga5quL$L&LgiM;U7#w8Mzc>;NsuabjszbZt$J=|@M>x0uMR@n z{)qf-5goZtaEPl`F3^dPv-A36)7^}3F0N>rVKGSu-3nxnS@xBj4P)ElQ-VV>01lE- zW>yW?g3*>!`;2lC8ntn(@h0~!X2^PSvhGTt1SxP^JX$}^JdE9%S6!~+c75$Of7Qk& zQ=KP2HFF_Lr*J0dy!-I`!DU_Z_r**q^f4_&fn1q)z9M^PtN)4r53d0Km#^OO3Jo3( z2Oj5Lm;wNb$dvqo#eMBLt$N2783+li0q`)|GE-^|s_1m%YzYZ!Zc}%FHJmf8bJ%+p zNCmF-INr+l50s$hH~kFI1++Rk?q#&%{bk&``R}o=<9YpJM*X8o@6={0oC3f!xH;=C z=^sI>{MCzVNL`O<0r|I^t?L(R;Se4q1u3U&xy+t#H$TbV<)8qGkDLjKzE>}1ij#l0 z0Ekk#qXxWOZMB}8B93GJPh8$(ssTZpdUO9e?g4OGsQ!29Ol|qwYca_8p2_-k@mYgwAKHfhpLc(G-}4Z zetPvnO{;}Y3sCj#fZ9>2$r;T^7^ z5-5%eAfnJTSg_#RrBH6`L)qSS_eZ*%cQvesGQcZ$YaZD4pPlien#YfW zQ-3${VCwhDBjHkuj6ZM97c_s*KPdGZt)!~GI^H%lot$(tt=Aa-zxpxRL6@-b^!|b5 zw*V)D8E1zX$%$Yyvb{)j8@WaUoskO1eN;%LN?q{frc&1}^!hn4OS4om43}txcP`&c zhRz5wFR=@OEG+g;G-i}ATgc;jKu8zK85kk+R8#(nO3=;T8E6`tj)R3fhLfNxw^$C5 z>V5DGiqh zFI@k(gPodXIRUP;>RbXG4qdirdwJf5mJL7?NT%l1NP|ePC)8gmXCU=nFLXEn`1klf zmgIju2^>5C0TCAm51&Vh28lrPe{BgIJlsb#LnOsEbrd{A`D{ zra^l5p6s8X9=~$?#l)yASZk6JOf@&c@MVy4Dw#M>GL`o?YKgG7@7xK6rD8;kr{bP} z6Lm>dgLHplJ*R4XuvOxx3kAkDVc{QpTaUc5^5}vUdKjEWjyG65dtHY6EO~jE|1Mrg zlqVbg+4`c1=okM|SgN=PllBZIci{Dy3eP>yS`N`iU^E&MdAJxKhfOa`oD{&Fu3G9% zjNBd1J-1&Ppoo+F`Z=#VqUMx@-IvR6m4BOGy|?q6`@7|N*$!pv={!acYs5{p;Q|ic z9>pKy3%t&R;+@lctT73xSqDyq0f&=C?lGA|i9$FEtrqDQ8pP2MDx>p_H-L9mHbD07 zEDIxn>Zg?~DrO0a4zQ>Lk$noc$~+9RSN?h$BHVqR=<_3oyv?S;#HDsXg(it0wX|Ay zz5)637~zP`pFza$5oBhU=U-CxyQAXryYhbz{G|9uDcT@KD3x4qINdpBn6-d}?#_s# zCh+qSvCnC)7}=FYS-Vog&_Sj>-_>=%`==q}SO6%Co(|cxv#5r*LSsJJv9sUY15vYv z&n2V*8QqMyqA^0xvqVoP!Gw~Jt)wd%ZH|cKiv6xO#QX08C$CM@k|o?Ao18)oGGzj& zr1d)mr<1(?SrvyCR~da#ZJL^}0{DxX@^J(o#Jyi%F|rItj5qIC=SE^lXSV&OTxMm+ zkEuSzZ(N0vjb-09SY*ydVXxg_*n02-Ws%M(iBo(8-eBU^FyUG)hE~_avniR9PXH&l zu=~8t<$O7w$iM`FE;~{eW04L~$KF#xhqV2Umk(;)%4%;}TLSeC(n6eibQI_lvsTH_ z#e`TYp3vK3@{a&n~StZ^k z=)F{Z^5dJ6rHa~#`cmM8C?_` zn-mTkvU5mvSEiD5qN|w97IfO7at*Y5S_`LvLYeW;mY^X$S=DG@lDEuYs%7d3g_pd3 ztK?sN_PNFFflg=9+*C8C_PCTCbkO+dNJpn<}-(BX|6?ys>(yo~)?p z$PD!o?S6!BoBL|yKV6QJwp@pvg{2}yJxFGS;_mYeg(BS$sGdTXU2AJq&z1UqN@w1L zNXrIOtG@i_*|?s5HLhc|6CJMe`R@5zzTcGLed^Y(7N&*_QYc+#t8}|5R}C=XPq!O6 zHz(xSIq5Tmhz95?YtTunfgM@b0@*z+k=VAFRGilxGemOm5?6M3b^O9k{-CoX_@Cf< z{Cy8MZuJrOFyA)|eKgYixb?E-o(ghi4eBvaf5r*{%;UJS|CRrF!=Q8cBtfNWrZ9!bxsJ! zK^!M3NTGI!HyBE410tt_ife0dF;OY4jbCr;j_W?R9)O~?JV3A_9Tr0ChFYAHZcdbS%R%}aD%uMz z&e)s1sqe#b@5zW$r=ohw%}8b_%(}(+<5ERNt z@8d-?nmAr3^!bW*9Q6&8yAxH0uRcwX<$q#+UniLWn*1Sila$ff1x^5q{fb(UAyLO9cD%L<7X?%Hof*%GLV|Bxi zSys6Kv0UbS)Fq)OH|vwjWE0*#eG6SMhEB9}D>cMcyqKw?qriLQZZ#On%c!AaaDx&-)sbdfu@#eayLlkxP;q%Onmd<7#^ZTK>-ym@d~u>K_xpAuy;(HtO}hi4 zj!V&QK48D-U!PJh1s~;#aqKYc$gxVqNnwQAV6AxT zw#4X#M}ttW*=-x2ap<>bqQ-YkqoPMP8HjLNoWdc&4QAW?M973A_1~-kWT}mY7ad;2 z8pZJqAjyCuOU7%Dbj4`&7q2vi9pJr4+177bz*E8LLeyl6gXDY<)9c6Jz}vo}EugLl zi*=&UcQT|eGI6R`4vP*p6Oax&ai`%G^*Cpur-AQJ0n z?xUiRrI5&9D)iEMU1ku=8gcu9Jd{eXKWoy5L4qhjjEFJ!ryi}VFE!v=Tg-AC<*D;88%KWFcBGmAEHE~T?>3e8 z`$@EjI6rg9-!}WrQkm|b2#;r%z$bclixfVa`3H}Ce`|kG9XFB2q3j`b;X7dnzZ1qg zf`5QVLV!m`_#Z5X!-c2C!Q(}sp_7u&4ozfnLQ%50$*l%o z29v(YC%CGJxw-c^>T!-3x1Lx9PZLks&Yo${mN_U>^e84moh<^yxxvuflZc`VgK{iz z8|@YPmNS>B1Qzj>&9e3#&2|h-M_ZKb=6)iJL!ltEu&$f6mY$Klq4Dc(e64EcW<39)lS0BQ&Z48^QPZK zPdiV+W3;-W=Kpt87X6gH=q>0o&Hf(~Ly}J9bYdqN5Ym^-MbD|xIQ$26`UZ$dE}qws z)Dpe6wVunu{CG(eLy+qIk!~=y^&>xc0%&V=C(_3C-|R72YVAFt2D`r_OOI{TEVNp7 zR>LU}RXjzpSVyo42DAOXmgD@hN@X3~>T+W@=`okqwoUnHHAQ@tCc!A%H!B)b#l6hd#|Gm#%O#`7Ef!d9x`=s9v-EU4;A# z27inQ&$Tq9`ekEMrJ&ZdTy6u&?I`q$sw~cOAb@_FL%w_qR=kYKjFXq_)LglxgZS#% z)uQu|wegYsC>{P{tHw3g;S~W|B|)sIVxV&au(ccr!k3M3xFk;Bs~c()b!1iIiY^OL zM@ZaK{IfjgqVhsl_XfC7$&lAh{Mk4J&CaRh(t`z39}!t@kaCZ)M5~DyM?_Dm&1eG~ z3$m3L8y2v+x|P>a_tkjkhai)k1l3ul#7on!l)kLSS}+6pGls=<-5Wd9dTZOV_?3a` z*5t?&U#r7W6^Etn@c_I3ARuEWLl~-o;APzvKYx-v#w}iV|8oQ95Tz&tBS;ygKuje=)g>e5B`fX6_waI z$@m%uk1ilJ7Uvj%eK7IS;7Wt zRbE6Nl@S7JSav-)o~U(Xt3G%Kny=&V2^q=HeN+ER9g&1DJpj$A%GIK`j~53oqv4b6 z#taC{HC6^Ak;MaDRh(|A7M(8UGhrYDLK}DE`wT&ZrPE)wE(Mi@V;sYmgl6fSrl>{v z;GsEDS}hL8Y}Mr>_1O!R;GboG_H*6Um~+I_s+uO#J~PYbPm_|Vng0r}(8;o{v}(K0 zQ}$8_uZF7VDol-9`qYxDj8kH<@v}03Rn;e?^%{fcguYIZvU?Oa8nHoEKF!b8@6Y2nNW^|MuaLbYJEgFtXEL~R{a|y6`HP#2{#&)8=IFo z>B^4al>3gaxAx=L_{5r#sOK-0JK6&5>_guMD4V?pY)bX#@CR+8SP+oN)>d*@$3eBE zJKvBfyFLumG(2*SiEi^_M`600wItU*jr2R46R#muuV1Oa14>BhG&Pr|PAFM=M;Lix z2K~2jZ0)^g0fsR8@@-Zf9meWXs-u0Uqtv)(tyFr#m;2IxQQ1E#aKu>>Jxwvn2_HRQW4UluMgUTG6_7okY#$>|z~PTxDY&?#AYltOdU3%$~78LJ6b&k}|f%`J8Hj2<^b1nF{JP4smyB zupf-==??NYc;A!w?F_f9)U9WJE3`zgVklu=<2|K8GYj!;JGU@$(U&pl4v>u6_I_3G z{VK6rL0f5mq8(H5!c)jd)MWOekB<6rdCM;uv8-oZy5y}0$W&ekL9JBo*>UFHP>~4S+ns{_SLmQ%EQHNa)Kh_XO)3fUR(>`3=DSAjEwQ z>e(NDtH zXa3+2*a3DC498N)2u)XJ22YhcB{x-XlbM>=-ZZLDpJg)8Z9dXD>-|s#J!Ob?C*_-b zCh(OP{LHt=YdKC7@wibFChdv4_Y(88vrc$do++D4{HL;REI28_P`5YDNn!rV%CT;* z{%7HW{6{Bp=BMoU|8DN$+V>w%o^>G=-O>~#J}Kk^?K zE_e)aRhg0IqX?>J$&)R!3mWvSUvzl)@UNk474x^O0adruD#gVg$@9mqK5%=@m8@1O@^G zZ237`xbPr(JTKE&!<;IIjx z%Pq836zGU|0b$>@@|rJQgneaDOCw(#AQj_1R2E6FLfIP0_MV<#J-qZ&!1JYXPEn@C z`D=%Yjcy4k>Om8XHv4v}62S&h>fZnJiOe?7HVz zBhpxzW93w<4yz2ANn>CN>_QqNOh==5NR2^^DGxi#_}2Th8`QSHG{K=$^dosTa^bQ75??mqfgu72B@c1&%Ko5sB%+ za2xy}SKDP##ikWP8*Tkojx;u#_);AVry4Ikn0ta#JWduiQiM1*se7zUe`gX0q3@^h zSfeJ9Flj@d=)?_bL4R>9Vci`ne&{^bt~Tf-TD(#BCYG(b7jfRA`XpvC_v}RIq{e8@ z*-vWYW;mx(fK!FkKp<;<5hCB_q{dbNejV)(?V zBW`f7ouY%C_DVvYPqbJ*fuYULUpydd1le(HwL>>Hp4bBdAr(sU6CkpXOMvEQ4Gg{? zm%Pe#=?i<#+3OG=1(AH4?sGnGqlkVWXR}lLElek{WY(k+B3sfpSbjKuuV+c>Oh6!7 zYy0ulOGrRu&T}K`a?8)SM`$o|yQX!v(XG*HpGe<>ZppprhU~HT-OsS#!2M6l^naVC z?_E)|sPQo7-PsPgKcmKF)}tcq3milg$dnAs)Qm1k6OPZxtpM`uTv51R zt~=`Ti!r{I`=X!W6ZU&^nE@f%FR};x>_1i49>c%|qI;9S{WdA5D}iP_K~rbW1-8%Z ztULLpTc5YYrz_YHcz*JrX!ceK86Wvs0iQ~tenS@#R3;uU=0${2Gm7&RuYGS6kHW3G zd)V@rF`@qsbP;LP*io{sd<=7|tUCGN29-pz!jy&4l!?xipM?L0&6^v)4jbJ56e)7d zpg}p^e&pSs9TPu!rqd=+=y9L7-=H8GU7-X$GNt|czScT^V?{yA!FBkZvZ`SP3tw-) z_!S+Z5@c|igyJ?f!Tp)Qzcb$((Hw|YkA%n7=+MomF~uPm2TdS1`+NP6nZn+8?Lr)p z%{M?yX1+3>Hx)zeEA!p-)Sx0;W}*Aw+RZ*&FXF#Hqnh3dZBwqU-kw*YJI_1qd;8BG zW~2CBYSj{~UA)@K+d`~i6A+cgb-6G>q>mqe{`Ol$tWiGDT@%_%W;7p~Y=HQ+Ia95x zBm;+Fpy%z-%c$(#pX?f&$mkh_^6?~x_@KBzBNF(los0`$woX_+n4Row$0h%|J9wqq$mORk+J6>eV{uS_N74|_-tHvdn3`=Uz zJlquJYk*i>8y^^Y$;;~YC#UKtAf|__SZs&F?D9WxWgvViJ#8Vx3N;q)AsIA@MMqtV z`q>~_8-ry34Iqe5@jQy}eo49Sjpr5z?OX#%G|2pvq4344wIAw4Kwgfn)kD4q!J8T04J97Dj{REt!O{#50fjQ5tx*$KwD$9#g1bx z;nIq&smx)C^D%=1H#4-%k#N}<$|tn$W1g96OIGwx9+{{}ZQnJHpf^_6t$wY&a_{6i_LJym$rfsRR?ak|AB?!%`J(TlpR>H)1c1+02?U8wAS=ocSNtu4;jmhtZ zRbg}KGrgxKtNjw8I^rRgp^*U8i;Lk8+8IK1hWH1 zngbY4?1!O-1A)0HAAxnvM{d!KHcV%{@MuP6Ic;ks0ud@e#&i#Ch=;$D#}tMkMMw5F zhmu>_4|&?`s>3{8zi(}9f+|K?WW(Lc1n2GORlmwsXjZSYGao~lzJY><`eT4-?dL(h zyljm?iOWq&kKCKx^DX_`CFCBt|oO8@LuyT@3UH5O$m8||Lo9Q6C& zhQhA1%B*?MEEM5~omea%@%tBni51YJ# zS>&!N94o#vRNlx|IvvC88K)A1vI);@a?Vv`DupgpCe}P~52Wx7@QIZuB;6_b<#Jw< z4(O451v^jL_m#L@<1*shZL&JDg6QawHtT%dLY;%UrJZr()N#28ZdlS_Ds$g!p&_gC zDA;xV7IKJi!F&U_tEqwd*tq+1mjP89`<#wHVbaN*UbL2y`1;lkkzyQ$BxvoSLaKnn zEK3uE;6|zqE0*M?IU61$n1W(U{R6r&*0lo6FHJ{30$UXc{HlH?n!ku17g9dT;f<;M zN~r;bL65vew^U^Hdt)Is5f}vDX@(Fj58*`&_tBm!(58ciIvH*g6YzUcK=O;K7-$;& zW-(e70d+-Ova@~G{+bMlpwhfEzwV4v(;m8#aHiK2A^Wy|#@TahBh(lTyC z&7w?ZlF7!X6`jbLq}}+IdhS?ANsO0@ZI!2d$f*vrTbc$`i_o5tuMMVtFXtL zP?57&Rci%OSFA+atqDky(8O-w>(CKei70u`kj>Eu(MyGW_l-N+CN5DXa6^RQehRVb zsOwz9eU18YxQ2N~DR++beK*Q+c$uQhqjX#M$LMLsEO97p=7#={)8Lta6nt73&IaGY zD6oA8sBmv->mvCpFB1&(T156{hZ8)v)}rgR`|YVnfiDsho#YD*CnI%Vjp$2${_a;P8g%opuf3rt>28vumx!($BGkSw zI(^6jXOlilHyNmoO~@WnDyt%7PnIE|*@*1Inie_}>4n?J4dzfYOiSXCp2GhC*No7R zx2&iZke0lWESJ5`f(LXfO0n2Fg!P2IblCj_aY0WA!uG*Gp*XT0A3H0l7m2@H3Ik*YqehcN>D6T>ZR-i;X z#vzJ=@iAfECsaeUu_xGGXg+(3^|-cGF@H7L!GF%5-scLM-o84|m-WjneCMvUPV_LY zOqDeGUjZpOvHCEgKFocK+!}llvML&}bG{06MrP5lbcT+l+BEgT8?^QsXPc(RrDNEq za)IwvQgGgSJhkW+Qay@=nqVHW0jW@=0klvygg`$=`D4~zeDNDtLxh~$*U3KAzB@#0 zh-JxsXH-O!mA=u+>kjQti6e9ga)#1&VlohVD{w}0jQ2UD z{Gvv;EeK(hs+u1(P4KkJ*jBd@rPd#8nv_0`n9~m!4;yaAva*sxH8px{ zLT2(1H8_!_)?ZXc6g)R!Owy;_Bl(p?H{6;_48}N5wVdPqHXlqH zodZMjh09g_=GauFY&l=I@WRY^jV3KT&ol|Q?T_P`wO}tZJzRprc6eQ?8+#W=-cS<{ zklMDMdl<&vW8_F{8#_rhtp|ecWpue;4O2jUt1y9m%{WnQ#wBYMwY!vD2$MeYA0LFL z@N06ip%qP1Cm#%P!J&u_0@OHLqy`G4ujSB*xQpcVid+Wuv*-GUZ4x z(ID@39LlgM54LIIOB9!CP&HIW+*+2yr+{7z4Msi@}7s=Qw3UX{9{F);8>vBYx3!ppXyFvW#33GZ^Xx ze`YW5vo-lr;;a>3lr720}|2`=Y9KXFHDEzS7sexy~p4cuNOJv zW9CCLo}{CoXbyNM$mt$*IT(~EBQ3a>lsW@FSZn8O#=5wV;*>M_UHpq|8Flefu~9rL zu5}HAwKg;F#hOL*R=;~eKAQ~*lYwb`ix*zMQ&r@@VanE#mOHqUTQ%i6u1u%)EdL-9 zRL6^H_X~ftFH4-Yq`2$Mu`S)a_SaXux8En>c6f>{gZfGd?oge+;CCrra(TD+9O$1R zyMES`7)C;Rvelaxg(Pg3PnTOo^E=ll7gV}$wllgMVHLr;+PE;kBM+H+Lu(LbV#}!I z6}*Q~PgHX&tz}qK8soFdoLCMyBufaT)+Rw{q;8)8x;NY=nPm22&!aWHK&oRAFvu*Y zwgPK!P3#VrSh||-`%6wtQz@vk!s|K3g%G(o|Ht^`diM~m*qTm7r;?YzUvaD}iWzWj zW-MPnL@TwYnZ>@|!;QT_^hi4ZEYc+I&E*Oj?jisN6o z;ewPi-vrvj!m!T0tSkB5Hr)9n!BV!@2Op{ABp}{-mH0*6QCi=lMu$J>-I07j<=CHTVla3pKLDmo*EGg=sGpA}csXO#T(n|MJ--g4=+ zv`@Mt(_as^%F-_=P(`h#8quyrPZN#*EDKXghzCBM`AA23(&^1vVrT2P_;>0LbGJ z^0yc*%ulkbbpQ2*$1;p*>U47Ph*?!v$BHN-{SB?MtB?KBHJ`7F(TLXO1xGPpI`U`P zmNp(Yky~PzOZim)@o9NmKcxk?aJN?dp2(C<^v@<4v$3!6^p~`LG*-#-Iu4f9YU1rZ zw6RM-D8yo!=lph@_@Fwnmgt;E+WUI#J64Ay+U#u!Iwx^MJa(TAOL)D)mv)v^?>B(< zdtXAEEo8T6HAO4=*XhWZku9>8VJ;tzt?B-qL9F0uY->Gl3`SA+fC5VR8Rno>)uMPt zq&w~#U?vuv5Y&5{eJHtk>mM`E?1xSK24L1ZAbv?G{MP)GC(>R27|(-eX=*e0V7&v` zBq>}(EHl&_9`5}R`xWkR&7k45ie6sQhAFJWlHBNJ9PPt6s_kf%I=MK=TGD|JugR{Z zYTW+tNMLIYo$T=E6YY;)Bo4=mTgBG&OKzGnf{LDR6A84?_O;^VtAphs@-=Vt7&u{WDl3OH6){BFXzYJ8SzP}JOK@kmEsf?mMulJz^~ zwpl8Ik94A0><``r9NA~0n+T?RwI|Wzg#4dJ@dQ)wAh$TvHz1({4{mxb?^rG*+Uw) z#&xuu6t-iZ0=G9Qz;VOZ0#4vZdYeh)BP$roXNQYpkuk)@?0C`q@U@ z@wz%z-(rJtSbXSXJFCN~-lL84X5&+G#O6MFiWO*TGwBTg)E>fhpjKr?D^$sXQYU|^ zZFo#d@*N(w=MZ>}OVY19K1_w#vh;2Kh`jV89> zwNf)NTKQJ+-8bffCnX9WkiVN@ABi{jRhVe*+8^J73`p6K|XT+xS%B*)GW}oB4s$F@%OZ_0oQrA!CGjDu$Iw4-Hm(Ok(={g8ax8&?k+Z%ue z3jmGn`mIV95&=X;RqD<8r%#>;qAFrLTXc5j2K&%*o937cb28QrcXbSzH-y94dV{mR zFw_wurK(sUBlfpvHxQ=9uaGe7-Ya=?Qma-ruh_zy^_>wdTT$YMkSAKporEVPsY(_| zP#Eo-94GJZt)kRxvg3X&n8uslR@B-M!7byH5o`3KV>{Y9R2&}BRr(WE&Ntcr$Eg*- z!e%t-^$;h$^2$g;c|_2%iWxQ$YuV_TfmK>J49#soF17BUeFRi3EO1#*a(EqTC zGU0=X4#P~Fsq#j|Crtyx2N5RK4GP4Vl3MpaM3AhR=iNO)KynZn@(4w1%^G%H^?`In{=14;wq`G?9^6+-57_e5 zoy5ctsdKxBfAro?!bnlo)!KyST-sS9UXAC42zR-s%O@VmGxG^`kJA&0EkI6y1 zFDv@;@6x;~HZx>1k6-#y@Le@Uo7%~jsXl|flHsg#?`xx|W^eR?6mqC*OXARAo z-Yd3F>{0m3s{7nzmGk@OQO$t;gPSqYo!;Gmy#99c;X@A$#GtQ_wtFjGGI>z~Gd?u5 z(aKBc;GuTz4Hq=zx zj!5lEl(e!w4EPg+bDEF5N%h!_*1lJu9^v_XLAN~!a;5N=EFPXmwV~`*)O`6WRHAvL z-I3wAWCC$QBpI3->U8(uCUWE+ii_HZFesb9p{L5xOc!j@Y_>x9*>aa-z*V86so$^miT<9nCOtKU@6FSDN zI4wwYPlpCX+KQ{_TJ~#rz@v@j^iP|+GA`(DeJ!f!R;|4xC(lB^&jQIe$dG&BRco;| zQ%vt&nleLjcX|TByG+ooHvm46rB7#4X;XcR55k&gHW~a;Yl7nm_k&Z&mblRs@*F#d zy(4cet`}j(&ZN6BhH>Nikcmorp&X7JQW`2efJu44Z-^*l%ZsaRO|&1J&KmtgBp#-J zZL<@z?>#8Gb9|pPI`rUo9FAWlt6Bf592$(_eX3yEa*OI7+q$gamWl&uIP*L(vU#_%EE zI%zhS7{Bg+OnDlu-wFCSP#1I&G;ZpJl2tF)l#SJDLm~GNZCgil&umaoXp^}ugu+dW zSJl@rc;@5yYN_`DYp)O8a@dSLehYw#ck{z@I%1R;7PZ8w`?znv2QY_Y=mKCrTd>B5_{2`p`?jv&3ieQTqh{^ z#DtDTn1z}MO*5hjN4)#nXYVaofnj67MPOL(_4_^8zTXWT-sw?n7zKta-iDL=^_f0< z$(Z91g)#xH3m$Vkhoo)Ne_41fEKBhdXH^-9Gga)!OG&%g+oHMWg*^cWo0P{9GZOzC zsncX3))@w@l zy{H)d!YrDk2Xe3?hZ{HQ4;=~vvCMJ+uUo#Tnh-ZEdL1FDKT;=C%wJ*^x!ya%-9t*I z+igUC=AS9av)KwFb>0BWe))Yl*@R0eIk$8pn|2k@=d&RVlDQJ=ZThxZO+m=ME{_EZ^bJ68r9!-`^J(#4La9o z4-=ZiDA<^X5(5JdyeFBd&W0+q|z;UZuyus*l7LswBV|io;Aj)!W4#+~9 zCui{o8M(EW=g%CcfODoUrX~v7FY-i=@P$3M&_nwybVFMu(2{pgGqd+{9P}?C#xh5~ z+SN>!k(?t%;G{k*)LmH1#0M&UguOWD$;2}03SUWDoFL`He+CW#C<4@uG0i#ZjN$`m z;SKAPJeWhWBsR!K6`w9W+s}Lpy!gF%nTH^YXfxzd?lNxxTtVKIZA})d5-x$0!#BW^ z-5+?IOe~oOW@AH3vN3LeTw1X;x4lv-N;VE@-s~CjV0+PX=E^{fPqBi_i=tOej}5W` zx-H&0uDmm-I4LeTC#jO3o`=vN2QGcmlHf7r4G>v6tHv8cIRKn$#&36b^o}LTWar_< z9Hdke`m6vz4j%8%T*V0#06B7RQ|%4jv01bV&5m1`G#+ZOW|6$t31fd<{GY@eQF%fP z9aYL}4L`MsU;T3ZA@JKwEAp7NW-y1-Ff(M5=ux)z3u9uZEETA%K7`-_W|>31Vsba7 z@KjJC;#$7;enUGN-d)PI58dH=)!A`t)r0R#dA0RaI30000000001 z5dsh)F(5Dl5&Y-e%oWE zG7>gxm(%F&R*$ETeYGsfA{ga~aco8RZ6`sSq|0F z89aMF_EoX9vX>}#(CRbrs+Q3?hEmX+h^7x=Ln@^~Yf?1~PRUCWFgR$4sTDFM6;lN^rvibo=wH`&l9|ovr1nVm zOv-X7c|CI^q*aoe$|V&w1v?jO(9{W{y5V4wm`vFS(AbC|fv9$HQ~e9Nq&Y)uQi+x( zUuJY5+F*=5JBw&@*dJQG*GfGa=I9ZR)a%!(eP^mL?NM~ zm63CTEhgSN(Ic7xi{XqGH@ zwZ(DRmTuAa@G7=4z>w^?F4-ro zq6K~Tp>`(&c3mI;!~iD|00II41_A;D0RaI300000009CK0}v7*Au$9nB0(}UQ9u(T z|Jncu0RjO5KLGbJ7(C&kD`gYt$Nc{Q&Ws<{h?SJ_5+N-_x;%yR{{RZyj1Sg`t}>jc zo3>y+NpyK1koK*}QD%ZsYtWAy^^qj$6P>ZM2l5|BllfTvX>xQ}rG1l!+TM>Df8ieG0>f-HPIygp`E2b{QGi_d~R zlFG=wNe?-6N2(NBD$t^+NTRtscc&3Fc8@uHb9=0^#UXOCY>b*Mv5%63idI`L$I;4( z7ifRO1=Aq@C!D@Hzf^k3^e#lw8z`)u*%(@~nq_cBEhdzo14b3=MjEJ|cj1){GQBY9 z`XtEE#91j(8@n8BS%$3*a%tewN+~NfD-*6y7arD@d8^>_v2syJrMnNIa7(g051goR zRB(`_h0#n)j!NLi=^atR~QK#gLYj{4`x@D@#nvx|kHpsmaQY9@>A#1U>sDn~O(jg|9j`bm_ zr9@)aER36xK5%wdsxRoVjo6|^QEFTdQsEZrh{3WkXl)52PNhVnDxz>?RWe_wWo3S- zu?;dY{+eiKC9*L@QZW^-4pJ>7ELvn`sTiq{i)@^02zftZVisH!heMf#D=ia`18qJ^ zOoOCNwqaqrwqn1FL&^JWQZ)`qjzg2n3RG(x?HV-^@ts)k Date: Tue, 23 Jul 2019 13:30:25 +0100 Subject: [PATCH 32/36] Fix some more simple rubocop issues --- app/helpers/spree/api/api_helpers.rb | 25 +++++++++++++------ .../spree/admin/products_controller_spec.rb | 13 +++++----- .../admin/reports/packing_report_spec.rb | 2 +- spec/models/product_importer_spec.rb | 7 +++--- 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/app/helpers/spree/api/api_helpers.rb b/app/helpers/spree/api/api_helpers.rb index 5341d92d2f..094d726336 100644 --- a/app/helpers/spree/api/api_helpers.rb +++ b/app/helpers/spree/api/api_helpers.rb @@ -12,7 +12,8 @@ module Spree end def product_attributes - [:id, :name, :description, :price, :available_on, :permalink, :meta_description, :meta_keywords, :shipping_category_id, :taxon_ids] + [:id, :name, :description, :price, :available_on, :permalink, :meta_description, + :meta_keywords, :shipping_category_id, :taxon_ids] end def product_property_attributes @@ -20,11 +21,13 @@ module Spree end def variant_attributes - [:id, :name, :sku, :price, :weight, :height, :width, :depth, :is_master, :cost_price, :permalink] + [:id, :name, :sku, :price, :weight, :height, :width, :depth, + :is_master, :cost_price, :permalink] end def image_attributes - [:id, :position, :attachment_content_type, :attachment_file_name, :type, :attachment_updated_at, :attachment_width, :attachment_height, :alt] + [:id, :position, :attachment_content_type, :attachment_file_name, + :type, :attachment_updated_at, :attachment_width, :attachment_height, :alt] end def option_value_attributes @@ -32,7 +35,9 @@ module Spree end def order_attributes - [:id, :number, :item_total, :total, :state, :adjustment_total, :user_id, :created_at, :updated_at, :completed_at, :payment_total, :shipment_state, :payment_state, :email, :special_instructions, :token] + [:id, :number, :item_total, :total, :state, :adjustment_total, :user_id, + :created_at, :updated_at, :completed_at, :payment_total, + :shipment_state, :payment_state, :email, :special_instructions, :token] end def line_item_attributes @@ -44,7 +49,8 @@ module Spree end def payment_attributes - [:id, :source_type, :source_id, :amount, :payment_method_id, :response_code, :state, :avs_response, :created_at, :updated_at] + [:id, :source_type, :source_id, :amount, :payment_method_id, + :response_code, :state, :avs_response, :created_at, :updated_at] end def payment_method_attributes @@ -80,11 +86,13 @@ module Spree end def adjustment_attributes - [:id, :source_type, :source_id, :adjustable_type, :adjustable_id, :originator_type, :originator_id, :amount, :label, :mandatory, :locked, :eligible, :created_at, :updated_at] + [:id, :source_type, :source_id, :adjustable_type, :adjustable_id, :originator_type, + :originator_id, :amount, :label, :mandatory, :locked, :eligible, :created_at, :updated_at] end def creditcard_attributes - [:id, :month, :year, :cc_type, :last_digits, :first_name, :last_name, :gateway_customer_profile_id, :gateway_payment_profile_id] + [:id, :month, :year, :cc_type, :last_digits, :first_name, :last_name, + :gateway_customer_profile_id, :gateway_payment_profile_id] end def user_attributes @@ -96,7 +104,8 @@ module Spree end def stock_location_attributes - [:id, :name, :address1, :address2, :city, :state_id, :state_name, :country_id, :zipcode, :phone, :active] + [:id, :name, :address1, :address2, :city, + :state_id, :state_name, :country_id, :zipcode, :phone, :active] end def stock_movement_attributes diff --git a/spec/controllers/spree/admin/products_controller_spec.rb b/spec/controllers/spree/admin/products_controller_spec.rb index 2f02e0215f..51c6983527 100644 --- a/spec/controllers/spree/admin/products_controller_spec.rb +++ b/spec/controllers/spree/admin/products_controller_spec.rb @@ -150,15 +150,16 @@ describe Spree::Admin::ProductsController, type: :controller do describe "when user uploads an image in an unsupported format" do it "does not throw an exception" do - product_image = ActionDispatch::Http::UploadedFile.new({ - :filename => 'unsupported_image_format.exr', - :content_type => 'application/octet-stream', - :tempfile => Tempfile.new('unsupported_image_format.exr') - }) + product_image = ActionDispatch::Http::UploadedFile.new( + filename: 'unsupported_image_format.exr', + content_type: 'application/octet-stream', + tempfile: Tempfile.new('unsupported_image_format.exr') + ) product_attrs_with_image = product_attrs.merge( images_attributes: { '0' => { attachment: product_image } - }) + } + ) expect do spree_put :create, product: product_attrs_with_image diff --git a/spec/features/admin/reports/packing_report_spec.rb b/spec/features/admin/reports/packing_report_spec.rb index e6af4e84bb..9ab6ff1150 100644 --- a/spec/features/admin/reports/packing_report_spec.rb +++ b/spec/features/admin/reports/packing_report_spec.rb @@ -27,7 +27,7 @@ feature "Packing Reports", js: true do select oc.name, from: "q_order_cycle_id_in" find('#q_completed_at_gt').click - select_date(Time.zone.today - 1.days) + select_date(Time.zone.today - 1.day) find('#q_completed_at_lt').click select_date(Time.zone.today) diff --git a/spec/models/product_importer_spec.rb b/spec/models/product_importer_spec.rb index d0487192c1..582297195d 100644 --- a/spec/models/product_importer_spec.rb +++ b/spec/models/product_importer_spec.rb @@ -250,7 +250,7 @@ describe ProductImport::ProductImporter do describe "updating an exiting variant" do let(:csv_data) { CSV.generate do |csv| - csv << ["name", "producer", "description" ,"category", "on_hand", "price", "units", "unit_type", "display_name", "shipping_category"] + csv << ["name", "producer", "description", "category", "on_hand", "price", "units", "unit_type", "display_name", "shipping_category"] csv << ["Hypothetical Cake", "Another Enterprise", "New Description", "Cake", "5", "5.50", "500", "g", "Preexisting Banana", shipping_category.name] end } @@ -530,7 +530,6 @@ describe ProductImport::ProductImporter do } let(:importer) { import_data csv_data, import_into: 'inventories' } - it "updates inventory item correctly" do importer.save_entries @@ -548,8 +547,8 @@ describe ProductImport::ProductImporter do let!(:inventory) { InventoryItem.create(variant_id: product4.variants.first.id, enterprise_id: enterprise2.id, visible: false) } let(:csv_data) { CSV.generate do |csv| - csv << ["name", "distributor", "producer", "on_hand", "price", "units", "variant_unit_name"] - csv << ["Cabbage", "Another Enterprise", "User Enterprise", "900", "", "1", "Whole"] + csv << ["name", "distributor", "producer", "on_hand", "price", "units", "variant_unit_name"] + csv << ["Cabbage", "Another Enterprise", "User Enterprise", "900", "", "1", "Whole"] end } let(:importer) { import_data csv_data, import_into: 'inventories' } From 96ce4deb45f47e15caccfb3342cc7ffe63b85815 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Tue, 23 Jul 2019 13:50:54 +0100 Subject: [PATCH 33/36] Transpec spec/support/api_helper.rb --- spec/support/api_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/support/api_helper.rb b/spec/support/api_helper.rb index 582f7ea341..2dd217117d 100644 --- a/spec/support/api_helper.rb +++ b/spec/support/api_helper.rb @@ -17,8 +17,8 @@ module OpenFoodNetwork end def assert_unauthorized! - json_response.should == { "error" => "You are not authorized to perform that action." } - response.status.should == 401 + expect(json_response).to eq("error" => "You are not authorized to perform that action.") + expect(response.status).to eq 401 end def image(filename) From e4a6b3880f9605720853a39ba4dbddcb73e02be5 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Tue, 23 Jul 2019 13:56:54 +0100 Subject: [PATCH 34/36] Fix some more simple rubocop issues --- app/helpers/spree/api/api_helpers.rb | 5 ++--- spec/factories/shipping_method_factory.rb | 2 +- spec/models/spree/order_spec.rb | 2 +- spec/support/controller_hacks.rb | 20 +++++++++++++------- spec/support/i18n_error_raising.rb | 2 +- spec/support/products_helper.rb | 6 +++--- 6 files changed, 21 insertions(+), 16 deletions(-) diff --git a/app/helpers/spree/api/api_helpers.rb b/app/helpers/spree/api/api_helpers.rb index 094d726336..e27d1164aa 100644 --- a/app/helpers/spree/api/api_helpers.rb +++ b/app/helpers/spree/api/api_helpers.rb @@ -2,7 +2,7 @@ module Spree module Api module ApiHelpers def required_fields_for(model) - required_fields = model._validators.select do |field, validations| + required_fields = model._validators.select do |_field, validations| validations.any? { |v| v.is_a?(ActiveModel::Validations::PresenceValidator) } end.map(&:first) # get fields that are invalid # Permalinks presence is validated, but are really automatically generated @@ -87,7 +87,7 @@ module Spree def adjustment_attributes [:id, :source_type, :source_id, :adjustable_type, :adjustable_id, :originator_type, - :originator_id, :amount, :label, :mandatory, :locked, :eligible, :created_at, :updated_at] + :originator_id, :amount, :label, :mandatory, :locked, :eligible, :created_at, :updated_at] end def creditcard_attributes @@ -118,4 +118,3 @@ module Spree end end end - diff --git a/spec/factories/shipping_method_factory.rb b/spec/factories/shipping_method_factory.rb index c98a62bf76..516dfb6b66 100644 --- a/spec/factories/shipping_method_factory.rb +++ b/spec/factories/shipping_method_factory.rb @@ -41,6 +41,6 @@ FactoryBot.modify do factory :shipping_method, parent: :base_shipping_method do distributors { [Enterprise.is_distributor.first || FactoryBot.create(:distributor_enterprise)] } display_on '' - zones { |a| [] } + zones { [] } end end diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 4327d5e782..893e450eff 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -385,7 +385,7 @@ describe Spree::Order do let(:distributor) { create(:distributor_enterprise) } let(:order_cycle) do - create(:order_cycle).tap do |record| + create(:order_cycle).tap do create(:exchange, variants: [v1], incoming: true) create(:exchange, variants: [v1], incoming: false, receiver: distributor) end diff --git a/spec/support/controller_hacks.rb b/spec/support/controller_hacks.rb index d57123c5bf..7645e87372 100644 --- a/spec/support/controller_hacks.rb +++ b/spec/support/controller_hacks.rb @@ -1,28 +1,34 @@ require 'active_support/all' module ControllerHacks - def api_get(action, params={}, session=nil, flash=nil) + def api_get(action, params = {}, session = nil, flash = nil) api_process(action, params, session, flash, "GET") end - def api_post(action, params={}, session=nil, flash=nil) + def api_post(action, params = {}, session = nil, flash = nil) api_process(action, params, session, flash, "POST") end - def api_put(action, params={}, session=nil, flash=nil) + def api_put(action, params = {}, session = nil, flash = nil) api_process(action, params, session, flash, "PUT") end - def api_delete(action, params={}, session=nil, flash=nil) + def api_delete(action, params = {}, session = nil, flash = nil) api_process(action, params, session, flash, "DELETE") end - def api_process(action, params={}, session=nil, flash=nil, method="get") + def api_process(action, params = {}, session = nil, flash = nil, method = "get") scoping = respond_to?(:resource_scoping) ? resource_scoping : {} - process(action, params.merge(scoping).reverse_merge!(:use_route => :spree, :format => :json), session, flash, method) + process(action, + params. + merge(scoping). + reverse_merge!(use_route: :spree, format: :json), + session, + flash, + method) end end RSpec.configure do |config| - config.include ControllerHacks, :type => :controller + config.include ControllerHacks, type: :controller end diff --git a/spec/support/i18n_error_raising.rb b/spec/support/i18n_error_raising.rb index 9cef014820..9d0610b6c3 100644 --- a/spec/support/i18n_error_raising.rb +++ b/spec/support/i18n_error_raising.rb @@ -1,5 +1,5 @@ # From: https://robots.thoughtbot.com/better-tests-through-internationalization -I18n.exception_handler = lambda do |exception, locale, key, options| +I18n.exception_handler = lambda do |_exception, _locale, key, _options| raise "missing translation: #{key}" end diff --git a/spec/support/products_helper.rb b/spec/support/products_helper.rb index 24ef6bfcac..fcadc55b62 100644 --- a/spec/support/products_helper.rb +++ b/spec/support/products_helper.rb @@ -11,17 +11,17 @@ module OpenFoodNetwork shared_examples "modifying product actions are restricted" do it "cannot create a new product if not an admin" do - api_post :create, :product => { :name => "Brand new product!" } + api_post :create, product: { name: "Brand new product!" } assert_unauthorized! end it "cannot update a product" do - api_put :update, :id => product.to_param, :product => { :name => "I hacked your store!" } + api_put :update, id: product.to_param, product: { name: "I hacked your store!" } assert_unauthorized! end it "cannot delete a product" do - api_delete :destroy, :id => product.to_param + api_delete :destroy, id: product.to_param assert_unauthorized! end end From 69a5527e24f76e2221fcae0a44de43de7b15621f Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Tue, 23 Jul 2019 14:14:37 +0100 Subject: [PATCH 35/36] Update/regenarate .rubocop_todo.yml --- .rubocop_todo.yml | 111 +++++++++++++++++++++++++++++++++------------- 1 file changed, 81 insertions(+), 30 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 3e44970301..712ceee1da 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config --exclude-limit 1400` -# on 2019-05-28 16:29:07 +0100 using RuboCop version 0.57.2. +# on 2019-07-23 14:09:18 +0100 using RuboCop version 0.57.2. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -32,15 +32,6 @@ Layout/EndAlignment: Layout/IndentHash: EnforcedStyle: consistent -# Offense count: 7 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle, IndentationWidth. -# SupportedStyles: aligned, indented, indented_relative_to_receiver -Layout/MultilineMethodCallIndentation: - Exclude: - - 'app/models/spree/line_item_decorator.rb' - - 'app/models/spree/product_decorator.rb' - # Offense count: 4 Lint/AmbiguousOperator: Exclude: @@ -55,7 +46,7 @@ Lint/DuplicateMethods: - 'lib/discourse/single_sign_on.rb' - 'lib/open_food_network/subscription_summary.rb' -# Offense count: 15 +# Offense count: 8 Lint/IneffectiveAccessModifier: Exclude: - 'app/models/column_preference.rb' @@ -79,7 +70,13 @@ Lint/UnderscorePrefixedVariableName: Exclude: - 'spec/support/cancan_helper.rb' -# Offense count: 6 +# Offense count: 1 +# Cop supports --auto-correct. +Lint/UnneededCopDisableDirective: + Exclude: + - 'app/models/product_import/entry_validator.rb' + +# Offense count: 5 # Configuration parameters: ContextCreatingMethods, MethodCreatingMethods. Lint/UselessAccessModifier: Exclude: @@ -89,28 +86,47 @@ Lint/UselessAccessModifier: - 'lib/open_food_network/reports/bulk_coop_report.rb' - 'spec/lib/open_food_network/reports/report_spec.rb' -# Offense count: 91 +# Offense count: 8 # Configuration parameters: CheckForMethodsWithNoSideEffects. Lint/Void: Exclude: - 'app/serializers/api/enterprise_serializer.rb' - 'spec/features/admin/bulk_product_update_spec.rb' - - 'spec/features/admin/enterprise_groups_spec.rb' - - 'spec/features/admin/enterprises/index_spec.rb' - - 'spec/features/admin/enterprises_spec.rb' - - 'spec/features/admin/order_cycles_spec.rb' - - 'spec/features/admin/payment_method_spec.rb' - - 'spec/features/admin/products_spec.rb' - - 'spec/features/admin/variant_overrides_spec.rb' - - 'spec/features/admin/variants_spec.rb' - 'spec/features/consumer/shopping/checkout_spec.rb' - 'spec/features/consumer/shopping/shopping_spec.rb' - 'spec/features/consumer/shopping/variant_overrides_spec.rb' -# Offense count: 109 +# Offense count: 15 +Metrics/AbcSize: + Max: 36 + +# Offense count: 13 # Configuration parameters: CountComments, ExcludedMethods. Metrics/BlockLength: - Max: 774 + Max: 115 + +# Offense count: 2 +# Configuration parameters: CountComments. +Metrics/ClassLength: + Max: 169 + +# Offense count: 1 +Metrics/CyclomaticComplexity: + Max: 8 + +# Offense count: 8 +# Configuration parameters: CountComments. +Metrics/MethodLength: + Max: 31 + +# Offense count: 2 +# Configuration parameters: CountComments. +Metrics/ModuleLength: + Max: 208 + +# Offense count: 2 +Metrics/PerceivedComplexity: + Max: 11 # Offense count: 7 Naming/AccessorMethodName: @@ -160,7 +176,7 @@ Naming/PredicateName: - 'lib/open_food_network/packing_report.rb' - 'lib/tasks/data.rake' -# Offense count: 12 +# Offense count: 11 # Configuration parameters: MinNameLength, AllowNamesEndingInNumbers, AllowedNames, ForbiddenNames. # AllowedNames: io, id, to, by, on, in, at Naming/UncommunicativeMethodParamName: @@ -288,13 +304,12 @@ Style/CaseEquality: - 'app/helpers/angular_form_helper.rb' - 'spec/models/spree/payment_spec.rb' -# Offense count: 79 +# Offense count: 78 # Cop supports --auto-correct. # Configuration parameters: AutoCorrect, EnforcedStyle. # SupportedStyles: nested, compact Style/ClassAndModuleChildren: Exclude: - - 'app/controllers/spree/store_controller_decorator.rb' - 'app/helpers/angular_form_helper.rb' - 'app/models/calculator/flat_percent_per_item.rb' - 'app/models/spree/concerns/payment_method_distributors.rb' @@ -379,11 +394,27 @@ Style/CommentedKeyword: Exclude: - 'app/controllers/application_controller.rb' +# Offense count: 4 +# Cop supports --auto-correct. +# Configuration parameters: EnforcedStyle, SingleLineConditionsOnly, IncludeTernaryExpressions. +# SupportedStyles: assign_to_condition, assign_inside_condition +Style/ConditionalAssignment: + Exclude: + - 'app/controllers/spree/api/products_controller.rb' + - 'app/controllers/spree/api/taxons_controller.rb' + - 'app/controllers/spree/api/variants_controller.rb' + # Offense count: 2 Style/DateTime: Exclude: - 'lib/open_food_network/users_and_enterprises_report.rb' +# Offense count: 1 +# Cop supports --auto-correct. +Style/EachWithObject: + Exclude: + - 'app/controllers/spree/api/base_controller.rb' + # Offense count: 5 # Configuration parameters: EnforcedStyle. # SupportedStyles: annotated, template, unannotated @@ -393,7 +424,7 @@ Style/FormatStringToken: - 'lib/open_food_network/sales_tax_report.rb' - 'spec/models/enterprise_spec.rb' -# Offense count: 69 +# Offense count: 68 # Configuration parameters: MinBodyLength. Style/GuardClause: Exclude: @@ -410,7 +441,9 @@ Style/GuardClause: - 'app/controllers/spree/admin/products_controller_decorator.rb' - 'app/controllers/spree/admin/resource_controller_decorator.rb' - 'app/controllers/spree/admin/variants_controller_decorator.rb' - - 'app/controllers/spree/orders_controller_decorator.rb' + - 'app/controllers/spree/api/base_controller.rb' + - 'app/controllers/spree/checkout_controller.rb' + - 'app/controllers/spree/orders_controller.rb' - 'app/controllers/spree/paypal_controller_decorator.rb' - 'app/jobs/products_cache_integrity_checker_job.rb' - 'app/models/enterprise.rb' @@ -434,12 +467,23 @@ Style/GuardClause: - 'spec/support/request/distribution_helper.rb' - 'spec/support/request/shop_workflow.rb' -# Offense count: 3 +# Offense count: 6 +# Cop supports --auto-correct. +# Configuration parameters: EnforcedStyle, UseHashRocketsWithSymbolValues, PreferHashRocketsForNonAlnumEndingSymbols. +# SupportedStyles: ruby19, hash_rockets, no_mixed_keys, ruby19_no_mixed_keys +Style/HashSyntax: + Exclude: + - 'app/controllers/spree/api/base_controller.rb' + - 'app/controllers/spree/checkout_controller.rb' + - 'app/controllers/spree/orders_controller.rb' + +# Offense count: 4 Style/IfInsideElse: Exclude: - 'app/controllers/admin/column_preferences_controller.rb' - 'app/controllers/admin/variant_overrides_controller.rb' - 'app/controllers/spree/admin/products_controller_decorator.rb' + - 'app/controllers/spree/api/taxons_controller.rb' # Offense count: 1 Style/MissingRespondToMissing: @@ -492,9 +536,10 @@ Style/RegexpLiteral: - 'spec/mailers/subscription_mailer_spec.rb' - 'spec/models/content_configuration_spec.rb' -# Offense count: 243 +# Offense count: 244 Style/Send: Exclude: + - 'app/controllers/spree/checkout_controller.rb' - 'app/models/spree/shipping_method_decorator.rb' - 'spec/controllers/admin/subscriptions_controller_spec.rb' - 'spec/controllers/checkout_controller_spec.rb' @@ -541,3 +586,9 @@ Style/Send: Style/StructInheritance: Exclude: - 'lib/open_food_network/enterprise_fee_applicator.rb' + +# Offense count: 1 +# Cop supports --auto-correct. +Style/UnlessElse: + Exclude: + - 'app/controllers/spree/api/variants_controller.rb' From b9ddb39edccbe326a5ca23ce690154b2fc3fb38e Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 14 Aug 2019 16:30:43 +0100 Subject: [PATCH 36/36] Re-add taxons jstree action to make taxonomies config page work again --- app/controllers/spree/api/taxons_controller.rb | 4 ++++ config/routes/spree.rb | 8 ++++++++ spec/controllers/spree/api/taxons_controller_spec.rb | 9 +++++++++ 3 files changed, 21 insertions(+) diff --git a/app/controllers/spree/api/taxons_controller.rb b/app/controllers/spree/api/taxons_controller.rb index e2450839cf..a6fe754184 100644 --- a/app/controllers/spree/api/taxons_controller.rb +++ b/app/controllers/spree/api/taxons_controller.rb @@ -21,6 +21,10 @@ module Spree respond_with(@taxon) end + def jstree + show + end + def create authorize! :create, Taxon @taxon = Taxon.new(params[:taxon]) diff --git a/config/routes/spree.rb b/config/routes/spree.rb index 05a709f0ff..1dc3c32071 100644 --- a/config/routes/spree.rb +++ b/config/routes/spree.rb @@ -92,6 +92,14 @@ Spree::Core::Engine.routes.prepend do end resources :taxons, :only => [:index] + + resources :taxonomies do + resources :taxons do + member do + get :jstree + end + end + end end namespace :admin do diff --git a/spec/controllers/spree/api/taxons_controller_spec.rb b/spec/controllers/spree/api/taxons_controller_spec.rb index efd5ec1880..03ec795ea1 100644 --- a/spec/controllers/spree/api/taxons_controller_spec.rb +++ b/spec/controllers/spree/api/taxons_controller_spec.rb @@ -56,6 +56,15 @@ module Spree expect(json_response['taxons'].count).to eq 1 end + it "gets all taxons in JSTree form" do + api_get :jstree, taxonomy_id: taxonomy.id, id: taxon.id + + response = json_response.first + response["data"].should eq(taxon2.name) + response["attr"].should eq("name" => taxon2.name, "id" => taxon2.id) + response["state"].should eq("closed") + end + it "can learn how to create a new taxon" do api_get :new, taxonomy_id: taxonomy.id expect(json_response["attributes"]).to eq(attributes.map(&:to_s))