#266: Exchange not sending attachments due to mistake during copy-paste

Fixed #266

lib/lazylead/email.rb

@@ -25,26 +25,6 @@
 require "tilt"
 
 module Lazylead
-  # Email notifications utilities.
-  module Emailing
-    # Construct html document from template and binds.
-    def make_body(opts)
-      Email.new(
-        opts["template"],
-        opts.merge(version: Lazylead::VERSION)
-      ).render
-    end
-
-    # Split text with email addresses by ',' and trim all elements if needed.
-    def split(type, opts)
-      if opts[type].include? ","
-        opts[type].split(",").map(&:strip).reject(&:empty?)
-      else
-        [opts[type]]
-      end
-    end
-  end
-
   # An email regarding tickets based on file with markup.
   #
   # The 'tilt' gem was used as a template engine.

lib/lazylead/exchange.rb

@@ -39,8 +39,6 @@ module Lazylead
   # Copyright:: Copyright (c) 2019-2020 Yurii Dubinka
   # License:: MIT
   class Exchange
-    include Emailing
-
     def initialize(
       log = Log.new, salt = Salt.new("exchange_salt"), opts = ENV.to_h
     )
@@ -52,38 +50,28 @@ def initialize(
     # Send an email.
     # :opts   :: the mail configuration like from, cc, subject, template.
     def send(opts)
-      to = opts["to"] || opts[:to]
-      to = [to] unless to.is_a? Array
-      if to.reject { |e| e.nil? || e.blank? }.empty?
-        @log.warn "Email can't be sent to '#{to}, more: '#{opts}'"
-        return
+      if opts.msg_to.empty?
+        @log.warn "ll-012: Email can't be sent to '#{opts.msg_to}," \
+                  " more: '#{opts}'"
+      else
+        msg = make_msg opts
+        cli.send_message msg
+        msg[:file_attachments].each(&:close) unless opts.msg_attachments.empty?
+        @log.debug "#{__FILE__} sent '#{opts['subject']}' to '#{opts.msg_to}'."
       end
-      msg = make_msg(to, opts)
-      msg.update(cc_recipients: opts["cc"]) if opts.key? "cc"
-      add_attachments(msg, opts)
-      cli.send_message msg
-      close_attachments msg
-      @log.debug "#{__FILE__} sent '#{opts['subject']}' to '#{to}'."
     end
 
-    def make_msg(to, opts)
-      {
+    def make_msg(opts)
+      msg = {
         subject: opts["subject"],
-        body: make_body(opts),
+        body: opts.msg_body,
         body_type: "HTML",
-        to_recipients: to
+        to_recipients: opts.msg_to
       }
-    end
-
-    def add_attachments(msg, opts)
-      return unless opts.key? "attachments"
-      files = split("attachments", opts).map { |f| File.open(f, "r") }
-      msg[:file_attachments] = files
-    end
-
-    def close_attachments(msg)
-      return if msg[:file_attachments].nil? || msg[:file_attachments].empty?
-      msg[:file_attachments].each(&:close)
+      files = opts.msg_attachments.map { |f| File.open(f, "r") }
+      msg[:file_attachments] = files unless files.empty?
+      msg[:cc_recipients] = opts["cc"] if opts.key? "cc"
+      msg
     end
 
     private

lib/lazylead/opts.rb

@@ -34,7 +34,8 @@ module Lazylead
   # License:: MIT
   class Opts
     extend Forwardable
-    def_delegators :@origin, :[], :[]=, :to_s, :key?, :fetch, :merge, :except
+    def_delegators :@origin, :[], :[]=, :to_s, :key?, :fetch, :except, :each,
+                   :each_pair, :sort_by
 
     def initialize(origin = {})
       @origin = origin
@@ -77,5 +78,51 @@ def decrypt(key, sid)
       return Salt.new(sid).decrypt(text) if ENV.key? sid
       text
     end
+
+    def merge(args)
+      return self unless args.is_a? Hash
+      Opts.new @origin.merge(args)
+    end
+
+    # Construct html document from template and binds.
+    def msg_body
+      Email.new(
+        to_h["template"],
+        to_h.merge(version: Lazylead::VERSION)
+      ).render
+    end
+
+    def msg_to(delim = ",")
+      sliced delim, :to, "to"
+    end
+
+    def msg_cc(delim = ",")
+      sliced delim, :cc, "cc"
+    end
+
+    def msg_from(delim = ",")
+      sliced delim, :from, "from"
+    end
+
+    def msg_attachments(delim = ",")
+      sliced(delim, :attachments, "attachments").select { |f| File.file? f }
+    end
+
+    #
+    # Find the option by key and split by delimiter
+    #   Opts.new("key" => "a,b").sliced(",", "key")     => [a, b]
+    #   Opts.new(key: "a,b").sliced(",", :key)          => [a, b]
+    #   Opts.new(key: "a,b").sliced(",", "key", :key)   => [a, b]
+    #   Opts.new(key: "").sliced ",", :key)             => []
+    #
+    def sliced(delim, *keys)
+      return [] if keys.empty?
+      key = keys.detect { |k| key? k }
+      val = to_h[key]
+      return [] if val.nil? || val.blank?
+      return val if val.is_a? Array
+      return [val] unless val.include? delim
+      slice key, delim
+    end
   end
 end

lib/lazylead/postman.rb

@@ -44,41 +44,36 @@ module Lazylead
   # Copyright:: Copyright (c) 2019-2020 Yurii Dubinka
   # License:: MIT
   class Postman
-    include Emailing
-
     def initialize(log = Log.new)
       @log = log
     end
 
     # Send an email.
     # :opts   :: the mail configuration like to, from, cc, subject, template.
     def send(opts)
-      mail = make_email(opts)
-      mail.deliver
-      @log.debug "#{__FILE__} sent '#{mail.subject}' to '#{mail.to}'."
+      if opts.msg_to.empty?
+        @log.warn "ll-013: Email can't be sent to '#{opts.msg_to}," \
+                  " more: '#{opts}'"
+      else
+        mail = make_email(opts)
+        mail.deliver
+        @log.debug "#{__FILE__} sent '#{mail.subject}' to '#{mail.to}'."
+      end
     end
 
     # Construct an email based on input arguments
     def make_email(opts)
       mail = Mail.new
-      mail.to opts[:to] || opts["to"]
-      mail.from opts["from"]
-      mail.cc opts["cc"] if opts.key? "cc"
+      mail.to opts.msg_to
+      mail.from opts.msg_from
+      mail.cc opts.msg_cc if opts.key? "cc"
       mail.subject opts["subject"]
-      html = make_body(opts)
       mail.html_part do
         content_type "text/html; charset=UTF-8"
-        body html
+        body opts.msg_body
       end
-      add_attachments mail, opts
+      opts.msg_attachments.each { |f| mail.add_file f }
       mail
     end
-
-    def add_attachments(mail, opts)
-      return unless opts.key?("attachments") || opts.key?(:attachments)
-      attach = opts["attachments"] || opts[:attachments]
-      return if attach.nil?
-      attach.select { |a| File.file? a }.each { |a| mail.add_file a }
-    end
   end
 end

test/lazylead/opts_test.rb

@@ -66,5 +66,40 @@ class OptsTest < Lazylead::Test
     test "except keys" do
       assert_equal 1, Opts.new("one" => "1", "two" => "2").except("one").size
     end
+
+    test "attachment is blank" do
+      assert_empty Opts.new("attachments" => "").msg_attachments
+    end
+
+    test "attachment is nil" do
+      assert_empty Opts.new("attachments" => nil).msg_attachments
+    end
+
+    test "attachment is absent" do
+      assert_empty Opts.new({}).msg_attachments
+    end
+
+    test "attachment is present" do
+      assert_equal %w[readme.md],
+                   Opts.new("attachments" => "readme.md").msg_attachments
+    end
+
+    test "attachments are present" do
+      assert_equal %w[readme.md license.txt],
+                   Opts.new("attachments" => "readme.md,license.txt")
+                       .msg_attachments
+    end
+
+    test "attachments are present as symbol" do
+      assert_equal %w[readme.md license.txt],
+                   Opts.new(attachments: " readme.md , license.txt ")
+                       .msg_attachments
+    end
+
+    test "attachments is present as array" do
+      assert_equal %w[readme.md license.txt],
+                   Opts.new(attachments: %w[readme.md license.txt])
+                       .msg_attachments
+    end
   end
 end

test/lazylead/postman_test.rb

@@ -24,6 +24,7 @@
 
 require_relative "../test"
 require_relative "../../lib/lazylead/log"
+require_relative "../../lib/lazylead/opts"
 require_relative "../../lib/lazylead/salt"
 require_relative "../../lib/lazylead/smtp"
 require_relative "../../lib/lazylead/postman"
@@ -46,11 +47,13 @@ class PostmanTest < Lazylead::Test
         smtp_pass: ENV["LL_SMTP_PASS"]
       ).enable
       Postman.new.send(
-        "to" => ENV["LL_SMTP_TO"],
-        "from" => ENV["LL_SMTP_FROM"],
-        "attachments" => ["readme.md"],
-        "subject" => "[LL] Attachments",
-        "template" => "lib/messages/savepoint.erb"
+        Opts.new(
+          "to" => ENV["LL_SMTP_TO"],
+          "from" => ENV["LL_SMTP_FROM"],
+          "attachments" => ["readme.md"],
+          "subject" => "[LL] Attachments",
+          "template" => "lib/messages/savepoint.erb"
+        )
       )
     end
   end

test/lazylead/task/savepoint_test.rb

@@ -24,6 +24,7 @@
 
 require_relative "../../test"
 require_relative "../../../lib/lazylead/smtp"
+require_relative "../../../lib/lazylead/opts"
 require_relative "../../../lib/lazylead/postman"
 require_relative "../../../lib/lazylead/task/savepoint"
 
@@ -36,10 +37,12 @@ class SavepointTest < Lazylead::Test
       Task::Savepoint.new.run(
         [],
         Postman.new,
-        "from" => "fake@email.com",
-        "subject" => "[LL] Configuration backup",
-        "template" => "lib/messages/savepoint.erb",
-        "to" => "big.boss@example.com"
+        Opts.new(
+          "from" => "fake@email.com",
+          "subject" => "[LL] Configuration backup",
+          "template" => "lib/messages/savepoint.erb",
+          "to" => "big.boss@example.com"
+        )
       )
       assert_equal 'text/markdown; filename="readme.md"',
                    Mail::TestMailer.deliveries