From 3f7794959e6e36976489093ef4c9c042866dcd87 Mon Sep 17 00:00:00 2001 From: Emiliano Frascaro Date: Thu, 10 Sep 2026 17:08:50 +0200 Subject: [PATCH] corretti bug sull'invio email --- app/controllers/mailings_controller.rb | 3 ++- app/models/mailing_recipient.rb | 7 ++++- app/services/mailings/audience_filters.rb | 3 ++- app/services/mailings/audience_query.rb | 1 + app/services/mailings/outbound_queue.rb | 7 ++++- app/views/mailings/audience.html.erb | 9 +++++-- test/services/mailings/outbound_queue_test.rb | 27 +++++++++++++++++++ test/services/mailings_audience_query_test.rb | 8 ++++++ 8 files changed, 59 insertions(+), 6 deletions(-) diff --git a/app/controllers/mailings_controller.rb b/app/controllers/mailings_controller.rb index 4a06b8a..ccd47e5 100644 --- a/app/controllers/mailings_controller.rb +++ b/app/controllers/mailings_controller.rb @@ -306,6 +306,7 @@ class MailingsController < ApplicationController def load_audience_options orgs = Organization.for_project(current_project) @sport_options = orgs.where.not(sport: [nil, ""]).distinct.order(:sport).pluck(:sport) + @country_options = orgs.where.not(country: [nil, ""]).distinct.order(:country).pluck(:country) @region_options = orgs.where.not(region: [nil, ""]).distinct.order(:region).pluck(:region) @province_options = orgs.where.not(province: [nil, ""]).distinct.order(:province).pluck(:province) @history_mailings = Mailing.for_project(current_project).where.not(id: @mailing.id).recent @@ -329,7 +330,7 @@ class MailingsController < ApplicationController ActionController::Parameters.new(copied).permit( :preset, :list_min, :list_max, :estimated_value, :history_kind, :history_days, :history_mailing_id, :never_contacted, :exclude_customers, :exclude_mailed_within_days, - sports: [], team_genders: [], regions: [], provinces: [], streaming_statuses: [], statuses: [], + sports: [], countries: [], team_genders: [], regions: [], provinces: [], streaming_statuses: [], statuses: [], send_statuses: [], ab_variants: [], pipeline_stages: [], exclude_received_mailing_ids: [], exclude_opened_mailing_ids: [] ) diff --git a/app/models/mailing_recipient.rb b/app/models/mailing_recipient.rb index ebd8e4a..3bfc694 100644 --- a/app/models/mailing_recipient.rb +++ b/app/models/mailing_recipient.rb @@ -115,7 +115,12 @@ class MailingRecipient < ApplicationRecord end MAX_SMTP_DEFERS = 8 - DEFER_PATTERN = /tentativo (\d+)\// + # Deve combaciare col testo generato sotto ("Da ritentare (N/8): ..."), + # altrimenti smtp_defer_count legge sempre 0 e il retry diventa infinito + # (bug osservato in produzione: un destinatario con SMTP EOF permanente + # veniva ritentato ogni 30s all'infinito, bloccando gli altri destinatari + # in errore della stessa mailing). + DEFER_PATTERN = /Da ritentare \((\d+)\// def defer_or_fail!(error) attempt = smtp_defer_count + 1 diff --git a/app/services/mailings/audience_filters.rb b/app/services/mailings/audience_filters.rb index 961dc01..e885fad 100644 --- a/app/services/mailings/audience_filters.rb +++ b/app/services/mailings/audience_filters.rb @@ -5,7 +5,7 @@ module Mailings VALUE_MODES = %w[any present blank].freeze ARRAY_KEYS = %w[ - sports team_genders regions provinces streaming_statuses statuses + sports countries team_genders regions provinces streaming_statuses statuses send_statuses ab_variants pipeline_stages exclude_received_mailing_ids exclude_opened_mailing_ids ].freeze @@ -46,6 +46,7 @@ module Mailings def summary_parts parts = [Catalog.label_for(Catalog::MAILING_AUDIENCES, preset)] parts << "sport: #{Array(@data["sports"]).join(", ")}" if @data["sports"].present? + parts << "nazione: #{Array(@data["countries"]).join(", ")}" if @data["countries"].present? parts << "M/F: #{Array(@data["team_genders"]).map { |g| Catalog.label_for(Catalog::TEAM_GENDERS, g) }.join(", ")}" if @data["team_genders"].present? parts << "regione: #{Array(@data["regions"]).join(", ")}" if @data["regions"].present? parts << "provincia: #{Array(@data["provinces"]).join(", ")}" if @data["provinces"].present? diff --git a/app/services/mailings/audience_query.rb b/app/services/mailings/audience_query.rb index 9bfd002..d551297 100644 --- a/app/services/mailings/audience_query.rb +++ b/app/services/mailings/audience_query.rb @@ -44,6 +44,7 @@ module Mailings def apply_organization_filters(scope) scope = scope.where(sport: @filters["sports"]) if @filters["sports"].present? + scope = scope.where(country: @filters["countries"]) if @filters["countries"].present? scope = scope.where(team_gender: @filters["team_genders"]) if @filters["team_genders"].present? scope = scope.where(region: @filters["regions"]) if @filters["regions"].present? scope = scope.where(province: @filters["provinces"]) if @filters["provinces"].present? diff --git a/app/services/mailings/outbound_queue.rb b/app/services/mailings/outbound_queue.rb index 07407e8..25e3de5 100644 --- a/app/services/mailings/outbound_queue.rb +++ b/app/services/mailings/outbound_queue.rb @@ -48,11 +48,16 @@ class Mailings::OutboundQueue end def next_queued + # Sceglie il destinatario piu vecchio (per priorita/updated_at) TRA le mailing + # la cui finestra di invio e attualmente aperta. Prima si sceglieva il piu vecchio + # in assoluto: se apparteneva a una mailing con finestra chiusa, drain_one! + # restituiva :closed e si fermava, bloccando l'intera coda condivisa anche per + # le mailing senza vincoli orari (bug: coda bloccata da mailing con orario chiuso). queued_scope.order( Arel.sql( "CASE WHEN mailing_recipients.error_message IS NULL OR mailing_recipients.error_message = '' THEN 0 ELSE 1 END, mailing_recipients.updated_at ASC, mailing_recipients.id ASC" ) - ).first + ).includes(:mailing).find { |r| Mailings::SendClock.new(r.mailing).open? } end def queued_scope diff --git a/app/views/mailings/audience.html.erb b/app/views/mailings/audience.html.erb index fa5e1b2..697cfa1 100644 --- a/app/views/mailings/audience.html.erb +++ b/app/views/mailings/audience.html.erb @@ -29,7 +29,7 @@

Chi deve ricevere questa email?

-

Lista di partenza, sport, regione e chi non ha mai ricevuto una email. Il resto è opzionale.

+

Lista di partenza, sport, nazione, regione e chi non ha mai ricevuto una email. Il resto è opzionale.

@@ -47,12 +47,17 @@ -
+
<%= render "mailings/audience_choices", name: "filters[sports]", choices: @sport_options.map { |sport| [sport, sport] }, selected: @filters["sports"] %>
+
+ + <%= render "mailings/audience_choices", name: "filters[countries]", + choices: @country_options.map { |country| [country, country] }, selected: @filters["countries"] %> +
<%= render "mailings/audience_choices", name: "filters[regions]", diff --git a/test/services/mailings/outbound_queue_test.rb b/test/services/mailings/outbound_queue_test.rb index 7e17392..cc8bc1d 100644 --- a/test/services/mailings/outbound_queue_test.rb +++ b/test/services/mailings/outbound_queue_test.rb @@ -85,6 +85,33 @@ class Mailings::OutboundQueueTest < ActiveSupport::TestCase assert_equal "queued", recipient_a.status assert_match(/Da ritentare \(1\/8\)/, recipient_a.error_message) assert_equal recipient_b.id, Mailings::OutboundQueue.next_queued.id + + # Il contatore deve avanzare sullo stesso destinatario (prima il regex + # non matchava "Da ritentare (N/8)" e restava sempre tentativo 1). + begin + gate.define_singleton_method(:deliver) { raise EOFError, "end of file reached" } + assert_equal :deferred, recipient_a.deliver_queued! + ensure + gate.define_singleton_method(:deliver, original) + end + assert_match(/Da ritentare \(2\/8\)/, recipient_a.reload.error_message) + end + + test "next_queued skips recipients whose send window is closed" do + open_org = create_campaign_org(name: "Club Aperto", email: "aperto@example.com") + closed_org = create_campaign_org(name: "Club Chiuso", email: "chiuso@example.com") + open_mailing = create_sending_mailing(name: "Finestra aperta") + closed_mailing = create_sending_mailing(name: "Finestra chiusa") + closed_mailing.update!(send_window_enabled: true, send_window_start_minutes: 8 * 60, send_window_end_minutes: 9 * 60) + open_mailing.update!(send_window_enabled: false) + + closed_recipient = enqueue_org(closed_mailing, closed_org) + open_recipient = enqueue_org(open_mailing, open_org) + closed_recipient.update_columns(updated_at: 1.hour.ago) + + travel_to Time.zone.local(2026, 9, 10, 20, 0, 0) do + assert_equal open_recipient.id, Mailings::OutboundQueue.next_queued.id + end end private diff --git a/test/services/mailings_audience_query_test.rb b/test/services/mailings_audience_query_test.rb index be145d2..214c434 100644 --- a/test/services/mailings_audience_query_test.rb +++ b/test/services/mailings_audience_query_test.rb @@ -30,6 +30,14 @@ class MailingsAudienceQueryTest < ActiveSupport::TestCase assert_not_includes ids, @volley.id end + test "filters by country" do + @volley.update!(country: "Francia") + @acme.update!(country: "Italia") + ids = query({ "preset" => "all", "countries" => ["Francia"] }).relation.pluck(:id) + assert_includes ids, @volley.id + assert_not_includes ids, @acme.id + end + test "filters by list position range" do ids = query({ "preset" => "all", "list_min" => 1, "list_max" => 5 }).relation.pluck(:id) assert_includes ids, @acme.id