Fixes

app/assets/javascripts/routes_layers.js

@@ -101,8 +101,19 @@ const popupModule = (function() {
   };
 
   const getPopupContent = function(url, marker, map) {
-    if (marker.getPopup())
+    // Reuse cached content when same context (url) - avoids reload on re-click after popup was closed (unbindPopup clears getPopup())
+    if (marker._popupLoadedUrl === url && marker._popupContent) {
+      marker.bindPopup(L.popup().setContent(marker._popupContent), {
+        minWidth: 200,
+        autoPan: false,
+        closeOnClick: false
+      }).addTo(map);
+      _previousPopup = marker.getPopup();
+      marker.openPopup();
       return;
+    }
+    if (marker.getPopup())
+      marker.unbindPopup();
 
     if (_ajaxRequest.current && _ajaxRequest.current.readyState !== 4)
       return;
@@ -111,26 +122,26 @@ const popupModule = (function() {
       url: url,
       beforeSend: beforeSendWaiting,
       success: function(data) {
-        var popup = marker.bindPopup(L.popup(), {
-          minWidth: 200,
-          autoPan: false,
-          closeOnClick: false
-        }).addTo(map).getPopup();
-
+        marker._popupLoadedUrl = url;
         data.i18n = mustache_i18n;
         data.routes = _context.options.routes.filter(function(route) {
           return route.vehicle_usage_id;
         }).map(function(route) {
           route.color = _context.options.colorsByRoute[route.route_id];
           return route;
-        }); // unnecessary to load all for each stop
+        });
         data.out_of_route_id = _context.options.outOfRouteId;
         data.number = marker.properties.number;
         if (_context.options.url_click2call) {
           phoneNumberCall(data, _context.options.url_click2call);
         }
         $.extend(data, _context.options.popupOptions);
-        popup.setContent(SMT['stops/show'](data));
+        marker._popupContent = SMT['stops/show'](data);
+        marker.bindPopup(L.popup().setContent(marker._popupContent), {
+          minWidth: 200,
+          autoPan: false,
+          closeOnClick: false
+        }).addTo(map);
 
         $('#isochrone_lat').val(data.lat);
         $('#isochrone_lng').val(data.lng);

app/jobs/importer_destinations.rb

@@ -185,6 +185,14 @@ def rows_to_json(rows)
   end
 
   def before_import(_name, data, options)
+    if options[:delete_plannings]
+      @customer.delete_all_plannings
+      @customer.plannings.reload
+    end
+    if options[:replace]
+      @customer.delete_all_destinations
+    end
+
     @common_tags = {}
     @tag_labels = Hash[@customer.tags.collect{ |tag| [tag.label, tag] }]
     @tag_ids = Hash[@customer.tags.collect{ |tag| [tag.id, tag] }]
@@ -204,13 +212,6 @@ def before_import(_name, data, options)
     @visit_ids = []
     @store_reload_ids = []
 
-    if options[:delete_plannings]
-      @customer.delete_all_plannings
-      @customer.plannings.reload
-    end
-    if options[:replace]
-      @customer.delete_all_destinations
-    end
     @plannings_hash = CaseInsensitiveHash[@customer.plannings.select(&:ref).map{ |plan| [plan.ref, plan] }]
 
     if options[:line_shift] == 1
@@ -240,13 +241,14 @@ def before_import(_name, data, options)
     @existing_store_reloads_by_ref = CaseInsensitiveHash.new
     @destinations_visits_attributes_by_ref = CaseInsensitiveHash.new
     @stores_store_reloads_attributes_by_ref = CaseInsensitiveHash.new
-    @customer.destinations.includes_visits.where.not(ref: nil).find_each{ |destination|
+    # Preload stop_visits to get route_ids without loading routes (avoids stale updates)
+    @customer.destinations.includes([{visits: [:tags, :stop_visits]}, :tags]).where.not(ref: nil).find_each{ |destination|
       @existing_destinations_by_ref[destination.ref] = destination
       @existing_visits_by_ref[destination.ref] = CaseInsensitiveHash[destination.visits.map{ |visit| [visit.ref, visit]}]
       @destinations_visits_attributes_by_ref[destination.ref] = CaseInsensitiveHash.new
       destination.visits.each{ |visit| @destinations_visits_attributes_by_ref[visit.destination.ref][visit.ref] = visit }
     }
-    @customer.stores.where.not(ref: nil).find_each{ |store|
+    @customer.stores.includes(:store_reloads).where.not(ref: nil).find_each{ |store|
       @existing_stores_by_ref[store.ref] = store
       @existing_store_reloads_by_ref[store.ref] = CaseInsensitiveHash[store.store_reloads.map{ |store_reload| [store_reload.ref, store_reload]}]
       @stores_store_reloads_attributes_by_ref[store.ref] = CaseInsensitiveHash.new
@@ -292,6 +294,9 @@ def before_import(_name, data, options)
 
     @nil_visit_available = CaseInsensitiveHash.new{ |h, k| h[k] = CaseInsensitiveHash.new(true) }
 
+    # Collect route_ids for deferred outdated (avoids loading routes during transaction)
+    @route_ids_to_outdate = []
+
     @tag_destinations = []
     @tag_visits = []
 
@@ -478,18 +483,19 @@ def after_import(name, _options)
     @store_ids = bulk_import_stores(@stores_attributes_without_ref)
     @store_ids += bulk_import_stores(@stores_attributes_by_ref.values)
     # bulk import do not support before_create or before_save callbacks
-    if @customer.destinations.size > max_lines
+    if @customer.destinations.count > max_lines
       raise(Exceptions::OverMaxLimitError.new(I18n.t('activerecord.errors.models.customer.attributes.destinations.over_max_limit')))
     end
     @visit_ids = bulk_import_visits
     tags_imported = bulk_import_tags
     @store_reload_ids = bulk_import_store_reloads
     @customer.reload
 
-    # Reload plannings with routes after bulk_import_tags to avoid stale route objects
-    # visit.save! in bulk_import_tags triggers visit.outdated which calls stop.route.save!
-    # This updates route.lock_version in database, making routes in memory stale
-    # Only reload if tags were imported (when tags_imported is true)
+    geocode_or_count_destinations
+    geocode_or_count_stores
+
+    sync_tags_deferred
+
     if tags_imported && @plannings_hash.any?
       reloaded_plannings = @customer.plannings.preload_route_details.where(ref: @plannings_hash.keys).index_by(&:ref)
       @plannings_hash.each{ |ref, planning|
@@ -499,11 +505,10 @@ def after_import(name, _options)
       }
     end
 
-    geocode_or_count_destinations
-    geocode_or_count_stores
-
     prepare_plannings(name, _options)
 
+    @customer.save! if tags_imported
+
     # Update destinations_count and visits_count as activerecord callbacks are not called
     Customer.where(id: @customer.id).update_all(
       destinations_count: @customer.destinations.count,
@@ -560,6 +565,7 @@ def save_plannings
   end
 
   def finalize_import(_name, _options)
+    outdate_routes_deferred
     if (@destinations_to_geocode_count > 0 || @stores_to_geocode_count > 0) && (!@synchronous && Planner::Application.config.delayed_job_use)
       save_plannings
       @customer.job_destination_geocoding = Delayed::Job.enqueue(GeocoderJob.new(@customer.id, !@plannings.empty? ? @plannings.map(&:id) : nil))
@@ -734,7 +740,8 @@ def bulk_import_visits
         import_result = Visit.import(
           visits_attributes,
           on_duplicate_key_update: { conflict_target: (keys.include?(:id) ? [:id] : [:destination_id, :ref]), columns: (Visit.column_names & keys.collect(&:to_s)) - ['id', 'updated_at'] },
-          validate: true, all_or_none: true, track_validation_failures: true
+          validate: true, all_or_none: true, track_validation_failures: true,
+          validate_with_context: :import
         )
 
         raise ImportBulkError.new(import_errors_with_indices(slice_lines, slice_index, import_result.failed_instances)) if import_result.failed_instances.any?
@@ -812,7 +819,8 @@ def bulk_import_store_reloads
         import_result = StoreReload.import(
           store_reloads_attributes,
           on_duplicate_key_update: { conflict_target: (keys.include?(:id) ? [:id] : [:store_id, :ref]), columns: (StoreReload.column_names & keys.collect(&:to_s)) - ['id', 'updated_at'] },
-          validate: true, all_or_none: true, track_validation_failures: true
+          validate: true, all_or_none: true, track_validation_failures: true,
+          validate_with_context: :import
         )
 
         raise ImportBulkError.new(import_errors_with_indices(slice_lines, slice_index, import_result.failed_instances)) if import_result.failed_instances.any?
@@ -828,6 +836,8 @@ def bulk_import_store_reloads
 
   def bulk_import_tags
     tags_imported = false
+    @tag_sync_affected_visit_ids = []
+
     if @tag_destinations.any?
       destination_ids_and_tag_ids = @tag_destinations.map{ |visit_index, tag_id|
         { destination_id: @destination_index_to_id_hash[visit_index], tag_id: tag_id }
@@ -839,14 +849,8 @@ def bulk_import_tags
       raise ImportBaseError.new(import_result.failed_instances.map(&:errors).uniq) if import_result.failed_instances.any?
 
       if @customer.plannings.any?
-        @customer.destinations.joins(:tags).where(
-          id: destination_ids_and_tag_ids.map{ |tag| tag[:destination_id] }.uniq).where(
-          tags: { id: destination_ids_and_tag_ids.map{ |tag| tag[:tag_id] }.uniq }
-        ).distinct.find_each{ |destination|
-          destination.update_tags_track(true)
-          destination.save!
-        }
-        @customer.save!
+        affected_destination_ids = destination_ids_and_tag_ids.map{ |t| t[:destination_id] }.uniq
+        @tag_sync_affected_visit_ids |= Visit.where(destination_id: affected_destination_ids).pluck(:id)
         tags_imported = true
       end
     end
@@ -861,17 +865,7 @@ def bulk_import_tags
       raise ImportBaseError.new(import_result.failed_instances.map(&:errors).uniq) if import_result.failed_instances.any?
 
       if @customer.plannings.any?
-        # Default scope requires destinations to be loaded
-        Destination.unscoped do
-          @customer.visits.joins(:tags).where(
-            id: visit_ids_and_tag_ids.map{ |tag| tag[:visit_id] }.uniq).where(
-            tags: { id: visit_ids_and_tag_ids.map{ |tag| tag[:tag_id] }.uniq }
-          ).distinct.find_each{ |visit|
-            visit.update_tags_track(true)
-            visit.save!
-          }
-        end
-        @customer.save!
+        @tag_sync_affected_visit_ids |= visit_ids_and_tag_ids.map{ |t| t[:visit_id] }.uniq
         tags_imported = true
       end
     end
@@ -991,9 +985,8 @@ def prepare_visit_with_destination_ref(row, line, destination, destination_index
         visit_attributes.merge!(destination_id: destination.id)
         if visit
           visit_attributes.merge!(id: visit.id)
-          # The visit is assumed to be changed
-          # TODO: Find a way to call the `update_outdated` callback during the transaction
-          visit.outdated
+          # Collect route_ids for deferred outdated - avoids loading routes (stale update)
+          @route_ids_to_outdate.concat(visit.stop_visits.map(&:route_id).compact)
 
           # Compact allows to avoid erasing nil fields
           visit_attributes.compact!
@@ -1138,6 +1131,36 @@ def prepare_plannings(name, _options)
     }
   end
 
+  def sync_tags_deferred
+    return if @tag_sync_affected_visit_ids.blank?
+
+    # Sync on in-memory planning objects (from @customer.plannings) so changes are reflected
+    # when autosave persists the customer. This mirrors the old Destination#update_tags /
+    # Visit#update_tags callbacks which operated on customer.plannings in memory.
+    @customer.plannings.each do |planning|
+      planning.sync_tags_for_visits(@tag_sync_affected_visit_ids, route_ids_collector: @route_ids_to_outdate)
+    end
+  end
+
+  def outdate_routes_deferred
+    return if @route_ids_to_outdate.blank?
+
+    route_ids = @route_ids_to_outdate.uniq.compact
+    return if route_ids.empty?
+
+    begin
+      ActiveRecord::Base.lock_optimistically = false
+      Route.where(id: route_ids).find_each do |route|
+        unless route.outdated
+          route.outdated = true
+          route.save!
+        end
+      end
+    ensure
+      ActiveRecord::Base.lock_optimistically = true
+    end
+  end
+
   def import_errors_with_indices(slice_lines, slice_index, failed_instances)
     failed_instances.group_by{ |index_in_dataset, object_with_errors|
       object_with_errors.errors.messages.map{ |key, message|

app/models/concerns/consistency.rb

@@ -25,8 +25,9 @@ module Consistency
 
   class_methods do
     # Attributes must have a defined #{attr_name}_changed? method
-    def validate_consistency(attributes, &block)
-      validate do |record|
+    # skip_contexts: skip validation when validation_context is in the list (e.g. skip_contexts: [:import])
+    def validate_consistency(attributes, skip_contexts: [], &block)
+      validate(unless: ->(record) { skip_contexts.any? && skip_contexts.include?(record.validation_context) }) do |record|
         consistent_value = block ? block.call(record) : record.send(:customer_id)
         if consistent_value
           attributes.each{ |attr|

app/models/destination.rb

@@ -33,7 +33,7 @@ class Destination < Location
   time_attr :duration
 
   include Consistency
-  validate_consistency [:tags]
+  validate_consistency [:tags], skip_contexts: [:import]
   validates :ref, uniqueness: { scope: :customer_id, case_sensitive: true }, allow_nil: true, allow_blank: true
 
   before_create :check_max_destination

app/models/planning.rb

@@ -44,7 +44,7 @@ class Planning < ApplicationRecord
   validate :begin_after_end_date
 
   include Consistency
-  validate_consistency [:vehicle_usage_set, :order_array, :zonings, :tags]
+  validate_consistency [:vehicle_usage_set, :order_array, :zonings, :tags], skip_contexts: [:import]
 
   before_create :update_zonings, :check_max_planning
   before_destroy :unlink_job_optimizer
@@ -363,6 +363,43 @@ def visit_remove(visit)
     }
   end
 
+  # Consolidated tag sync for import: apply add/remove logic for affected visits
+  # route_ids_collector: array to append modified route_ids (reuses importer's @route_ids_to_outdate)
+  def sync_tags_for_visits(visit_ids, route_ids_collector: nil)
+    return if visit_ids.blank?
+
+    plan_tags = tags.to_a
+    routes_with_stops = routes.includes(:stops)
+    modified = false
+
+    Visit.where(id: visit_ids).includes(:tags, :stop_visits, destination: :tags).find_each do |visit|
+      combined_tags = (visit.tags.to_a | visit.destination.tags.to_a)
+      in_planning = visits_include?(visit)
+
+      if in_planning
+        should_remove = if tag_operation == '_or'
+          (plan_tags & combined_tags).empty?
+        else
+          (plan_tags & combined_tags) != plan_tags
+        end
+        if should_remove
+          route_ids_collector&.concat(visit.stop_visits.map(&:route_id) & routes_with_stops.map(&:id))
+          visit_remove(visit)
+          modified = true
+        end
+      elsif tags_compatible?(combined_tags)
+        route = routes_with_stops.find{ |r| !r.vehicle_usage? }
+        if route
+          route_ids_collector&.<<(route.id)
+          visit_add(visit)
+          modified = true
+        end
+      end
+    end
+
+    save! if modified
+  end
+
   def default_empty_routes(ignore_errors = false)
     delete_all_routes
     new_routes = [

app/models/store_reload.rb

@@ -44,7 +44,7 @@ class StoreReload < ApplicationRecord
   validate :time_window_end_after_time_window_start
 
   include Consistency
-  validate_consistency([]) { |store_reload| store_reload.store.try :customer_id }
+  validate_consistency([], skip_contexts: [:import]) { |store_reload| store_reload.store.try :customer_id }
 
   before_update :update_outdated
 

app/models/visit.rb

@@ -66,7 +66,7 @@ class Visit < ApplicationRecord
   validates :revenue, numericality: {only_float: true, greater_than_or_equal_to: 0}, allow_nil: true
 
   include Consistency
-  validate_consistency([:tags]) { |visit| visit.destination.try :customer_id }
+  validate_consistency([:tags], skip_contexts: [:import]) { |visit| visit.destination.try :customer_id }
 
   before_save :update_tags, unless: :internal_skip
   before_save :create_orders