Review Board 1.5.4

Handling IPN Cancel messages

Updated 2 years, 3 months ago

Tim Perrett Reviewers
wip_tim_issue_88
88 dchenbecker, dpp, marius, heiko
None LiftWeb-archive 
Patch to handle Cancel message from IPN not having the payment status parameter. Boxed the value so that now it returns Empty for a cancel message. Sweet. 
Sample testing with the IPN Simulator on developer.paypal.com - when i figure out lift-testkit i'll write some specs for this stuff.
Derek Chen-Becker
Posted 2 years, 3 months ago (October 9th, 2009, 9:52 p.m.)
Generally looks good but I have a few comments.
Very minor, but that's a typo on incoming.
  1. Fair enough, i'll change it - the comments need rewriting anyway I think.
What's the point of having a private ref here?
  1. Its private because all the possible options that could be passed by paypal are account for as val properties. No one has ever said they need direct access to the parameters but if that happened and had a good use case id happily open it up. 
  2. Sorry, I just meant that because there's already a "val" keyword on the params constructor parameter, isn't that already accessible? If you really want to hide the underlying HasParams then I think we need to remove that "val" keyword.
  3. Ah crap yeah - well spotted batman! I'll remove the val assignment and then we should be good to go. w00t.
I don't think that this file should be committed as part of this patch if the contents are entirely commented out.
  1. This is something im going to work on and based on the other discussion had on the ML i thought that tests didn't need to go through reviewboard? This only replaces some commented out stub tests so at least this is a move in the right direction! 
  2. Good point. It just looked funny to have a fully commented out file :)
Review request changed
Updated 2 years, 3 months ago (October 11th, 2009, 4:41 a.m.)
Removed the paramater val
Ship it!
Derek Chen-Becker
Posted 2 years, 3 months ago (October 11th, 2009, 10:28 a.m.)
Good to go.