From 73e954c7a7019f9f8dc2a6b054354b3e7c652264 Mon Sep 17 00:00:00 2001
From: Andrew White <andrew.white@unboxedconsulting.com>
Date: Wed, 2 Dec 2015 16:37:45 +0000
Subject: [PATCH 1/4] The CSS value 'none' is not valid for padding-right

---
 app/assets/stylesheets/petitions/admin/_list.scss | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/assets/stylesheets/petitions/admin/_list.scss b/app/assets/stylesheets/petitions/admin/_list.scss
index c8c7a56aa..c0f1c9991 100644
--- a/app/assets/stylesheets/petitions/admin/_list.scss
+++ b/app/assets/stylesheets/petitions/admin/_list.scss
@@ -19,6 +19,6 @@
   }
 
   th:last-child, td:last-child {
-    padding-right: none;
+    padding-right: 0;
   }
 }

From 15cd3088db8d835a97068ff0c343ce9a4c5eef73 Mon Sep 17 00:00:00 2001
From: Andrew White <andrew.white@unboxedconsulting.com>
Date: Wed, 2 Dec 2015 16:39:01 +0000
Subject: [PATCH 2/4] Remove extraneous update definition

---
 app/controllers/admin/debate_outcomes_controller.rb | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/app/controllers/admin/debate_outcomes_controller.rb b/app/controllers/admin/debate_outcomes_controller.rb
index ea25b6046..1d112a600 100644
--- a/app/controllers/admin/debate_outcomes_controller.rb
+++ b/app/controllers/admin/debate_outcomes_controller.rb
@@ -7,15 +7,6 @@ def show
     render 'admin/petitions/show'
   end
 
-  def update
-    if @debate_outcome.update(debate_outcome_params)
-      EmailDebateOutcomesJob.run_later_tonight(petition: @petition)
-      redirect_to [:admin, @petition], notice: 'Email will be sent overnight'
-    else
-      render 'admin/petitions/show'
-    end
-  end
-
   def update
     if @debate_outcome.update(debate_outcome_params)
       if send_email_to_petitioners?

From b4863e640cdfbacb162af285043b08739a5b7c7c Mon Sep 17 00:00:00 2001
From: Andrew White <andrew.white@unboxedconsulting.com>
Date: Wed, 2 Dec 2015 17:43:39 +0000
Subject: [PATCH 3/4] Rescue deserialization errors

There is no clean way at the moment to cancel an Active Job based upon
the petition email being deleted. Therefore it's easiest just to catch
the deserialization error, log it and move on. It's unlikely that any
other kind of deserialization error would be recoverable anyway.
---
 app/jobs/email_petitioners_job.rb       | 13 +++++++++++++
 spec/jobs/email_petitioners_job_spec.rb | 26 +++++++++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/app/jobs/email_petitioners_job.rb b/app/jobs/email_petitioners_job.rb
index 7a538260e..906eff25c 100644
--- a/app/jobs/email_petitioners_job.rb
+++ b/app/jobs/email_petitioners_job.rb
@@ -6,6 +6,11 @@ class EmailPetitionersJob < ActiveJob::Base
 
   attr_reader :email
 
+  # It's likely that the email got deleted so we just log the error and move on
+  rescue_from ActiveJob::DeserializationError do |exception|
+    log_exception(exception)
+  end
+
   def perform(**args)
     @email = args[:email]
     super
@@ -16,4 +21,12 @@ def perform(**args)
   def mailer_arguments(signature)
     super.merge(email: email)
   end
+
+  def log_exception(exception)
+    logger.info(log_message(exception))
+  end
+
+  def log_message(exception)
+    "#{exception.class.name} while running #{self.class.name}"
+  end
 end
diff --git a/spec/jobs/email_petitioners_job_spec.rb b/spec/jobs/email_petitioners_job_spec.rb
index 280529c12..38cd293fc 100644
--- a/spec/jobs/email_petitioners_job_spec.rb
+++ b/spec/jobs/email_petitioners_job_spec.rb
@@ -15,4 +15,30 @@
 
   it_behaves_like "job to enqueue signatory mailing jobs"
 
+  context "when the petition email has been deleted" do
+    before do
+      email.destroy
+    end
+
+    it "enqueues a job" do
+      described_class.run_later_tonight(**arguments)
+      expect(enqueued_jobs.size).to eq(1)
+    end
+
+    it "doesn't raise an error" do
+      expect {
+        perform_enqueued_jobs {
+          described_class.run_later_tonight(**arguments)
+        }
+      }.not_to raise_error
+    end
+
+    it "doesn't send any email" do
+      expect {
+        perform_enqueued_jobs {
+          described_class.run_later_tonight(**arguments)
+        }
+      }.not_to change { deliveries.size }
+    end
+  end
 end

From 630149da7446ac76dd8c46fb8cfb1c84cf9180fb Mon Sep 17 00:00:00 2001
From: Andrew White <andrew.white@unboxedconsulting.com>
Date: Wed, 2 Dec 2015 19:50:44 +0000
Subject: [PATCH 4/4] Add petition email crud feature

This adds the ability to edit, update and delete items of other
parliamentary business. For the email to send it needs to be queued
by pressing the 'Email {n} petitioners' at least once - multiple
presses don't cause multiple emails to be sent and editing an email
and pressing 'Save without emailing' won't prevent the sending of
a previously queued email. The only way to stop an email after it
has been sent is to delete it. This causes the job to error which
is rescued and ignored.
---
 .../stylesheets/petitions/admin/_list.scss    |  16 +-
 .../admin/petition_emails_controller.rb       |  58 +-
 ...petition_action_email_petitioners.html.erb |   2 +-
 ...petition_action_email_petitioners.html.erb |  26 +-
 app/views/admin/petition_emails/edit.html.erb |  29 +
 config/routes.rb                              |   2 +-
 features/admin/petition_email.feature         |  53 +-
 .../step_definitions/petition_email_steps.rb  |  27 +
 .../admin/petition_emails_controller_spec.rb  | 856 +++++++++++++++---
 9 files changed, 950 insertions(+), 119 deletions(-)
 create mode 100644 app/views/admin/petition_emails/edit.html.erb

diff --git a/app/assets/stylesheets/petitions/admin/_list.scss b/app/assets/stylesheets/petitions/admin/_list.scss
index c0f1c9991..bd1b30537 100644
--- a/app/assets/stylesheets/petitions/admin/_list.scss
+++ b/app/assets/stylesheets/petitions/admin/_list.scss
@@ -1,7 +1,11 @@
 .admin {
-  .petition-list th, .petition-list td {
-    width: 12.5%;
-    text-align: right;
+  .petition-list {
+    margin-bottom: em(24, 16);
+
+    th, td {
+      width: 12.5%;
+      text-align: right;
+    }
   }
 
   th.petition-list-petition-action, td.petition-list-petition-action {
@@ -21,4 +25,10 @@
   th:last-child, td:last-child {
     padding-right: 0;
   }
+
+  .petition-emails {
+    th:first-child, td:first-child {
+      text-align: left;
+    }
+  }
 }
diff --git a/app/controllers/admin/petition_emails_controller.rb b/app/controllers/admin/petition_emails_controller.rb
index 925fb21dc..ca4422ab6 100644
--- a/app/controllers/admin/petition_emails_controller.rb
+++ b/app/controllers/admin/petition_emails_controller.rb
@@ -1,7 +1,8 @@
 class Admin::PetitionEmailsController < Admin::AdminController
   respond_to :html
   before_action :fetch_petition
-  before_action :build_email
+  before_action :build_email, only: [:new, :create]
+  before_action :fetch_email, only: [:edit, :update, :destroy]
 
   def new
     render 'admin/petitions/show'
@@ -9,15 +10,47 @@ def new
 
   def create
     if @email.update(email_params)
-      EmailPetitionersJob.run_later_tonight(petition: @petition, email: @email)
-      PetitionMailer.email_signer(@petition, feedback_signature, @email).deliver_now
+      if send_email_to_petitioners?
+        schedule_email_petitioners_job
+        message = 'Email will be sent overnight'
+      else
+        message = 'Created other parliamentary business successfully'
+      end
 
-      redirect_to [:admin, @petition], notice: 'Email will be sent overnight'
+      redirect_to [:admin, @petition], notice: message
     else
       render 'admin/petitions/show'
     end
   end
 
+  def edit
+  end
+
+  def update
+    if @email.update(email_params)
+      if send_email_to_petitioners?
+        schedule_email_petitioners_job
+        message = 'Email will be sent overnight'
+      else
+        message = 'Updated other parliamentary business successfully'
+      end
+
+      redirect_to [:admin, @petition], notice: message
+    else
+      render 'admin/petitions/show'
+    end
+  end
+
+  def destroy
+    if @email.destroy
+      message = 'Deleted other parliamentary business successfully'
+    else
+      message = 'Unable to delete other parliamentary business - please contact support'
+    end
+
+    redirect_to [:admin, @petition], notice: message
+  end
+
   private
 
   def fetch_petition
@@ -25,14 +58,27 @@ def fetch_petition
   end
 
   def build_email
-    @email = @petition.emails.build(sent_by: current_user.pretty_name)
+    @email = @petition.emails.build
+  end
+
+  def fetch_email
+    @email = @petition.emails.find(params[:id])
   end
 
   def email_params
-    params.require(:petition_email).permit(:subject, :body)
+    params.require(:petition_email).permit(:subject, :body).merge(sent_by: current_user.pretty_name)
   end
 
   def feedback_signature
     FeedbackSignature.new(@petition)
   end
+
+  def send_email_to_petitioners?
+    params.key?(:save_and_email)
+  end
+
+  def schedule_email_petitioners_job
+    EmailPetitionersJob.run_later_tonight(petition: @petition, email: @email)
+    PetitionMailer.email_signer(@petition, feedback_signature, @email).deliver_now
+  end
 end
diff --git a/app/views/admin/admin/_petition_action_email_petitioners.html.erb b/app/views/admin/admin/_petition_action_email_petitioners.html.erb
index a2a72b533..fe8f9f172 100644
--- a/app/views/admin/admin/_petition_action_email_petitioners.html.erb
+++ b/app/views/admin/admin/_petition_action_email_petitioners.html.erb
@@ -1 +1 @@
-<%= link_to 'Add an item of parliamentary business', new_admin_petition_email_path(@petition), class: 'petition-action-heading' %>
+<%= link_to 'Other parliamentary business', new_admin_petition_email_path(@petition), class: 'petition-action-heading' %>
diff --git a/app/views/admin/petition_emails/_petition_action_email_petitioners.html.erb b/app/views/admin/petition_emails/_petition_action_email_petitioners.html.erb
index 316f26ac1..e61c09b54 100644
--- a/app/views/admin/petition_emails/_petition_action_email_petitioners.html.erb
+++ b/app/views/admin/petition_emails/_petition_action_email_petitioners.html.erb
@@ -1,4 +1,27 @@
-<h2 class="petition-action-heading">Add an item of parliamentary business</h2>
+<h2 class="petition-action-heading">Other parliamentary business</h2>
+
+<% if petition.emails.any?(&:persisted?) %>
+  <table class="petition-list petition-emails">
+    <thead>
+      <tr>
+        <th>Subject</th>
+        <th>Actions</th>
+      </tr>
+    </thead>
+    <tbody>
+      <% petition.emails.select(&:persisted?).each do |email| %>
+        <tr>
+          <td><%= email.subject %></td>
+          <td>
+            <%= button_to 'Edit', edit_admin_petition_email_path(petition, email), method: :get, class: 'button' %>
+            <%= button_to 'Delete', admin_petition_email_path(petition, email), method: :delete, class: 'button-warning', data: { confirm: 'Delete other parliamentary business?' } %>
+          </td>
+        </tr>
+      <% end %>
+    </tbody>
+  </table>
+<% end %>
+
 <%= form_for @email, url: admin_petition_emails_path(petition), method: :post do |f| -%>
   <%= form_row :for => [f.object, :subject] do %>
     <%= f.label :subject, 'Subject', class: 'form-label' %>
@@ -15,6 +38,7 @@
   <% end %>
 
   <%= email_petitioners_with_count_submit_button(f, petition) %>
+  <%= f.submit "Save without emailing", name: 'save', class: 'button-secondary' %>
 <% end -%>
 
 <%= javascript_include_tag 'character-counter' %>
diff --git a/app/views/admin/petition_emails/edit.html.erb b/app/views/admin/petition_emails/edit.html.erb
new file mode 100644
index 000000000..6031adc8a
--- /dev/null
+++ b/app/views/admin/petition_emails/edit.html.erb
@@ -0,0 +1,29 @@
+<div class="grid-row">
+  <div class="column-two-thirds extra-gutter">
+    <h1>Edit other parliamentary business</h1>
+
+    <%= form_for @email, url: admin_petition_email_path(@petition, @email), method: :patch do |f| -%>
+      <%= form_row :for => [f.object, :subject] do %>
+        <%= f.label :subject, 'Subject', class: 'form-label' %>
+        <%= error_messages_for_field f.object, :subject %>
+        <%= f.text_area :subject, rows: 2, cols: 70, tabindex: increment, data: { max_length: 100 }, class: 'form-control' %>
+        <p class="character-count">100 characters max</p>
+      <% end %>
+
+      <%= form_row :for => [f.object, :body] do %>
+        <%= f.label :body, 'Body', class: 'form-label' %>
+        <%= error_messages_for_field f.object, :body %>
+        <%= f.text_area :body, rows: 8, cols: 70, tabindex: increment, data: { max_length: 5000 }, class: 'form-control' %>
+        <p class="character-count">5000 characters max</p>
+      <% end %>
+
+      <%= email_petitioners_with_count_submit_button(f, @petition) %>
+      <%= f.submit "Save without emailing", name: 'save', class: 'button-secondary' %>
+    <% end -%>
+  </div>
+  <div class="petition-meta column-third">
+    <%= render 'admin/petitions/petition_details' %>
+  </div>
+</div>
+
+<%= javascript_include_tag 'character-counter' %>
diff --git a/config/routes.rb b/config/routes.rb
index a37a969cb..c666491ea 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -76,7 +76,7 @@
       resources :admin_users
       resources :petitions, :only => [:show, :index] do
         resource 'debate-outcome', only: [:show, :update], as: :debate_outcome, controller: :debate_outcomes
-        resources :emails, only: [:new, :create], controller: :petition_emails
+        resources :emails, only: [:new, :create, :edit, :update, :destroy], controller: :petition_emails
         resource :petition_details, :only => [:show, :update]
         resource :moderation, :only => [:update], controller: :moderation
         resource :notes, :only => [:show, :update]
diff --git a/features/admin/petition_email.feature b/features/admin/petition_email.feature
index d9dc27227..a359cef0a 100644
--- a/features/admin/petition_email.feature
+++ b/features/admin/petition_email.feature
@@ -9,7 +9,7 @@ Feature: Emailing petitioner supporters
     And I am logged in as a sysadmin with the email "admin@example.com", first_name "Admin", last_name "User"
     When I am on the admin all petitions page
     And I follow "Ban Badger Baiting"
-    And I follow "Add an item of parliamentary business"
+    And I follow "Other parliamentary business"
     Then I should be on the admin email petitioners form page for "Ban Badger Baiting"
     And the markup should be valid
     When I press "Email 6 petitioners"
@@ -21,3 +21,54 @@ Feature: Emailing petitioner supporters
     And the petition creator should have been emailed with the update
     And all the signatories of the petition should have been emailed with the update
     And the feedback email address should have been emailed a copy
+
+  Scenario: Saving an email to all petitioners
+    Given an open petition "Ban Badger Baiting" with some signatures
+    And I am logged in as a sysadmin with the email "admin@example.com", first_name "Admin", last_name "User"
+    When I am on the admin all petitions page
+    And I follow "Ban Badger Baiting"
+    And I follow "Other parliamentary business"
+    Then I should be on the admin email petitioners form page for "Ban Badger Baiting"
+    And the markup should be valid
+    When I press "Save without emailing"
+    Then the petition should not have any emails
+    And I should see an error
+    When I fill in the email details
+    And press "Save without emailing"
+    Then the petition should have the email details I provided
+    And the petition creator should not have been emailed with the update
+    And all the signatories of the petition should not have been emailed with the update
+    And the feedback email address should not have been emailed a copy
+
+  Scenario: Updating an email to all petitioners
+    Given an open petition "Ban Badger Baiting" with some signatures
+    And it has an existing petition email "This will be debated"
+    And I am logged in as a sysadmin with the email "admin@example.com", first_name "Admin", last_name "User"
+    When I am on the admin all petitions page
+    And I follow "Ban Badger Baiting"
+    And I follow "Other parliamentary business"
+    Then I should be on the admin email petitioners form page for "Ban Badger Baiting"
+    And the markup should be valid
+    And I should see "This will be debated"
+    When I press "Edit"
+    Then I should see "Edit other parliamentary business"
+    When I fill in "Subject" with "This will not be debated"
+    And I press "Save without emailing"
+    Then I should see "Updated other parliamentary business successfully"
+    When I follow "Other parliamentary business"
+    Then I should see "This will not be debated"
+
+  Scenario: Deleting an email to all petitioners
+    Given an open petition "Ban Badger Baiting" with some signatures
+    And it has an existing petition email "This will be debated"
+    And I am logged in as a sysadmin with the email "admin@example.com", first_name "Admin", last_name "User"
+    When I am on the admin all petitions page
+    And I follow "Ban Badger Baiting"
+    And I follow "Other parliamentary business"
+    Then I should be on the admin email petitioners form page for "Ban Badger Baiting"
+    And the markup should be valid
+    And I should see "This will be debated"
+    When I press "Delete"
+    Then I should see "Deleted other parliamentary business successfully"
+    When I follow "Other parliamentary business"
+    Then I should not see "This will be debated"
diff --git a/features/step_definitions/petition_email_steps.rb b/features/step_definitions/petition_email_steps.rb
index 459ed5ace..52736cf52 100644
--- a/features/step_definitions/petition_email_steps.rb
+++ b/features/step_definitions/petition_email_steps.rb
@@ -3,6 +3,10 @@
   fill_in "Body", :with => "Petition email body"
 end
 
+Given(/^it has an existing petition email "(.*?)"$/) do |subject|
+  @email = FactoryGirl.create(:petition_email, petition: @petition, subject: subject)
+end
+
 Then(/^the petition should not have any emails$/) do
   @petition.reload
   expect(@petition.emails).to be_empty
@@ -27,6 +31,13 @@
   )
 end
 
+Then(/^the petition creator should not have been emailed with the update$/) do
+  @petition.reload
+  steps %Q(
+    Then "#{@petition.creator_signature.email}" should receive no emails
+  )
+end
+
 Then(/^all the signatories of the petition should have been emailed with the update$/) do
   @petition.reload
   @petition.signatures.notify_by_email.validated.where.not(id: @petition.creator_signature.id).each do |signatory|
@@ -40,6 +51,15 @@
   end
 end
 
+Then(/^all the signatories of the petition should not have been emailed with the update$/) do
+  @petition.reload
+  @petition.signatures.notify_by_email.validated.where.not(id: @petition.creator_signature.id).each do |signatory|
+    steps %Q(
+      Then "#{signatory.email}" should receive no emails
+    )
+  end
+end
+
 Then(/^the feedback email address should have been emailed a copy$/) do
   signatory = FeedbackSignature.new(@petition)
   steps %Q(
@@ -50,3 +70,10 @@
     Then I should be on the petition page for "#{@petition.action}"
   )
 end
+
+Then(/^the feedback email address should not have been emailed a copy$/) do
+  signatory = FeedbackSignature.new(@petition)
+  steps %Q(
+    Then "#{signatory.email}" should receive no emails
+  )
+end
diff --git a/spec/controllers/admin/petition_emails_controller_spec.rb b/spec/controllers/admin/petition_emails_controller_spec.rb
index 6bca82232..959d893d1 100644
--- a/spec/controllers/admin/petition_emails_controller_spec.rb
+++ b/spec/controllers/admin/petition_emails_controller_spec.rb
@@ -1,10 +1,11 @@
 require 'rails_helper'
 
 RSpec.describe Admin::PetitionEmailsController, type: :controller, admin: true do
-
   let!(:petition) { FactoryGirl.create(:open_petition) }
 
   describe 'not logged in' do
+    let(:email) { FactoryGirl.create(:petition_email, petition: petition) }
+
     describe 'GET /new' do
       it 'redirects to the login page' do
         get :new, petition_id: petition.id
@@ -14,14 +15,37 @@
 
     describe 'POST /' do
       it 'redirects to the login page' do
-        patch :create, petition_id: petition.id
+        post :create, petition_id: petition.id
+        expect(response).to redirect_to('https://moderate.petition.parliament.uk/admin/login')
+      end
+    end
+
+    describe 'GET /:id/edit' do
+      it 'redirects to the login page' do
+        get :edit, petition_id: petition.id, id: email.id
+        expect(response).to redirect_to('https://moderate.petition.parliament.uk/admin/login')
+      end
+    end
+
+    describe 'PATCH /:id' do
+      it 'redirects to the login page' do
+        patch :update, petition_id: petition.id, id: email.id
+        expect(response).to redirect_to('https://moderate.petition.parliament.uk/admin/login')
+      end
+    end
+
+    describe 'DELETE /:id' do
+      it 'redirects to the login page' do
+        patch :destroy, petition_id: petition.id, id: email.id
         expect(response).to redirect_to('https://moderate.petition.parliament.uk/admin/login')
       end
     end
   end
 
   context 'logged in as moderator user but need to reset password' do
+    let(:email) { FactoryGirl.create(:petition_email, petition: petition) }
     let(:user) { FactoryGirl.create(:moderator_user, force_password_reset: true) }
+
     before { login_as(user) }
 
     describe 'GET /new' do
@@ -33,7 +57,28 @@
 
     describe 'POST /' do
       it 'redirects to edit profile page' do
-        patch :create, petition_id: petition.id
+        post :create, petition_id: petition.id
+        expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit")
+      end
+    end
+
+    describe 'GET /:id/edit' do
+      it 'redirects to the login page' do
+        get :edit, petition_id: petition.id, id: email.id
+        expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit")
+      end
+    end
+
+    describe 'PATCH /:id' do
+      it 'redirects to the login page' do
+        patch :update, petition_id: petition.id, id: email.id
+        expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit")
+      end
+    end
+
+    describe 'DELETE /:id' do
+      it 'redirects to the login page' do
+        patch :destroy, petition_id: petition.id, id: email.id
         expect(response).to redirect_to("https://moderate.petition.parliament.uk/admin/profile/#{user.id}/edit")
       end
     end
@@ -89,158 +134,757 @@
         }
       end
 
-      def do_post(overrides = {})
-        params = { petition_id: petition.id, petition_email: petition_email_attributes }
-        post :create, params.merge(overrides)
-      end
+      context 'when clicking the Email button' do
+        def do_post(overrides = {})
+          params = {
+            petition_id: petition.id,
+            petition_email: petition_email_attributes,
+            save_and_email: "Email"
+          }
 
-      describe 'for an open petition' do
-        it 'fetches the requested petition' do
-          do_post
-          expect(assigns(:petition)).to eq petition
+          post :create, params.merge(overrides)
         end
 
-        describe 'with valid params' do
-          it 'redirects to the petition show page' do
+        describe 'for an open petition' do
+          it 'fetches the requested petition' do
             do_post
-            expect(response).to redirect_to "https://moderate.petition.parliament.uk/admin/petitions/#{petition.id}"
+            expect(assigns(:petition)).to eq petition
           end
 
-          it 'tells the moderator that their email will be sent overnight' do
-            do_post
-            expect(flash[:notice]).to eq 'Email will be sent overnight'
-          end
+          describe 'with valid params' do
+            it 'redirects to the petition show page' do
+              do_post
+              expect(response).to redirect_to "https://moderate.petition.parliament.uk/admin/petitions/#{petition.id}"
+            end
 
-          it 'stores the supplied email details in the db' do
-            do_post
-            petition.reload
-            email = petition.emails.last
-            expect(email).to be_present
-            expect(email.subject).to eq "Petition email subject"
-            expect(email.body).to eq "Petition email body"
-            expect(email.sent_by).to eq user.pretty_name
-          end
-
-          context "emails out the petition email" do
-            before do
-              3.times do |i|
-                attributes = {
-                  name: "Laura #{i}",
-                  email: "laura_#{i}@example.com",
-                  notify_by_email: true,
-                  petition: petition
-                }
-                s = FactoryGirl.create(:pending_signature, attributes)
-                s.validate!
+            it 'tells the moderator that their email will be sent overnight' do
+              do_post
+              expect(flash[:notice]).to eq 'Email will be sent overnight'
+            end
+
+            it 'stores the supplied email details in the db' do
+              do_post
+              petition.reload
+              email = petition.emails.last
+              expect(email).to be_present
+              expect(email.subject).to eq "Petition email subject"
+              expect(email.body).to eq "Petition email body"
+              expect(email.sent_by).to eq user.pretty_name
+            end
+
+            context "emails out the petition email" do
+              before do
+                3.times do |i|
+                  attributes = {
+                    name: "Laura #{i}",
+                    email: "laura_#{i}@example.com",
+                    notify_by_email: true,
+                    petition: petition
+                  }
+                  s = FactoryGirl.create(:pending_signature, attributes)
+                  s.validate!
+                end
+                2.times do |i|
+                  attributes = {
+                    name: "Sarah #{i}",
+                    email: "sarah_#{i}@example.com",
+                    notify_by_email: false,
+                    petition: petition
+                  }
+
+                  s = FactoryGirl.create(:pending_signature, attributes)
+                  s.validate!
+                end
+                2.times do |i|
+                  attributes = {
+                    name: "Brian #{i}",
+                    email: "brian_#{i}@example.com",
+                    notify_by_email: true,
+                    petition: petition
+                  }
+                  FactoryGirl.create(:pending_signature, attributes)
+                end
+                petition.reload
               end
-              2.times do |i|
-                attributes = {
-                  name: "Sarah #{i}",
-                  email: "sarah_#{i}@example.com",
-                  notify_by_email: false,
-                  petition: petition
-                }
-
-                s = FactoryGirl.create(:pending_signature, attributes)
-                s.validate!
+
+              it "queues a job to process the emails" do
+                assert_enqueued_jobs 1 do
+                  do_post
+                end
               end
-              2.times do |i|
-                attributes = {
-                  name: "Brian #{i}",
-                  email: "brian_#{i}@example.com",
-                  notify_by_email: true,
-                  petition: petition
-                }
-                FactoryGirl.create(:pending_signature, attributes)
+
+              it "stamps the 'petition_email' email sent receipt on each signature when the job runs" do
+                perform_enqueued_jobs do
+                  do_post
+                  petition.reload
+                  petition_timestamp = petition.get_email_requested_at_for('petition_email')
+                  expect(petition_timestamp).not_to be_nil
+                  petition.signatures.validated.notify_by_email.each do |signature|
+                    expect(signature.get_email_sent_at_for('petition_email')).to eq(petition_timestamp)
+                  end
+                end
               end
+
+              it "should email out to the validated signees who have opted in when the delayed job runs" do
+                perform_enqueued_jobs do
+                  do_post
+                  expect(deliveries.length).to eq 5
+                  expect(deliveries.map(&:to)).to eq([
+                    [petition.creator_signature.email],
+                    ['laura_0@example.com'],
+                    ['laura_1@example.com'],
+                    ['laura_2@example.com'],
+                    ['petitionscommittee@parliament.uk']
+                  ])
+                end
+              end
+            end
+          end
+
+          describe 'with invalid params' do
+            let(:petition_email_attributes) do
+              { subject: "", body: "" }
+            end
+
+            it 're-renders the petitions/show template' do
+              do_post
+              expect(response).to be_success
+              expect(response).to render_template('petitions/show')
+            end
+
+            it 'leaves the in-memory instance with errors' do
+              do_post
+              expect(assigns(:email)).to be_present
+              expect(assigns(:email).errors).not_to be_empty
+            end
+
+            it 'does not stores the email details in the db' do
+              do_post
               petition.reload
+              expect(petition.emails).to be_empty
             end
+          end
+        end
 
-            it "queues a job to process the emails" do
-              assert_enqueued_jobs 1 do
-                do_post
-              end
+        shared_examples_for 'trying to email supporters of a petition in the wrong state' do
+          it 'raises a 404 error' do
+            expect {
+              do_post
+            }.to raise_error ActiveRecord::RecordNotFound
+          end
+
+          it 'does not stores the supplied email details in the db' do
+            suppress(ActiveRecord::RecordNotFound) { do_post }
+            petition.reload
+            expect(petition.emails).to be_empty
+          end
+        end
+
+        describe 'for a pending petition' do
+          before { petition.update_column(:state, Petition::PENDING_STATE) }
+          it_behaves_like 'trying to email supporters of a petition in the wrong state'
+        end
+
+        describe 'for a validated petition' do
+          before { petition.update_column(:state, Petition::VALIDATED_STATE) }
+          it_behaves_like 'trying to email supporters of a petition in the wrong state'
+        end
+
+        describe 'for a sponsored petition' do
+          before { petition.update_column(:state, Petition::SPONSORED_STATE) }
+          it_behaves_like 'trying to email supporters of a petition in the wrong state'
+        end
+      end
+
+      context 'when clicking the Save button' do
+        def do_post(overrides = {})
+          params = {
+            petition_id: petition.id,
+            petition_email: petition_email_attributes,
+            save: "Save"
+          }
+
+          post :create, params.merge(overrides)
+        end
+
+        describe 'for an open petition' do
+          it 'fetches the requested petition' do
+            do_post
+            expect(assigns(:petition)).to eq petition
+          end
+
+          describe 'with valid params' do
+            it 'redirects to the petition show page' do
+              do_post
+              expect(response).to redirect_to "https://moderate.petition.parliament.uk/admin/petitions/#{petition.id}"
             end
 
-            it "stamps the 'petition_email' email sent receipt on each signature when the job runs" do
-              perform_enqueued_jobs do
-                do_post
-                petition.reload
-                petition_timestamp = petition.get_email_requested_at_for('petition_email')
-                expect(petition_timestamp).not_to be_nil
-                petition.signatures.validated.notify_by_email.each do |signature|
-                  expect(signature.get_email_sent_at_for('petition_email')).to eq(petition_timestamp)
+            it 'tells the moderator that their changes were saved' do
+              do_post
+              expect(flash[:notice]).to eq 'Created other parliamentary business successfully'
+            end
+
+            it 'stores the supplied email details in the db' do
+              do_post
+              petition.reload
+              email = petition.emails.last
+              expect(email).to be_present
+              expect(email.subject).to eq "Petition email subject"
+              expect(email.body).to eq "Petition email body"
+              expect(email.sent_by).to eq user.pretty_name
+            end
+
+            context "does not email out the petition email" do
+              before do
+                3.times do |i|
+                  attributes = {
+                    name: "Laura #{i}",
+                    email: "laura_#{i}@example.com",
+                    notify_by_email: true,
+                    petition: petition
+                  }
+                  s = FactoryGirl.create(:pending_signature, attributes)
+                  s.validate!
+                end
+                2.times do |i|
+                  attributes = {
+                    name: "Sarah #{i}",
+                    email: "sarah_#{i}@example.com",
+                    notify_by_email: false,
+                    petition: petition
+                  }
+
+                  s = FactoryGirl.create(:pending_signature, attributes)
+                  s.validate!
                 end
+                2.times do |i|
+                  attributes = {
+                    name: "Brian #{i}",
+                    email: "brian_#{i}@example.com",
+                    notify_by_email: true,
+                    petition: petition
+                  }
+                  FactoryGirl.create(:pending_signature, attributes)
+                end
+                petition.reload
               end
-            end
 
-            it "should email out to the validated signees who have opted in when the delayed job runs" do
-              ActionMailer::Base.deliveries.clear
-              perform_enqueued_jobs do
-                do_post
-                expect(ActionMailer::Base.deliveries.length).to eq 5
-                expect(ActionMailer::Base.deliveries.map(&:to)).to eq([
-                  [petition.creator_signature.email],
-                  ['laura_0@example.com'],
-                  ['laura_1@example.com'],
-                  ['laura_2@example.com'],
-                  ['petitionscommittee@parliament.uk']
-                ])
+              it "does not queue a job to process the emails" do
+                assert_enqueued_jobs 0 do
+                  do_post
+                end
               end
             end
           end
-        end
 
-        describe 'with invalid params' do
-          before { petition_email_attributes.delete(:subject) }
+          describe 'with invalid params' do
+            let(:petition_email_attributes) do
+              { subject: "", body: "" }
+            end
 
-          it 're-renders the petitions/show template' do
-            do_post
-            expect(response).to be_success
-            expect(response).to render_template('petitions/show')
+            it 're-renders the petitions/show template' do
+              do_post
+              expect(response).to be_success
+              expect(response).to render_template('petitions/show')
+            end
+
+            it 'leaves the in-memory instance with errors' do
+              do_post
+              expect(assigns(:email)).to be_present
+              expect(assigns(:email).errors).not_to be_empty
+            end
+
+            it 'does not stores the email details in the db' do
+              do_post
+              petition.reload
+              expect(petition.emails).to be_empty
+            end
           end
+        end
 
-          it 'leaves the in-memory instance with errors' do
-            do_post
-            expect(assigns(:email)).to be_present
-            expect(assigns(:email).errors).not_to be_empty
+        shared_examples_for 'trying to email supporters of a petition in the wrong state' do
+          it 'raises a 404 error' do
+            expect {
+              do_post
+            }.to raise_error ActiveRecord::RecordNotFound
           end
 
-          it 'does not stores the email details in the db' do
-            do_post
+          it 'does not store the supplied email details in the db' do
+            suppress(ActiveRecord::RecordNotFound) { do_post }
             petition.reload
             expect(petition.emails).to be_empty
           end
         end
+
+        describe 'for a pending petition' do
+          before { petition.update_column(:state, Petition::PENDING_STATE) }
+          it_behaves_like 'trying to email supporters of a petition in the wrong state'
+        end
+
+        describe 'for a validated petition' do
+          before { petition.update_column(:state, Petition::VALIDATED_STATE) }
+          it_behaves_like 'trying to email supporters of a petition in the wrong state'
+        end
+
+        describe 'for a sponsored petition' do
+          before { petition.update_column(:state, Petition::SPONSORED_STATE) }
+          it_behaves_like 'trying to email supporters of a petition in the wrong state'
+        end
+      end
+    end
+
+    describe 'GET /:id/edit' do
+      let(:email) do
+        FactoryGirl.create(
+          :petition_email,
+          petition: petition,
+          subject: "Petition email subject",
+          body: "Petition email body"
+        )
+      end
+
+      describe 'for an open petition' do
+        it 'fetches the requested petition' do
+          get :edit, petition_id: petition.id, id: email.id
+          expect(assigns(:petition)).to eq petition
+        end
+
+        it 'fetches the requested email' do
+          get :edit, petition_id: petition.id, id: email.id
+          expect(assigns(:email)).to eq email
+        end
+
+        it 'responds successfully and renders the petition_emails/edit template' do
+          get :edit, petition_id: petition.id, id: email.id
+          expect(response).to be_success
+          expect(response).to render_template('petition_emails/edit')
+        end
       end
 
-      shared_examples_for 'trying to email supporters of a petition in the wrong state' do
+      shared_examples_for 'trying to view the email petitioners form of a petition in the wrong state' do
         it 'raises a 404 error' do
           expect {
-            do_post
+            get :new, petition_id: petition.id, id: email.id
           }.to raise_error ActiveRecord::RecordNotFound
         end
-
-        it 'does not stores the supplied email details in the db' do
-          suppress(ActiveRecord::RecordNotFound) { do_post }
-          petition.reload
-          expect(petition.emails).to be_empty
-        end
       end
 
       describe 'for a pending petition' do
         before { petition.update_column(:state, Petition::PENDING_STATE) }
-        it_behaves_like 'trying to email supporters of a petition in the wrong state'
+        it_behaves_like 'trying to view the email petitioners form of a petition in the wrong state'
       end
 
       describe 'for a validated petition' do
         before { petition.update_column(:state, Petition::VALIDATED_STATE) }
-        it_behaves_like 'trying to email supporters of a petition in the wrong state'
+        it_behaves_like 'trying to view the email petitioners form of a petition in the wrong state'
       end
 
       describe 'for a sponsored petition' do
         before { petition.update_column(:state, Petition::SPONSORED_STATE) }
-        it_behaves_like 'trying to email supporters of a petition in the wrong state'
+        it_behaves_like 'trying to view the email petitioners form of a petition in the wrong state'
+      end
+    end
+
+    describe 'PATCH /:id' do
+      let(:email) do
+        FactoryGirl.create(
+          :petition_email,
+          petition: petition,
+          subject: "Petition email subject",
+          body: "Petition email body"
+        )
+      end
+
+      let(:petition_email_attributes) do
+        {
+          subject: "New petition email subject",
+          body: "New petition email body"
+        }
+      end
+
+      context 'when clicking the Email button' do
+        def do_patch(overrides = {})
+          params = {
+            petition_id: petition.id,
+            id: email.id,
+            petition_email: petition_email_attributes,
+            save_and_email: "Email"
+          }
+
+          patch :update, params.merge(overrides)
+        end
+
+        describe 'for an open petition' do
+          it 'fetches the requested petition' do
+            do_patch
+            expect(assigns(:petition)).to eq petition
+          end
+
+          it 'fetches the requested email' do
+            do_patch
+            expect(assigns(:email)).to eq email
+          end
+
+          describe 'with valid params' do
+            it 'redirects to the petition show page' do
+              do_patch
+              expect(response).to redirect_to "https://moderate.petition.parliament.uk/admin/petitions/#{petition.id}"
+            end
+
+            it 'tells the moderator that their email will be sent overnight' do
+              do_patch
+              expect(flash[:notice]).to eq 'Email will be sent overnight'
+            end
+
+            it 'stores the supplied email details in the db' do
+              do_patch
+              petition.reload
+              email = petition.emails.last
+              expect(email).to be_present
+              expect(email.subject).to eq "New petition email subject"
+              expect(email.body).to eq "New petition email body"
+              expect(email.sent_by).to eq user.pretty_name
+            end
+
+            context "emails out the petition email" do
+              before do
+                3.times do |i|
+                  attributes = {
+                    name: "Laura #{i}",
+                    email: "laura_#{i}@example.com",
+                    notify_by_email: true,
+                    petition: petition
+                  }
+                  s = FactoryGirl.create(:pending_signature, attributes)
+                  s.validate!
+                end
+                2.times do |i|
+                  attributes = {
+                    name: "Sarah #{i}",
+                    email: "sarah_#{i}@example.com",
+                    notify_by_email: false,
+                    petition: petition
+                  }
+
+                  s = FactoryGirl.create(:pending_signature, attributes)
+                  s.validate!
+                end
+                2.times do |i|
+                  attributes = {
+                    name: "Brian #{i}",
+                    email: "brian_#{i}@example.com",
+                    notify_by_email: true,
+                    petition: petition
+                  }
+                  FactoryGirl.create(:pending_signature, attributes)
+                end
+                petition.reload
+              end
+
+              it "queues a job to process the emails" do
+                assert_enqueued_jobs 1 do
+                  do_patch
+                end
+              end
+
+              it "stamps the 'petition_email' email sent receipt on each signature when the job runs" do
+                perform_enqueued_jobs do
+                  do_patch
+                  petition.reload
+                  petition_timestamp = petition.get_email_requested_at_for('petition_email')
+                  expect(petition_timestamp).not_to be_nil
+                  petition.signatures.validated.notify_by_email.each do |signature|
+                    expect(signature.get_email_sent_at_for('petition_email')).to eq(petition_timestamp)
+                  end
+                end
+              end
+
+              it "should email out to the validated signees who have opted in when the delayed job runs" do
+                perform_enqueued_jobs do
+                  do_patch
+                  expect(deliveries.length).to eq 5
+                  expect(deliveries.map(&:to)).to eq([
+                    [petition.creator_signature.email],
+                    ['laura_0@example.com'],
+                    ['laura_1@example.com'],
+                    ['laura_2@example.com'],
+                    ['petitionscommittee@parliament.uk']
+                  ])
+                end
+              end
+            end
+          end
+
+          describe 'with invalid params' do
+            let(:petition_email_attributes) do
+              { subject: "", body: "" }
+            end
+
+            it 're-renders the petitions/show template' do
+              do_patch
+              expect(response).to be_success
+              expect(response).to render_template('petitions/show')
+            end
+
+            it 'leaves the in-memory instance with errors' do
+              do_patch
+              expect(assigns(:email)).to be_present
+              expect(assigns(:email).errors).not_to be_empty
+            end
+
+            it 'does not stores the email details in the db' do
+              do_patch
+              email.reload
+              expect(email).to be_present
+              expect(email.subject).to eq "Petition email subject"
+              expect(email.body).to eq "Petition email body"
+            end
+          end
+        end
+
+        shared_examples_for 'trying to email supporters of a petition in the wrong state' do
+          it 'raises a 404 error' do
+            expect {
+              do_patch
+            }.to raise_error ActiveRecord::RecordNotFound
+          end
+
+          it 'does not stores the supplied email details in the db' do
+            suppress(ActiveRecord::RecordNotFound) { do_patch }
+            email.reload
+            expect(email).to be_present
+            expect(email.subject).to eq "Petition email subject"
+            expect(email.body).to eq "Petition email body"
+          end
+        end
+
+        describe 'for a pending petition' do
+          before { petition.update_column(:state, Petition::PENDING_STATE) }
+          it_behaves_like 'trying to email supporters of a petition in the wrong state'
+        end
+
+        describe 'for a validated petition' do
+          before { petition.update_column(:state, Petition::VALIDATED_STATE) }
+          it_behaves_like 'trying to email supporters of a petition in the wrong state'
+        end
+
+        describe 'for a sponsored petition' do
+          before { petition.update_column(:state, Petition::SPONSORED_STATE) }
+          it_behaves_like 'trying to email supporters of a petition in the wrong state'
+        end
+      end
+
+      context 'when clicking the Save button' do
+        def do_patch(overrides = {})
+          params = {
+            petition_id: petition.id,
+            id: email.id,
+            petition_email: petition_email_attributes,
+            save: "Save"
+          }
+
+          patch :update, params.merge(overrides)
+        end
+
+        describe 'for an open petition' do
+          it 'fetches the requested petition' do
+            do_patch
+            expect(assigns(:petition)).to eq petition
+          end
+
+          it 'fetches the requested email' do
+            do_patch
+            expect(assigns(:email)).to eq email
+          end
+
+          describe 'with valid params' do
+            it 'redirects to the petition show page' do
+              do_patch
+              expect(response).to redirect_to "https://moderate.petition.parliament.uk/admin/petitions/#{petition.id}"
+            end
+
+            it 'tells the moderator that their changes were saved' do
+              do_patch
+              expect(flash[:notice]).to eq 'Updated other parliamentary business successfully'
+            end
+
+            it 'stores the supplied email details in the db' do
+              do_patch
+              email.reload
+              expect(email).to be_present
+              expect(email.subject).to eq "New petition email subject"
+              expect(email.body).to eq "New petition email body"
+              expect(email.sent_by).to eq user.pretty_name
+            end
+
+            context "does not email out the petition email" do
+              before do
+                3.times do |i|
+                  attributes = {
+                    name: "Laura #{i}",
+                    email: "laura_#{i}@example.com",
+                    notify_by_email: true,
+                    petition: petition
+                  }
+                  s = FactoryGirl.create(:pending_signature, attributes)
+                  s.validate!
+                end
+                2.times do |i|
+                  attributes = {
+                    name: "Sarah #{i}",
+                    email: "sarah_#{i}@example.com",
+                    notify_by_email: false,
+                    petition: petition
+                  }
+
+                  s = FactoryGirl.create(:pending_signature, attributes)
+                  s.validate!
+                end
+                2.times do |i|
+                  attributes = {
+                    name: "Brian #{i}",
+                    email: "brian_#{i}@example.com",
+                    notify_by_email: true,
+                    petition: petition
+                  }
+                  FactoryGirl.create(:pending_signature, attributes)
+                end
+                petition.reload
+              end
+
+              it "does not queue a job to process the emails" do
+                assert_enqueued_jobs 0 do
+                  do_patch
+                end
+              end
+            end
+          end
+
+          describe 'with invalid params' do
+            let(:petition_email_attributes) do
+              { subject: "", body: "" }
+            end
+
+            it 're-renders the petitions/show template' do
+              do_patch
+              expect(response).to be_success
+              expect(response).to render_template('petitions/show')
+            end
+
+            it 'leaves the in-memory instance with errors' do
+              do_patch
+              expect(assigns(:email)).to be_present
+              expect(assigns(:email).errors).not_to be_empty
+            end
+
+            it 'does not stores the email details in the db' do
+              do_patch
+              email.reload
+              expect(email.subject).to eq("Petition email subject")
+              expect(email.body).to eq("Petition email body")
+            end
+          end
+        end
+
+        shared_examples_for 'trying to email supporters of a petition in the wrong state' do
+          it 'raises a 404 error' do
+            expect {
+              do_patch
+            }.to raise_error ActiveRecord::RecordNotFound
+          end
+
+          it 'does not store the supplied email details in the db' do
+            suppress(ActiveRecord::RecordNotFound) { do_patch }
+            email.reload
+            expect(email.subject).to eq("Petition email subject")
+            expect(email.body).to eq("Petition email body")
+          end
+        end
+
+        describe 'for a pending petition' do
+          before { petition.update_column(:state, Petition::PENDING_STATE) }
+          it_behaves_like 'trying to email supporters of a petition in the wrong state'
+        end
+
+        describe 'for a validated petition' do
+          before { petition.update_column(:state, Petition::VALIDATED_STATE) }
+          it_behaves_like 'trying to email supporters of a petition in the wrong state'
+        end
+
+        describe 'for a sponsored petition' do
+          before { petition.update_column(:state, Petition::SPONSORED_STATE) }
+          it_behaves_like 'trying to email supporters of a petition in the wrong state'
+        end
+      end
+    end
+
+    describe 'DELETE /:id' do
+      let(:email) do
+        FactoryGirl.create(
+          :petition_email,
+          petition: petition,
+          subject: "Petition email subject",
+          body: "Petition email body"
+        )
+      end
+
+      def do_delete(overrides = {})
+        params = { petition_id: petition.id, id: email.id }
+        delete :destroy, params.merge(overrides)
+      end
+
+      describe 'for an open petition' do
+        let(:moderated) { double(:moderated) }
+        let(:emails) { double(:emails) }
+
+        before do
+          expect(Petition).to receive(:moderated).and_return(moderated)
+          expect(moderated).to receive(:find).with("#{petition.id}").and_return(petition)
+          expect(petition).to receive(:emails).and_return(emails)
+          expect(emails).to receive(:find).with("#{email.id}").and_return(email)
+        end
+
+        it 'fetches the requested petition' do
+          do_delete
+          expect(assigns(:petition)).to eq petition
+        end
+
+        it 'fetches the requested email' do
+          do_delete
+          expect(assigns(:email)).to eq email
+        end
+
+        context "when the delete is successful" do
+          before do
+            expect(email).to receive(:destroy).and_return(true)
+          end
+
+          it 'redirects to the petition show page' do
+            do_delete
+            expect(response).to redirect_to "https://moderate.petition.parliament.uk/admin/petitions/#{petition.id}"
+          end
+
+          it 'tells the moderator that the record was deleted' do
+            do_delete
+            expect(flash[:notice]).to eq 'Deleted other parliamentary business successfully'
+          end
+        end
+
+        context "when the delete is unsuccessful" do
+          before do
+            expect(email).to receive(:destroy).and_return(false)
+          end
+
+          it 'redirects to the petition show page' do
+            do_delete
+            expect(response).to redirect_to "https://moderate.petition.parliament.uk/admin/petitions/#{petition.id}"
+          end
+
+          it 'tells the moderator to contact support' do
+            do_delete
+            expect(flash[:notice]).to eq 'Unable to delete other parliamentary business - please contact support'
+          end
+        end
       end
     end
   end