From 5d471a29443fd2b579a57717bbc00a3c1dc0b200 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gw=C3=A9na=C3=ABl=20Rault?= Date: Tue, 17 Feb 2026 11:12:55 +0100 Subject: [PATCH 1/2] Avoid popup data recall --- app/assets/javascripts/routes_layers.js | 29 +++++++++++++++++-------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/routes_layers.js b/app/assets/javascripts/routes_layers.js index fe759bfb2..0d266ae7c 100644 --- a/app/assets/javascripts/routes_layers.js +++ b/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); From 5875dd88413b4081e4047775cd1eba59c4af8c58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gw=C3=A9na=C3=ABl=20Rault?= Date: Tue, 17 Feb 2026 13:59:57 +0100 Subject: [PATCH 2/2] Import Destinations - Performance improvements in update --- app/jobs/importer_destinations.rb | 105 ++++++++++++++++++----------- app/models/concerns/consistency.rb | 5 +- app/models/destination.rb | 2 +- app/models/planning.rb | 39 ++++++++++- app/models/store_reload.rb | 2 +- app/models/visit.rb | 2 +- 6 files changed, 108 insertions(+), 47 deletions(-) diff --git a/app/jobs/importer_destinations.rb b/app/jobs/importer_destinations.rb index 0fa2fbdbb..e9db30bd3 100644 --- a/app/jobs/importer_destinations.rb +++ b/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,7 +483,7 @@ 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 @@ -486,10 +491,11 @@ def after_import(name, _options) @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| diff --git a/app/models/concerns/consistency.rb b/app/models/concerns/consistency.rb index 7ad0b87ef..5f7791336 100644 --- a/app/models/concerns/consistency.rb +++ b/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| diff --git a/app/models/destination.rb b/app/models/destination.rb index a2fdad7a5..358653899 100644 --- a/app/models/destination.rb +++ b/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 diff --git a/app/models/planning.rb b/app/models/planning.rb index 31ef25a6b..8ecaf1d71 100644 --- a/app/models/planning.rb +++ b/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 = [ diff --git a/app/models/store_reload.rb b/app/models/store_reload.rb index ed504cdab..663a567a3 100644 --- a/app/models/store_reload.rb +++ b/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 diff --git a/app/models/visit.rb b/app/models/visit.rb index 50e4c182a..c111d753e 100644 --- a/app/models/visit.rb +++ b/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