Move family location sharing to its own controller

app/controllers/api/v1/families/locations_controller.rb

@@ -2,10 +2,6 @@
 
 class Api::V1::Families::LocationsController < ApiController
   before_action :ensure_family_feature_enabled!
-  
-  # Skip API key auth for toggle action (use session auth instead for browser)
-  skip_before_action :authenticate_api_key, only: [:toggle]
-  before_action :authenticate_user!, only: [:toggle]
   before_action :ensure_user_in_family!
 
   def index
@@ -18,21 +14,10 @@ class Api::V1::Families::LocationsController < ApiController
     }
   end
 
-  def toggle
-    result = Families::UpdateLocationSharing.new(
-      user: current_user,
-      enabled: params[:enabled],
-      duration: params[:duration]
-    ).call
-
-    render json: result.payload, status: result.status
-  end
-
   private
 
   def ensure_user_in_family!
-    user = action_name == 'toggle' ? current_user : current_api_user
-    return if user&.in_family?
+    return if current_api_user&.in_family?
 
     render json: { error: 'User is not part of a family' }, status: :forbidden
   end

app/controllers/api/v1/families_controller.rb

@@ -1,16 +0,0 @@
-# frozen_string_literal: true
-
-# This controller is kept for future family-level API endpoints
-# Location-specific endpoints have been moved to Api::V1::Families::LocationsController
-class Api::V1::FamiliesController < ApiController
-  before_action :ensure_family_feature_enabled!
-  before_action :ensure_user_in_family!
-
-  private
-
-  def ensure_user_in_family!
-    return if current_api_user.in_family?
-
-    render json: { error: 'User is not part of a family' }, status: :forbidden
-  end
-end

app/controllers/family/location_sharing_controller.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+class Family::LocationSharingController < ApplicationController
+  before_action :authenticate_user!
+  before_action :ensure_family_feature_enabled!
+  before_action :ensure_user_in_family!
+
+  def update
+    result = Families::UpdateLocationSharing.new(
+      user: current_user,
+      enabled: params[:enabled],
+      duration: params[:duration]
+    ).call
+
+    render json: result.payload, status: result.status
+  end
+
+  private
+
+  def ensure_user_in_family!
+    return if current_user.in_family?
+
+    render json: { error: 'User is not part of a family' }, status: :forbidden
+  end
+end

app/javascript/controllers/location_sharing_toggle_controller.js

@@ -62,7 +62,7 @@ export default class extends Controller {
     try {
       const csrfToken = document.querySelector('meta[name="csrf-token"]').getAttribute('content');
 
-      const response = await fetch(`/api/v1/families/locations/toggle`, {
+      const response = await fetch(`/family/location_sharing`, {
         method: 'PATCH',
         headers: {
           'Accept': 'application/json',

config/routes.rb

@@ -62,6 +62,8 @@ Rails.application.routes.draw do
     resource :family, only: %i[show new create edit update destroy] do
       resources :invitations, except: %i[edit update], controller: 'family/invitations'
       resources :members, only: %i[destroy], controller: 'family/memberships'
+
+      patch 'location_sharing', to: 'family/location_sharing#update', as: :location_sharing
     end
 
     get 'invitations/:token', to: 'family/invitations#show', as: :public_invitation
@@ -170,11 +172,7 @@ Rails.application.routes.draw do
       end
 
       namespace :families do
-        resources :locations, only: [:index] do
-          collection do
-            patch :toggle
-          end
-        end
+        resources :locations, only: [:index]
       end
 
       post 'subscriptions/callback', to: 'subscriptions#callback'

spec/requests/api/v1/families/locations_spec.rb

@@ -66,109 +66,4 @@ RSpec.describe 'Api::V1::Families::Locations', type: :request do
     end
   end
 
-  describe 'PATCH /api/v1/families/locations/toggle' do
-    before { sign_in user }
-
-    context 'with valid session authentication' do
-      context 'when enabling location sharing' do
-        around do |example|
-          travel_to(Time.zone.local(2024, 1, 1, 12, 0, 0)) { example.run }
-        end
-
-        it 'enables location sharing with duration' do
-          patch '/api/v1/families/locations/toggle',
-                params: { enabled: true, duration: '1h' },
-                as: :json
-
-          expect(response).to have_http_status(:ok)
-          json_response = JSON.parse(response.body)
-          expect(json_response['success']).to be true
-          expect(json_response['enabled']).to be true
-          expect(json_response['duration']).to eq('1h')
-          expect(json_response['message']).to eq('Location sharing enabled for 1 hour')
-          expect(json_response['expires_at']).to eq(1.hour.from_now.iso8601)
-        end
-
-        it 'enables location sharing permanently' do
-          patch '/api/v1/families/locations/toggle',
-                params: { enabled: true, duration: 'permanent' },
-                as: :json
-
-          expect(response).to have_http_status(:ok)
-          json_response = JSON.parse(response.body)
-          expect(json_response['success']).to be true
-          expect(json_response['enabled']).to be true
-          expect(json_response['duration']).to eq('permanent')
-          expect(json_response).not_to have_key('expires_at')
-        end
-      end
-
-      context 'when disabling location sharing' do
-        before do
-          user.update_family_location_sharing!(true, duration: '1h')
-        end
-
-        it 'disables location sharing' do
-          patch '/api/v1/families/locations/toggle',
-                params: { enabled: false },
-                as: :json
-
-          expect(response).to have_http_status(:ok)
-          json_response = JSON.parse(response.body)
-          expect(json_response['success']).to be true
-          expect(json_response['enabled']).to be false
-          expect(json_response['message']).to eq('Location sharing disabled')
-        end
-      end
-
-      context 'when user is not in a family' do
-        let(:solo_user) { create(:user) }
-
-        before do
-          sign_out user
-          sign_in solo_user
-        end
-
-        it 'returns forbidden' do
-          patch '/api/v1/families/locations/toggle',
-                params: { enabled: true, duration: '1h' },
-                as: :json
-
-          expect(response).to have_http_status(:forbidden)
-          json_response = JSON.parse(response.body)
-          expect(json_response['error']).to eq('User is not part of a family')
-        end
-      end
-
-      context 'when update fails' do
-        before do
-          allow_any_instance_of(User).to receive(:update_family_location_sharing!)
-            .and_raise(StandardError, 'Database error')
-        end
-
-        it 'returns internal server error' do
-          patch '/api/v1/families/locations/toggle',
-                params: { enabled: true, duration: '1h' },
-                as: :json
-
-          expect(response).to have_http_status(:internal_server_error)
-          json_response = JSON.parse(response.body)
-          expect(json_response['success']).to be false
-          expect(json_response['message']).to eq('An error occurred while updating location sharing')
-        end
-      end
-    end
-
-    context 'without authentication' do
-      before { sign_out user }
-
-      it 'returns unauthorized' do
-        patch '/api/v1/families/locations/toggle',
-              params: { enabled: true, duration: '1h' },
-              as: :json
-
-        expect(response).to have_http_status(:unauthorized)
-      end
-    end
-  end
 end

spec/requests/family/location_sharing_spec.rb

@@ -0,0 +1,117 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe 'Family::LocationSharing', type: :request do
+  include ActiveSupport::Testing::TimeHelpers
+
+  let(:user) { create(:user) }
+  let(:family) { create(:family, creator: user) }
+  let!(:user_membership) { create(:family_membership, user: user, family: family, role: :owner) }
+
+  before { sign_in user }
+
+  describe 'PATCH /family/location_sharing' do
+    context 'when enabling location sharing' do
+      around do |example|
+        travel_to(Time.zone.local(2024, 1, 1, 12, 0, 0)) { example.run }
+      end
+
+      it 'enables location sharing with duration' do
+        patch '/family/location_sharing',
+              params: { enabled: true, duration: '1h' },
+              as: :json
+
+        expect(response).to have_http_status(:ok)
+        json_response = JSON.parse(response.body)
+        expect(json_response['success']).to be true
+        expect(json_response['enabled']).to be true
+        expect(json_response['duration']).to eq('1h')
+        expect(json_response['message']).to eq('Location sharing enabled for 1 hour')
+        expect(json_response['expires_at']).to eq(1.hour.from_now.iso8601)
+      end
+
+      it 'enables location sharing permanently' do
+        patch '/family/location_sharing',
+              params: { enabled: true, duration: 'permanent' },
+              as: :json
+
+        expect(response).to have_http_status(:ok)
+        json_response = JSON.parse(response.body)
+        expect(json_response['success']).to be true
+        expect(json_response['enabled']).to be true
+        expect(json_response['duration']).to eq('permanent')
+        expect(json_response).not_to have_key('expires_at')
+      end
+    end
+
+    context 'when disabling location sharing' do
+      before do
+        user.update_family_location_sharing!(true, duration: '1h')
+      end
+
+      it 'disables location sharing' do
+        patch '/family/location_sharing',
+              params: { enabled: false },
+              as: :json
+
+        expect(response).to have_http_status(:ok)
+        json_response = JSON.parse(response.body)
+        expect(json_response['success']).to be true
+        expect(json_response['enabled']).to be false
+        expect(json_response['message']).to eq('Location sharing disabled')
+      end
+    end
+
+    context 'when user is not in a family' do
+      let(:solo_user) { create(:user) }
+
+      before do
+        sign_out user
+        sign_in solo_user
+      end
+
+      it 'returns forbidden' do
+        patch '/family/location_sharing',
+              params: { enabled: true, duration: '1h' },
+              as: :json
+
+        expect(response).to have_http_status(:forbidden)
+        json_response = JSON.parse(response.body)
+        expect(json_response['error']).to eq('User is not part of a family')
+      end
+    end
+
+    context 'when update fails' do
+      before do
+        allow_any_instance_of(User).to receive(:update_family_location_sharing!)
+          .and_raise(StandardError, 'Database error')
+      end
+
+      it 'returns internal server error' do
+        patch '/family/location_sharing',
+              params: { enabled: true, duration: '1h' },
+              as: :json
+
+        expect(response).to have_http_status(:internal_server_error)
+        json_response = JSON.parse(response.body)
+        expect(json_response['success']).to be false
+        expect(json_response['message']).to eq('An error occurred while updating location sharing')
+      end
+    end
+
+    context 'without authentication' do
+      before { sign_out user }
+
+      it 'returns unauthorized' do
+        patch '/family/location_sharing',
+              params: { enabled: true, duration: '1h' },
+              as: :json
+
+        expect(response).to have_http_status(:unauthorized)
+        json_response = JSON.parse(response.body)
+        expect(json_response['error']).to eq('You need to sign in or sign up before continuing.')
+      end
+    end
+  end
+end