Merge pull request #411 from alphagov/add-petition-email-crud

Add petition email crud

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 {
@@ -19,6 +23,12 @@
   }
 
   th:last-child, td:last-child {
-    padding-right: none;
+    padding-right: 0;
+  }
+
+  .petition-emails {
+    th:first-child, td:first-child {
+      text-align: left;
+    }
   }
 }

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?

app/controllers/admin/petition_emails_controller.rb

@@ -1,38 +1,84 @@
 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'
   end
 
   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
     @petition = Petition.moderated.find(params[:petition_id])
   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

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

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' %>

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' %>

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' %>

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]

features/admin/petition_email.feature

@@ -9,7 +9,7 @@ Feature: Emailing petitioner supporters
     And I am logged in as a sysadmin with the email "[email protected]", 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 "[email protected]", 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 "[email protected]", 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 "[email protected]", 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"

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