Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve DSL for notifications #14

Closed
abitdodgy opened this issue May 15, 2013 · 2 comments
Closed

Improve DSL for notifications #14

abitdodgy opened this issue May 15, 2013 · 2 comments

Comments

@abitdodgy
Copy link

This is really minor but it makes sense. When processing a notification, you need to call @notification.id to pull up the reference number (order number you provided).

I think it readers better if it were @notification.order_id or @notification.reference_id to stay consistent with PagSeguro's API. Check this out:

class PagSeguroNotification
  def initialize(notification_code)
    @notification = PagSeguro::Notification.new(PAGSEGURO_EMAIL, PAGSEGURO_TOKEN, notification_code)
  end

  def get
    # I'm grabbing the order by calling @notification.id, which can be confusing
    # Calling @payment.id doesn't read better either 
    order = Order.find(@payment.id)
    # ...
  end
end
@teonimesic
Copy link
Member

I guess we could create an alias to id as reference (and not reference_id, since reference is the actual attr name that pagseguro returns). I dont like order_id, because users may not name it Order. It could be Invoice, Payment or something else completely.

But i dont intend to remove id, i really think that it makes sense for a notification's id to return the reference, since it is supposed to be a unique payment identifier on the system. It also is very well documented, both on specs and README.

@teonimesic
Copy link
Member

I added a reference method to both payment and notifications. So you can now do something like:

PagSeguro::Payment.new email, token, reference: "some id reference"
notification.reference # => "some id reference"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants