Be wary of hard coding array positions

This is part of the Semicolon&Sons Code Diary - consisting of lessons learned on the job. You're in the workflows category.

Last Updated: 2024-04-18

Whenever a customer pays on PayPal, the Sale object needed for me to verify payment completion server side resides deeply nested within a separate Payment object (also from PayPal).

I deployed this accessor code:

def successful_paypal_rest_sale
  return unless paypal_rest_payment

  paypal_rest_payment.transactions[0].related_resources[0].sale
end

After a week, I noticed that some customers with valid sales were having issues paying. This is because I had naively assumed that there would only be one transaction via this line paypal_rest_payment.transactions[0]. In fact, if a customer attempts to pay three times, e.g. with different credit cards, there will be multiple transactions and the one at index 0 may be a failed attempt.

The lesson here is to be super vigilant whenever you are about to hard-code an array position. Instead you need to iterate as below:

def paypal_rest_sales
  return unless paypal_rest_payment

  paypal_rest_payment.transactions.map do |transaction|
    transaction.related_resources[0].sale
  end
end

def successful_paypal_rest_sale
  return unless paypal_rest_payment

  paypal_rest_sales.find { |sale| sale.state == 'completed' }
end