Add a field to track errors of notify_by_email

Author: ozovalihasan

app/controllers/api/v2/version_controller.rb

@@ -21,7 +21,7 @@ def features
       },
       pushes: {
         enabled: true,
-        email_auto_dispatch: Settings.mail.smtp_address.present?,
+        email_auto_dispatch: Settings.notify_by_email_available?,
         file_attachments: {
           enabled: Settings.enable_file_pushes,
           requires_authentication: true

app/jobs/send_push_created_email_job.rb

@@ -4,7 +4,25 @@ class SendPushCreatedEmailJob < ApplicationJob
   queue_as :default
 
   def perform(notify_by_email_id)
-    notify_by_email = NotifyByEmail.find_by!(id: notify_by_email_id)
+    notify_by_email = NotifyByEmail.find_by(id: notify_by_email_id)
+
+    if notify_by_email.nil?
+      Rails.logger.error "[SendPushCreatedEmailJob] NotifyByEmail not found: #{notify_by_email_id}"
+
+      return
+    end
+
+    if notify_by_email.recipients.blank?
+      notify_by_email.update(status: :failed, error_message: _("No recipients found."))
+
+      return
+    end
+
+    unless Settings.notify_by_email_available?
+      notify_by_email.update(status: :failed, error_message: _("Email notifications are not available."))
+
+      return
+    end
 
     return unless notify_by_email.pending?
 
@@ -14,6 +32,12 @@ def perform(notify_by_email_id)
     locale = notify_by_email.locale
     recipients = notify_by_email.recipients.split(",").map(&:strip)
 
+    if push.expired?
+      notify_by_email.update(status: :failed, error_message: _("Push already expired."))
+
+      return
+    end
+
     successful_sends = []
     recipients.each do |recipient|
       mail = PushCreatedMailer.with(push: push, recipient: recipient, locale: locale).notify
@@ -23,14 +47,19 @@ def perform(notify_by_email_id)
       Rails.logger.error "[SendPushCreatedEmailJob] Error sending email: #{e.message}"
     end
 
-    status = if successful_sends.size == recipients.size
-      :completed
+    status, error_message = if successful_sends.size == recipients.size
+      [:completed, nil]
     elsif successful_sends.empty?
-      :fully_failed
+      [:failed, I18n._("No emails were sent successfully.")]
     else
-      :partially_failed
+      [:partially_failed, I18n._("Some emails could not be sent.")]
     end
 
-    notify_by_email.update!(successful_sends: successful_sends.join(","), status: status, proceed_at: Time.current)
+    notify_by_email.update!(successful_sends: successful_sends.join(","), status: status, error_message: error_message, proceed_at: Time.current)
   end
+
+rescue => e
+  Rails.logger.error "[SendPushCreatedEmailJob] Error sending email: #{e.message}"
+
+  notify_by_email.update(status: :failed, error_message: e.message)
 end

app/models/concerns/pwpush/notifiable_by_email.rb

@@ -18,7 +18,7 @@ module NotifiableByEmail
       validate :notify_by_email_limit
 
       def notify_by_email_available?(cur_user)
-        Settings.mail.smtp_address.present? && !Settings.disable_logins && cur_user.present? && (!persisted? || (cur_user == user))
+        Settings.notify_by_email_available? && cur_user.present? && (!persisted? || (cur_user == user))
       end
     end
 
@@ -46,7 +46,7 @@ def total_notify_by_emails_count
     end
 
     def notify_by_email_availability
-      if Settings.disable_logins || Settings.mail.smtp_address.blank?
+      unless Settings.notify_by_email_available?
         errors.add(:base, _("Notifying by email is not available"))
       end
 

app/models/notify_by_email.rb

@@ -3,14 +3,14 @@
 class NotifyByEmail < ApplicationRecord
   attr_readonly :recipients, :recipients_count, :locale
 
-  enum :status, [:pending, :processing, :completed, :partially_failed, :fully_failed], validate: true
+  enum :status, [:pending, :processing, :completed, :partially_failed, :failed], validate: true
 
   before_create :set_recipients_count
 
   belongs_to :audit_log
 
   has_one :push, through: :audit_log
-  has_encrypted :recipients, :locale, :successful_sends
+  has_encrypted :recipients, :locale, :successful_sends, :error
 
   private
 

app/views/audit_logs/_log_creation_email_send.html.erb

@@ -4,7 +4,7 @@
     when "completed" then "list-group-item-success"
     when "processing", "pending" then "list-group-item-info"
     when "partially_failed" then "list-group-item-warning"
-    when "fully_failed" then "list-group-item-danger"
+    when "failed" then "list-group-item-danger"
     else "list-group-item-info"
   end %>
 
@@ -13,7 +13,7 @@
     when "processing" then "bi-hourglass-split"
     when "pending" then "bi-envelope"
     when "partially_failed" then "bi-envelope-exclamation"
-    when "fully_failed" then "bi-envelope-x"
+    when "failed" then "bi-envelope-x"
     else "bi-envelope"
   end %>
 
@@ -39,6 +39,12 @@
       </div>
     <% end %>
 
+    <% if notify_by_email.error_message.present? %>
+      <div>
+        <strong><%= _('Error:') %></strong> <span><%= notify_by_email.error_message %></span>
+      </div>
+    <% end %>
+
     <div>
       <strong><%= _('Proceed At:') %></strong> <span><%= notify_by_email.proceed_at.blank? ? _('Not yet') : local_time(notify_by_email.proceed_at, :long) %></span>
     </div>

config/initializers/config.rb

@@ -58,3 +58,9 @@
   #
   # config.evaluate_erb_yaml = true
 end
+
+Rails.application.config.to_prepare do
+  Settings.define_singleton_method(:notify_by_email_available?) do
+    !Settings.disable_logins && Settings.mail&.smtp_address.present?
+  end
+end

db/migrate/20260427190300_create_notify_by_emails.rb

@@ -9,6 +9,7 @@ def change
       t.text :successful_sends_ciphertext
       t.string :locale_ciphertext
       t.integer :status, default: 0, null: false
+      t.text :error_message
 
       t.datetime :proceed_at
       t.timestamps

db/schema.rb

@@ -65,6 +65,7 @@
   create_table "notify_by_emails", force: :cascade do |t|
     t.integer "audit_log_id", null: false
     t.datetime "created_at", null: false
+    t.text "error_message"
     t.string "locale_ciphertext"
     t.datetime "proceed_at"
     t.text "recipients_ciphertext", null: false

test/unit/send_push_created_email_job_test.rb

@@ -28,7 +28,7 @@ class SendPushCreatedEmailJobTest < ActiveJob::TestCase
     assert_equal "one@example.com", @notify_by_email.successful_sends
   end
 
-  test "perform update notify_by_email status to fully_failed after sending" do
+  test "perform update notify_by_email status to failed after sending" do
     failing_mail = Minitest::Mock.new
     failing_mail.expect(:deliver_now, -> { raise StandardError, "test error" })
 
@@ -37,8 +37,9 @@ class SendPushCreatedEmailJobTest < ActiveJob::TestCase
     end
 
     @notify_by_email.reload
-    assert_equal "fully_failed", @notify_by_email.status
+    assert_equal "failed", @notify_by_email.status
     assert_equal "", @notify_by_email.successful_sends
+    assert_equal "No emails were sent successfully.", @notify_by_email.error_message
   end
 
   test "perform does not send mail when notify_by_email is not pending" do
@@ -54,15 +55,21 @@ class SendPushCreatedEmailJobTest < ActiveJob::TestCase
     SendPushCreatedEmailJob.perform_now(@notify_by_email.id)
 
     @notify_by_email.reload
-    assert_equal "completed", @notify_by_email.status
+    assert_equal "failed", @notify_by_email.status
     assert @notify_by_email.successful_sends.blank?
+    assert_equal "No recipients found.", @notify_by_email.error_message
   end
 
-  test "perform raises an error if notify_by_email is not found" do
-    assert_raise ActiveRecord::RecordNotFound do
-      invalid_id = -1
+  test "perform logs error and does not send mail if notify_by_email is not found" do
+    invalid_id = -1
+    logger = Minitest::Mock.new
+    logger.expect(:error, nil, ["[SendPushCreatedEmailJob] NotifyByEmail not found: #{invalid_id}"])
+
+    Rails.stub(:logger, logger) do
       SendPushCreatedEmailJob.perform_now(invalid_id)
     end
+
+    assert logger.verify
   end
 
   test "perform uses default queue" do