Refactor tab controller and improve localization for RSVP management

Author: rsmithlal

app/controllers/better_together/agreements_controller.rb

@@ -9,7 +9,7 @@ class AgreementsController < FriendlyResourceController
     # return only the fragment wrapped in the expected <turbo-frame id="agreement_modal_frame">...</turbo-frame>
     # so Turbo can swap it into the frame. For normal requests, fall back to the
     # default rendering (with layout).
-    def show
+    def show # rubocop:todo Metrics/MethodLength
       if @agreement.page
         @page = @agreement.page
         @content_blocks = @page.content_blocks
@@ -19,10 +19,10 @@ def show
 
       # Check if this is a Turbo Frame request
       if request.headers['Turbo-Frame'].present?
-        Rails.logger.debug "Rendering turbo frame response"
+        Rails.logger.debug 'Rendering turbo frame response'
         render partial: 'modal_content', layout: false
       else
-        Rails.logger.debug "Rendering normal response"
+        Rails.logger.debug 'Rendering normal response'
         # Normal full-page rendering continues with the view
       end
     end

app/controllers/better_together/events_controller.rb

@@ -2,7 +2,7 @@
 
 module BetterTogether
   # CRUD for BetterTogether::Event
-  class EventsController < FriendlyResourceController
+  class EventsController < FriendlyResourceController # rubocop:todo Metrics/ClassLength
     before_action if: -> { Rails.env.development? } do
       # Make sure that all subclasses are loaded in dev to generate type selector
       Rails.application.eager_load!
@@ -46,14 +46,14 @@ def rsvp_going
     def rsvp_cancel
       @event = set_resource_instance
       authorize @event, :show?
-      
+
       # Ensure current_person exists
       current_person = helpers.current_person
       unless current_person
         redirect_to @event, alert: t('better_together.events.login_required', default: 'Please log in to manage RSVPs.')
         return
       end
-      
+
       attendance = BetterTogether::EventAttendance.find_by(event: @event, person: current_person)
       attendance&.destroy
       redirect_to @event, notice: t('better_together.events.rsvp_cancelled', default: 'RSVP cancelled')
@@ -124,7 +124,7 @@ def rsvp_update(status) # rubocop:todo Metrics/AbcSize, Metrics/MethodLength
     # Override base controller method to add performance optimizations
     def set_resource_instance
       super
-      
+
       # Preload associations needed for event show page to avoid N+1 queries
       preload_event_associations! unless json_request?
     end
@@ -133,34 +133,40 @@ def json_request?
       request.format.json?
     end
 
-    def preload_event_associations!
+    # rubocop:todo Metrics/AbcSize
+    # rubocop:todo Metrics/MethodLength
+    def preload_event_associations! # rubocop:todo Metrics/CyclomaticComplexity, Metrics/AbcSize
       return unless @event
 
       # Preload categories and their translations to avoid N+1 queries
       @event.categories.includes(:string_translations).load
-      
+
       # Preload event hosts and their associated models
       @event.event_hosts.includes(:host).load
-      
+
       # Preload event attendances to avoid count queries in view
       @event.event_attendances.includes(:person).load
-      
+
       # Preload current person's attendance for RSVP buttons
       if current_person
-        @current_attendance = @event.event_attendances.find { |a| a.person_id == current_person.id }
+        @current_attendance = @event.event_attendances.find do |a|
+          a.person_id == current_person.id
+        end
       end
-      
+
       # Preload translations for the event itself
       @event.string_translations.load
       @event.text_translations.load
-      
+
       # Preload cover image attachment to avoid attachment queries
       @event.cover_image_attachment&.blob&.load if @event.cover_image.attached?
-      
+
       # Preload location if present
-      @event.location&.reload if @event.location
-      
+      @event.location&.reload
+
       self
     end
+    # rubocop:enable Metrics/MethodLength
+    # rubocop:enable Metrics/AbcSize
   end
 end

app/controllers/better_together/people_controller.rb

@@ -17,7 +17,7 @@ def show
                           :string_translations,
                           blocks: { background_image_file_attachment: :blob }
                         )
-      
+
       # Preload calendar associations to avoid N+1 queries
       @person.preload_calendar_associations!
     end

app/helpers/better_together/application_helper.rb

@@ -206,18 +206,22 @@ def better_together_url_helper?(method)
     end
 
     # Returns the appropriate icon and color for an event based on the person's relationship to it
-    def event_relationship_icon(person, event)
+    def event_relationship_icon(person, event) # rubocop:todo Metrics/MethodLength
       relationship = person.event_relationship_for(event)
-      
+
       case relationship
       when :created
-        { icon: 'fas fa-user-edit', color: '#28a745', tooltip: t('better_together.events.relationship.created', default: 'Created by you') }
+        { icon: 'fas fa-user-edit', color: '#28a745',
+          tooltip: t('better_together.events.relationship.created', default: 'Created by you') }
       when :going
-        { icon: 'fas fa-check-circle', color: '#007bff', tooltip: t('better_together.events.relationship.going', default: 'You\'re going') }
+        { icon: 'fas fa-check-circle', color: '#007bff',
+          tooltip: t('better_together.events.relationship.going', default: 'You\'re going') }
       when :interested
-        { icon: 'fas fa-heart', color: '#e91e63', tooltip: t('better_together.events.relationship.interested', default: 'You\'re interested') }
+        { icon: 'fas fa-heart', color: '#e91e63',
+          tooltip: t('better_together.events.relationship.interested', default: 'You\'re interested') }
       else
-        { icon: 'fas fa-circle', color: '#6c757d', tooltip: t('better_together.events.relationship.calendar', default: 'Calendar event') }
+        { icon: 'fas fa-circle', color: '#6c757d',
+          tooltip: t('better_together.events.relationship.calendar', default: 'Calendar event') }
       end
     end
   end

app/javascript/controllers/better_together/tabs_controller.js

@@ -6,56 +6,45 @@ export default class extends Controller {
   static targets = ["tab"];
 
   connect() {
+    // Get all available tabs (both via Stimulus targets and manual selection)
+    this.allTabs = this.tabTargets.length > 0 ? this.tabTargets : 
+                   Array.from(this.element.querySelectorAll('[data-bs-toggle="tab"]'));
+    
     this.activateTabFromHash();
     this.setupTabChangeListener();
   }
 
   activateTabFromHash() {
     const hash = window.location.hash;
     if (hash) {
-      let selectedTab = this.element.querySelector(`[data-bs-target="${hash}"]`);
-      while (selectedTab) {
-        // Skip tabs inside the localized-fields class
-        if (selectedTab.closest('.localized-fields')) break;
-
-        const tabTarget = this.element.querySelector(`${selectedTab.dataset.bsTarget}`);
-        const tabPanes = this.element.querySelectorAll('.nav-tab-pane');
-
-        this.tabTargets.forEach((tab) => {
-          tab.classList.remove('active');
-        });
-        selectedTab.classList.add('active');
-
-        tabPanes.forEach((pane) => {
-          pane.classList.remove('active');
-          pane.classList.remove('show');
-        });
-
-        if (tabTarget) {
-          tabTarget.classList.add('active');
-          tabTarget.classList.add('show');
-        }
-
-        // Check if the selected tab is nested and activate its parent tab
-        const parentTabPane = selectedTab.closest('.nav-tab-pane');
-        if (parentTabPane) {
-          const parentTab = this.element.querySelector(`[data-bs-target="#${parentTabPane.id}"]`);
-          selectedTab = parentTab; // Move up to the parent tab for the next iteration
-        } else {
-          selectedTab = null; // Exit the loop if no parent tab exists
-        }
+      // Look for tabs that target this hash with either href or data-bs-target
+      let selectedTab = this.element.querySelector(`[href="${hash}"]`) || 
+                       this.element.querySelector(`[data-bs-target="${hash}"]`);
+      
+      if (selectedTab && !selectedTab.closest('.localized-fields')) {
+        console.log('Activating tab from hash:', hash, selectedTab);
+        
+        // Let Bootstrap handle the tab activation
+        const tabInstance = new bootstrap.Tab(selectedTab);
+        tabInstance.show();
       }
     }
   }
 
   setupTabChangeListener() {
-    this.tabTargets.forEach((link) => {
+    // Use the unified collection of all available tabs
+    const tabsToSetup = this.allTabs || [];
+    
+    tabsToSetup.forEach((link) => {
       if (link.closest('.localized-fields')) return;
 
-      link.addEventListener("shown.bs.tab", (event) => {
-        const targetHash = event.target.getAttribute("data-bs-target");
-        if (targetHash) {
-          history.pushState({}, "", targetHash); // Add the hash to the address bar
+      // Primary: Listen for click events
+      link.addEventListener("click", (event) => {
+        const targetHash = event.target.getAttribute("href") || event.target.getAttribute("data-bs-target");
+        
+        if (targetHash && targetHash.startsWith('#')) {
+          // Update immediately - no delay needed
+          history.pushState({}, "", targetHash);
         }
       });
     });

app/models/better_together/person.rb

@@ -70,7 +70,6 @@ def self.primary_community_delegation_attrs
            joinable_type: 'platform'
 
     slugged :identifier, use: %i[slugged mobility], dependent: :delete_all
-
     store_attributes :preferences do
       locale String, default: I18n.default_locale.to_s
       time_zone String, default: ENV.fetch('APP_TIME_ZONE', 'Newfoundland')
@@ -164,23 +163,23 @@ def after_record_created
 
     # Returns all events relevant to this person's calendar view
     # Combines events they're going to, created, and interested in
-    def all_calendar_events
+    def all_calendar_events # rubocop:todo Metrics/AbcSize, Metrics/MethodLength
       @all_calendar_events ||= begin
         # Build a single query to get all relevant events with proper includes
         event_ids = Set.new
-        
+
         # Get event IDs from calendar entries (going events)
         calendar_event_ids = primary_calendar.calendar_entries.pluck(:event_id)
         event_ids.merge(calendar_event_ids)
-        
+
         # Get event IDs from attendances (interested events)
         attendance_event_ids = event_attendances.pluck(:event_id)
         event_ids.merge(attendance_event_ids)
-        
+
         # Get event IDs from created events
         created_event_ids = Event.where(creator_id: id).pluck(:id)
         event_ids.merge(created_event_ids)
-        
+
         # Single query to fetch all events with necessary includes
         if event_ids.any?
           Event.includes(:string_translations, :text_translations)
@@ -197,33 +196,33 @@ def all_calendar_events
     def event_relationship_for(event)
       # Check if they created it first (highest priority)
       return :created if event.creator_id == id
-      
+
       # Use memoized attendances to avoid N+1 queries
       @_event_attendances_by_event_id ||= event_attendances.index_by(&:event_id)
       attendance = @_event_attendances_by_event_id[event.id]
-      
+
       return attendance.status.to_sym if attendance
-      
+
       # Check if it's in their calendar (for events added directly to calendar)
       @_calendar_event_ids ||= Set.new(primary_calendar.calendar_entries.pluck(:event_id))
       return :going if @_calendar_event_ids.include?(event.id)
-      
+
       :calendar # Default for calendar events
     end
 
     # Preloads associations needed for calendar display to avoid N+1 queries
     def preload_calendar_associations!
       # Preload event attendances
       event_attendances.includes(:event).load
-      
+
       # Preload calendar entries
       primary_calendar.calendar_entries.includes(:event).load
-      
+
       # Reset memoized variables
       @all_calendar_events = nil
       @_event_attendances_by_event_id = nil
       @_calendar_event_ids = nil
-      
+
       self
     end