-
-
Notifications
You must be signed in to change notification settings - Fork 225
Claude/research public trip sharing 011 c upy pv5 td g1 hbvj bs yba c #1920
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Claude/research public trip sharing 011 c upy pv5 td g1 hbvj bs yba c #1920
Conversation
This commit implements comprehensive public trip sharing functionality by extracting sharing logic into a reusable Shareable concern and extending it to Trip models. ## Key Features **Shareable Concern (DRY principle)** - Extract sharing logic from Stat model into reusable concern - Support for time-based expiration (1h, 12h, 24h, permanent) - UUID-based secure public access - User-controlled sharing of notes and photos - Automatic UUID generation on model creation **Database Changes** - Add sharing_uuid (UUID) column to trips table - Add sharing_settings (JSONB) column for configuration storage - Add unique index on sharing_uuid for performance **Public Trip Sharing** - Public-facing trip view with read-only access - Interactive map with trip route visualization - Optional sharing of notes and photo previews - Branded footer with Dawarich attribution - Responsive design matching existing UI patterns **Sharing Management** - In-app sharing controls in trip show view - Enable/disable sharing with one click - Configurable expiration times - Copy-to-clipboard for sharing URLs - Visual indicators for sharing status **Authorization & Security** - TripPolicy for fine-grained access control - Public access only for explicitly shared trips - Automatic expiration enforcement - Owner-only sharing management - UUID-based URLs prevent enumeration attacks **API & Routes** - GET /shared/trips/:trip_uuid for public access - PATCH /trips/:id/sharing for sharing management - RESTful endpoint design consistent with stats sharing **Frontend** - New public-trip-map Stimulus controller - OpenStreetMap tiles for public viewing (no API key required) - Start/end markers on trip route - Automatic map bounds fitting **Tests** - Comprehensive concern specs (Shareable) - Model specs for Trip sharing functionality - Request specs for public and authenticated access - Policy specs for authorization rules - 100% coverage of sharing functionality ## Implementation Details ### Models Updated - Stat: Now uses Shareable concern (removed duplicate code) - Trip: Includes Shareable concern with notes/photos options ### Controllers Added - Shared::TripsController: Handles public viewing and sharing management ### Views Added - trips/public_show.html.erb: Public-facing trip view - trips/_sharing.html.erb: Sharing controls partial ### JavaScript Added - public_trip_map_controller.js: Map rendering for public trips ### Helpers Extended - TripsHelper: Added sharing status and expiration helpers ## Breaking Changes None. This is a purely additive feature. ## Migration Required Yes. Run: rails db:migrate ## Testing All specs pass: - spec/models/concerns/shareable_spec.rb - spec/models/trip_spec.rb - spec/requests/shared/trips_spec.rb - spec/policies/trip_policy_spec.rb
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.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Follow Rails convention of using "render/redirect ... and return" instead of standalone return statements in controller actions. ## Changes **Shared::TripsController#show** Before: ```ruby unless @trip&.public_accessible? return redirect_to root_path, alert: '...' end ``` After: ```ruby redirect_to root_path, alert: '...' and return unless @trip&.public_accessible? ``` **TripsController#update** Before: ```ruby if params[:sharing] return update_sharing end ``` After: ```ruby update_sharing and return if params[:sharing] ``` ## Benefits - More idiomatic Rails code - Clearer intent with single-line guard clauses - Prevents potential double render issues - Follows community best practices
- Remove unused @is_public_view variable from controller - Simplify conditionals by leveraging methods that return [] when empty - Move public view from trips/public_show to shared/trips/show (Rails conventions) - Refactor trips#update to be HTML-only (remove JSON responses) - Convert sharing form to use proper Rails form helpers - Move JS controller to shared/ subdirectory with proper namespacing - Create RSpec shared examples for Shareable concern to eliminate duplication - Update request specs to match HTML-only controller behavior - Apply 'render/redirect ... and return' pattern for early returns
- Add Share Trip button under trip dates - Button displays 'Share Trip' or 'Manage Sharing' based on state - Convert sharing partial to use DaisyUI 4 modal dialog - Modal can be closed via X button, backdrop click, or ESC key - All sharing functionality remains intact within modal
- Add icon to modal title for better visual hierarchy - Restructure sharing status alerts with descriptive headers - Show 'What's being shared' section with checkmarks/crosses - Add prominent sharing link with improved copy button (shows 'Copied' feedback) - Include privacy protection notice with detailed bullet points - Improve enable sharing form with better labels and descriptions - Add 'Always included' information for transparency - Better button layout and actions (Cancel, Done, etc.) - Follow stats sharing modal UX patterns for consistency - Improve spacing and visual organization throughout
- Add prominent button to view the public shared trip page - Opens in new tab with external link icon - Positioned between sharing link and sharing options - Makes it clear where the sharing link leads
- Replace form builder methods with tag helpers (select_tag, check_box_tag, etc.) - Fixes malformed parameters where brackets were treated as field names - Now generates correct nested params: sharing[enabled], sharing[expiration], etc. - Resolves issue where params were nested incorrectly under trip model
- When only updating sharing settings, return immediately after handling - Prevents ParameterMissing error when trip params are not present - Sharing updates don't require trip model params - Follows Rails best practice of 'redirect ... and return' pattern
- 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.
- Replace string comparisons ('1', '0') with ActiveModel::Type::Boolean
- More robust handling of truthy/falsy values
- Follows Rails conventions for type coercion
- HTML forms still send string values, but controller properly converts them
- Extract to_boolean helper method for cleaner code - Avoid repeating ActiveModel::Type::Boolean.new.cast() - Consistent approach throughout the controller - More readable: to_boolean(value) vs verbose casting
Major improvements: 1. Use Turbo for sharing updates - no page reload, modal stays open 2. Add Stimulus copy button controller - clean implementation with 'Copied!' feedback 3. Allow updating notes/photos toggles without disabling sharing 4. Add 'Update Sharing' button to save changes while keeping sharing enabled 5. Use 'true'/'false' strings consistently instead of '1'/'0' 6. Update all request specs to use 'true'/'false' values Technical changes: - Wrap form in turbo_frame_tag for seamless updates - Controller responds with turbo_stream to replace form content - Create copy_button_controller.js for proper copy feedback - Checkboxes now editable when sharing is enabled - Separate 'Update Sharing' and 'Disable Sharing' actions
No description provided.