Refactor: Move trip sharing management to trips#update

Simplifies architecture by using the existing trips#update route for
sharing settings management instead of a separate route.

## Changes

**Routes**
- Removed: PATCH /trips/:id/sharing → shared/trips#update
- Now uses: PATCH /trips/:id (existing route) with sharing params

**Controllers**
- Shared::TripsController: Simplified to only handle public view (show)
- TripsController: Added update_sharing private method to handle
  sharing params when present

**Views**
- Updated JavaScript in _sharing.html.erb to use trip_path with
  nested sharing params

**Tests**
- Updated request specs to use trip_path instead of sharing_trip_path
- All params now nested under sharing key

## Benefits
- Cleaner namespace separation (Shared:: only for public access)
- Follows Rails conventions (one update route handles everything)
- Simpler routing structure
- Reduced code duplication

## Backwards Compatibility
This is a breaking change for the sharing API endpoint, but since
this feature was just implemented and hasn't been released yet,
no migration path is needed.
This commit is contained in:
Claude 2025-11-05 15:54:42 +00:00
parent ce5e57a691
commit 9fba3ce4ca
No known key found for this signature in database
5 changed files with 71 additions and 71 deletions

View file

@ -1,9 +1,6 @@
# frozen_string_literal: true
class Shared::TripsController < ApplicationController
before_action :authenticate_user!, except: [:show]
before_action :authenticate_active_user!, only: [:update]
def show
@trip = Trip.find_by(sharing_uuid: params[:trip_uuid])
@ -20,44 +17,6 @@ class Shared::TripsController < ApplicationController
render 'trips/public_show'
end
def update
@trip = current_user.trips.find(params[:id])
return head :not_found unless @trip
if params[:enabled] == '1'
sharing_options = {
expiration: params[:expiration] || '24h'
}
# Add optional sharing settings
sharing_options[:share_notes] = params[:share_notes] == '1'
sharing_options[:share_photos] = params[:share_photos] == '1'
@trip.enable_sharing!(**sharing_options)
sharing_url = shared_trip_url(@trip.sharing_uuid)
render json: {
success: true,
sharing_url: sharing_url,
message: 'Sharing enabled successfully'
}
else
@trip.disable_sharing!
render json: {
success: true,
message: 'Sharing disabled successfully'
}
end
rescue StandardError => e
render json: {
success: false,
message: 'Failed to update sharing settings',
error: e.message
}, status: :unprocessable_content
end
private
def extract_coordinates

View file

@ -39,6 +39,12 @@ class TripsController < ApplicationController
end
def update
# Handle sharing settings update (JSON response)
if params[:sharing]
return update_sharing
end
# Handle regular trip update
if @trip.update(trip_params)
redirect_to @trip, notice: 'Trip was successfully updated.', status: :see_other
else
@ -64,6 +70,40 @@ class TripsController < ApplicationController
).map { [_1.to_f, _2.to_f, _3.to_s, _4.to_s, _5.to_s, _6.to_s, _7.to_s, _8.to_s] }
end
def update_sharing
if params[:sharing][:enabled] == '1'
sharing_options = {
expiration: params[:sharing][:expiration] || '24h'
}
# Add optional sharing settings
sharing_options[:share_notes] = params[:sharing][:share_notes] == '1'
sharing_options[:share_photos] = params[:sharing][:share_photos] == '1'
@trip.enable_sharing!(**sharing_options)
sharing_url = shared_trip_url(@trip.sharing_uuid)
render json: {
success: true,
sharing_url: sharing_url,
message: 'Sharing enabled successfully'
}
else
@trip.disable_sharing!
render json: {
success: true,
message: 'Sharing disabled successfully'
}
end
rescue StandardError => e
render json: {
success: false,
message: 'Failed to update sharing settings',
error: e.message
}, status: :unprocessable_content
end
def trip_params
params.require(:trip).permit(:name, :started_at, :ended_at, :notes)
end

View file

@ -140,17 +140,19 @@ function enableTripSharing(tripId) {
const shareNotes = document.getElementById(`share-notes-${tripId}`).checked ? '1' : '0';
const sharePhotos = document.getElementById(`share-photos-${tripId}`).checked ? '1' : '0';
fetch(`/trips/${tripId}/sharing`, {
fetch(`/trips/${tripId}`, {
method: 'PATCH',
headers: {
'Content-Type': 'application/json',
'X-CSRF-Token': document.querySelector('meta[name="csrf-token"]').content
},
body: JSON.stringify({
enabled: '1',
expiration: expiration,
share_notes: shareNotes,
share_photos: sharePhotos
sharing: {
enabled: '1',
expiration: expiration,
share_notes: shareNotes,
share_photos: sharePhotos
}
})
})
.then(response => response.json())
@ -171,14 +173,16 @@ function disableTripSharing(tripId) {
return;
}
fetch(`/trips/${tripId}/sharing`, {
fetch(`/trips/${tripId}`, {
method: 'PATCH',
headers: {
'Content-Type': 'application/json',
'X-CSRF-Token': document.querySelector('meta[name="csrf-token"]').content
},
body: JSON.stringify({
enabled: '0'
sharing: {
enabled: '0'
}
})
})
.then(response => response.json())

View file

@ -92,14 +92,11 @@ Rails.application.routes.draw do
get 'shared/month/:uuid', to: 'shared/stats#show', as: :shared_stat
get 'shared/trips/:trip_uuid', to: 'shared/trips#show', as: :shared_trip
# Sharing management endpoints (require auth)
# Sharing management endpoint for stats (require auth)
patch 'stats/:year/:month/sharing',
to: 'shared/stats#update',
as: :sharing_stats,
constraints: { year: /\d{4}/, month: /\d{1,2}/ }
patch 'trips/:id/sharing',
to: 'shared/trips#update',
as: :sharing_trip
root to: 'home#index'

View file

@ -112,14 +112,14 @@ RSpec.describe 'Shared::Trips', type: :request do
end
end
describe 'PATCH /trips/:id/sharing' do
describe 'PATCH /trips/:id (sharing update)' do
context 'when user is signed in' do
before { sign_in user }
context 'enabling sharing' do
it 'enables sharing and returns success' do
patch sharing_trip_path(trip),
params: { enabled: '1', expiration: '24h' },
patch trip_path(trip),
params: { sharing: { enabled: '1', expiration: '24h' } },
as: :json
expect(response).to have_http_status(:success)
@ -135,8 +135,8 @@ RSpec.describe 'Shared::Trips', type: :request do
end
it 'enables sharing with notes option' do
patch sharing_trip_path(trip),
params: { enabled: '1', expiration: '24h', share_notes: '1' },
patch trip_path(trip),
params: { sharing: { enabled: '1', expiration: '24h', share_notes: '1' } },
as: :json
expect(response).to have_http_status(:success)
@ -147,8 +147,8 @@ RSpec.describe 'Shared::Trips', type: :request do
end
it 'enables sharing with photos option' do
patch sharing_trip_path(trip),
params: { enabled: '1', expiration: '24h', share_photos: '1' },
patch trip_path(trip),
params: { sharing: { enabled: '1', expiration: '24h', share_photos: '1' } },
as: :json
expect(response).to have_http_status(:success)
@ -159,8 +159,8 @@ RSpec.describe 'Shared::Trips', type: :request do
end
it 'sets custom expiration when provided' do
patch sharing_trip_path(trip),
params: { enabled: '1', expiration: '1h' },
patch trip_path(trip),
params: { sharing: { enabled: '1', expiration: '1h' } },
as: :json
expect(response).to have_http_status(:success)
@ -170,8 +170,8 @@ RSpec.describe 'Shared::Trips', type: :request do
end
it 'enables permanent sharing' do
patch sharing_trip_path(trip),
params: { enabled: '1', expiration: 'permanent' },
patch trip_path(trip),
params: { sharing: { enabled: '1', expiration: 'permanent' } },
as: :json
expect(response).to have_http_status(:success)
@ -187,8 +187,8 @@ RSpec.describe 'Shared::Trips', type: :request do
end
it 'disables sharing and returns success' do
patch sharing_trip_path(trip),
params: { enabled: '0' },
patch trip_path(trip),
params: { sharing: { enabled: '0' } },
as: :json
expect(response).to have_http_status(:success)
@ -204,8 +204,8 @@ RSpec.describe 'Shared::Trips', type: :request do
context 'when trip does not exist' do
it 'returns not found' do
patch sharing_trip_path(id: 999999),
params: { enabled: '1' },
patch trip_path(id: 999999),
params: { sharing: { enabled: '1' } },
as: :json
expect(response).to have_http_status(:not_found)
@ -217,8 +217,8 @@ RSpec.describe 'Shared::Trips', type: :request do
let(:other_trip) { create(:trip, user: other_user, name: 'Other Trip') }
it 'returns not found' do
patch sharing_trip_path(other_trip),
params: { enabled: '1' },
patch trip_path(other_trip),
params: { sharing: { enabled: '1' } },
as: :json
expect(response).to have_http_status(:not_found)
@ -228,8 +228,8 @@ RSpec.describe 'Shared::Trips', type: :request do
context 'when user is not signed in' do
it 'returns unauthorized' do
patch sharing_trip_path(trip),
params: { enabled: '1' },
patch trip_path(trip),
params: { sharing: { enabled: '1' } },
as: :json
expect(response).to have_http_status(:unauthorized)