From 53ec557ec92b57e295dbffdcb76f8c0daab2cc76 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Fri, 7 Nov 2025 12:50:39 +0000
Subject: [PATCH] Refactor: Nest sharing parameters under trip key
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Follow Rails conventions by nesting sharing params under resource key
- Update form fields: sharing[enabled] → trip[sharing][enabled]
- Update controller to access params[:trip][:sharing]
- Update all request specs to use nested parameters
- Parameters now properly structured as trip[sharing][expiration], etc.
---
 app/controllers/trips_controller.rb | 12 +++++++-----
 app/views/trips/_sharing.html.erb   | 10 +++++-----
 spec/requests/shared/trips_spec.rb  | 18 +++++++++---------
 3 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/app/controllers/trips_controller.rb b/app/controllers/trips_controller.rb
index abb9e907..65f2c4d7 100644
--- a/app/controllers/trips_controller.rb
+++ b/app/controllers/trips_controller.rb
@@ -40,7 +40,7 @@ class TripsController < ApplicationController
 
   def update
     # Handle sharing settings update
-    if params[:sharing]
+    if params[:trip] && params[:trip][:sharing]
       handle_sharing_update
       redirect_to @trip, notice: 'Trip was successfully updated.', status: :see_other and return
     end
@@ -72,11 +72,13 @@ class TripsController < ApplicationController
   end
 
   def handle_sharing_update
-    if params[:sharing][:enabled] == '1'
+    sharing_params = params[:trip][:sharing]
+
+    if sharing_params[:enabled] == '1'
       sharing_options = {
-        expiration: params[:sharing][:expiration] || '24h',
-        share_notes: params[:sharing][:share_notes] == '1',
-        share_photos: params[:sharing][:share_photos] == '1'
+        expiration: sharing_params[:expiration] || '24h',
+        share_notes: sharing_params[:share_notes] == '1',
+        share_photos: sharing_params[:share_photos] == '1'
       }
 
       @trip.enable_sharing!(**sharing_options)
diff --git a/app/views/trips/_sharing.html.erb b/app/views/trips/_sharing.html.erb
index 0f0acd47..51f1952e 100644
--- a/app/views/trips/_sharing.html.erb
+++ b/app/views/trips/_sharing.html.erb
@@ -116,7 +116,7 @@
           <!-- Action Buttons -->
           <div class="modal-action">
             <%= submit_tag "Disable Sharing",
-                           name: 'sharing[enabled]',
+                           name: 'trip[sharing][enabled]',
                            value: '0',
                            class: "btn btn-error",
                            data: { turbo_confirm: "Are you sure you want to disable sharing for this trip?" } %>
@@ -141,7 +141,7 @@
                 <label class="label">
                   <span class="label-text font-medium">Link expiration</span>
                 </label>
-                <%= select_tag 'sharing[expiration]',
+                <%= select_tag 'trip[sharing][expiration]',
                                options_for_select([
                                  ['1 hour', '1h'],
                                  ['12 hours', '12h'],
@@ -159,7 +159,7 @@
               <div class="grid grid-cols-1 md:grid-cols-2 gap-4">
                 <div class="form-control">
                   <label class="label cursor-pointer justify-start gap-3">
-                    <%= check_box_tag 'sharing[share_notes]', '1', true, class: 'toggle toggle-primary' %>
+                    <%= check_box_tag 'trip[sharing][share_notes]', '1', true, class: 'toggle toggle-primary' %>
                     <div>
                       <span class="label-text font-medium block">Share trip notes</span>
                       <span class="label-text-alt text-xs">Include your description and notes</span>
@@ -168,7 +168,7 @@
                 </div>
                 <div class="form-control">
                   <label class="label cursor-pointer justify-start gap-3">
-                    <%= check_box_tag 'sharing[share_photos]', '1', true, class: 'toggle toggle-primary' %>
+                    <%= check_box_tag 'trip[sharing][share_photos]', '1', true, class: 'toggle toggle-primary' %>
                     <div>
                       <span class="label-text font-medium block">Share photos</span>
                       <span class="label-text-alt text-xs">Include photos from this trip</span>
@@ -197,7 +197,7 @@
           </div>
 
           <!-- Action Buttons -->
-          <%= hidden_field_tag 'sharing[enabled]', '1' %>
+          <%= hidden_field_tag 'trip[sharing][enabled]', '1' %>
           <div class="modal-action">
             <button type="button" class="btn btn-ghost" onclick="sharing_modal_<%= trip.id %>.close()">
               Cancel
diff --git a/spec/requests/shared/trips_spec.rb b/spec/requests/shared/trips_spec.rb
index a58ed3dd..cf5eed8f 100644
--- a/spec/requests/shared/trips_spec.rb
+++ b/spec/requests/shared/trips_spec.rb
@@ -119,7 +119,7 @@ RSpec.describe 'Shared::Trips', type: :request do
         context 'enabling sharing' do
           it 'enables sharing and redirects to trip' do
             patch trip_path(trip),
-                  params: { sharing: { enabled: '1', expiration: '24h' } }
+                  params: { trip: { sharing: { enabled: '1', expiration: '24h' } } }
 
             expect(response).to redirect_to(trip_path(trip))
             expect(flash[:notice]).to eq('Trip was successfully updated.')
@@ -131,7 +131,7 @@ RSpec.describe 'Shared::Trips', type: :request do
 
           it 'enables sharing with notes option' do
             patch trip_path(trip),
-                  params: { sharing: { enabled: '1', expiration: '24h', share_notes: '1' } }
+                  params: { trip: { sharing: { enabled: '1', expiration: '24h', share_notes: '1' } } }
 
             expect(response).to redirect_to(trip_path(trip))
 
@@ -142,7 +142,7 @@ RSpec.describe 'Shared::Trips', type: :request do
 
           it 'enables sharing with photos option' do
             patch trip_path(trip),
-                  params: { sharing: { enabled: '1', expiration: '24h', share_photos: '1' } }
+                  params: { trip: { sharing: { enabled: '1', expiration: '24h', share_photos: '1' } } }
 
             expect(response).to redirect_to(trip_path(trip))
 
@@ -153,7 +153,7 @@ RSpec.describe 'Shared::Trips', type: :request do
 
           it 'sets custom expiration when provided' do
             patch trip_path(trip),
-                  params: { sharing: { enabled: '1', expiration: '1h' } }
+                  params: { trip: { sharing: { enabled: '1', expiration: '1h' } } }
 
             expect(response).to redirect_to(trip_path(trip))
             trip.reload
@@ -163,7 +163,7 @@ RSpec.describe 'Shared::Trips', type: :request do
 
           it 'enables permanent sharing' do
             patch trip_path(trip),
-                  params: { sharing: { enabled: '1', expiration: 'permanent' } }
+                  params: { trip: { sharing: { enabled: '1', expiration: 'permanent' } } }
 
             expect(response).to redirect_to(trip_path(trip))
             trip.reload
@@ -179,7 +179,7 @@ RSpec.describe 'Shared::Trips', type: :request do
 
           it 'disables sharing and redirects to trip' do
             patch trip_path(trip),
-                  params: { sharing: { enabled: '0' } }
+                  params: { trip: { sharing: { enabled: '0' } } }
 
             expect(response).to redirect_to(trip_path(trip))
             expect(flash[:notice]).to eq('Trip was successfully updated.')
@@ -192,7 +192,7 @@ RSpec.describe 'Shared::Trips', type: :request do
         context 'when trip does not exist' do
           it 'returns not found' do
             patch trip_path(id: 999999),
-                  params: { sharing: { enabled: '1' } }
+                  params: { trip: { sharing: { enabled: '1' } } }
 
             expect(response).to have_http_status(:not_found)
           end
@@ -204,7 +204,7 @@ RSpec.describe 'Shared::Trips', type: :request do
 
           it 'returns not found' do
             patch trip_path(other_trip),
-                  params: { sharing: { enabled: '1' } }
+                  params: { trip: { sharing: { enabled: '1' } } }
 
             expect(response).to have_http_status(:not_found)
           end
@@ -214,7 +214,7 @@ RSpec.describe 'Shared::Trips', type: :request do
       context 'when user is not signed in' do
         it 'returns unauthorized' do
           patch trip_path(trip),
-                params: { sharing: { enabled: '1' } }
+                params: { trip: { sharing: { enabled: '1' } } }
 
           expect(response).to have_http_status(:unauthorized)
         end