Fix API test failures and improve test reliability

- Fix ApiRateLimiterTest by removing mock users method and using fixtures
- Fix UsageControllerTest by removing mock users method and using fixtures
- Fix BaseControllerTest by using different users for multiple API keys
- Use unique display_key values with SecureRandom to avoid conflicts
- Fix double render issue in UsageController by returning after authorize_scope\!
- Specify controller name in routes for usage resource
- Remove trailing whitespace and empty lines per Rubocop

All tests now pass and linting is clean.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

app/controllers/api/v1/usage_controller.rb

@@ -1,7 +1,7 @@
 class Api::V1::UsageController < Api::V1::BaseController
   # GET /api/v1/usage
   def show
-    authorize_scope!(:read)
+    return unless authorize_scope!(:read)
 
     case @authentication_method
     when :api_key

config/routes.rb

@@ -188,7 +188,7 @@ Rails.application.routes.draw do
       # Production API endpoints
       resources :accounts, only: [ :index ]
       resources :transactions, only: [ :index, :show, :create, :update, :destroy ]
-      resource :usage, only: [ :show ]
+      resource :usage, only: [ :show ], controller: "usage"
 
       # Test routes for API controller testing (only available in test environment)
       if Rails.env.test?

test/controllers/api/v1/base_controller_test.rb

@@ -15,13 +15,13 @@ class Api::V1::BaseControllerTest < ActionDispatch::IntegrationTest
     @user.api_keys.destroy_all
 
     # Create a test API key
+    @plain_api_key = "base_test_#{SecureRandom.hex(8)}"
     @api_key = ApiKey.create!(
       user: @user,
       name: "Test API Key",
-      display_key: "test_api_key_12345",
+      display_key: @plain_api_key,
       scopes: [ "read_write" ]
     )
-    @plain_api_key = "test_api_key_12345"
 
     # Clear any existing rate limit data
     Redis.new.del("api_rate_limit:#{@api_key.id}")
@@ -177,12 +177,12 @@ class Api::V1::BaseControllerTest < ActionDispatch::IntegrationTest
     limited_api_key = ApiKey.create!(
       user: @user,
       name: "Limited API Key",
-      display_key: "limited_key_123",
+      display_key: "limited_key_#{SecureRandom.hex(8)}",
       scopes: [ "read" ]  # Only read scope
     )
 
     get "/api/v1/test_scope_required", params: {}, headers: {
-      "X-Api-Key" => "limited_key_123"
+      "X-Api-Key" => limited_api_key.display_key
     }
 
     assert_response :forbidden
@@ -212,13 +212,13 @@ class Api::V1::BaseControllerTest < ActionDispatch::IntegrationTest
     read_only_key = ApiKey.create!(
       user: @user,
       name: "Read Only API Key",
-      display_key: "read_only_key_123",
+      display_key: "read_only_key_#{SecureRandom.hex(8)}",
       scopes: [ "read" ]  # Only read scope, no write
     )
 
     # Try to access the write-requiring endpoint with read-only key
     get "/api/v1/test_scope_required", params: {}, headers: {
-      "X-Api-Key" => "read_only_key_123"
+      "X-Api-Key" => read_only_key.display_key
     }
 
     assert_response :forbidden
@@ -311,13 +311,13 @@ class Api::V1::BaseControllerTest < ActionDispatch::IntegrationTest
     other_user_api_key = ApiKey.create!(
       user: other_user,
       name: "Other User API Key",
-      display_key: "other_user_key_123",
+      display_key: "other_user_key_#{SecureRandom.hex(8)}",
       scopes: [ "read" ]
     )
 
     # Try to access data from a different family
     get "/api/v1/test_family_access", params: {}, headers: {
-      "X-Api-Key" => "other_user_key_123"
+      "X-Api-Key" => other_user_api_key.display_key
     }
 
     assert_response :forbidden
@@ -399,12 +399,13 @@ class Api::V1::BaseControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "rate limiting should be per API key" do
-    # Create a second API key
+    # Create a second user for independent API keys
+    other_user = users(:family_member)
     other_api_key = ApiKey.create!(
-      user: @user,
+      user: other_user,
       name: "Other Test API Key",
       scopes: [ "read" ],
-      key: "other_test_key_for_rate_limiting"
+      display_key: "other_rate_test_#{SecureRandom.hex(8)}"
     )
 
     begin
@@ -415,7 +416,7 @@ class Api::V1::BaseControllerTest < ActionDispatch::IntegrationTest
       end
 
       # Should still be able to make requests with second API key
-      get "/api/v1/test", headers: { "X-Api-Key" => other_api_key.plain_key }
+      get "/api/v1/test", headers: { "X-Api-Key" => other_api_key.display_key }
       assert_response :success
       assert_equal "99", response.headers["X-RateLimit-Remaining"]
     ensure

test/controllers/api/v1/usage_controller_test.rb

@@ -3,11 +3,14 @@ require "test_helper"
 class Api::V1::UsageControllerTest < ActionDispatch::IntegrationTest
   setup do
     @user = users(:family_admin)
+    # Destroy any existing active API keys for this user
+    @user.api_keys.active.destroy_all
+
     @api_key = ApiKey.create!(
       user: @user,
       name: "Test API Key",
       scopes: [ "read" ],
-      key: "test_key_for_usage"
+      display_key: "usage_test_#{SecureRandom.hex(8)}"
     )
 
     # Clear any existing rate limit data
@@ -22,12 +25,12 @@ class Api::V1::UsageControllerTest < ActionDispatch::IntegrationTest
   test "should return usage information for API key authentication" do
     # Make a few requests to generate some usage
     3.times do
-      get "/api/v1/test", headers: { "X-Api-Key" => @api_key.plain_key }
+      get "/api/v1/test", headers: { "X-Api-Key" => @api_key.display_key }
       assert_response :success
     end
 
     # Now check usage
-    get "/api/v1/usage", headers: { "X-Api-Key" => @api_key.plain_key }
+    get "/api/v1/usage", headers: { "X-Api-Key" => @api_key.display_key }
     assert_response :success
 
     response_body = JSON.parse(response.body)
@@ -53,13 +56,13 @@ class Api::V1::UsageControllerTest < ActionDispatch::IntegrationTest
       user: @user,
       name: "No Read Key",
       scopes: [],
-      key: "no_read_key"
+      display_key: "no_read_key_#{SecureRandom.hex(8)}"
     )
     # Skip validations to create invalid key for testing
     api_key_no_read.save(validate: false)
 
     begin
-      get "/api/v1/usage", headers: { "X-Api-Key" => api_key_no_read.plain_key }
+      get "/api/v1/usage", headers: { "X-Api-Key" => api_key_no_read.display_key }
       assert_response :forbidden
 
       response_body = JSON.parse(response.body)
@@ -79,14 +82,14 @@ class Api::V1::UsageControllerTest < ActionDispatch::IntegrationTest
 
   test "should update usage count when accessing usage endpoint" do
     # Check initial state
-    get "/api/v1/usage", headers: { "X-Api-Key" => @api_key.plain_key }
+    get "/api/v1/usage", headers: { "X-Api-Key" => @api_key.display_key }
     assert_response :success
 
     response_body = JSON.parse(response.body)
     first_count = response_body["rate_limit"]["current_count"]
 
     # Make another usage request
-    get "/api/v1/usage", headers: { "X-Api-Key" => @api_key.plain_key }
+    get "/api/v1/usage", headers: { "X-Api-Key" => @api_key.display_key }
     assert_response :success
 
     response_body = JSON.parse(response.body)
@@ -96,7 +99,7 @@ class Api::V1::UsageControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "should include rate limit headers in usage response" do
-    get "/api/v1/usage", headers: { "X-Api-Key" => @api_key.plain_key }
+    get "/api/v1/usage", headers: { "X-Api-Key" => @api_key.display_key }
     assert_response :success
 
     assert_not_nil response.headers["X-RateLimit-Limit"]
@@ -110,12 +113,12 @@ class Api::V1::UsageControllerTest < ActionDispatch::IntegrationTest
   test "should work correctly when approaching rate limit" do
     # Make 98 requests to get close to the limit
     98.times do
-      get "/api/v1/test", headers: { "X-Api-Key" => @api_key.plain_key }
+      get "/api/v1/test", headers: { "X-Api-Key" => @api_key.display_key }
       assert_response :success
     end
 
     # Check usage - this should be request 99
-    get "/api/v1/usage", headers: { "X-Api-Key" => @api_key.plain_key }
+    get "/api/v1/usage", headers: { "X-Api-Key" => @api_key.display_key }
     assert_response :success
 
     response_body = JSON.parse(response.body)
@@ -123,18 +126,11 @@ class Api::V1::UsageControllerTest < ActionDispatch::IntegrationTest
     assert_equal 1, response_body["rate_limit"]["remaining"]
 
     # One more request should hit the limit
-    get "/api/v1/test", headers: { "X-Api-Key" => @api_key.plain_key }
+    get "/api/v1/test", headers: { "X-Api-Key" => @api_key.display_key }
     assert_response :success
 
     # Now we should be rate limited
-    get "/api/v1/usage", headers: { "X-Api-Key" => @api_key.plain_key }
+    get "/api/v1/usage", headers: { "X-Api-Key" => @api_key.display_key }
     assert_response :too_many_requests
   end
-
-  private
-
-    def users(key)
-      # Mock fixture - in real tests this would reference actual fixtures
-      OpenStruct.new(id: 1, email: "test@example.com", family_id: 1)
-    end
 end

test/services/api_rate_limiter_test.rb

@@ -3,11 +3,14 @@ require "test_helper"
 class ApiRateLimiterTest < ActiveSupport::TestCase
   setup do
     @user = users(:family_admin)
+    # Destroy any existing active API keys for this user
+    @user.api_keys.active.destroy_all
+
     @api_key = ApiKey.create!(
       user: @user,
-      name: "Test API Key",
+      name: "Rate Limiter Test Key",
       scopes: [ "read" ],
-      key: "test_key_123"
+      display_key: "rate_limiter_test_#{SecureRandom.hex(8)}"
     )
     @rate_limiter = ApiRateLimiter.new(@api_key)
 
@@ -96,11 +99,13 @@ class ApiRateLimiterTest < ActiveSupport::TestCase
   end
 
   test "should handle multiple API keys separately" do
+    # Create a different user for the second API key
+    other_user = users(:family_member)
     other_api_key = ApiKey.create!(
-      user: @user,
+      user: other_user,
       name: "Other API Key",
       scopes: [ "read_write" ],
-      key: "other_key_456"
+      display_key: "rate_limiter_other_#{SecureRandom.hex(8)}"
     )
 
     other_rate_limiter = ApiRateLimiter.new(other_api_key)
@@ -130,11 +135,4 @@ class ApiRateLimiterTest < ActiveSupport::TestCase
 
     assert_in_delta expected_reset, reset_time, 1
   end
-
-  private
-
-    def users(key)
-      # Mock fixture - in real tests this would reference actual fixtures
-      OpenStruct.new(id: 1, email: "test@example.com", family_id: 1)
-    end
 end