Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Track emails about parliamentary business #889

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/jobs/archive_petition_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ def perform(petition)
e.subject = email.subject
e.body = email.body
e.sent_by = email.sent_by
e.email_count = email.email_count
e.emails_enqueued_at = email.emails_enqueued_at
e.created_at = email.created_at
e.updated_at = email.updated_at
end
Expand Down
7 changes: 7 additions & 0 deletions app/jobs/archived/email_petitioners_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ class EmailPetitionersJob < ApplicationJob
log_exception(exception)
end

after_enqueue_send_email_jobs do
email.update_columns(
email_count: petition.signatures.subscribers,
emails_enqueued_at: Time.current
)
end

def perform(**args)
@email = args[:email]
super
Expand Down
18 changes: 16 additions & 2 deletions app/jobs/concerns/email_all_petition_signatories.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ module EmailAllPetitionSignatories
attr_reader :petition, :requested_at, :scope

queue_as :high_priority

with_options(skip_after_callbacks_if_terminated: true) do
define_callbacks :enqueue_send_email_jobs
end
end

module ClassMethods
Expand All @@ -23,6 +27,14 @@ def run_later_tonight(**args)
set(wait_until: later_tonight).perform_later(**args.merge(requested_at: requested_at_iso8601))
end

def before_enqueue_send_email_jobs(*filters, &blk)
set_callback(:enqueue_send_email_jobs, :before, *filters, &blk)
end

def after_enqueue_send_email_jobs(*filters, &blk)
set_callback(:enqueue_send_email_jobs, :after, *filters, &blk)
end

private

def requested_at
Expand Down Expand Up @@ -64,8 +76,10 @@ def perform(**args)
# and enqueues a job to do the actual sending
def enqueue_send_email_jobs
Appsignal.without_instrumentation do
signatures_to_email.find_each do |signature|
email_delivery_job_class.perform_later(**mailer_arguments(signature))
run_callbacks :enqueue_send_email_jobs do
signatures_to_email.find_each do |signature|
email_delivery_job_class.perform_later(**mailer_arguments(signature))
end
end
end
end
Expand Down
7 changes: 7 additions & 0 deletions app/jobs/email_petitioners_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ class EmailPetitionersJob < ApplicationJob
log_exception(exception)
end

after_enqueue_send_email_jobs do
email.update_columns(
email_count: petition.signatures.subscribers,
emails_enqueued_at: Time.current
)
end

def perform(**args)
@email = args[:email]
super
Expand Down
4 changes: 4 additions & 0 deletions app/models/archived/signature.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ def subscribed
where(notify_by_email: true)
end

def subscribers
validated.subscribed.count
end

def validated
where(state: VALIDATED_STATE)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
<thead>
<tr>
<th>Subject</th>
<th>Count</th>
<th>Sent</th>
<th>Actions</th>
</tr>
</thead>
Expand All @@ -14,6 +16,8 @@
<td><%= email.subject %></td>
<td>
<%= button_to 'Edit', edit_admin_archived_petition_email_path(petition, email), method: :get, class: 'button' %>
<td><%= number_with_delimiter(email.email_count) || "–" %></td>
<td><%= date_format(email.emails_enqueued_at) || "–" %></td>
<%= button_to 'Delete', admin_archived_petition_email_path(petition, email), method: :delete, class: 'button-warning', data: { confirm: 'Delete other parliamentary business?' } %>
</td>
</tr>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,17 @@
<thead>
<tr>
<th>Subject</th>
<th>Count</th>
<th>Sent</th>
<th>Actions</th>
</tr>
</thead>
<tbody>
<% petition.emails.select(&:persisted?).each do |email| %>
<tr>
<td><%= email.subject %></td>
<td><%= number_with_delimiter(email.email_count) || "–" %></td>
<td><%= date_format(email.emails_enqueued_at) || "–" %></td>
<td>
<%= button_to 'Edit', edit_admin_petition_email_path(petition, email), method: :get, class: 'button', disabled: @petition.editing_disabled? %>
<%= button_to 'Delete', admin_petition_email_path(petition, email), method: :delete, class: 'button-warning', disabled: @petition.editing_disabled?, data: { confirm: 'Delete other parliamentary business?' } %>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class AddEmailCountToOtherParliamentaryBusiness < ActiveRecord::Migration[6.1]
def change
add_column :petition_emails, :email_count, :integer
add_column :petition_emails, :emails_enqueued_at, :datetime
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class AddEmailCountToOtherParliamentaryBusinessArchived < ActiveRecord::Migration[6.1]
def change
add_column :archived_petition_emails, :email_count, :integer
add_column :archived_petition_emails, :emails_enqueued_at, :datetime
end
end
6 changes: 5 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 2023_03_27_145255) do
ActiveRecord::Schema.define(version: 2024_01_17_150804) do

# These are extensions that must be enabled in order to support this database
enable_extension "intarray"
Expand Down Expand Up @@ -112,6 +112,8 @@
t.string "sent_by"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.integer "email_count"
t.datetime "emails_enqueued_at"
t.index ["petition_id"], name: "index_archived_petition_emails_on_petition_id"
end

Expand Down Expand Up @@ -475,6 +477,8 @@
t.string "sent_by"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.integer "email_count"
t.datetime "emails_enqueued_at"
t.index ["petition_id"], name: "index_petition_emails_on_petition_id"
end

Expand Down
11 changes: 10 additions & 1 deletion spec/factories.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@
opened_at { 2.years.ago }
closed_at { 1.year.ago }

trait :creator do
association :creator, :validated, :creator, factory: :archived_signature
end

after(:build) do |petition, evaluator|
petition.parliament ||= Parliament.archived.first || FactoryBot.create(:parliament, :archived)

Expand Down Expand Up @@ -258,6 +262,7 @@
creator_name { nil }
creator_email { nil }
creator_attributes { {} }
email_attributes { nil }
sponsors_signed { nil }
sponsor_count { Site.minimum_number_of_sponsors }
increment { true }
Expand Down Expand Up @@ -294,6 +299,10 @@
if evaluator.admin_notes
petition.build_note details: evaluator.admin_notes
end

if evaluator.email_attributes
petition.emails << build(:petition_email, petition: petition, **evaluator.email_attributes)
end
end

after(:create) do |petition, evaluator|
Expand All @@ -319,7 +328,7 @@

trait :archived do
archived_at { 3.day.ago }
archiving_started_at { 4.days.ago }
archiving_started_at { 4.days.ago }
end

trait :with_additional_details do
Expand Down
27 changes: 22 additions & 5 deletions spec/jobs/archive_petition_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@
end
end

context "with a petition that has an government_response email scheduled" do
context "with a petition that has a government_response email scheduled" do
let(:petition) do
FactoryBot.create(:closed_petition, :email_requested, email_requested_for_government_response_at: Time.current)
end
Expand All @@ -297,7 +297,7 @@
end
end

context "with a petition that has an debate_scheduled email scheduled" do
context "with a petition that has a debate_scheduled email scheduled" do
let(:petition) do
FactoryBot.create(:closed_petition, :email_requested, email_requested_for_debate_scheduled_at: Time.current)
end
Expand All @@ -307,7 +307,7 @@
end
end

context "with a petition that has an debate_outcome email scheduled" do
context "with a petition that has a debate_outcome email scheduled" do
let(:petition) do
FactoryBot.create(:closed_petition, :email_requested, email_requested_for_debate_outcome_at: Time.current)
end
Expand All @@ -317,7 +317,7 @@
end
end

context "with a petition that has an petition_email email scheduled" do
context "with a petition that has a petition_email email scheduled" do
let(:petition) do
FactoryBot.create(:closed_petition, :email_requested, email_requested_for_petition_email_at: Time.current)
end
Expand All @@ -327,7 +327,24 @@
end
end

context "with a petition that has an petition_mailshot email scheduled" do
context "with a petition that has a petition_email email that has been sent" do
let(:petition) do
FactoryBot.create(:closed_petition, email_attributes: { email_count: 216, emails_enqueued_at: 6.months.ago })
end

let(:email) { petition.emails.first }
let(:archived_email) { archived_petition.emails.first }

it "copies the email count to the archived petition email" do
expect(archived_email.email_count).to eq(email.email_count)
end

it "copies the emails enqueued timestamp to the archived petition email" do
expect(archived_email.emails_enqueued_at).to be_usec_precise_with(email.emails_enqueued_at)
end
end

context "with a petition that has a petition_mailshot email scheduled" do
let(:petition) do
FactoryBot.create(:closed_petition, :email_requested, email_requested_for_petition_mailshot_at: Time.current)
end
Expand Down
28 changes: 26 additions & 2 deletions spec/jobs/archived/email_petitioners_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

RSpec.describe Archived::EmailPetitionersJob, type: :job do
let(:email_requested_at) { Time.current }
let(:petition) { FactoryBot.create(:archived_petition) }
let(:petition) { FactoryBot.create(:archived_petition, :creator) }
let(:signature) { FactoryBot.create(:archived_signature, petition: petition) }
let(:email) { FactoryBot.create(:archived_petition_email, petition: petition) }
let(:arguments) { { petition: petition, email: email } }
Expand All @@ -13,8 +13,32 @@
allow(petition).to receive_message_chain(:signatures_to_email_for, :find_each).and_yield(signature)
end

it_behaves_like "job to enqueue signatory mailing jobs"
it_behaves_like "job to enqueue signatory mailing jobs", -> {
subject.perform(petition: petition, email: email, requested_at: requested_at_as_string)
}

context "when sending other parliamentary business emails" do
it "records the number of emails sent" do
expect {
perform_enqueued_jobs {
described_class.run_later_tonight(**arguments)
}
}.to change {
email.reload.email_count
}.from(nil).to(2)
end

it "records when the emails were enqueued by" do
expect {
perform_enqueued_jobs {
described_class.run_later_tonight(**arguments)
}
}.to change {
email.reload.emails_enqueued_at
}.from(nil).to(be_within(1.second).of(Time.current))
end
end

context "when the petition email has been deleted" do
before do
email.destroy
Expand Down
26 changes: 25 additions & 1 deletion spec/jobs/email_petitioners_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,31 @@
allow(petition).to receive_message_chain(:signatures_to_email_for, :find_each).and_yield(signature)
end

it_behaves_like "job to enqueue signatory mailing jobs"
it_behaves_like "job to enqueue signatory mailing jobs", -> {
subject.perform(petition: petition, email: email, requested_at: requested_at_as_string)
}

context "when sending other parliamentary business emails" do
it "records the number of emails sent" do
expect {
perform_enqueued_jobs {
described_class.run_later_tonight(**arguments)
}
}.to change {
email.reload.email_count
}.from(nil).to(2)
end

it "records when the emails were enqueued by" do
expect {
perform_enqueued_jobs {
described_class.run_later_tonight(**arguments)
}
}.to change {
email.reload.emails_enqueued_at
}.from(nil).to(be_within(1.second).of(Time.current))
end
end

context "when the petition email has been deleted" do
before do
Expand Down
16 changes: 9 additions & 7 deletions spec/jobs/shared_examples.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
RSpec.shared_examples_for "job to enqueue signatory mailing jobs" do
RSpec.shared_examples_for "job to enqueue signatory mailing jobs" do |work_proc|
let(:requested_at) { Time.current.change(usec: 0) }
let(:requested_at_as_string) { requested_at.getutc.iso8601(6) }

def do_work
subject.perform(petition: petition, requested_at: requested_at_as_string)
unless work_proc.present?
work_proc = -> {
subject.perform(petition: petition, requested_at: requested_at_as_string)
}
end

describe '.run_later_tonight' do
Expand Down Expand Up @@ -47,18 +49,18 @@ def do_work

context "when the petition has not been updated" do
it "enqueues a job to send an email to each signatory" do
do_work
instance_exec(&work_proc)
expect(enqueued_jobs.size).to eq(1)
end

it "the job is the correct type for the type of notification" do
do_work
instance_exec(&work_proc)
job = enqueued_jobs.first
expect(job[:job]).to eq(subject.email_delivery_job_class)
end

it "the job has the expected arguments" do
do_work
instance_exec(&work_proc)
args = enqueued_jobs.first[:args].first

expect(args["timestamp_name"]).to eq(subject.timestamp_name)
Expand All @@ -72,7 +74,7 @@ def do_work
end

it "does not enqueue any jobs to send emails" do
do_work
instance_exec(&work_proc)
expect(enqueued_jobs).to be_empty
end
end
Expand Down