0

I need to generate invoices once per month, according to the EST/EDT timezone (the clients are throughout the country but in this industry billing happens at the same timezone).

I'm creating a GenerateInvoicesJob but I'm having trouble reasoning about a 100% perfect way to generate invoices so that there isn't any possible duplicate/confusion with regards to:

  • Generate invoices only once per month
  • Let the job run every day
  • Make the job idempotent

Then the final point which is the hard one for me, how do I ensure that there are no bugs with EST/DST and 1 hour slipping through.

Here is my clock.rb:

every(1.day, 'GenerateInvoicesJob', tz: 'America/New_York', at: '04:00') do
  Delayed::Job.enqueue GenerateInvoicesJob.new, queue: 'high'
end

And here's the top of my job:

Unit.where(enabled: true)
  .joins(:user)
  .where('last_invoice_generated_at <= ?', Time.now.utc.end_of_month)
  .each do |unit|

  ActiveRecord::Base.transaction do
    unit.update_attributes(
      last_invoice_generated_at: Time.now.utc
    )
    invoice = Invoice.create!(
      ...
    )
    line_item = LineItem.create!(
      ...
    )
  end

I realize the direct conditional logic might be wrong, so that's not entirely my question... my main addition to that question is whats the best way overall to do this so I can make sure that all times in EST are 100% accounted for, including weird off-by-1-hour bugs, etc. This job is super important so I'm hesitant on a way to make it perfect.

On top of that I"m not sure whether I should store UTC in the database.... normally I know you always are supposed to store UTC, but I know UTC doesnt have DST so I'm afraid if I store it like that, the job could run one time and invoices would be not run properly

Tallboy
  • 12,847
  • 13
  • 82
  • 173

1 Answers1

0

I would do something like this in the worker:

# `beginning_of_month` because we want to load units that haven't 
# been billed this month
units_to_bill = Unit.where(enabled: true)
  .where('last_invoice_generated_at < ?', Time.current.beginning_of_month)

# `find_each` because it needs less memory
units_to_bill.find_each do |unit|

  # Beginn a transaction to ensure all or nothing is updated
  Unit.transaction do

    # reload the unit, because it might have been updated by another 
    # task in the meantime
    unit.reload

    # lock the current unit for updates
    unit.lock!

    # check if the condition is still true
    if unit.last_invoice_generated_at < 1.month.ago

      # generate invoices
      # invoice = Invoice.create!(
      # line_item = LineItem.create!(

      # last step update unit
      unit.update_attributes(
        last_invoice_generated_at: Time.current
      )
    end
  end
end
spickermann
  • 100,941
  • 9
  • 101
  • 131
  • Hmm, how is Time.current different than Time.now.utc (if my server is set to UTC. I had just been tacking on 'utc' just to be save, even though its not needed) – Tallboy Jan 11 '17 at 18:19
  • Also, does the transaction finishing automatically unlock a record? Ive never used that method, looks great – Tallboy Jan 11 '17 at 18:26
  • 1
    `Time.current` is [preferred over](http://stackoverflow.com/a/12748804/2483313) `Time.now`. And yes, the end of the transaction also releases the lock on the `unit`. – spickermann Jan 11 '17 at 18:54
  • Ahh interesting. And so does Time.current (with Rails set to `UTC`) still ackwnoledge +/- 1 hour DST? – Tallboy Jan 11 '17 at 18:57
  • No, it doesn't but that not important in the context of your worker, because the worker only cares about not generating duplicate invoices. But it it the external clock's responsibility to start the worker on time and it does (as far at it looks from your code above). – spickermann Jan 11 '17 at 19:02