Merge pull request #1837 from Freika/fix/minor-bugs-2025-10

Small fixes

CHANGELOG.md

@@ -6,10 +6,20 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 # [UNRELEASED]
 
+## Changed
+
+- On the Trip page, instead of list of visited countries, a number of them is being shown. Clicking on it opens a modal with a list of countries visited during the trip. #1731
+
 ## Fixed
 
 - `GET /api/v1/stats` endpoint now returns correct 0 instead of null if no points were tracked in the requested period.
 - User import data now being streamed instead of loaded into memory all at once. This should prevent large imports from exhausting memory or hitting IO limits while reading export archives.
+- Popup for manual visit creation now looks better in both light and dark modes. #1835
+- Fixed a bug where visit circles were not interactive on the map page. #1833
+- Fixed a bug with stats sharing settings being not filled. #1826
+- Fixed a bug where user could not be deleted due to counter cache on points. #1818
+- Introduce apt-get upgrade before installing new packages in the docker image to prevent vulnerabilities. #1793
+- Fixed time shift when creating visits manually. #1679
 
 # [0.33.0] - 2025-09-29
 

app/javascript/controllers/add_visit_controller.js

@@ -197,39 +197,45 @@ export default class extends Controller {
     const startTime = formatDateTime(now);
     const endTime = formatDateTime(oneHourLater);
 
-    // Create form HTML
+    // Create form HTML using DaisyUI classes for automatic theme support
     const formHTML = `
       <div class="visit-form" style="min-width: 280px;">
-        <h3 style="margin-top: 0; margin-bottom: 15px; font-size: 16px; color: #333;">Add New Visit</h3>
+        <h3 class="text-base font-semibold mb-4">Add New Visit</h3>
 
-        <form id="add-visit-form" style="display: flex; flex-direction: column; gap: 10px;">
+        <form id="add-visit-form" class="space-y-3">
-          <div>
+          <div class="form-control">
-            <label for="visit-name" style="display: block; margin-bottom: 5px; font-weight: bold; font-size: 14px;">Name:</label>
+            <label for="visit-name" class="label">
+              <span class="label-text font-medium">Name:</span>
+            </label>
             <input type="text" id="visit-name" name="name" required
-                   style="width: 100%; padding: 8px; border: 1px solid #ccc; border-radius: 4px; font-size: 14px;"
+                   class="input input-bordered w-full"
                    placeholder="Enter visit name">
           </div>
 
-          <div>
+          <div class="form-control">
-            <label for="visit-start" style="display: block; margin-bottom: 5px; font-weight: bold; font-size: 14px;">Start Time:</label>
+            <label for="visit-start" class="label">
+              <span class="label-text font-medium">Start Time:</span>
+            </label>
             <input type="datetime-local" id="visit-start" name="started_at" required value="${startTime}"
-                   style="width: 100%; padding: 8px; border: 1px solid #ccc; border-radius: 4px; font-size: 14px;">
+                   class="input input-bordered w-full">
           </div>
 
-          <div>
+          <div class="form-control">
-            <label for="visit-end" style="display: block; margin-bottom: 5px; font-weight: bold; font-size: 14px;">End Time:</label>
+            <label for="visit-end" class="label">
+              <span class="label-text font-medium">End Time:</span>
+            </label>
             <input type="datetime-local" id="visit-end" name="ended_at" required value="${endTime}"
-                   style="width: 100%; padding: 8px; border: 1px solid #ccc; border-radius: 4px; font-size: 14px;">
+                   class="input input-bordered w-full">
           </div>
 
           <input type="hidden" name="latitude" value="${lat}">
           <input type="hidden" name="longitude" value="${lng}">
 
-          <div style="display: flex; gap: 10px; margin-top: 15px;">
+          <div class="flex gap-2 mt-4">
-            <button type="submit" style="flex: 1; background: #28a745; color: white; border: none; padding: 10px; border-radius: 4px; cursor: pointer; font-weight: bold;">
+            <button type="submit" class="btn btn-success flex-1">
               Create Visit
             </button>
-            <button type="button" id="cancel-visit" style="flex: 1; background: #dc3545; color: white; border: none; padding: 10px; border-radius: 4px; cursor: pointer; font-weight: bold;">
+            <button type="button" id="cancel-visit" class="btn btn-error flex-1">
               Cancel
             </button>
           </div>

app/javascript/controllers/maps_controller.js

@@ -167,25 +167,28 @@ export default class extends BaseController {
     // Create a proper Leaflet layer for fog
     this.fogOverlay = new (createFogOverlay())();
 
-    // Create custom pane for areas
+    // Create custom panes with proper z-index ordering
+    // Leaflet default panes: tilePane=200, overlayPane=400, shadowPane=500, markerPane=600, tooltipPane=650, popupPane=700
+
+    // Areas pane - below visits so they don't block interaction
     this.map.createPane('areasPane');
-    this.map.getPane('areasPane').style.zIndex = 650;
+    this.map.getPane('areasPane').style.zIndex = 605; // Above markerPane but below visits
-    this.map.getPane('areasPane').style.pointerEvents = 'all';
+    this.map.getPane('areasPane').style.pointerEvents = 'none'; // Don't block clicks, let them pass through
 
-    // Create custom panes for visits
+    // Legacy visits pane for backward compatibility
-    // Note: We'll still create visitsPane for backward compatibility
     this.map.createPane('visitsPane');
-    this.map.getPane('visitsPane').style.zIndex = 600;
+    this.map.getPane('visitsPane').style.zIndex = 615;
-    this.map.getPane('visitsPane').style.pointerEvents = 'all';
+    this.map.getPane('visitsPane').style.pointerEvents = 'auto';
-
-    // Create separate panes for confirmed and suggested visits
-    this.map.createPane('confirmedVisitsPane');
-    this.map.getPane('confirmedVisitsPane').style.zIndex = 450;
-    this.map.getPane('confirmedVisitsPane').style.pointerEvents = 'all';
 
+    // Suggested visits pane - interactive layer
     this.map.createPane('suggestedVisitsPane');
-    this.map.getPane('suggestedVisitsPane').style.zIndex = 460;
+    this.map.getPane('suggestedVisitsPane').style.zIndex = 610;
-    this.map.getPane('suggestedVisitsPane').style.pointerEvents = 'all';
+    this.map.getPane('suggestedVisitsPane').style.pointerEvents = 'auto';
+
+    // Confirmed visits pane - on top of suggested, interactive
+    this.map.createPane('confirmedVisitsPane');
+    this.map.getPane('confirmedVisitsPane').style.zIndex = 620;
+    this.map.getPane('confirmedVisitsPane').style.pointerEvents = 'auto';
 
     // Initialize areasLayer as a feature group and add it to the map immediately
     this.areasLayer = new L.FeatureGroup();

app/javascript/maps/visits.js

@@ -12,14 +12,17 @@ export class VisitsManager {
     this.userTheme = userTheme;
 
     // Create custom panes for different visit types
-    if (!map.getPane('confirmedVisitsPane')) {
+    // Leaflet default panes: tilePane=200, overlayPane=400, shadowPane=500, markerPane=600, tooltipPane=650, popupPane=700
-      map.createPane('confirmedVisitsPane');
-      map.getPane('confirmedVisitsPane').style.zIndex = 450; // Above default overlay pane (400)
-    }
-
     if (!map.getPane('suggestedVisitsPane')) {
       map.createPane('suggestedVisitsPane');
-      map.getPane('suggestedVisitsPane').style.zIndex = 460; // Below confirmed visits but above base layers
+      map.getPane('suggestedVisitsPane').style.zIndex = 610; // Above markerPane (600), below tooltipPane (650)
+      map.getPane('suggestedVisitsPane').style.pointerEvents = 'auto'; // Ensure interactions work
+    }
+
+    if (!map.getPane('confirmedVisitsPane')) {
+      map.createPane('confirmedVisitsPane');
+      map.getPane('confirmedVisitsPane').style.zIndex = 620; // Above suggested visits
+      map.getPane('confirmedVisitsPane').style.pointerEvents = 'auto'; // Ensure interactions work
     }
 
     this.visitCircles = L.layerGroup();
@@ -1324,38 +1327,31 @@ export class VisitsManager {
       // Create popup content with form and dropdown
       const defaultName = visit.name;
       const popupContent = `
-        <div class="p-4 bg-base-100 text-base-content rounded-lg shadow-lg">
+        <div style="min-width: 280px;">
-          <div class="mb-4">
+          <h3 class="text-base font-semibold mb-3">${dateTimeDisplay.trim()}</h3>
-            <div class="text-sm mb-2 text-base-content/80 font-medium">
+
-              ${dateTimeDisplay.trim()}
+          <div class="space-y-1 mb-4 text-sm">
-            </div>
+            <div>Duration: ${durationText}</div>
-            <div class="space-y-1">
+            <div class="${statusColorClass} font-semibold">Status: ${visit.status.charAt(0).toUpperCase() + visit.status.slice(1)}</div>
-              <div class="text-sm text-base-content/60">
+            <div class="text-xs opacity-60 font-mono">${visit.place.latitude}, ${visit.place.longitude}</div>
-                Duration: ${durationText}
-              </div>
-              <div class="text-sm ${statusColorClass} font-semibold">
-                Status: ${visit.status.charAt(0).toUpperCase() + visit.status.slice(1)}
-              </div>
-              <div class="text-xs text-base-content/50 font-mono">
-                ${visit.place.latitude}, ${visit.place.longitude}
-              </div>
-            </div>
           </div>
+
           <form class="visit-name-form space-y-3" data-visit-id="${visit.id}">
             <div class="form-control">
               <label class="label">
-                <span class="label-text text-sm font-medium">Visit Name</span>
+                <span class="label-text font-medium">Visit Name:</span>
               </label>
               <input type="text"
-                     class="input input-bordered input-sm w-full bg-base-200 text-base-content placeholder:text-base-content/50"
+                     class="input input-bordered w-full"
                      value="${defaultName}"
                      placeholder="Enter visit name">
             </div>
+
             <div class="form-control">
               <label class="label">
-                <span class="label-text text-sm font-medium">Location</span>
+                <span class="label-text font-medium">Location:</span>
               </label>
-              <select class="select select-bordered select-sm text-xs w-full bg-base-200 text-base-content" name="place">
+              <select class="select select-bordered w-full" name="place">
                 ${possiblePlaces.length > 0 ? possiblePlaces.map(place => `
                   <option value="${place.id}" ${place.id === visit.place.id ? 'selected' : ''}>
                     ${place.name}
@@ -1367,36 +1363,24 @@ export class VisitsManager {
                 `}
               </select>
             </div>
-            <div class="flex gap-2 mt-4 pt-2 border-t border-base-300">
+
-              <button type="submit" class="btn btn-sm btn-primary flex-1">
+            <div class="grid grid-cols-3 gap-2">
-                <svg class="w-4 h-4 mr-1" fill="none" stroke="currentColor" viewBox="0 0 24 24">
+              <button type="submit" class="btn btn-primary btn-sm">
-                  <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M5 13l4 4L19 7"></path>
-                </svg>
                 Save
               </button>
               ${visit.status !== 'confirmed' ? `
-                <button type="button" class="btn btn-sm btn-success confirm-visit" data-id="${visit.id}">
+                <button type="button" class="btn btn-success btn-sm confirm-visit" data-id="${visit.id}">
-                  <svg class="w-4 h-4 mr-1" fill="none" stroke="currentColor" viewBox="0 0 24 24">
-                    <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M9 12l2 2 4-4"></path>
-                  </svg>
                   Confirm
                 </button>
-                <button type="button" class="btn btn-sm btn-error decline-visit" data-id="${visit.id}">
+                <button type="button" class="btn btn-error btn-sm decline-visit" data-id="${visit.id}">
-                  <svg class="w-4 h-4 mr-1" fill="none" stroke="currentColor" viewBox="0 0 24 24">
-                    <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M6 18L18 6M6 6l12 12"></path>
-                  </svg>
                   Decline
                 </button>
-              ` : ''}
+              ` : '<div class="col-span-2"></div>'}
-            </div>
-            <div class="mt-2">
-              <button type="button" class="btn btn-sm btn-outline btn-error w-full delete-visit" data-id="${visit.id}">
-                <svg class="w-4 h-4 mr-1" fill="none" stroke="currentColor" viewBox="0 0 24 24">
-                  <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M19 7l-.867 12.142A2 2 0 0116.138 21H7.862a2 2 0 01-1.995-1.858L5 7m5 4v6m4-6v6m1-10V4a1 1 0 00-1-1h-4a1 1 0 00-1 1v3M4 7h16"></path>
-                </svg>
-                Delete Visit
-              </button>
             </div>
+
+            <button type="button" class="btn btn-outline btn-error btn-sm w-full delete-visit" data-id="${visit.id}">
+              Delete Visit
+            </button>
           </form>
         </div>
       `;

app/models/stat.rb

@@ -33,14 +33,14 @@ class Stat < ApplicationRecord
   end
 
   def sharing_enabled?
-    sharing_settings['enabled'] == true
+    sharing_settings.try(:[], 'enabled') == true
   end
 
   def sharing_expired?
-    expiration = sharing_settings['expiration']
+    expiration = sharing_settings.try(:[], 'expiration')
     return false if expiration.blank?
 
-    expires_at_value = sharing_settings['expires_at']
+    expires_at_value = sharing_settings.try(:[], 'expires_at')
     return true if expires_at_value.blank?
 
     expires_at = begin

app/services/visits/create.rb

@@ -71,16 +71,16 @@ module Visits
     end
 
     def create_visit(place)
-      started_at = DateTime.parse(params[:started_at])
+      started_at = Time.zone.parse(params[:started_at])
-      ended_at = DateTime.parse(params[:ended_at])
+      ended_at = Time.zone.parse(params[:ended_at])
-      duration_minutes = (ended_at - started_at) * 24 * 60
+      duration_minutes = ((ended_at - started_at) / 60).to_i
 
       @visit = user.visits.create!(
         name: params[:name],
         place: place,
         started_at: started_at,
         ended_at: ended_at,
-        duration: duration_minutes.to_i,
+        duration: duration_minutes,
         status: :confirmed
       )
 

app/views/trips/_countries.html.erb

@@ -11,12 +11,13 @@
       <div class="stat-value text-lg"><%= trip_duration(trip) %></div>
     </div>
   </div>
-  <div class="card bg-base-200 shadow-lg">
+  <div class="card bg-base-200 shadow-lg cursor-pointer hover:bg-base-300 transition-colors"
+       onclick="countries_modal_<%= trip.id %>.showModal()">
     <div class="card-body p-4">
       <div class="stat-title text-xs">Countries</div>
       <div class="stat-value text-lg">
         <% if trip.visited_countries.any? %>
-          <%= trip.visited_countries.join(', ') %>
+          <%= trip.visited_countries.count %>
         <% else %>
           <span class="loading loading-dots loading-sm"></span>
         <% end %>
@@ -24,3 +25,27 @@
     </div>
   </div>
 </div>
+
+<!-- Countries Modal -->
+<dialog id="countries_modal_<%= trip.id %>" class="modal">
+  <div class="modal-box">
+    <form method="dialog">
+      <button class="btn btn-sm btn-circle btn-ghost absolute right-2 top-2">✕</button>
+    </form>
+    <h3 class="font-bold text-lg mb-4">Visited Countries</h3>
+    <% if trip.visited_countries.any? %>
+      <div class="space-y-2">
+        <% trip.visited_countries.sort.each do |country| %>
+          <div class="p-3 bg-base-200 rounded-lg">
+            <%= country %>
+          </div>
+        <% end %>
+      </div>
+    <% else %>
+      <p class="text-base-content/70">No countries data available yet.</p>
+    <% end %>
+  </div>
+  <form method="dialog" class="modal-backdrop">
+    <button>close</button>
+  </form>
+</dialog>

spec/models/stat_spec.rb

@@ -262,5 +262,223 @@ RSpec.describe Stat, type: :model do
         end
       end
     end
+
+    describe 'sharing settings' do
+      let(:user) { create(:user) }
+      let(:stat) { create(:stat, year: 2024, month: 6, user: user) }
+
+      describe '#sharing_enabled?' do
+        context 'when sharing_settings is nil' do
+          before { stat.update_column(:sharing_settings, nil) }
+
+          it 'returns false' do
+            expect(stat.sharing_enabled?).to be false
+          end
+        end
+
+        context 'when sharing_settings is empty hash' do
+          before { stat.update(sharing_settings: {}) }
+
+          it 'returns false' do
+            expect(stat.sharing_enabled?).to be false
+          end
+        end
+
+        context 'when enabled is false' do
+          before { stat.update(sharing_settings: { 'enabled' => false }) }
+
+          it 'returns false' do
+            expect(stat.sharing_enabled?).to be false
+          end
+        end
+
+        context 'when enabled is true' do
+          before { stat.update(sharing_settings: { 'enabled' => true }) }
+
+          it 'returns true' do
+            expect(stat.sharing_enabled?).to be true
+          end
+        end
+
+        context 'when enabled is a string "true"' do
+          before { stat.update(sharing_settings: { 'enabled' => 'true' }) }
+
+          it 'returns false (strict boolean check)' do
+            expect(stat.sharing_enabled?).to be false
+          end
+        end
+      end
+
+      describe '#sharing_expired?' do
+        context 'when sharing_settings is nil' do
+          before { stat.update_column(:sharing_settings, nil) }
+
+          it 'returns false' do
+            expect(stat.sharing_expired?).to be false
+          end
+        end
+
+        context 'when expiration is blank' do
+          before { stat.update(sharing_settings: { 'enabled' => true }) }
+
+          it 'returns false' do
+            expect(stat.sharing_expired?).to be false
+          end
+        end
+
+        context 'when expiration is present but expires_at is blank' do
+          before do
+            stat.update(sharing_settings: {
+              'enabled' => true,
+              'expiration' => '1h'
+            })
+          end
+
+          it 'returns true' do
+            expect(stat.sharing_expired?).to be true
+          end
+        end
+
+        context 'when expires_at is in the future' do
+          before do
+            stat.update(sharing_settings: {
+              'enabled' => true,
+              'expiration' => '1h',
+              'expires_at' => 1.hour.from_now.iso8601
+            })
+          end
+
+          it 'returns false' do
+            expect(stat.sharing_expired?).to be false
+          end
+        end
+
+        context 'when expires_at is in the past' do
+          before do
+            stat.update(sharing_settings: {
+              'enabled' => true,
+              'expiration' => '1h',
+              'expires_at' => 1.hour.ago.iso8601
+            })
+          end
+
+          it 'returns true' do
+            expect(stat.sharing_expired?).to be true
+          end
+        end
+
+        context 'when expires_at is 1 second in the future' do
+          before do
+            stat.update(sharing_settings: {
+              'enabled' => true,
+              'expiration' => '1h',
+              'expires_at' => 1.second.from_now.iso8601
+            })
+          end
+
+          it 'returns false (not yet expired)' do
+            expect(stat.sharing_expired?).to be false
+          end
+        end
+
+        context 'when expires_at is invalid date string' do
+          before do
+            stat.update(sharing_settings: {
+              'enabled' => true,
+              'expiration' => '1h',
+              'expires_at' => 'invalid-date'
+            })
+          end
+
+          it 'returns true (treats as expired)' do
+            expect(stat.sharing_expired?).to be true
+          end
+        end
+
+        context 'when expires_at is nil' do
+          before do
+            stat.update(sharing_settings: {
+              'enabled' => true,
+              'expiration' => '1h',
+              'expires_at' => nil
+            })
+          end
+
+          it 'returns true' do
+            expect(stat.sharing_expired?).to be true
+          end
+        end
+
+        context 'when expires_at is empty string' do
+          before do
+            stat.update(sharing_settings: {
+              'enabled' => true,
+              'expiration' => '1h',
+              'expires_at' => ''
+            })
+          end
+
+          it 'returns true' do
+            expect(stat.sharing_expired?).to be true
+          end
+        end
+      end
+
+      describe '#public_accessible?' do
+        context 'when sharing_settings is nil' do
+          before { stat.update_column(:sharing_settings, nil) }
+
+          it 'returns false' do
+            expect(stat.public_accessible?).to be false
+          end
+        end
+
+        context 'when sharing is not enabled' do
+          before { stat.update(sharing_settings: { 'enabled' => false }) }
+
+          it 'returns false' do
+            expect(stat.public_accessible?).to be false
+          end
+        end
+
+        context 'when sharing is enabled but expired' do
+          before do
+            stat.update(sharing_settings: {
+              'enabled' => true,
+              'expiration' => '1h',
+              'expires_at' => 1.hour.ago.iso8601
+            })
+          end
+
+          it 'returns false' do
+            expect(stat.public_accessible?).to be false
+          end
+        end
+
+        context 'when sharing is enabled and not expired' do
+          before do
+            stat.update(sharing_settings: {
+              'enabled' => true,
+              'expiration' => '1h',
+              'expires_at' => 1.hour.from_now.iso8601
+            })
+          end
+
+          it 'returns true' do
+            expect(stat.public_accessible?).to be true
+          end
+        end
+
+        context 'when sharing is enabled with no expiration' do
+          before do
+            stat.update(sharing_settings: { 'enabled' => true })
+          end
+
+          it 'returns true' do
+            expect(stat.public_accessible?).to be true
+          end
+        end
+      end
+    end
   end
 end

spec/services/visits/create_spec.rb

@@ -166,6 +166,31 @@ RSpec.describe Visits::Create do
           expect(service.visit.duration).to eq(36 * 60) # 36 hours in minutes
         end
       end
+
+      context 'when datetime-local input is provided without timezone' do
+        let(:params) do
+          valid_params.merge(
+            started_at: '2023-12-01T19:54',
+            ended_at: '2023-12-01T20:54'
+          )
+        end
+        subject(:service) { described_class.new(user, params) }
+
+        it 'parses the datetime in the application timezone' do
+          service.call
+          visit = service.visit
+
+          expect(visit.started_at.hour).to eq(19)
+          expect(visit.started_at.min).to eq(54)
+          expect(visit.ended_at.hour).to eq(20)
+          expect(visit.ended_at.min).to eq(54)
+        end
+
+        it 'calculates correct duration' do
+          service.call
+          expect(service.visit.duration).to eq(60) # 1 hour in minutes
+        end
+      end
     end
   end
 end