From 5dad30faa8f2c3a2fc9d3d60df7ecb545fad4b52 Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Wed, 21 May 2025 11:46:56 +0200 Subject: [PATCH] Refactor "current_api_user" into instacached helper In the same spirit as the previous commit, which refactored `@current_user_roles` into a helper method, we do the same for the `current_api_user`. Also fixes a spec that previously compared a user object with a string, hoping them not to be identical (they never would be). --- .../controllers/spree/api/base_controller.rb | 24 ++++++++++++------- api/lib/spree/api/testing_support/helpers.rb | 2 +- api/spec/requests/spree/api/orders_spec.rb | 2 +- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/api/app/controllers/spree/api/base_controller.rb b/api/app/controllers/spree/api/base_controller.rb index a4a60ea67b2..39eb481831b 100644 --- a/api/app/controllers/spree/api/base_controller.rb +++ b/api/app/controllers/spree/api/base_controller.rb @@ -21,9 +21,7 @@ class BaseController < ActionController::Base class_attribute :admin_metadata_attributes self.admin_metadata_attributes = [{ admin_metadata: {} }] - attr_accessor :current_api_user - - before_action :load_user + before_action :deprecated_load_user before_action :authorize_for_order, if: proc { order_token.present? } before_action :authenticate_user # This is deprecated and will be removed in Spree 5.0 @@ -36,7 +34,7 @@ class BaseController < ActionController::Base rescue_from StateMachines::InvalidTransition, with: :invalid_transition helper Spree::Api::ApiHelpers - helper_method :current_user_roles + helper_method :current_user_roles, :current_api_user private @@ -63,12 +61,12 @@ def permitted_user_attributes can?(:admin, Spree.user_class) ? super + admin_metadata_attributes : super end - def load_user - @current_api_user ||= Spree.user_class.find_by(spree_api_key: api_key.to_s) + def current_api_user + @_current_api_user ||= Spree.user_class.find_by(spree_api_key: api_key.to_s) end def authenticate_user - unless @current_api_user + unless current_api_user if requires_authentication? && api_key.blank? && order_token.blank? render "spree/api/errors/must_specify_api_key", status: :unauthorized elsif order_token.blank? && (requires_authentication? || api_key.present?) @@ -85,9 +83,17 @@ def load_deprecated_user_roles end end + def deprecated_load_user + @current_api_user = if Rails.version < Gem::Version.new("7.2.0") + ActiveSupport::Deprecation::DeprecatedInstanceVariableProxy.new(self, :current_api_user, :@current_api_user, Spree.deprecator) + else + ActiveSupport::Deprecation::DeprecatedInstanceVariableProxy.new(self, :current_api_user, :@current_api_user, deprecator: Spree.deprecator) + end + end + def current_user_roles - @_current_user_roles ||= if @current_api_user - @current_api_user.spree_roles.pluck(:name) + @_current_user_roles ||= if current_api_user + current_api_user.spree_roles.pluck(:name) else [] end diff --git a/api/lib/spree/api/testing_support/helpers.rb b/api/lib/spree/api/testing_support/helpers.rb index f6e3278f0c7..ab6818bb853 100644 --- a/api/lib/spree/api/testing_support/helpers.rb +++ b/api/lib/spree/api/testing_support/helpers.rb @@ -30,7 +30,7 @@ def stub_authentication! # This method can be overridden (with a let block) inside a context # For instance, if you wanted to have an admin user instead. def current_api_user - @current_api_user ||= stub_model(Spree::LegacyUser, email: "solidus@example.com", spree_roles: []) + @_current_api_user ||= stub_model(Spree::LegacyUser, email: "solidus@example.com", spree_roles: []) end def image(filename) diff --git a/api/spec/requests/spree/api/orders_spec.rb b/api/spec/requests/spree/api/orders_spec.rb index c1e6f711c93..80bbad26fe2 100644 --- a/api/spec/requests/spree/api/orders_spec.rb +++ b/api/spec/requests/spree/api/orders_spec.rb @@ -512,7 +512,7 @@ module Spree::Api it "assigns email when creating a new order" do post spree.api_orders_path, params: { order: { email: "guest@solidus.io" } } - expect(json_response['email']).not_to eq controller.current_api_user + expect(json_response['email']).not_to eq current_api_user.email expect(json_response['email']).to eq "guest@solidus.io" end