heavenstudio / pag_seguro

A Ruby Client to deal with PagSeguro's API
http://heavenstudio.com.br
71 stars 38 forks source link

Sandbox #20

Closed marcosteixeira closed 10 years ago

marcosteixeira commented 10 years ago

Add PagSeguro sandbox support.

In order to get PagSeguro's urls we can call PagSeguro::Url.site_url("/some/path") or PagSeguro::Url.api_url("/some/path") .

This methods returns the api or site url based on configured environment.

To change the environment:

# eg: config/initializers/pagseguro.rb 
PagSeguro::Url.environment= :sandbox  #production by default

I update some gems and the rspec sintax.

teonimesic commented 10 years ago

Awesome pull request! There are a few things i would like you to do if possible before merging it:

First, there's a change to PAGSEGURO_TRANSACTIONS_URL, and it doens't seems there is a unit test for this constant anywhere, so it would be very cool if you could add a test to it, which should be very similar to your sandbox ::CHECKOUT_URL spec.

Second, i think this changes requires some sort of change in the README so people can understand how to use the sandbox feature. Can you describe where can you download a current pag seguro server simulator, and how to change the sandbox url to match it? And possible caveats, if any.

Finally, in future pull requests try to add the feature itself in one pull request, and then send a second pull request with refactor and gem / syntax changes. It makes it easier to understand what changes were actually introduced in the pull request if it only does 1 thing :)

marcosteixeira commented 10 years ago

I remove PAGSEGURO_TRANSACTIONS_URL and CHECKOUT_URL constants. I use PagSeguro::Payment.checkout_url and PagSeguro::Transaction.transactions_url class methods instead. These changes facilitated because this methods do not record the environment value and we can change at any time.

I do not know if the readme is good. I can change that.

Thanks for the pull requests tip. I'm starting in ruby and contributing in opensource projects. I was too scared to send this pull request :)

abitdodgy commented 10 years ago

Will this be merged?