Updating an auto-generated invoice (future) when current plan changes I've built-out a mechanism to go and find an auto-generated invoice and update it with new information when a plan is changed or if users are added to the plan for the current billing period. It will make it a lot easier than having to remember to change it a month, quarter, 6 months, or even a year from now. And this also reduces the burden on the finance department from having to go find the future invoice and manually updating it. I hope my comments in the code are clear.

I'm mainly looking for feedback on my code. It does work. I will need to write tests for this though.

Model

def new_amount(max_users, item)
  amount = ((max_users.to_i * item.amount) / item.quantity)
  # Returns the new adjusted amount
  case term_type
  when 'month'      then amount / 12
  when 'quarter'    then amount / 4
  when 'semiannual' then amount / 2
  when 'annual'     then amount / 1
  else                   amount
  end
end


Controller

  # Finds the upcoming / unpaid system invoice.
  manual_invoice = @subscription.manual_invoices.system.not_paid.latest.first

  # TODO: This is very hacky. If I use dup or clone on manual_invoice, and
  #   when change_plan is executed, the calculation runs, regardless if
  #   there were any updates. Also, chaining these scopes brings back all
  #   of the records when using .latest... so, I end up with an invoice
  #   where paid_at is nil.
  first_manual_invoice = @subscription.manual_invoices.system.paid.first

  # Guards changing of plan from blowing up if manual_invoice.nil?
  if manual_invoice
    # Iterate through the invoice's items.
    first_manual_invoice.items.each do |item|
      # Set new_amount by taking the new max_users quantity and multiply it
      #   by the previous amount. Then divide it by the previous max_users
      #   quantity. Finally, divide by payment frequency per term.
      new_amount = @subscription.new_amount(max_users, item)
      # Update the manual_invoice items with the calcuated new_amount and
      #   user quantity.
      manual_invoice.items.first.update_attributes(quantity: max_users,
                                                   amount: new_amount)
      # Update the manual_invoice.
      manual_invoice.update_attributes(amount_in_cents: new_amount,
                                       term_type: term_type,
                                       term_start_date: term_start_date,
                                       term_end_date: term_end_date,
                                       due_on: term_end_date)
    end
  end
Your code is very readable. Given the provided snippets of code, I won't focus this review on overall architecture (as I'm not sure what the entire system does). I'd like focus on two areas of this code: comments and method names.

When reading your comments I asked my self the following question (taken directly from Code Complete 2).

Do comments explain the code's intent or summarize what the code does, rather than just repeating the code?

# Returns the new adjusted amount
# Iterate through the invoice's items.
# Update the manual_invoice.


These comments repeat what your code does. Removing them would improve the readability and reduce the length of your code.

Next, the following comment and method invocation:

# Set new_amount by taking the new max_users quantity and multiply it
#   by the previous amount. Then divide it by the previous max_users
#   quantity. Finally, divide by payment frequency per term.
new_amount = @subscription.new_amount(max_users, item)


A 3 line comment before a method call such as this is a great opportunity to rename your method name to be more descriptive and possibly avoid a comment altogether. Here is an idea of a possible name:

adjusted_amount = @subscription.adjust_amount(max_users, item)


What other method names would better describe what this method does? Could you move this method to another Model? 

Don't be afraid to use 3 or more words to describe a method if that's what it takes. Better to have a long method name instead of a 3-line comment.

I believe you could improve these two lines:

# Guards changing of plan from blowing up if manual_invoice.nil?
if manual_invoice


By replacing them with one line:

unless manual_invoice.nil?


Finally, inside the method new_amount I believe referencing model attributes with self would the improve the readability of your code by differentiating parameters from model attributes. 

case self.term_type
# ...
end