improvement: track email devivery more accurately

app/jobs/mail_delivery_job.rb

@@ -2,6 +2,6 @@
 
 class MailDeliveryJob < ActionMailer::MailDeliveryJob
  retry_on Net::SMTPFatalError, Net::SMTPAuthenticationError, Net::ReadTimeout, wait: 30.seconds
- discard_on ActiveJob::DeserializationError
+ # discard_on ActiveJob::DeserializationError
  sidekiq_options retry: 5
 end

app/mailers/organisation_mailer.rb

@@ -1,6 +1,9 @@
 # frozen_string_literal: true
 
 class OrganisationMailer < ApplicationMailer
+ BOOKING_HEADER_NAME = 'X-Heimv-Booking-Id'
+ NOTIFICATION_HEADER_NAME = 'X-Heimv-Notification-Id'
+
  default from: -> { (@organisation.mail_from || @organisation.email) },
  reply_to: -> { @organisation.email },
  charset: 'UTF-8'
@@ -11,8 +14,10 @@ class OrganisationMailer < ApplicationMailer
  def booking_email(notification)
  @organisation = notification.organisation
  @notification = notification
- attach_active_storage_attachments(notification.attachments)
+ return if @notification.blank?
 
+ set_headers
+ attach_active_storage_attachments(notification.attachments)
  mail(to: notification.to, cc: notification.cc, bcc: notification.bcc, subject: notification.subject) do |format|
  format.text { render plain: @notification.text }
  # rubocop:disable Rails/OutputSafety
@@ -35,6 +40,11 @@ def attach_active_storage_attachments(values)
  end
  end
 
+ def set_headers
+ headers[NOTIFICATION_HEADER_NAME] = @notification.to_param
+ headers[BOOKING_HEADER_NAME] = @notification.booking.to_param
+ end
+
  def set_delivery_options
  return unless Rails.env.production? && @organisation.smtp_settings.present?
 
@@ -46,4 +56,16 @@ def set_organisation
  @organisation ||= params
  end
  # rubocop:enable Naming/MemoizedInstanceVariableName
+
+ class DeliveryObserver
+ def self.delivered_email(mail)
+ notification = Notification.find_by(id: mail.header[NOTIFICATION_HEADER_NAME]&.value)
+
+ # rubocop:disable Rails/SkipsModelValidations
+ notification&.touch(:sent_at)
+ # rubocop:enable Rails/SkipsModelValidations
+ end
+ end
+
+ register_observer DeliveryObserver
 end

app/models/booking_question_response.rb

@@ -31,18 +31,22 @@ class BookingQuestionResponse < ApplicationRecord
  end
 
  def self.process_nested_attributes(booking, attributes)
- existing_responses = booking.booking_question_responses.index_by(&:booking_question_id)
+ existing_responses = indexed_by_booking_question_id(booking)
  questions = BookingQuestion.applying_to_booking(booking).index_by(&:id)
  attributes&.values&.map do |attribute_set|
  question = questions[attribute_set[:booking_question_id]&.to_i]
- next unless question.present?
+ next if question.blank?
 
- response = existing_responses.fetch(question.id, nil) || booking.booking_question_responses.build
- response&.assign_attributes(booking_question: question, value: question.cast(attribute_set[:value]))
+ response = existing_responses.fetch(question.id, booking.booking_question_responses.build)
+ response.assign_attributes(booking_question: question, value: question.cast(attribute_set[:value]))
  response
  end
  end
 
+ def self.indexed_by_booking_question_id(booking)
+ (booking&.booking_question_responses.presence || []).index_by(&:booking_question_id)
+ end
+
  def self.prepare_booking(booking)
  booking.booking_questions = BookingQuestion.applying_to_booking(booking)
  end

app/models/data_digest_template.rb

@@ -81,7 +81,7 @@ def digest(period = nil)
  end
 
  def columns
- @columns ||= (columns_config || self.class::DEFAULT_COLUMN_CONFIG).map do |config|
+ @columns ||= (columns_config.presence || self.class::DEFAULT_COLUMN_CONFIG).map do |config|
  config.symbolize_keys!
  column_type = config.fetch(:type, :default)&.to_sym
  self.class.column_types.fetch(column_type, ColumnType.new).column_from_config(config)

app/models/notification.rb

@@ -57,9 +57,7 @@ def deliverable?
  end
 
  def deliver
- return false unless deliverable? && update!(sent_at: Time.zone.now)
-
- message_delivery.tap(&:deliver_later)
+ deliverable? && save && message_delivery.tap(&:deliver_later)
  end
 
  def attach(*files_or_documents_to_attach)

spec/models/notification_spec.rb

@@ -78,11 +78,13 @@
  subject(:message) { notification.deliver }
 
  it do
- is_expected.to be_truthy
+ expect(notification.sent_at).to be_nil
+ expect { message }.to change { ActionMailer::Base.deliveries.count }.by(1)
  expect(message.to).to eq(notification.to)
  expect(message.cc).to eq(notification.cc)
  expect(message.subject).to eq(notification.subject)
- expect(notification.sent_at).not_to be nil
+ notification.reload
+ expect(notification.sent_at).not_to be_nil
  end
  end