Fix tag layers restoration and filtering logic

app/controllers/api/v1/places_controller.rb

@@ -65,7 +65,7 @@ module Api
       private
 
       def set_place
-        @place = current_api_user.places.find(params[:id])
+        @place = current_api_user.places.includes(:tags, :visits).find(params[:id])
       end
 
       def place_params

app/javascript/controllers/maps_controller.js

@@ -636,6 +636,21 @@ export default class extends BaseController {
 
     // Add event listeners for overlay layer changes to keep routes/tracks selector in sync
     this.map.on('overlayadd', (event) => {
+      // Track place tag layer restoration
+      if (this.isRestoringLayers && event.layer && this.placesFilteredLayers) {
+        // Check if this is a place tag layer being restored
+        const isPlaceTagLayer = Object.values(this.placesFilteredLayers).includes(event.layer);
+        if (isPlaceTagLayer && this.restoredPlaceTagLayers !== undefined) {
+          const tagId = event.layer._placeTagId;
+          this.restoredPlaceTagLayers.add(tagId);
+
+          // Check if all expected place tag layers have been restored
+          if (this.restoredPlaceTagLayers.size >= this.expectedPlaceTagLayerCount) {
+            this.isRestoringLayers = false;
+          }
+        }
+      }
+
       // Save enabled layers whenever a layer is added (unless we're restoring from settings)
       if (!this.isRestoringLayers) {
         this.saveEnabledLayers();
@@ -1784,42 +1799,30 @@ export default class extends BaseController {
       }
     });
 
-    // Handle place tag layers (format: "place_tag:ID" or "place_tag:untagged")
+    // Place tag layers will be restored by updateTreeControlCheckboxes
-    const placeTagLayers = enabledLayers.filter(key => key.startsWith('place_tag:'));
+    // which triggers the tree control's change events to properly add/remove layers
-    if (placeTagLayers.length > 0) {
-      // Set flag once before restoring all place tag layers
-      this.isRestoringLayers = true;
 
-      placeTagLayers.forEach(layerKey => {
+    // Track expected place tag layers to be restored
-        const tagId = layerKey.replace('place_tag:', '');
+    const expectedPlaceTagLayers = enabledLayers.filter(key => key.startsWith('place_tag:'));
-        let layer;
+    this.restoredPlaceTagLayers = new Set();
+    this.expectedPlaceTagLayerCount = expectedPlaceTagLayers.length;
 
-        if (tagId === 'untagged') {
+    // Set flag to prevent saving during layer restoration
-          // Find untagged layer
+    this.isRestoringLayers = true;
-          layer = Object.values(this.placesFilteredLayers || {}).find(l => l._placeTagId === 'untagged');
-        } else {
-          // Find layer by tag ID
-          const tagIdNum = parseInt(tagId);
-          layer = Object.values(this.placesFilteredLayers || {}).find(l => l._placeTagId === tagIdNum);
-        }
-
-        if (layer && !this.map.hasLayer(layer)) {
-          layer.addTo(this.map);
-          console.log(`Enabled place tag layer: ${tagId}`);
-        }
-      });
-
-      // Reset flag after all layers have been added and events processed
-      setTimeout(() => {
-        this.isRestoringLayers = false;
-        console.log('[initializeLayersFromSettings] Finished restoring place tag layers');
-      }, 150);
-    }
 
     // Update the tree control checkboxes to reflect the layer states
+    // The tree control will handle adding/removing layers when checkboxes change
     // Wait a bit for the tree control to be fully initialized
     setTimeout(() => {
       this.updateTreeControlCheckboxes(enabledLayers);
+
+      // Set a fallback timeout in case not all layers get added
+      setTimeout(() => {
+        if (this.isRestoringLayers) {
+          console.warn('[initializeLayersFromSettings] Timeout reached, forcing restoration complete');
+          this.isRestoringLayers = false;
+        }
+      }, 2000);
     }, 200);
   }
 
@@ -1849,19 +1852,30 @@ export default class extends BaseController {
         // Check if this is a standard layer
         let shouldBeEnabled = enabledLayers.includes(layerName);
 
-        // Check if this is a place tag layer by finding the layer object
+        // Also check if this is a place tag layer
-        if (!shouldBeEnabled && this.placesFilteredLayers) {
+        let placeLayer = null;
-          const placeLayer = this.placesFilteredLayers[layerName];
+        if (this.placesFilteredLayers) {
+          placeLayer = this.placesFilteredLayers[layerName];
           if (placeLayer && placeLayer._placeTagId !== undefined) {
-            shouldBeEnabled = enabledTagIds.has(placeLayer._placeTagId);
+            // This is a place tag layer - check if it should be enabled
+            const placeLayerEnabled = enabledTagIds.has(placeLayer._placeTagId);
+            if (placeLayerEnabled) {
+              shouldBeEnabled = true;
+            }
           }
         }
 
         // Skip group headers that might have checkboxes
         if (layerName && !layerName.includes('Map Styles') && !layerName.includes('Layers')) {
           if (shouldBeEnabled !== input.checked) {
-            input.checked = shouldBeEnabled;
+            // Checkbox state needs to change - simulate a click to trigger tree control
-            console.log(`Updated checkbox for ${layerName}: ${shouldBeEnabled}`);
+            // The tree control listens for click events, not change events
+            input.click();
+          } else if (shouldBeEnabled && placeLayer && !this.map.hasLayer(placeLayer)) {
+            // Checkbox is already checked but layer isn't on map (edge case)
+            // This can happen if the checkbox was checked in HTML but layer wasn't added
+            // Manually add the layer since clicking won't help (checkbox is already checked)
+            placeLayer.addTo(this.map);
           }
         }
       }

app/javascript/maps/places.js

@@ -25,7 +25,6 @@ export class PlacesManager {
       this.loadPlaces();
     });
 
-    console.log("[PlacesManager] Initializing, loading places for first time...");
     await this.loadPlaces();
     this.setupMapClickHandler();
     this.setupEventListeners();
@@ -55,14 +54,19 @@ export class PlacesManager {
     });
   }
 
-  async loadPlaces(tagIds = null) {
+  async loadPlaces(tagIds = null, untaggedOnly = false) {
     try {
       const url = new URL('/api/v1/places', window.location.origin);
-      if (tagIds && tagIds.length > 0) {
+
+      if (untaggedOnly) {
+        // Load only untagged places
+        url.searchParams.append('untagged', 'true');
+      } else if (tagIds && tagIds.length > 0) {
+        // Load places with specific tags
         tagIds.forEach(id => url.searchParams.append('tag_ids[]', id));
       }
+      // If neither untaggedOnly nor tagIds, load all places
 
-      console.log("[PlacesManager] loadPlaces called, fetching from:", url.toString());
       const response = await fetch(url, {
         headers: { 'Authorization': `Bearer ${this.apiKey}` }
       });
@@ -261,9 +265,9 @@ export class PlacesManager {
     }
   }
 
-  filterByTags(tagIds) {
+  filterByTags(tagIds, untaggedOnly = false) {
-    this.selectedTags = new Set(tagIds);
+    this.selectedTags = new Set(tagIds || []);
-    this.loadPlaces(tagIds.length > 0 ? tagIds : null);
+    this.loadPlaces(tagIds && tagIds.length > 0 ? tagIds : null, untaggedOnly);
   }
 
   /**
@@ -278,11 +282,9 @@ export class PlacesManager {
 
     // Add event listener to load places when layer is added to map
     filteredLayer.on('add', () => {
-      console.log(`[PlacesManager] Filtered layer added to map, tagIds:`, tagIds);
       this.loadPlacesIntoLayer(filteredLayer, tagIds);
     });
 
-    console.log(`[PlacesManager] Created filtered layer for tagIds:`, tagIds);
     return filteredLayer;
   }
 
@@ -291,21 +293,20 @@ export class PlacesManager {
    */
   async loadPlacesIntoLayer(layer, tagIds) {
     try {
-      console.log(`[PlacesManager] loadPlacesIntoLayer called with tagIds:`, tagIds);
+      const url = new URL('/api/v1/places', window.location.origin);
-      let url = `/api/v1/places?api_key=${this.apiKey}`;
 
       if (Array.isArray(tagIds) && tagIds.length > 0) {
         // Specific tags requested
-        url += `&tag_ids=${tagIds.join(',')}`;
+        tagIds.forEach(id => url.searchParams.append('tag_ids[]', id));
       } else if (Array.isArray(tagIds) && tagIds.length === 0) {
         // Empty array means untagged places only
-        url += '&untagged=true';
+        url.searchParams.append('untagged', 'true');
       }
 
-      console.log(`[PlacesManager] Fetching from URL:`, url);
+      const response = await fetch(url, {
-      const response = await fetch(url);
+        headers: { 'Authorization': `Bearer ${this.apiKey}` }
+      });
       const data = await response.json();
-      console.log(`[PlacesManager] Received ${data.length} places for tagIds:`, tagIds);
 
       // Clear existing markers in this layer
       layer.clearLayers();
@@ -315,8 +316,6 @@ export class PlacesManager {
         const marker = this.createPlaceMarker(place);
         layer.addLayer(marker);
       });
-
-      console.log(`[PlacesManager] Added ${data.length} markers to layer`);
     } catch (error) {
       console.error('Error loading places into layer:', error);
     }
@@ -330,14 +329,12 @@ export class PlacesManager {
   ensurePlacesLayerVisible() {
     // Check if the main places layer is already on the map
     if (this.map.hasLayer(this.placesLayer)) {
-      console.log('Places layer already visible');
       return;
     }
 
     // Try to find and enable the Places checkbox in the tree control
     const layerControl = document.querySelector('.leaflet-control-layers');
     if (!layerControl) {
-      console.log('Layer control not found, adding places layer directly');
       this.map.addLayer(this.placesLayer);
       return;
     }
@@ -356,7 +353,6 @@ export class PlacesManager {
 
             input.checked = true;
             input.dispatchEvent(new Event('change', { bubbles: true }));
-            console.log('Enabled Places layer in tree control');
 
             // Reset the flag after a short delay to allow the event to process
             setTimeout(() => {

app/javascript/maps/places_control.js

@@ -171,21 +171,18 @@ export function createPlacesControl(placesManager, tags, userTheme = 'dark') {
     applyCurrentFilters: function() {
       if (!this.placesEnabled) return;
 
-      // If no specific filters, show all places
+      // Build filter criteria
-      if (this.activeFilters.size === 0 && !this.showUntagged) {
+      const tagIds = Array.from(this.activeFilters);
-        this.placesManager.filterByTags(null);
-      } else {
-        // Build filter criteria
-        const tagIds = Array.from(this.activeFilters);
 
-        // For now, just filter by tags
+      if (this.showUntagged && tagIds.length === 0) {
-        // TODO: Add support for untagged filter in PlacesManager
+        // Show only untagged places
-        if (tagIds.length > 0) {
+        this.placesManager.filterByTags(null, true);
-          this.placesManager.filterByTags(tagIds);
+      } else if (tagIds.length > 0) {
-        } else if (this.showUntagged) {
+        // Show places with specific tags
-          // Show only untagged places
+        this.placesManager.filterByTags(tagIds, false);
-          this.placesManager.filterByTags([]);
+      } else {
-        }
+        // Show all places (no filters)
+        this.placesManager.filterByTags(null, false);
       }
     },
 

app/models/concerns/omniauthable.rb

@@ -15,23 +15,23 @@ module Omniauthable
       return user if user
 
       # If not found, try to find by email
-      user = find_by(email: data['email'])
+      user = find_by(email: data['email']) if data['email'].present?
 
       if user
         # Update provider and uid for existing user (first-time linking)
         user.update!(provider: provider, uid: uid)
+
         return user
       end
 
-      # Create new user if not found
+      return nil unless data['email'].present?
-      user = create(
+
+      create(
         email: data['email'],
         password: Devise.friendly_token[0, 20],
         provider: provider,
         uid: uid
       )
-
-      user
     end
   end
 end

config/initializers/01_constants.rb

@@ -42,7 +42,7 @@ METRICS_PASSWORD = ENV.fetch('METRICS_PASSWORD', 'prometheus')
 OMNIAUTH_PROVIDERS =
   if SELF_HOSTED
     # Self-hosted: only OpenID Connect
-    ENV['OIDC_CLIENT_ID'].present? ? %i[openid_connect] : []
+    (ENV['OIDC_CLIENT_ID'].present? && ENV['OIDC_CLIENT_SECRET'].present?) ? %i[openid_connect] : []
   else
     # Cloud: only GitHub and Google
     providers = []

spec/requests/shared/stats_spec.rb

@@ -3,8 +3,6 @@
 require 'rails_helper'
 
 RSpec.describe 'Shared::Stats', type: :request do
-
-
   context 'public sharing' do
     let(:user) { create(:user) }
     let(:stat) { create(:stat, :with_sharing_enabled, user:, year: 2024, month: 6) }