From 668e272cfd5e3901f5c93797c01f2b55efd590bf Mon Sep 17 00:00:00 2001
From: Martin Edenhofer <me@znuny.com>
Date: Tue, 9 Apr 2019 11:30:43 +0200
Subject: [PATCH] Fixed issue #2541 - Existing scheduler email notification
 without `body` will raise an exception.

---
 ...541_fix_notification_email_without_body.rb | 29 +++++++++
 ...ix_notification_email_without_body_spec.rb | 61 +++++++++++++++++++
 2 files changed, 90 insertions(+)
 create mode 100644 db/migrate/20190408000001_issue_2541_fix_notification_email_without_body.rb
 create mode 100644 spec/db/migrate/issue_2541_fix_notification_email_without_body_spec.rb

diff --git a/db/migrate/20190408000001_issue_2541_fix_notification_email_without_body.rb b/db/migrate/20190408000001_issue_2541_fix_notification_email_without_body.rb
new file mode 100644
index 000000000..bbf8e5c55
--- /dev/null
+++ b/db/migrate/20190408000001_issue_2541_fix_notification_email_without_body.rb
@@ -0,0 +1,29 @@
+class Issue2541FixNotificationEmailWithoutBody < ActiveRecord::Migration[5.1]
+  def up
+
+    # return if it's a new setup
+    return if !Setting.find_by(name: 'system_init_done')
+
+    # update jobs and triggers
+    [::Job, ::Trigger].each do |model|
+      model.all.each do |record|
+        next if record.perform.blank?
+
+        %w[notification.email notification.sms].each do |action|
+          next if record.perform[action].blank?
+          next if record.perform[action]['body'].present?
+
+          record.perform[action]['body'] = '-'
+          record.save!
+        end
+      end
+    end
+
+    # re-enable jobs again
+    scheduler = Scheduler.find_by(method: 'Job.run')
+    return if !scheduler
+    return if scheduler.active?
+
+    scheduler.update!(active: true)
+  end
+end
diff --git a/spec/db/migrate/issue_2541_fix_notification_email_without_body_spec.rb b/spec/db/migrate/issue_2541_fix_notification_email_without_body_spec.rb
new file mode 100644
index 000000000..2d7bb9ecc
--- /dev/null
+++ b/spec/db/migrate/issue_2541_fix_notification_email_without_body_spec.rb
@@ -0,0 +1,61 @@
+require 'rails_helper'
+
+RSpec.describe Issue2541FixNotificationEmailWithoutBody, type: :db_migration do
+
+  context 'when migrating Triggers' do
+
+    before(:all) { Trigger.skip_callback(:create, :before, :validate_perform) }
+
+    it "updates empty perform['notification.email']['body'] attribute" do
+      trigger = create(:trigger,
+                       perform: {
+                         'notification.email' => {
+                           'body'      => '',
+                           'recipient' => 'article_last_sender',
+                           'subject'   => 'Thanks for your inquiry (#{ticket.title})', # rubocop:disable Lint/InterpolationCheck
+                         },
+                       })
+
+      expect { migrate }.to change { trigger.reload.perform['notification.email']['body'] }.from('').to('-')
+    end
+
+    it "updates empty perform['notification.sms']['body'] attribute" do
+      trigger = create(:trigger,
+                       perform: {
+                         'notification.sms' => {
+                           'body'      => '',
+                           'recipient' => 'article_last_sender',
+                         },
+                       })
+
+      expect { migrate }.to change { trigger.reload.perform['notification.sms']['body'] }.from('').to('-')
+    end
+  end
+
+  context 'when migrating Jobs' do
+
+    before(:all) { Job.skip_callback(:create, :before, :validate_perform) }
+
+    it "updates empty perform['notification.email']['body'] attribute" do
+
+      job = create(:job,
+                   perform: {
+                     'notification.email' => {
+                       'body'      => '',
+                       'recipient' => 'article_last_sender',
+                       'subject'   => 'Thanks for your inquiry (#{ticket.title})', # rubocop:disable Lint/InterpolationCheck
+                     },
+                   },)
+
+      expect { migrate }.to change { job.reload.perform['notification.email']['body'] }.from('').to('-')
+    end
+  end
+
+  it "re-enables 'Job.run' Scheduler" do
+    scheduler = Scheduler.find_by(method: 'Job.run')
+    scheduler.update!(active: false)
+
+    expect { migrate }.to change { scheduler.reload.active }.to(true)
+  end
+
+end