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' diff --git a/app/controllers/spree/admin/products_controller_decorator.rb b/app/controllers/spree/admin/products_controller_decorator.rb index 0007c0c59e..b265ba64fd 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/base_controller.rb b/app/controllers/spree/api/base_controller.rb new file mode 100644 index 0000000000..3f28596fec --- /dev/null +++ b/app/controllers/spree/api/base_controller.rb @@ -0,0 +1,130 @@ +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]}(#{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 || + !requires_authentication? + + return if api_key.present? + render("spree/api/errors/must_specify_api_key", status: :unauthorized) && return + end + + def authenticate_user + 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: :unauthorized) && return + end + + def error_during_processing(exception) + render(text: { exception: exception.message }.to_json, + status: :unprocessable_entity) && return + end + + def requires_authentication? + true + end + + def not_found + render("spree/api/errors/not_found", status: :not_found) && 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: :unprocessable_entity + end + + def api_key + request.headers["X-Spree-Token"] || params[:token] + end + helper_method :api_key + + def find_product(id) + product_scope.find_by_permalink!(id.to_s) + rescue ActiveRecord::RecordNotFound + product_scope.find(id) + 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 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/app/controllers/spree/api/products_controller.rb b/app/controllers/spree/api/products_controller.rb new file mode 100644 index 0000000000..615596383e --- /dev/null +++ b/app/controllers/spree/api/products_controller.rb @@ -0,0 +1,149 @@ +require 'open_food_network/permissions' + +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.zone.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.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]) + 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 + # 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 + 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 new file mode 100644 index 0000000000..4963c204a4 --- /dev/null +++ b/app/controllers/spree/api/shipments_controller.rb @@ -0,0 +1,108 @@ +require 'open_food_network/scope_variant_to_hub' + +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 = 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 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: :unprocessable_entity + 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 = 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 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 + + 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/taxons_controller.rb b/app/controllers/spree/api/taxons_controller.rb new file mode 100644 index 0000000000..a6fe754184 --- /dev/null +++ b/app/controllers/spree/api/taxons_controller.rb @@ -0,0 +1,75 @@ +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) && 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 + 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 diff --git a/app/controllers/spree/api/variants_controller.rb b/app/controllers/spree/api/variants_controller.rb new file mode 100644 index 0000000000..fe0fa1d793 --- /dev/null +++ b/app/controllers/spree/api/variants_controller.rb @@ -0,0 +1,83 @@ +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 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]) + @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 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 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]) diff --git a/app/helpers/spree/api/api_helpers.rb b/app/helpers/spree/api/api_helpers.rb new file mode 100644 index 0000000000..e27d1164aa --- /dev/null +++ b/app/helpers/spree/api/api_helpers.rb @@ -0,0 +1,120 @@ +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 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/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 %>; 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 diff --git a/config/routes/spree.rb b/config/routes/spree.rb index 5613ae9c11..1dc3c32071 100644 --- a/config/routes/spree.rb +++ b/config/routes/spree.rb @@ -76,8 +76,29 @@ 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] + + resources :taxonomies do + resources :taxons do + member do + get :jstree + end + end end end @@ -105,6 +126,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 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 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/admin/products_controller_spec.rb b/spec/controllers/spree/admin/products_controller_spec.rb index 7209b31551..0466ec6564 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/controllers/spree/api/base_controller_spec.rb b/spec/controllers/spree/api/base_controller_spec.rb new file mode 100644 index 0000000000..cab638c74e --- /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 + allow(controller).to receive_messages try_spree_current_user: + double(email: "spree@example.com") + end + + it "can make a request" do + api_get :index + 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 + 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, {} + 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" + expect(json_response).to eq( "error" => "Invalid API key (fake_key) specified." ) + end + end + + it 'handles exceptions' do + 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 }, + 'name' => 'test order' } + + mapped = subject.map_nested_attributes_keys(klass, attributes) + expect(mapped.key?('line_items_attributes')).to be_truthy + expect(mapped.key?('name')).to be_truthy + 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 diff --git a/spec/controllers/spree/api/products_controller_spec.rb b/spec/controllers/spree/api/products_controller_spec.rb index 89fb08c40c..c726487052 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.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] } + 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,120 @@ module Spree spree_get :managed, template: 'bulk_index', format: :json assert_unauthorized! end + + it "retrieves a list of products" do + api_get :index + 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 "retrieves a list of products by id" do + 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] + expect(json_response["count"]).to eq(0) + end + + it "does not list unavailable products" do + api_get :index + 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 + 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 + 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' + 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" } + 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.set_property("spree", "rocks") + 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") } + + before do + product.update_attribute(:permalink, "#{other_product.id}-and-1-ways") + end + + specify do + 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 + expect(json_response["permalink"]).to match(/droids/) + end + end + + it "cannot see inactive products" do + 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" + 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 + 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"] + 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" end context "as an enterprise user" do @@ -38,15 +154,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 +181,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 +209,91 @@ 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 + 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) + end + + it "does not include deleted products" do + api_get :index + expect(json_response["products"].count).to eq(2) + end + + it "can include deleted products" do + 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" } + 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: {} + 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.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!" } + 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: "" } + 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 + 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 @@ -124,7 +305,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 +318,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 +341,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/controllers/spree/api/shipments_controller_spec.rb b/spec/controllers/spree/api/shipments_controller_spec.rb index 3c71618c50..2b02eb8534 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 + 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 + 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 + allow_any_instance_of(Spree::Order).to receive_messages(paid?: false) + api_put :ready + + 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 } } + + it 'adds a variant to a shipment' do + 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 + 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 + 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) + 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 + shipment.reload + 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 + context 'for a completed order with shipment' do let(:order) { create :completed_order_with_totals } 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..03ec795ea1 --- /dev/null +++ b/spec/controllers/spree/api/taxons_controller_spec.rb @@ -0,0 +1,135 @@ +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 + 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 + let(:current_api_user) { build(:user) } + + 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 "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)) + 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 + let(:current_api_user) { build(:admin_user) } + + 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 diff --git a/spec/controllers/spree/api/variants_controller_spec.rb b/spec/controllers/spree/api/variants_controller_spec.rb index 19f4bccc95..e32f34809a 100644 --- a/spec/controllers/spree/api/variants_controller_spec.rb +++ b/spec/controllers/spree/api/variants_controller_spec.rb @@ -9,6 +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] + } before do allow(controller).to receive(:spree_current_user) { current_api_user } @@ -17,8 +21,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,12 +38,133 @@ 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 + + it "can see a paginated list of variants" do + api_get :index + + keys = json_response["variants"].first.keys.map(&:to_sym) + expect(standard_attributes.all?{ |attr| keys.include? attr }).to eq(true) + 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 + + 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' } + + 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"] + 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")) + + api_get :index + + 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.zone.now) + end + + it "is not returned in the results" do + api_get :index + 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 + 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 + + keys = json_response["variants"].first.keys.map(&:to_sym) + expect(standard_attributes.all?{ |attr| keys.include? attr }).to eq(true) + 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 + + 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"] + 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 + + 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_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 + + 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" } + + 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! + expect { variant.reload }.not_to raise_error + end end context "as an enterprise user" do @@ -44,6 +177,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 @@ -51,6 +185,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 @@ -71,9 +206,11 @@ 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 + expect(response.status).to eq(204) expect { variant.reload }.not_to raise_error expect(variant.deleted_at).not_to be_nil @@ -82,7 +219,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 @@ -97,6 +233,41 @@ module Spree spree_delete :soft_delete, variant_id: variant.id, product_id: variant.product.permalink, format: :json end end + + context "deleted variants" do + before do + variant.update_column(:deleted_at, Time.zone.now) + end + + it "are visible by admin" do + api_get :index, show_deleted: 1 + + 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" } + + expect(standard_attributes.all?{ |attr| json_response.include? attr.to_s }).to eq(true) + 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" } + + expect(response.status).to eq(200) + end + + it "can delete a variant" do + api_delete :destroy, id: variant.to_param + + expect(response.status).to eq(204) + expect { Spree::Variant.find(variant.id) }.to raise_error(ActiveRecord::RecordNotFound) + end end end end 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/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/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' } 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/spec_helper.rb b/spec/spec_helper.rb index 499a810302..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' @@ -127,8 +126,6 @@ RSpec.configure do |config| spree_config.shipping_instructions = true spree_config.auto_capture = true end - - Spree::Api::Config[:requires_authentication] = true end # Helpers @@ -140,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..2dd217117d 100644 --- a/spec/support/api_helper.rb +++ b/spec/support/api_helper.rb @@ -11,5 +11,18 @@ 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! + expect(json_response).to eq("error" => "You are not authorized to perform that action.") + expect(response.status).to eq 401 + end + + def image(filename) + File.open(Rails.root + "spec/support/fixtures" + filename) + end end end diff --git a/spec/support/controller_hacks.rb b/spec/support/controller_hacks.rb new file mode 100644 index 0000000000..7645e87372 --- /dev/null +++ b/spec/support/controller_hacks.rb @@ -0,0 +1,34 @@ +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 diff --git a/spec/support/fixtures/thinking-cat.jpg b/spec/support/fixtures/thinking-cat.jpg new file mode 100644 index 0000000000..7e8524d367 Binary files /dev/null and b/spec/support/fixtures/thinking-cat.jpg differ 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 8e9ebc1c98..fcadc55b62 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