Refactor NotificationFactory and support custom templates (replaces #…

…2081)

lib/notification_factory.rb

@@ -1,4 +1,6 @@
 module NotificationFactory
+  TEMPLATE_PATH_STRING = Rails.root.join('app', 'views', '%<type>s', '%<template>s', '%<filename>s').to_s.freeze
+  APPLICATION_TEMPLATE_PATH_STRING = Rails.root.join('app', 'views', '%<type>s', 'application.%<format>s.erb').to_s.freeze
 
 =begin
 
@@ -28,39 +30,28 @@ module NotificationFactory
 =end
 
   def self.template_read(data)
+    template = File.readlines(template_path(data))
 
-    template_subject = nil
-    template_body = ''
+    { subject: template.shift, body: template.join }
+  end
+
+  def self.template_path(data)
+    template_filenames(data)
+      .map { |filename| data.merge(filename: filename) }
+      .map { |data_hash| TEMPLATE_PATH_STRING % data_hash }
+      .find(&File.method(:exist?))
+  end
+  private_class_method :template_path
+
+  def self.template_filenames(data)
     locale = data[:locale] || Setting.get('locale_default') || 'en-us'
-    template = data[:template]
-    format = data[:format]
-    type = data[:type]
-    root = Rails.root
-    location = "#{root}/app/views/#{type}/#{template}/#{locale}.#{format}.erb"
-
-    # as fallback, use 2 char locale
-    if !File.exist?(location)
-      locale = locale[0, 2]
-      location = "#{root}/app/views/#{type}/#{template}/#{locale}.#{format}.erb"
-    end
-
-    # as fallback, use en
-    if !File.exist?(location)
-      location = "#{root}/app/views/#{type}/#{template}/en.#{format}.erb"
-    end
-
-    File.open(location, 'r:UTF-8').each do |line|
-      if !template_subject
-        template_subject = line
-        next
-      end
-      template_body += line
-    end
-    {
-      subject: template_subject,
-      body: template_body,
-    }
+
+    [locale, locale[0, 2], 'en']
+      .uniq
+      .map { |locale_code| "#{locale_code}.#{data[:format]}.erb" }
+      .map { |basename| ["#{basename}.custom", basename] }.flatten
   end
+  private_class_method :template_filenames
 
 =begin
 
@@ -83,14 +74,7 @@ def self.template_read(data)
 =end
 
   def self.application_template_read(data)
-    format = data[:format]
-    type = data[:type]
-    root = Rails.root
-    application_template = nil
-    File.open("#{root}/app/views/#{type}/application.#{format}.erb", 'r:UTF-8') do |file|
-      application_template = file.read
-    end
-    application_template
+    File.read(APPLICATION_TEMPLATE_PATH_STRING % data)
   end
 
 end

spec/lib/notification_factory_spec.rb

@@ -0,0 +1,97 @@
+require 'rails_helper'
+
+RSpec.describe NotificationFactory do
+  # WARNING: This spec relies on the presence of
+  #          *actual* view templates in the app/ directory.
+  #          Deleting them from the repo will break the tests!
+  describe '::template_read' do
+    let(:rendered_locale) { 'en' }
+    let(:parsed_template) { { subject: template_lines.first, body: template_lines.drop(1).join } }
+    let(:template_lines) { File.readlines(template_path) }
+    let(:template_path) { Rails.root.join('app', 'views', 'mailer', 'signup', "#{rendered_locale}.html.erb") }
+
+    let(:read_params) do
+      { type: 'mailer', template: 'signup', locale: 'en', format: 'html' }
+    end
+
+    it 'returns template file content as { subject: <first line>, body: <rest of file> }' do
+      expect(described_class.template_read(read_params))
+        .to eq(parsed_template)
+    end
+
+    context 'when selecting a template file to render' do
+      # see https://github.com/zammad/zammad/issues/845#issuecomment-395084348
+      context 'and file with ‘.custom’ suffix is available' do
+        let(:template_path) { Rails.root.to_s + "/app/views/mailer/signup/#{rendered_locale}.html.erb.custom" }
+
+        it 'uses that file' do
+          begin
+            File.write(template_path, "Subject\nBody\nbody\n")
+
+            expect(described_class.template_read(read_params))
+              .to eq({ subject: "Subject\n", body: "Body\nbody\n" })
+          ensure
+            File.delete(template_path)
+          end
+        end
+      end
+
+      context 'if no locale given in arguments, and no default locale is set' do
+        before { Setting.set('locale_default', nil) }
+
+        it 'renders en-us template' do
+          expect(described_class.template_read(read_params.except(:locale)))
+            .to eq(parsed_template)
+        end
+      end
+
+      context 'if no locale given in arguments, but default locale is set' do
+        before { Setting.set('locale_default', 'de-de') }
+        let(:rendered_locale) { 'de' }
+
+        it 'tries template for default locale' do
+          expect(described_class.template_read(read_params.except(:locale)))
+            .to eq(parsed_template)
+        end
+
+        context 'and no such template exists' do
+          before { Setting.set('locale_default', 'xx') }
+          let(:rendered_locale) { 'en' }
+
+          it 'falls back to en template' do
+            expect(described_class.template_read(read_params.except(:locale)))
+              .to eq(parsed_template)
+          end
+        end
+      end
+
+      context 'if locale given in arguments' do
+        let(:rendered_locale) { 'de' }
+
+        it 'tries template for given locale' do
+          expect(described_class.template_read(read_params.merge(locale: 'de-de')))
+            .to eq(parsed_template)
+        end
+
+        context 'and no such template exists' do
+          let(:rendered_locale) { 'en' }
+
+          it 'falls back to en template' do
+            expect(described_class.template_read(read_params.merge(locale: 'xx')))
+              .to eq(parsed_template)
+          end
+        end
+      end
+    end
+  end
+
+  describe '::application_template_read' do
+    let(:read_params) { { type: 'mailer', format: 'html' } }
+    let(:template_path) { Rails.root.join('app', 'views', 'mailer', 'application.html.erb') }
+
+    it 'returns template file content as string' do
+      expect(described_class.application_template_read(read_params))
+        .to eq(File.read(template_path))
+    end
+  end
+end