Skip to content

Commit 5bdf6fd

Browse files
committed
Use only new delievery_status and last_delivery_attempt columns
This removes references and usage on the old column names.
1 parent 12d688f commit 5bdf6fd

11 files changed

Lines changed: 26 additions & 111 deletions

app/jobs/delete_submissions_job.rb

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@ def perform(*_args)
88

99
delete_submissions_sent_before_time = Settings.retain_submissions_for_seconds.seconds.ago
1010
submissions_to_delete = Submission
11-
.where(sent_at: ..delete_submissions_sent_before_time)
12-
.or(Submission.where(last_delivery_attempt: ..delete_submissions_sent_before_time))
11+
.where(last_delivery_attempt: ..delete_submissions_sent_before_time)
1312
.not_bounced
1413

1514
submissions_to_delete.find_each { |submission| delete_submission_data(submission) }

app/jobs/send_submission_job.rb

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,7 @@ def perform(submission)
2424

2525
submission.update!(
2626
mail_message_id: message_id,
27-
mail_status: :pending,
28-
delivery_status: :delivery_pending,
29-
sent_at: Time.zone.now,
27+
delivery_status: :pending,
3028
last_delivery_attempt: Time.zone.now,
3129
)
3230

app/models/submission.rb

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,15 @@
11
class Submission < ApplicationRecord
2+
self.ignored_columns += %w[mail_status sent_at]
3+
24
delegate :preview?, to: :mode_object
35

46
encrypts :answers
57

6-
enum :mail_status, {
8+
enum :delivery_status, {
79
pending: "pending",
810
bounced: "bounced",
911
}
1012

11-
enum :delivery_status, {
12-
delivery_pending: "pending",
13-
delivery_bounced: "bounced",
14-
}
15-
16-
def pending?
17-
mail_status == "pending" || delivery_status == "delivery_pending"
18-
end
19-
20-
def bounced?
21-
mail_status == "bounced" || delivery_status == "delivery_bounced"
22-
end
23-
24-
scope :not_bounced, -> { where.not(mail_status: :bounced).and(where.not(delivery_status: :delivery_bounced)) }
25-
scope :not_pending, -> { where.not(mail_status: :pending).where.not(delivery_status: :delivery_pending) }
26-
27-
scope :pending, -> { where(mail_status: :pending, delivery_status: :delivery_pending) }
28-
scope :bounced, -> { where(mail_status: :bounced).or(where(delivery_status: :delivery_bounced)) }
29-
30-
def pending!
31-
update(mail_status: "pending", delivery_status: "delivery_pending")
32-
end
33-
3413
def journey
3514
@journey ||= Flow::Journey.new(answer_store:, form:)
3615
end

spec/factories/submissions.rb

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@
1616
end
1717
mode { "live" }
1818
form_document { build :v2_form_document }
19-
mail_status { :pending }
20-
delivery_status { :delivery_pending }
19+
delivery_status { :pending }
2120

2221
trait :sent do
2322
mail_message_id { Faker::Alphanumeric.alphanumeric }

spec/jobs/delete_submissions_job_spec.rb

Lines changed: 4 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -48,41 +48,16 @@
4848
form_document: form_without_file_upload,
4949
last_delivery_attempt: 6.days.ago
5050
end
51-
let!(:sent_submission_sent_at_7_days_ago) do
52-
create :submission,
53-
:sent,
54-
reference: "SENT7DAYS",
55-
form_id: form_without_file_upload.id,
56-
form_document: form_without_file_upload,
57-
sent_at: 7.days.ago
58-
end
59-
let!(:sent_submission_sent_at_6_days_ago) do
60-
create :submission,
61-
:sent,
62-
reference: "SENT6DAYS",
63-
form_id: form_without_file_upload.id,
64-
form_document: form_without_file_upload,
65-
sent_at: 6.days.ago
66-
end
67-
let!(:bounced_submission_mail_status) do
68-
create :submission,
69-
:sent,
70-
reference: "BOUNCED",
71-
form_id: form_without_file_upload.id,
72-
form_document: form_without_file_upload,
73-
last_delivery_attempt: 7.days.ago,
74-
mail_status: :bounced
75-
end
7651
let!(:bounced_submission) do
7752
create :submission,
7853
:sent,
7954
reference: "BOUNCED",
8055
form_id: form_without_file_upload.id,
8156
form_document: form_without_file_upload,
8257
last_delivery_attempt: 7.days.ago,
83-
delivery_status: :delivery_bounced
58+
delivery_status: :bounced
8459
end
85-
let!(:unsent_submission) { create :submission, reference: "UNSENT", sent_at: nil }
60+
let!(:unsent_submission) { create :submission, reference: "UNSENT", last_delivery_attempt: nil }
8661

8762
let(:output) { StringIO.new }
8863
let(:logger) do
@@ -117,16 +92,14 @@
11792
end
11893

11994
it "destroys the sent submissions updated more than 7 days ago" do
120-
expect { perform_enqueued_jobs }.to change(Submission, :count).by(-3)
95+
expect { perform_enqueued_jobs }.to change(Submission, :count).by(-2)
12196
expect(Submission.exists?(sent_submission_sent_8_days_ago.id)).to be false
12297
expect(Submission.exists?(sent_submission_sent_7_days_ago.id)).to be false
123-
expect(Submission.exists?(sent_submission_sent_at_7_days_ago.id)).to be false
12498
end
12599

126100
it "does not destroy the sent submission updated more recently than 7 days ago" do
127101
perform_enqueued_jobs
128102
expect(Submission.exists?(sent_submission_sent_6_days_ago.id)).to be true
129-
expect(Submission.exists?(sent_submission_sent_at_6_days_ago.id)).to be true
130103
end
131104

132105
it "does not destroy the submission that hasn't been sent" do
@@ -139,11 +112,6 @@
139112
expect(Submission.exists?(bounced_submission.id)).to be true
140113
end
141114

142-
it "does not destroy the submission that marked bounced under mail status" do
143-
perform_enqueued_jobs
144-
expect(Submission.exists?(bounced_submission_mail_status.id)).to be true
145-
end
146-
147115
it "logs deletion" do
148116
perform_enqueued_jobs
149117
expect(log_lines).to include(
@@ -208,9 +176,8 @@
208176
end
209177

210178
it "continues to delete the next submission" do
211-
expect { perform_enqueued_jobs }.to change(Submission, :count).by(-2)
179+
expect { perform_enqueued_jobs }.to change(Submission, :count).by(-1)
212180
expect(Submission.exists?(sent_submission_sent_7_days_ago.id)).to be false
213-
expect(Submission.exists?(sent_submission_sent_at_7_days_ago.id)).to be false
214181
end
215182

216183
it "sends cloudwatch metric" do

spec/jobs/receive_submission_bounces_and_complaints_job_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
let(:mail_message_id) { "mail-message-id" }
3030
let(:reference) { "submission-reference" }
3131
let!(:submission) { create :submission, mail_message_id:, reference:, form_id: form_with_file_upload.id, form_document: form_with_file_upload, answers: form_with_file_upload_answers }
32-
let!(:other_submission) { create :submission, mail_message_id: "abc", delivery_status: :delivery_pending, reference: "other-submission-reference", form_id: 2, answers: form_with_file_upload_answers }
32+
let!(:other_submission) { create :submission, mail_message_id: "abc", delivery_status: :pending, reference: "other-submission-reference", form_id: 2, answers: form_with_file_upload_answers }
3333

3434
let(:output) { StringIO.new }
3535
let(:logger) do

spec/jobs/receive_submission_deliveries_job_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
let(:mail_message_id) { "mail-message-id" }
3131
let(:reference) { "submission-reference" }
3232
let!(:submission) { create :submission, created_at: Time.zone.parse("2025-05-09T10:25:35.001Z"), mail_message_id:, reference:, form_id: form_with_file_upload.id, form_document: form_with_file_upload, answers: form_with_file_upload_answers }
33-
let!(:other_submission) { create :submission, created_at: Time.zone.parse("2025-05-09T10:25:35.001Z"), mail_message_id: "abc", delivery_status: :delivery_bounced, reference: "other-submission-reference", form_id: 2, answers: form_with_file_upload_answers }
33+
let!(:other_submission) { create :submission, created_at: Time.zone.parse("2025-05-09T10:25:35.001Z"), mail_message_id: "abc", delivery_status: :bounced, reference: "other-submission-reference", form_id: 2, answers: form_with_file_upload_answers }
3434

3535
let(:output) { StringIO.new }
3636
let(:logger) do

spec/jobs/send_submission_job_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
include ActiveJob::TestHelper
66

77
let(:submission_created_at) { Time.utc(2022, 12, 14, 13, 0o0, 0o0) }
8-
let(:submission) { create :submission, form_document: form, delivery_status: :delivery_pending, created_at: submission_created_at }
8+
let(:submission) { create :submission, form_document: form, delivery_status: :pending, created_at: submission_created_at }
99
let(:form) { build(:form, id: 1, name: "Form 1") }
1010
let(:question) { build :text, question_text: "What is the meaning of life?", text: "42" }
1111
let(:step) { build :step, question: }
@@ -53,7 +53,7 @@
5353
end
5454

5555
it "updates the sent at time" do
56-
expect(submission.reload.sent_at).to be_within(1.second).of(@job_ran_at)
56+
expect(submission.reload.last_delivery_attempt).to be_within(1.second).of(@job_ran_at)
5757
end
5858

5959
it "sends cloudwatch metric for the submission being sent" do

spec/lib/tasks/submissions.rake_spec.rb

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
end
1717

1818
before do
19-
create :submission, :sent, delivery_status: :delivery_pending, reference: "test_ref"
19+
create :submission, :sent, delivery_status: :pending, reference: "test_ref"
2020
end
2121

2222
it "displays submission data when found" do
@@ -41,11 +41,11 @@
4141
before do
4242
create :submission,
4343
:sent,
44-
delivery_status: :delivery_pending
44+
delivery_status: :pending
4545

4646
create_list :submission, 2,
4747
:sent,
48-
delivery_status: :delivery_bounced
48+
delivery_status: :bounced
4949
end
5050

5151
it "logs how many submissions there are for each mail status" do
@@ -69,20 +69,20 @@
6969
create :submission,
7070
:sent,
7171
form_id:,
72-
delivery_status: :delivery_bounced
72+
delivery_status: :bounced
7373
end
7474
let!(:pending_submission) do
7575
create :submission,
7676
:sent,
7777
form_id:,
78-
delivery_status: :delivery_pending
78+
delivery_status: :pending
7979
end
8080

8181
before do
8282
create :submission,
8383
:sent,
8484
form_id: other_form_id,
85-
delivery_status: :delivery_pending
85+
delivery_status: :pending
8686
end
8787

8888
context "with valid arguments" do
@@ -146,7 +146,7 @@
146146
end
147147

148148
let(:form_id) { 1 }
149-
let(:delivery_status) { :delivery_bounced }
149+
let(:delivery_status) { :bounced }
150150
let(:reference) { "submission-reference" }
151151
let(:args) { [reference] }
152152

@@ -188,7 +188,7 @@
188188
end
189189

190190
context "when the submission has not bounced" do
191-
let(:delivery_status) { :delivery_pending }
191+
let(:delivery_status) { :pending }
192192

193193
it "logs that there the submission hasn't bounced" do
194194
allow(Rails.logger).to receive(:info)

spec/models/submission_spec.rb

Lines changed: 3 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,12 @@
3030

3131
describe "validations" do
3232
it "is valid for a submission's delivery_status to be pending" do
33-
submission.delivery_pending!
33+
submission.pending!
3434
expect(submission).to be_valid
3535
end
3636

3737
it "is valid for a submission's delivery_status to be bounced" do
38-
submission.delivery_bounced!
38+
submission.bounced!
3939
expect(submission).to be_valid
4040
end
4141

@@ -46,36 +46,9 @@
4646

4747
describe "delivery_status enum" do
4848
it "returns a list of delivery statuses" do
49-
expect(described_class.delivery_statuses.keys).to eq(%w[delivery_pending delivery_bounced])
49+
expect(described_class.delivery_statuses.keys).to eq(%w[pending bounced])
5050
expect(described_class.delivery_statuses.values).to eq(%w[pending bounced])
5151
end
5252
end
5353
end
54-
55-
describe "mail_status" do
56-
let(:submission) { create :submission }
57-
58-
describe "validations" do
59-
it "is valid for a submission's mail_status to be pending" do
60-
submission.pending!
61-
expect(submission).to be_valid
62-
end
63-
64-
it "is valid for a submission's mail_status to be bounced" do
65-
submission.bounced!
66-
expect(submission).to be_valid
67-
end
68-
69-
it "is not valid for a submission's mail_status to be something else" do
70-
expect { submission.mail_status = "some other string" }.to raise_error(ArgumentError).with_message(/is not a valid mail_status/)
71-
end
72-
end
73-
74-
describe "mail_status enum" do
75-
it "returns a list of mail statuses" do
76-
expect(described_class.mail_statuses.keys).to eq(%w[pending bounced])
77-
expect(described_class.mail_statuses.values).to eq(%w[pending bounced])
78-
end
79-
end
80-
end
8154
end

0 commit comments

Comments
 (0)