From 8ec81e80e997c7d8bce1d042970f9ea11c0e32d7 Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Wed, 23 Feb 2022 15:22:35 -0500 Subject: [PATCH] Removing duplicate logic and adding docs (#16681) As I was investigating an approach for #16488, I stumbled upon two methods partially doing the same thing. This helps consolidate the logic and provides some guiding documentation. --- app/controllers/api/v0/api_controller.rb | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/v0/api_controller.rb b/app/controllers/api/v0/api_controller.rb index ef606a255..81d082a99 100644 --- a/app/controllers/api/v0/api_controller.rb +++ b/app/controllers/api/v0/api_controller.rb @@ -43,15 +43,21 @@ module Api end # Checks if the user is authenticated, sets @user to nil otherwise + # + # @return [User, NilClass] def authenticate_with_api_key_or_current_user @user = authenticate_with_api_key || current_user end - # Checks if the user is authenticated, if so sets the variable @user - # Returns HTTP 401 Unauthorized otherwise + # Checks if the user is authenticated, if not respond with an HTTP 401 Unauthorized + # + # @see {authenticate_with_api_key_or_current_user} def authenticate_with_api_key_or_current_user! - @user = authenticate_with_api_key || current_user - error_unauthorized unless @user + # [@jeremyf] Note, I'm not relying on the other method setting the instance variable, but + # instead relying on the returned value. This insulates us from an implementation detail + # (namely should we use @user or current_user, which is a bit soupy in the API controller). + user = authenticate_with_api_key_or_current_user + error_unauthorized unless user end private