Merge pull request #1954 from Freika/fix/family-location-toggle

Fix family location sharing toggle

CHANGELOG.md

@@ -25,6 +25,7 @@ OIDC_REDIRECT_URI=https://your-dawarich-url.com/users/auth/openid_connect/callba
 ## Fixed
 
 - The map settings panel is now scrollable
+- Fixed a bug where family location sharing settings were not being updated correctly. #1940
 
 ## Changed
 

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

@@ -1,10 +1,10 @@
 # frozen_string_literal: true
 
-class Api::V1::FamiliesController < ApiController
+class Api::V1::Families::LocationsController < ApiController
   before_action :ensure_family_feature_enabled!
   before_action :ensure_user_in_family!
 
-  def locations
+  def index
     family_locations = Families::Locations.new(current_api_user).call
 
     render json: {
@@ -17,7 +17,7 @@ class Api::V1::FamiliesController < ApiController
   private
 
   def ensure_user_in_family!
-    return if current_api_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/families_controller.rb

@@ -3,7 +3,7 @@
 class FamiliesController < ApplicationController
   before_action :authenticate_user!
   before_action :ensure_family_feature_enabled!
-  before_action :set_family, only: %i[show edit update destroy update_location_sharing]
+  before_action :set_family, only: %i[show edit update destroy]
 
   def show
     authorize @family
@@ -76,18 +76,6 @@ class FamiliesController < ApplicationController
     end
   end
 
-  def update_location_sharing
-    authorize @family, :update_location_sharing?
-
-    result = Families::UpdateLocationSharing.new(
-      user: current_user,
-      enabled: params[:enabled],
-      duration: params[:duration]
-    ).call
-
-    render json: result.payload, status: result.status
-  end
-
   private
 
   def set_family

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(`/family/update_location_sharing`, {
+      const response = await fetch(`/family/location_sharing`, {
         method: 'PATCH',
         headers: {
           'Accept': 'application/json',

config/routes.rb

@@ -60,10 +60,10 @@ Rails.application.routes.draw do
   # Family management routes (only if feature is enabled)
   if DawarichSettings.family_feature_enabled?
     resource :family, only: %i[show new create edit update destroy] do
-      patch :update_location_sharing, on: :member
-
       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
@@ -171,10 +171,8 @@ Rails.application.routes.draw do
         end
       end
 
-      resources :families, only: [] do
+      namespace :families do
-        collection do
+        resources :locations, only: [:index]
-          get :locations
-        end
       end
 
       post 'subscriptions/callback', to: 'subscriptions#callback'

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

@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe 'Api::V1::Families::Locations', type: :request do
+  include ActiveSupport::Testing::TimeHelpers
+
+  let(:user) { create(:user) }
+  let(:other_user) { create(:user) }
+  let(:family) { create(:family, creator: user) }
+  let!(:user_membership) { create(:family_membership, user: user, family: family, role: :owner) }
+
+  describe 'GET /api/v1/families/locations' do
+    context 'with valid API key' do
+      before do
+        create(:family_membership, user: other_user, family: family, role: :member)
+        other_user.update_family_location_sharing!(true, duration: 'permanent')
+      end
+
+      it 'returns family member locations' do
+        get '/api/v1/families/locations', params: { api_key: user.api_key }
+
+        expect(response).to have_http_status(:ok)
+        json_response = JSON.parse(response.body)
+        expect(json_response).to have_key('locations')
+        expect(json_response).to have_key('updated_at')
+        expect(json_response).to have_key('sharing_enabled')
+      end
+
+      it 'includes sharing status' do
+        user.update_family_location_sharing!(true, duration: 'permanent')
+
+        get '/api/v1/families/locations', params: { api_key: user.api_key }
+
+        json_response = JSON.parse(response.body)
+        expect(json_response['sharing_enabled']).to be true
+      end
+    end
+
+    context 'without API key' do
+      it 'returns unauthorized' do
+        get '/api/v1/families/locations'
+
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+
+    context 'with invalid API key' do
+      it 'returns unauthorized' do
+        get '/api/v1/families/locations', params: { api_key: 'invalid' }
+
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+
+    context 'when user is not in a family' do
+      let(:solo_user) { create(:user) }
+
+      it 'returns forbidden' do
+        get '/api/v1/families/locations', params: { api_key: solo_user.api_key }
+
+        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
+  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