From 97dd5b4322f8ea6063e40b596d20ba041fadade1 Mon Sep 17 00:00:00 2001
From: Artur Beljajev <artur.beljajev@internet.ee>
Date: Sun, 7 Apr 2019 19:10:11 +0300
Subject: [PATCH] Refactor invoice PDF generation, download and delivery

- Remove `Que::Mailer` (#895)
- Extract controllers
- Extract translations
- Convert HAML to ERB
- Add mailer preview
- Improve UI
- Remove unused routes
- Add tests
---
 .../admin/invoices/delivery_controller.rb     | 13 ++++++++
 app/controllers/admin/invoices_controller.rb  | 31 +++----------------
 app/controllers/concerns/deliverable.rb       | 26 ++++++++++++++++
 .../registrar/invoices/delivery_controller.rb | 13 ++++++++
 .../registrar/invoices_controller.rb          | 30 +++---------------
 app/mailers/invoice_mailer.rb                 | 20 ++++--------
 app/models/invoice.rb                         | 27 ++++------------
 app/models/invoice/pdf_generator.rb           | 22 +++++++++++++
 .../admin/invoices/delivery/new.html.erb      | 25 +++++++++++++++
 app/views/admin/invoices/forward.haml         | 15 ---------
 app/views/admin/invoices/show.haml            |  4 +--
 .../{registrar/invoices => invoice}/pdf.haml  |  0
 .../registrar/invoices/delivery/new.html.erb  | 25 +++++++++++++++
 app/views/registrar/invoices/forward.haml     | 15 ---------
 app/views/registrar/invoices/show.haml        |  4 +--
 config/locales/admin/invoices.en.yml          |  7 +++++
 config/locales/admin/invoices/delivery.en.yml | 10 ++++++
 config/locales/en.yml                         |  5 ---
 config/locales/mailers/invoice.en.yml         |  4 +++
 config/locales/registrar/invoices.en.yml      |  6 ++++
 .../registrar/invoices/delivery.en.yml        | 10 ++++++
 config/routes.rb                              | 18 ++++++-----
 test/fixtures/invoices.yml                    |  1 +
 test/fixtures/registrars.yml                  |  2 +-
 test/integration/admin_area/invoices_test.rb  | 10 ++++--
 .../registrar_area/invoices_test.rb           | 10 ++++--
 test/mailers/invoice_mailer_test.rb           | 22 +++++++++++++
 .../previews/invoice_mailer_preview.rb        |  6 ++++
 test/system/admin_area/invoices_test.rb       | 23 ++++++++++++++
 test/system/registrar_area/invoices_test.rb   | 23 ++++++++++++++
 30 files changed, 288 insertions(+), 139 deletions(-)
 create mode 100644 app/controllers/admin/invoices/delivery_controller.rb
 create mode 100644 app/controllers/concerns/deliverable.rb
 create mode 100644 app/controllers/registrar/invoices/delivery_controller.rb
 create mode 100644 app/models/invoice/pdf_generator.rb
 create mode 100644 app/views/admin/invoices/delivery/new.html.erb
 delete mode 100644 app/views/admin/invoices/forward.haml
 rename app/views/{registrar/invoices => invoice}/pdf.haml (100%)
 create mode 100644 app/views/registrar/invoices/delivery/new.html.erb
 delete mode 100644 app/views/registrar/invoices/forward.haml
 create mode 100644 config/locales/admin/invoices/delivery.en.yml
 create mode 100644 config/locales/mailers/invoice.en.yml
 create mode 100644 config/locales/registrar/invoices/delivery.en.yml
 create mode 100644 test/mailers/invoice_mailer_test.rb
 create mode 100644 test/mailers/previews/invoice_mailer_preview.rb

diff --git a/app/controllers/admin/invoices/delivery_controller.rb b/app/controllers/admin/invoices/delivery_controller.rb
new file mode 100644
index 0000000000..b53f802697
--- /dev/null
+++ b/app/controllers/admin/invoices/delivery_controller.rb
@@ -0,0 +1,13 @@
+module Admin
+  module Invoices
+    class DeliveryController < BaseController
+      include Deliverable
+
+      private
+
+      def redirect_url
+        admin_invoice_path(@invoice)
+      end
+    end
+  end
+end
\ No newline at end of file
diff --git a/app/controllers/admin/invoices_controller.rb b/app/controllers/admin/invoices_controller.rb
index 10610983dc..c9f1d0c469 100644
--- a/app/controllers/admin/invoices_controller.rb
+++ b/app/controllers/admin/invoices_controller.rb
@@ -2,8 +2,6 @@ module Admin
   class InvoicesController < BaseController
     load_and_authorize_resource
 
-    before_action :set_invoice, only: [:forward, :download_pdf]
-
     def new
       @deposit = Deposit.new
     end
@@ -28,33 +26,16 @@ def index
       @invoices = @q.result.page(params[:page])
     end
 
-    def show
-      @invoice = Invoice.find(params[:id])
-    end
+    def show; end
 
     def cancel
       @invoice.cancel
       redirect_to [:admin, @invoice], notice: t('.cancelled')
     end
 
-    def forward
-      @invoice.billing_email = @invoice.buyer.billing_email
-
-      return unless request.post?
-
-      @invoice.billing_email = params[:invoice][:billing_email]
-
-      if @invoice.forward(render_to_string('registrar/invoices/pdf', layout: false))
-        flash[:notice] = t(:invoice_forwared)
-        redirect_to([:admin, @invoice])
-      else
-        flash.now[:alert] = t(:failed_to_forward_invoice)
-      end
-    end
-
-    def download_pdf
-      pdf = @invoice.pdf(render_to_string('registrar/invoices/pdf', layout: false))
-      send_data pdf, filename: @invoice.pdf_name
+    def download
+      filename = "invoice-#{@invoice.number}.pdf"
+      send_data @invoice.as_pdf, filename: filename
     end
 
     private
@@ -62,9 +43,5 @@ def download_pdf
     def deposit_params
       params.require(:deposit).permit(:amount, :description, :registrar_id)
     end
-
-    def set_invoice
-      @invoice = Invoice.find(params[:invoice_id])
-    end
   end
 end
diff --git a/app/controllers/concerns/deliverable.rb b/app/controllers/concerns/deliverable.rb
new file mode 100644
index 0000000000..0ceae00674
--- /dev/null
+++ b/app/controllers/concerns/deliverable.rb
@@ -0,0 +1,26 @@
+module Deliverable
+  extend ActiveSupport::Concern
+
+  included do
+    before_action :find_invoice
+  end
+
+  def new
+    authorize! :manage, @invoice
+    @recipient = @invoice.buyer.billing_email
+  end
+
+  def create
+    authorize! :manage, @invoice
+
+    InvoiceMailer.invoice_email(invoice: @invoice, recipient: params[:recipient]).deliver_now
+
+    redirect_to redirect_url, notice: t('.delivered')
+  end
+
+  private
+
+  def find_invoice
+    @invoice = Invoice.find(params[:invoice_id])
+  end
+end
\ No newline at end of file
diff --git a/app/controllers/registrar/invoices/delivery_controller.rb b/app/controllers/registrar/invoices/delivery_controller.rb
new file mode 100644
index 0000000000..bdee237990
--- /dev/null
+++ b/app/controllers/registrar/invoices/delivery_controller.rb
@@ -0,0 +1,13 @@
+class Registrar
+  module Invoices
+    class DeliveryController < BaseController
+      include Deliverable
+
+      private
+
+      def redirect_url
+        registrar_invoice_path(@invoice)
+      end
+    end
+  end
+end
\ No newline at end of file
diff --git a/app/controllers/registrar/invoices_controller.rb b/app/controllers/registrar/invoices_controller.rb
index 3736bfa6ab..9df92d1410 100644
--- a/app/controllers/registrar/invoices_controller.rb
+++ b/app/controllers/registrar/invoices_controller.rb
@@ -2,8 +2,6 @@ class Registrar
   class InvoicesController < BaseController
     load_and_authorize_resource
 
-    before_action :set_invoice, only: [:show, :forward, :download_pdf]
-
     def index
       params[:q] ||= {}
       invoices = current_registrar_user.registrar.invoices.includes(:items, :account_activity)
@@ -15,40 +13,20 @@ def index
       end
     end
 
-    def show;
-    end
-
-    def forward
-      @invoice.billing_email = @invoice.buyer.billing_email
-
-      return unless request.post?
-
-      @invoice.billing_email = params[:invoice][:billing_email]
-
-      if @invoice.forward(render_to_string('pdf', layout: false))
-        flash[:notice] = t(:invoice_forwared)
-        redirect_to([:registrar, @invoice])
-      else
-        flash.now[:alert] = t(:failed_to_forward_invoice)
-      end
-    end
+    def show; end
 
     def cancel
       @invoice.cancel
       redirect_to [:registrar, @invoice], notice: t('.cancelled')
     end
 
-    def download_pdf
-      pdf = @invoice.pdf(render_to_string('pdf', layout: false))
-      send_data pdf, filename: @invoice.pdf_name
+    def download
+      filename = "invoice-#{@invoice.number}.pdf"
+      send_data @invoice.as_pdf, filename: filename
     end
 
     private
 
-    def set_invoice
-      @invoice = Invoice.find(params[:id])
-    end
-
     def normalize_search_parameters
       params[:q][:total_gteq].gsub!(',', '.') if params[:q][:total_gteq]
       params[:q][:total_lteq].gsub!(',', '.') if params[:q][:total_lteq]
diff --git a/app/mailers/invoice_mailer.rb b/app/mailers/invoice_mailer.rb
index 74821c25a9..4f01b2c15a 100644
--- a/app/mailers/invoice_mailer.rb
+++ b/app/mailers/invoice_mailer.rb
@@ -1,17 +1,9 @@
 class InvoiceMailer < ApplicationMailer
-  include Que::Mailer
+  def invoice_email(invoice:, recipient:)
+    @invoice = invoice
 
-  def invoice_email(invoice_id, html, billing_email)
-    @invoice = Invoice.find_by(id: invoice_id)
-    billing_email ||= @invoice.billing_email
-    return unless @invoice
-    return if whitelist_blocked?(billing_email)
-
-    kit = PDFKit.new(html)
-    pdf = kit.to_pdf
-    invoice = @invoice
-
-    attachments[invoice.pdf_name] = pdf
-    mail(to: format(billing_email), subject: invoice)
+    subject = default_i18n_subject(invoice_number: invoice.number)
+    attachments["invoice-#{invoice.number}.pdf"] = invoice.as_pdf
+    mail(to: recipient, subject: subject)
   end
-end
+end
\ No newline at end of file
diff --git a/app/models/invoice.rb b/app/models/invoice.rb
index 6942ce5b85..98fc67f022 100644
--- a/app/models/invoice.rb
+++ b/app/models/invoice.rb
@@ -26,9 +26,6 @@ class Invoice < ActiveRecord::Base
 
   scope :overdue, -> { unpaid.non_cancelled.where('due_date < ?', Time.zone.today) }
 
-  attr_accessor :billing_email
-  validates :billing_email, email_format: { message: :invalid }, allow_blank: true
-
   validates :issue_date, presence: true
   validates :due_date, :currency, :seller_name,
             :seller_iban, :buyer_name, :items, presence: true
@@ -84,23 +81,6 @@ def order
     "Order nr. #{number}"
   end
 
-  def pdf(html)
-    kit = PDFKit.new(html)
-    kit.to_pdf
-  end
-
-  def pdf_name
-    "invoice-#{number}.pdf"
-  end
-
-  def forward(html)
-    return false unless valid?
-    return false unless billing_email.present?
-
-    InvoiceMailer.invoice_email(id, html, billing_email).deliver
-    true
-  end
-
   def subtotal
     items.map(&:item_sum_without_vat).reduce(:+)
   end
@@ -119,6 +99,11 @@ def each
     items.each { |item| yield item }
   end
 
+  def as_pdf
+    generator = PdfGenerator.new(self)
+    generator.as_pdf
+  end
+
   private
 
   def apply_default_vat_rate
@@ -132,4 +117,4 @@ def apply_default_buyer_vat_no
   def calculate_total
     self.total = subtotal + vat_amount
   end
-end
+end
\ No newline at end of file
diff --git a/app/models/invoice/pdf_generator.rb b/app/models/invoice/pdf_generator.rb
new file mode 100644
index 0000000000..2078fad204
--- /dev/null
+++ b/app/models/invoice/pdf_generator.rb
@@ -0,0 +1,22 @@
+class Invoice
+  class PdfGenerator
+    attr_reader :invoice
+
+    def initialize(invoice)
+      @invoice = invoice
+    end
+
+    def as_pdf
+      generator = PDFKit.new(invoice_html)
+      generator.to_pdf
+    end
+
+    private
+
+    def invoice_html
+      view = ActionView::Base.new(ActionController::Base.view_paths, invoice: invoice)
+      view.class_eval { include ApplicationHelper }
+      view.render(file: 'invoice/pdf', layout: false)
+    end
+  end
+end
\ No newline at end of file
diff --git a/app/views/admin/invoices/delivery/new.html.erb b/app/views/admin/invoices/delivery/new.html.erb
new file mode 100644
index 0000000000..413e09f051
--- /dev/null
+++ b/app/views/admin/invoices/delivery/new.html.erb
@@ -0,0 +1,25 @@
+<ol class="breadcrumb">
+    <li><%= link_to t('admin.invoices.index.header'), admin_invoices_path %></li>
+    <li><%= link_to @invoice, admin_invoice_path(@invoice) %></li>
+</ol>
+
+<div class="page-header">
+    <h1><%= t '.header' %></h1>
+</div>
+
+<%= form_tag(admin_invoice_delivery_path(@invoice)) do %>
+    <div class="row">
+        <div class="col-md-4 col-md-offset-4">
+            <div class="form-group">
+                <%= label_tag :recipient %>
+                <%= email_field_tag :recipient, @recipient, required: true, autofocus: true,
+                                    class: 'form-control' %>
+            </div>
+            <div class="row">
+                <div class="col-md-12 text-right">
+                    <%= submit_tag t('.submit_btn'), class: 'btn btn-warning' %>
+                </div>
+            </div>
+        </div>
+    </div>
+<% end %>
\ No newline at end of file
diff --git a/app/views/admin/invoices/forward.haml b/app/views/admin/invoices/forward.haml
deleted file mode 100644
index 25f59d3ad9..0000000000
--- a/app/views/admin/invoices/forward.haml
+++ /dev/null
@@ -1,15 +0,0 @@
-- content_for :actions do
-  = link_to(t(:back_to_invoice), admin_invoice_path(@invoice), class: 'btn btn-default')
-= render 'shared/title', name: t(:forward_invoice)
-
-= form_for([:admin, @invoice], url: { action: :forward }, method: :post) do |f|
-  .row
-    .col-md-4.col-md-offset-4
-      = render 'shared/full_errors', object: @invoice
-      .form-group
-        = f.label :billing_email
-        = f.text_field :billing_email, class: 'form-control', autocomplete: 'off'
-
-      .row
-        .col-md-12.text-right
-          = button_tag(t(:forward), class: 'btn btn-warning')
diff --git a/app/views/admin/invoices/show.haml b/app/views/admin/invoices/show.haml
index bdeb69d6a5..e3627c1589 100644
--- a/app/views/admin/invoices/show.haml
+++ b/app/views/admin/invoices/show.haml
@@ -6,8 +6,8 @@
     %h1.text-right.text-center-xs
       - if @invoice.unpaid?
         = link_to(t(:payment_received), new_admin_bank_statement_path(invoice_id: @invoice.id), class: 'btn btn-default')
-      = link_to(t(:download), admin_invoice_download_pdf_path(@invoice), class: 'btn btn-default')
-      = link_to(t(:forward), admin_invoice_forward_path(@invoice), class: 'btn btn-default')
+      = link_to(t('.download_btn'), download_admin_invoice_path(@invoice), class: 'btn btn-default')
+      = link_to(t('.deliver_btn'), new_admin_invoice_delivery_path(@invoice), class: 'btn btn-default')
       - if @invoice.cancellable?
         = link_to(t(:cancel), cancel_admin_invoice_path(@invoice), method: :patch, class: 'btn btn-warning')
       = link_to(t(:back), admin_invoices_path, class: 'btn btn-default')
diff --git a/app/views/registrar/invoices/pdf.haml b/app/views/invoice/pdf.haml
similarity index 100%
rename from app/views/registrar/invoices/pdf.haml
rename to app/views/invoice/pdf.haml
diff --git a/app/views/registrar/invoices/delivery/new.html.erb b/app/views/registrar/invoices/delivery/new.html.erb
new file mode 100644
index 0000000000..20db814189
--- /dev/null
+++ b/app/views/registrar/invoices/delivery/new.html.erb
@@ -0,0 +1,25 @@
+<ol class="breadcrumb">
+    <li><%= link_to t('registrar.invoices.index.header'), registrar_invoices_path %></li>
+    <li><%= link_to @invoice, registrar_invoice_path(@invoice) %></li>
+</ol>
+
+<div class="page-header">
+    <h1><%= t '.header' %></h1>
+</div>
+
+<%= form_tag(registrar_invoice_delivery_path(@invoice)) do %>
+    <div class="row">
+        <div class="col-md-4 col-md-offset-4">
+            <div class="form-group">
+                <%= label_tag :recipient %>
+                <%= email_field_tag :recipient, @recipient, required: true, autofocus: true,
+                                    class: 'form-control' %>
+            </div>
+            <div class="row">
+                <div class="col-md-12 text-right">
+                    <%= submit_tag t('.submit_btn'), class: 'btn btn-warning' %>
+                </div>
+            </div>
+        </div>
+    </div>
+<% end %>
\ No newline at end of file
diff --git a/app/views/registrar/invoices/forward.haml b/app/views/registrar/invoices/forward.haml
deleted file mode 100644
index 2f2b3a66f1..0000000000
--- a/app/views/registrar/invoices/forward.haml
+++ /dev/null
@@ -1,15 +0,0 @@
-- content_for :actions do
-  = link_to(t(:back_to_invoice), registrar_invoice_path(@invoice), class: 'btn btn-default')
-= render 'shared/title', name: t(:forward_invoice)
-
-= form_for([:registrar, @invoice], url: { action: :forward }, method: :post) do |f|
-  .row
-    .col-md-4.col-md-offset-4
-      = render 'shared/full_errors', object: @invoice
-      .form-group
-        = f.label :billing_email
-        = f.text_field :billing_email, class: 'form-control', autocomplete: 'off'
-
-      .row
-        .col-md-12.text-right
-          = button_tag(t(:forward), class: 'btn btn-warning')
diff --git a/app/views/registrar/invoices/show.haml b/app/views/registrar/invoices/show.haml
index f44660c9a4..66a025eaf9 100644
--- a/app/views/registrar/invoices/show.haml
+++ b/app/views/registrar/invoices/show.haml
@@ -1,6 +1,6 @@
 - content_for :actions do
-  = link_to(t(:download), download_pdf_registrar_invoice_path(@invoice), class: 'btn btn-default')
-  = link_to(t(:forward), forward_registrar_invoice_path(@invoice), class: 'btn btn-default')
+  = link_to(t('.download_btn'), download_registrar_invoice_path(@invoice), class: 'btn btn-default')
+  = link_to(t('.deliver_btn'), new_registrar_invoice_delivery_path(@invoice), class: 'btn btn-default')
   - if @invoice.cancellable?
     = link_to(t(:cancel), cancel_registrar_invoice_path(@invoice), method: :patch, class: 'btn btn-warning')
   = link_to(t(:back), registrar_invoices_path, class: 'btn btn-default')
diff --git a/config/locales/admin/invoices.en.yml b/config/locales/admin/invoices.en.yml
index fba6ea26d9..6ec73ca9c6 100644
--- a/config/locales/admin/invoices.en.yml
+++ b/config/locales/admin/invoices.en.yml
@@ -1,5 +1,12 @@
 en:
   admin:
     invoices:
+      index:
+        header: Invoices
+
+      show:
+        download_btn: Download
+        deliver_btn: Send
+
       cancel:
         cancelled: Invoice has been cancelled
\ No newline at end of file
diff --git a/config/locales/admin/invoices/delivery.en.yml b/config/locales/admin/invoices/delivery.en.yml
new file mode 100644
index 0000000000..a678c7f29b
--- /dev/null
+++ b/config/locales/admin/invoices/delivery.en.yml
@@ -0,0 +1,10 @@
+en:
+  admin:
+    invoices:
+      delivery:
+        new:
+          header: Send invoice
+          submit_btn: Send
+
+        create:
+          delivered: Invoice has been sent
\ No newline at end of file
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 3e6e01a1cb..12cc56ab90 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -558,11 +558,6 @@ en:
   registrant_head_title: 'EIS Registrant'
   registrant_head_title_sufix: ' - EIS Registrant'
   bind_manually: 'Bind manually'
-  forward_invoice: 'Forward invoice'
-  forward: 'Forward'
-  back_to_invoice: 'Back to invoice'
-  invoice_forwared: 'Invoice forwarded'
-  failed_to_forward_invoice: 'Failed to forward invoice'
   client: 'Client'
   you_have_a_new_invoice: 'You have a new invoice.'
   sincerely: 'Sincerely'
diff --git a/config/locales/mailers/invoice.en.yml b/config/locales/mailers/invoice.en.yml
new file mode 100644
index 0000000000..000dfb9cb4
--- /dev/null
+++ b/config/locales/mailers/invoice.en.yml
@@ -0,0 +1,4 @@
+en:
+  invoice_mailer:
+    invoice_email:
+      subject: Invoice no. %{invoice_number}
\ No newline at end of file
diff --git a/config/locales/registrar/invoices.en.yml b/config/locales/registrar/invoices.en.yml
index 8e79ee2e04..dfa7657540 100644
--- a/config/locales/registrar/invoices.en.yml
+++ b/config/locales/registrar/invoices.en.yml
@@ -7,8 +7,14 @@ en:
       go_to_intermediary: 'Go to intermediary'
       pay_by_credit_card: Pay by credit card
       payment_complete: Credit Card payment Complete
+
       index:
+        header: Invoices
         reset_btn: Reset
 
+      show:
+        download_btn: Download
+        deliver_btn: Send
+
       cancel:
         cancelled: Invoice has been cancelled
\ No newline at end of file
diff --git a/config/locales/registrar/invoices/delivery.en.yml b/config/locales/registrar/invoices/delivery.en.yml
new file mode 100644
index 0000000000..5178b2a719
--- /dev/null
+++ b/config/locales/registrar/invoices/delivery.en.yml
@@ -0,0 +1,10 @@
+en:
+  registrar:
+    invoices:
+      delivery:
+        new:
+          header: Send invoice
+          submit_btn: Send
+
+        create:
+          delivered: Invoice has been sent
\ No newline at end of file
diff --git a/config/routes.rb b/config/routes.rb
index 1c9350327c..0f093d45b9 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -53,10 +53,11 @@
       post 'mid' => 'sessions#mid'
     end
 
-    resources :invoices do
+    resources :invoices, except: %i[new create edit update destroy] do
+      resource :delivery, controller: 'invoices/delivery', only: %i[new create]
+
       member do
-        get 'download_pdf'
-        match 'forward', via: [:post, :get]
+        get 'download'
         patch 'cancel'
       end
     end
@@ -199,10 +200,13 @@
       patch 'bind', on: :member
     end
 
-    resources :invoices do
-      get 'download_pdf'
-      patch 'cancel', on: :member
-      match 'forward', via: [:post, :get]
+    resources :invoices, except: %i[edit update destroy] do
+      resource :delivery, controller: 'invoices/delivery', only: %i[new create]
+
+      member do
+        get 'download'
+        patch 'cancel'
+      end
     end
 
     resources :domains, except: %i[new create destroy] do
diff --git a/test/fixtures/invoices.yml b/test/fixtures/invoices.yml
index d163ba732a..9957f787ae 100644
--- a/test/fixtures/invoices.yml
+++ b/test/fixtures/invoices.yml
@@ -9,6 +9,7 @@ one:
   vat_rate: 0.1
   total: 16.50
   reference_no: 13
+  number: 1
 
 for_payments_test:
   number: 1
diff --git a/test/fixtures/registrars.yml b/test/fixtures/registrars.yml
index 7afaf8edf3..7f7f97ad2e 100644
--- a/test/fixtures/registrars.yml
+++ b/test/fixtures/registrars.yml
@@ -10,7 +10,7 @@ bestnames:
   country_code: US
   accounting_customer_code: bestnames
   language: en
-  billing_email: billing@example.com
+  billing_email: billing@bestnames.test
   website:  https://bestnames.test
   reference_no: 13
 
diff --git a/test/integration/admin_area/invoices_test.rb b/test/integration/admin_area/invoices_test.rb
index bd315d960f..204f953d8d 100644
--- a/test/integration/admin_area/invoices_test.rb
+++ b/test/integration/admin_area/invoices_test.rb
@@ -6,8 +6,14 @@ class AdminAreaInvoicesIntegrationTest < ApplicationIntegrationTest
     sign_in users(:admin)
   end
 
-  def test_download_invoice_pdf
-    get admin_invoice_download_pdf_path(@invoice)
+  def test_downloads_invoice
+    assert_equal 1, @invoice.number
+
+    get download_admin_invoice_path(@invoice)
+
     assert_response :ok
+    assert_equal 'application/pdf', response.headers['Content-Type']
+    assert_equal 'attachment; filename="invoice-1.pdf"', response.headers['Content-Disposition']
+    assert_not_empty response.body
   end
 end
\ No newline at end of file
diff --git a/test/integration/registrar_area/invoices_test.rb b/test/integration/registrar_area/invoices_test.rb
index 5e5a91b46a..1f43a5287b 100644
--- a/test/integration/registrar_area/invoices_test.rb
+++ b/test/integration/registrar_area/invoices_test.rb
@@ -6,8 +6,14 @@ class RegistrarAreaInvoicesIntegrationTest < ApplicationIntegrationTest
     sign_in users(:api_bestnames)
   end
 
-  def test_download_invoice_pdf
-    get download_pdf_registrar_invoice_path(@invoice)
+  def test_downloads_invoice
+    assert_equal 1, @invoice.number
+
+    get download_registrar_invoice_path(@invoice)
+
     assert_response :ok
+    assert_equal 'application/pdf', response.headers['Content-Type']
+    assert_equal 'attachment; filename="invoice-1.pdf"', response.headers['Content-Disposition']
+    assert_not_empty response.body
   end
 end
\ No newline at end of file
diff --git a/test/mailers/invoice_mailer_test.rb b/test/mailers/invoice_mailer_test.rb
new file mode 100644
index 0000000000..f84a9d5789
--- /dev/null
+++ b/test/mailers/invoice_mailer_test.rb
@@ -0,0 +1,22 @@
+require 'test_helper'
+
+class InvoiceMailerTest < ActiveSupport::TestCase
+  include ActionMailer::TestHelper
+
+  setup do
+    @invoice = invoices(:one)
+    ActionMailer::Base.deliveries.clear
+  end
+
+  def test_delivers_invoice_email
+    assert_equal 1, @invoice.number
+
+    email = InvoiceMailer.invoice_email(invoice: @invoice, recipient: 'billing@bestnames.test')
+              .deliver_now
+
+    assert_emails 1
+    assert_equal ['billing@bestnames.test'], email.to
+    assert_equal 'Invoice no. 1', email.subject
+    assert email.attachments['invoice-1.pdf']
+  end
+end
\ No newline at end of file
diff --git a/test/mailers/previews/invoice_mailer_preview.rb b/test/mailers/previews/invoice_mailer_preview.rb
new file mode 100644
index 0000000000..d00f1c9298
--- /dev/null
+++ b/test/mailers/previews/invoice_mailer_preview.rb
@@ -0,0 +1,6 @@
+class InvoiceMailerPreview < ActionMailer::Preview
+  def invoice_email
+    invoice = Invoice.first
+    InvoiceMailer.invoice_email(invoice: invoice, recipient: 'billing@registrar.test')
+  end
+end
\ No newline at end of file
diff --git a/test/system/admin_area/invoices_test.rb b/test/system/admin_area/invoices_test.rb
index 081bbb8507..8fef3cddb3 100644
--- a/test/system/admin_area/invoices_test.rb
+++ b/test/system/admin_area/invoices_test.rb
@@ -1,9 +1,13 @@
 require 'test_helper'
 
 class AdminAreaInvoicesTest < ApplicationSystemTestCase
+  include ActionMailer::TestHelper
+
   setup do
     sign_in users(:admin)
     @invoice = invoices(:one)
+
+    ActionMailer::Base.deliveries.clear
   end
 
   def test_cancels_an_invoice
@@ -17,4 +21,23 @@ def test_cancels_an_invoice
     assert @invoice.cancelled?
     assert_text 'Invoice has been cancelled'
   end
+
+  def test_invoice_delivery_form_is_pre_populated_with_billing_email_of_a_registrar
+    assert_equal 'billing@bestnames.test', @invoice.buyer.billing_email
+    visit new_admin_invoice_delivery_url(@invoice)
+    assert_field 'Recipient', with: 'billing@bestnames.test'
+  end
+
+  def test_delivers_an_invoice
+    visit admin_invoice_url(@invoice)
+    click_on 'Send'
+    fill_in 'Recipient', with: 'billing@registrar.test'
+    click_on 'Send'
+
+    assert_emails 1
+    email = ActionMailer::Base.deliveries.first
+    assert_equal ['billing@registrar.test'], email.to
+    assert_current_path admin_invoice_path(@invoice)
+    assert_text 'Invoice has been sent'
+  end
 end
\ No newline at end of file
diff --git a/test/system/registrar_area/invoices_test.rb b/test/system/registrar_area/invoices_test.rb
index a8fa6891f2..02df5da825 100644
--- a/test/system/registrar_area/invoices_test.rb
+++ b/test/system/registrar_area/invoices_test.rb
@@ -1,9 +1,13 @@
 require 'test_helper'
 
 class RegistrarAreaInvoicesTest < ApplicationSystemTestCase
+  include ActionMailer::TestHelper
+
   setup do
     sign_in users(:api_bestnames)
     @invoice = invoices(:one)
+
+    ActionMailer::Base.deliveries.clear
   end
 
   def test_cancels_an_invoice
@@ -17,4 +21,23 @@ def test_cancels_an_invoice
     assert @invoice.cancelled?
     assert_text 'Invoice has been cancelled'
   end
+
+  def test_invoice_delivery_form_is_pre_populated_with_billing_email_of_a_registrar
+    assert_equal 'billing@bestnames.test', @invoice.buyer.billing_email
+    visit new_registrar_invoice_delivery_url(@invoice)
+    assert_field 'Recipient', with: 'billing@bestnames.test'
+  end
+
+  def test_delivers_an_invoice
+    visit registrar_invoice_url(@invoice)
+    click_on 'Send'
+    fill_in 'Recipient', with: 'billing@registrar.test'
+    click_on 'Send'
+
+    assert_emails 1
+    email = ActionMailer::Base.deliveries.first
+    assert_equal ['billing@registrar.test'], email.to
+    assert_current_path registrar_invoice_path(@invoice)
+    assert_text 'Invoice has been sent'
+  end
 end
\ No newline at end of file