Why doesn't my Object update?

Question

I have this test:

 describe 'check_account_status' do
  it 'should send the correct reminder email one week prior to account disablement' do
  # Three weeks since initial email
  reverification = create(:reverification)
  initial_notification = reverification.twitter_reverification_sent_at.to_datetime
  ActionMailer::Base.deliveries.clear
  Timecop.freeze(initial_notification + 21) do
    Reverification.check_account_status
    ActionMailer::Base.deliveries.size.must_equal 1
    ActionMailer::Base.deliveries.first.subject.must_equal I18n.t('.account_mailer.one_week_left.subject')
    reverification.reminder_sent_at.class.must_equal ActiveSupport::TimeWithZone
    reverification.notification_counter.must_equal 1
    must_render_template 'reverification.html.haml'
  end
end

This test produces this error:

check_account_status#test_0001_should send the correct reminder email one week prior to account disablement [/Users/drubio/vms/ohloh-ui/test/models/reverification_test.rb:67]:
Expected: ActiveSupport::TimeWithZone
Actual: NilClass

Here is my code:

class Reverification < ActiveRecord::Base
  belongs_to :account

  FIRST_NOTIFICATION_ERROR = []

  class << self
    def check_account_status
      Reverification.where(twitter_reverified: false).each do |reverification|
        calculate_status(reverification)
        one_week_left(reverification)
      end
    end

    private

    def calculate_status(reverification)
      @right_now = Time.now.utc.to_datetime
      @initial_email_date = reverification.twitter_reverification_sent_at.to_datetime
      @notification_counter = reverification.notification_counter
    end

    def one_week_left(reverification)
    # Check to see if three weeks have passed since the initial email
    # and check to see if its before the one day notification before
    # marking an account as spam.
      if (@right_now.to_i >= (@initial_email_date + 21).to_i) && (@right_now.to_i < (@initial_email_date + 29).to_i)
        begin
          AccountMailer.one_week_left(reverification.account).deliver_now
        rescue
          FIRST_NOTIFICATION_FAILURE << account.id
          return
        end
        update_reverification_fields(reverification)
      end
    end

    def update_reverification_fields(reverification)
      reverification.notification_counter += 1
      reverification.reminder_sent_at = Time.now.utc
      reverification.save!
      reverification.reload
    end
  end

Forgive the indentation, but what seems to be the problem, is that my reverification object doesn't update when it leaves the check_account_status method. I've placed puts statements through out the code and I can see without a doubt that the reverification object is indeed updating. However after it leaves the update_reverification_fields and returns to the test block, the fields are not updated. Why is that? Has anyone encountered this?

score 1 · Accepted Answer · edited May 23 '17 at 11:58

1

I believe you have a scope issue, the methods you call from check_account_status certainly don't return the updated object back to your method and Ruby only passes parameters by value.

Try something like this instead:

def check_account_status
  Reverification.where(twitter_reverified: false).each do |reverification|
    reverification = calculate_status(reverification)
    reverification = one_week_left(reverification)
  end
end

private 
  def calculate_status(reverification)
    # ...
    reverification
  end
  def one_week_left(reverification)
    # ...
    reverification = update_reverification_fields(reverification)
    reverification
  end
  def update_reverification_fields(reverification)
    # ...
    reverification
  end

edited May 23 '17 at 11:58

Community

1
1

answered Sep 22 '15 at 19:29

sjagr

15,983
5
40
67

Thanks for the reply sjagr. Could you please take a look at the edit I made to the Reverification code. I realized the indentation mattered because I defined the methods as Class methods not instance methods. This should clarify the scope issue. – Dan Rubio Sep 22 '15 at 19:35
@DanRubio It's still a scoping issue to me – sjagr Sep 22 '15 at 19:43

Sergio Tulentsev · Answer 2 · 2015-09-22T20:13:49.560

1

The problem is that reverification object in your test and objects inside of check_account_status are different instances of the same model.

def update_reverification_fields(reverification)
  reverification.notification_counter += 1
  reverification.reminder_sent_at = Time.now.utc
  reverification.save!
  reverification.reload
end

This reload here, it's doing nothing. Let's walk through your test.

# check_account_status runs a DB query, finds some objects and does things to them
Reverification.check_account_status

# this expectation succeeds because you're accessing `deliveries` for the 
# first time and you don't have it cached. So you get the actual value
ActionMailer::Base.deliveries.size.must_equal 1

# this object, on the other hand, was instantiated before 
# `check_account_status` was called and, naturally, it doesn't see 
# the database changes that completely bypassed it.
reverification.reminder_sent_at.class.must_equal ActiveSupport::TimeWithZone

So, before making expectations on reverification, reload it, so that it pulls latest data from the DB.

reverification.reload # here
reverification.reminder_sent_at.class.must_equal ActiveSupport::TimeWithZone

edited Sep 22 '15 at 20:13

answered Sep 22 '15 at 20:08

Sergio Tulentsev

226,338
43
373
367

Also a good solution - but can cover symptoms. If `one_week_left` depended on an updated `reverification` in the future from `calculate_status`, for example, then `one_week_left` could produce inaccurate results. – sjagr Sep 22 '15 at 20:18
@sjagr: yeah, that part of the code is horrible. No need to pass reverification as a parameter at all. Just do things on `self`. But this type of error is quite common (forgetting to refresh the object under test) – Sergio Tulentsev Sep 22 '15 at 20:21
@sjagr: while we're at it, I don't see how your answer will fix his spec :) (which is, you know, the question) – Sergio Tulentsev Sep 22 '15 at 20:23
RE "just do things on self" - I see that all those are class methods. So yes, this kinda justifies the parameter. – Sergio Tulentsev Sep 22 '15 at 20:26
@sjagr: that reassignment of `reverification` suggested in your answer - it's doing nothing. If `calculate_status` were to update its parameter, `one_week_left` would see the changes. Because `reverification` is passed-by-value-which-is-reference – Sergio Tulentsev Sep 22 '15 at 20:30
Well I do agree. `one_week_left` and `update_reverification_fields` don't need to be class methods and would work better as instance methods, then the `reload` isn't necessary at all. The question was asking why the fields weren't updated - I answered in a way that helped OP understand scope. Different flavours to the approach. Yours is still quite correct hence my upvote. – sjagr Sep 22 '15 at 20:30

Why doesn't my Object update?

2 Answers2