diff --git a/app/controllers/api/v1/families/locations_controller.rb b/app/controllers/api/v1/families/locations_controller.rb index e57d90a4..7d3d7fbb 100644 --- a/app/controllers/api/v1/families/locations_controller.rb +++ b/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 diff --git a/app/controllers/api/v1/families_controller.rb b/app/controllers/api/v1/families_controller.rb deleted file mode 100644 index b8cdb2c9..00000000 --- a/app/controllers/api/v1/families_controller.rb +++ /dev/null @@ -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 diff --git a/app/controllers/family/location_sharing_controller.rb b/app/controllers/family/location_sharing_controller.rb new file mode 100644 index 00000000..cc18a6af --- /dev/null +++ b/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 diff --git a/app/javascript/controllers/location_sharing_toggle_controller.js b/app/javascript/controllers/location_sharing_toggle_controller.js index a6d52c85..ff17b0d8 100644 --- a/app/javascript/controllers/location_sharing_toggle_controller.js +++ b/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', diff --git a/config/routes.rb b/config/routes.rb index fd482f42..60684cfb 100644 --- a/config/routes.rb +++ b/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' diff --git a/spec/requests/api/v1/families/locations_spec.rb b/spec/requests/api/v1/families/locations_spec.rb index 8b0879e6..a62bbb90 100644 --- a/spec/requests/api/v1/families/locations_spec.rb +++ b/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 diff --git a/spec/requests/family/location_sharing_spec.rb b/spec/requests/family/location_sharing_spec.rb new file mode 100644 index 00000000..3355919d --- /dev/null +++ b/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