Skip to content

Commit

Permalink
Use new unsubscribed_at attribute for more accuracy and simplicity
Browse files Browse the repository at this point in the history
Currently we work out when someone unsubscribed by looking at
updated_at time for unsubscribed alerts. This might not always be
accurate. It's also adds extra unexpected behaviour for us to understand
and maintain.

This commit swaps that calculation out and instead simply uses the new
unsubscribed_at attribute.

Also adds a factory for unsubscribed alerts.
  • Loading branch information
equivalentideas committed Nov 3, 2016
1 parent 901d0b7 commit c18ebcc
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 14 deletions.
6 changes: 3 additions & 3 deletions app/models/alert.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ def self.count_of_new_unique_email_created_on_date(date)
end

def self.count_of_email_completely_unsubscribed_on_date(date)
emails = where("date(updated_at) = ?", date).where(unsubscribed: true).
distinct.
pluck(:email)
emails = where("date(unsubscribed_at) = ?", date).where(unsubscribed: true).
distinct.
pluck(:email)

emails.reject do |email|
active.where(email: email).where("date(created_at) <= ?", date).any?
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/performance_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
before do
alert = create :confirmed_alert, email: "mary@example.org", created_at: DateTime.new(2016,6,19)

Timecop.freeze(Time.utc(2016, 8, 23)) { alert.update_attribute(:unsubscribed, true) }
Timecop.freeze(Time.utc(2016, 8, 23)) { alert.unsubscribe! }

get(:alerts, format: :json)
end
Expand Down
5 changes: 5 additions & 0 deletions spec/factories.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@
factory :confirmed_alert do
confirmed true
confirm_id "1234"

factory :unsubscribed_alert do
unsubscribed true
unsubscribed_at Time.now
end
end
end

Expand Down
20 changes: 10 additions & 10 deletions spec/models/alert_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@
end

Timecop.freeze(Date.new(2016, 8, 24)) do
a.update_attribute(:unsubscribed, true)
a.unsubscribe!
end

a
Expand All @@ -255,16 +255,16 @@
end

it "is 0 when there is no complete unsubscribes" do
create(:confirmed_alert, email: "foo@email.com", unsubscribed: true)
create(:confirmed_alert, email: "foo@email.com", unsubscribed: true)
create(:confirmed_alert, email: "foo@email.com", unsubscribed: false)
create(:unsubscribed_alert, email: "foo@email.com")
create(:unsubscribed_alert, email: "foo@email.com")
create(:confirmed_alert, email: "foo@email.com")

expect(Alert.count_of_email_completely_unsubscribed_on_date(Date.today)).to eql 0
end

it "only counts unique emails" do
create(:confirmed_alert, email: "foo@email.com", unsubscribed: true)
create(:confirmed_alert, email: "foo@email.com", unsubscribed: true)
create(:unsubscribed_alert, email: "foo@email.com")
create(:unsubscribed_alert, email: "foo@email.com")

expect(Alert.count_of_email_completely_unsubscribed_on_date(Date.today)).to eql 1
end
Expand All @@ -273,8 +273,8 @@
alert1 = create(:confirmed_alert, email: "foo@email.com")
alert2 = create(:confirmed_alert, email: "bar@email.com")

Timecop.freeze(Time.utc(2016, 8, 23)) { alert1.update_attribute(:unsubscribed, true) }
Timecop.freeze(Time.utc(2016, 8, 24)) { alert2.update_attribute(:unsubscribed, true) }
Timecop.freeze(Time.utc(2016, 8, 23)) { alert1.unsubscribe! }
Timecop.freeze(Time.utc(2016, 8, 24)) { alert2.unsubscribe! }

expect(Alert.count_of_email_completely_unsubscribed_on_date(Date.new(2016, 8, 23))).to eql 1
end
Expand All @@ -285,7 +285,7 @@
create(:confirmed_alert, email: "foo@email.com")
end

Timecop.freeze(Time.utc(2016, 8, 23)) { alert.update_attribute(:unsubscribed, true) }
Timecop.freeze(Time.utc(2016, 8, 23)) { alert.unsubscribe! }

Timecop.freeze(Time.utc(2016, 8, 28)) do
create(:confirmed_alert, email: "foo@email.com")
Expand All @@ -302,7 +302,7 @@
create(:confirmed_alert, email: "foo@email.com")
end

Timecop.freeze(Time.utc(2016, 9, 1)) { alert.update_attribute(:unsubscribed, true) }
Timecop.freeze(Time.utc(2016, 9, 1)) { alert.unsubscribe! }
end

it "doesn't count their unsubscribe" do
Expand Down

0 comments on commit c18ebcc

Please sign in to comment.