Skip to content

Refactor "current_api_user" into instacached helper #6246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tvdeyen merged 1 commit into from
May 30, 2025
Merged

Refactor "current_api_user" into instacached helper #6246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 15 additions & 9 deletions api/app/controllers/spree/api/base_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@
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
Expand All @@ -36,7 +34,7 @@
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

Expand All @@ -63,12 +61,12 @@
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?)
Expand All @@ -85,9 +83,17 @@
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)

Check warning on line 88 in api/app/controllers/spree/api/base_controller.rb

View check run for this annotation

Codecov / codecov/patch

api/app/controllers/spree/api/base_controller.rb#L88 

Added line #L88 was not covered by tests
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
Expand Down
2 changes: 1 addition & 1 deletion api/lib/spree/api/testing_support/helpers.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion api/spec/requests/spree/api/orders_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down