search

Sailias / bitcoin_payable

A rails bitcoin payment processing gem
MIT License
121 stars 31 forks source link

Improvements on top of PR33 #34

Closed maesitos closed 6 years ago

maesitos commented 6 years ago

Small changes on top pf https://github.com/Sailias/bitcoin_payable/pull/33

Sailias commented 6 years ago

@maesitos I'm not a big fan of aggregating all notifications under a single name. A webhook contains many events which can be handled and updated in bulk.

I don't ever see a need for that and actually find it quite dangerous. If all hooks are created individually for each payment, it creates more separation which I feel can be good for many reasons. It can even allow us to create unique urls for better debugging.

POST /bitcoin_payable/notify_transaction/:payment_id

maesitos commented 6 years ago

I have no particular preference and I was trying to follow the way Blocktrail, in my view, thought the users will use the API. Nonetheless I have tested and I was able to create almost 1200+ webhooks so the API can handle a decent amount of pending payments with your proposal and by the way, the user interface does paginate.

screen shot 2018-03-15 at 19 07 44

I'd suggest to create new state for the payments canceled so that if the paren app decides to cancel the order, effectively never expecting to receive any payments, the web-hook can be cleaned up.

maesitos commented 6 years ago

@Sailias fixed. So basically the meat for this PR is the commit cdb93c5 The API wrapper raised an error if you try to set a web-hook that already exists.