Ryan, Ignore my last email please - i've just tested the change using the IPN simulator and it now handles the Cancel message properly by passing Empty.
The change is on my branch here: http://github.com/dpp/liftweb/commit/451dd3cb97e562a063da5cfe046badf1f9d8ad4c Cheers, Tim On Oct 10, 1:05 am, Timothy Perrett <timo...@getintheloop.eu> wrote: > Ryan, > > Looking at it, the strange thing is actually why it compiles now, not > why it doesn't compile with the change you suggested. > > Given: > > for (info <- buildInfo(resp, r); > // stat is going to be a Box[PaypalTransactionStatus.Value] anyway > // because of L489. > stat <- info.paymentStatus) yield { > actions((stat, info, r)) > true > > } > > So, it appears that adding the Box[] to the partial function > definition would just make the type exactly right. Im starting to > write some mock tests etc as this is going to need testing > programatically. > > More to come soon. > > Cheers, Tim > > On Oct 9, 7:03 pm, Ryan Donahue <donahu...@gmail.com> wrote: > > > > > Here's a diff showing the changes I made. Notice I added a case to the > > SimplePaypal.actions method that I'd think would fail compilation but does > > not. > > > On Fri, Oct 9, 2009 at 1:08 PM, Ryan Donahue <donahu...@gmail.com> wrote: > > > Well, I am a scala newb, but I know maven all too well. I ran "mvn clean > > > install" from the lift-paypal dir to install lift-paypal-1.1-SNAPSHOT.jar > > > to > > > my local repo. > > > > To be sure, I changed the signature as follows which does cause errors: > > > def actions: PartialFunction[(PayPalInfo, Req), Unit] > > > > Change back to def actions: > > > PartialFunction[(Box[PaypalTransactionStatus.value], PayPalInfo, Req), > > > Unit] > > > and no errors. > > > > On Fri, Oct 9, 2009 at 12:35 PM, Timothy Perrett > > > <timo...@getintheloop.eu>wrote: > > > >> Hey Ryan, > > > >> How *exactly* did you locally do the build? If you had done the > > >> install of your altered lift-paypal then you would certainly get a > > >> compile error because the signature has changed. The new syntax should > > >> be: > > > >> object MyIPN extends PaypalIPN { > > >> def actions = { > > >> case (Full(CompletedPayment), info, req) => // do something > > >> } > > >> } > > > >> The only exclusion would be if you had a implicit conversion to Box > > >> PaypalTransactionStatus types that were unboxed. > > > >> Cheers, Tim > > > >> On Oct 9, 3:46 pm, Ryan Donahue <donahu...@gmail.com> wrote: > > >> > Tim, > > > >> > I locally changed the PaypalIPN.actions method return type to > > >> > trait PaypalIPN { > > >> > def actions: PartialFunction[(Box[PaypalTransactionStatus.Value], > > >> > PayPalInfo, Req), Unit] > > > >> > } > > > >> > Apparently this does not cause any compilation errors for user > > >> > implementing their own IPN handler as follows > > >> > object MyIPN extends PaypalIPN { > > >> > import PaypalTransactionStatus._ > > >> > def actions = { > > >> > case (CompletedPayment, info, req) => // do something > > >> > } > > > >> > } > > > >> > This is not good since I assume the result is that the case won't > > >> > match anymore but we won't have a compilation error to tell us to > > >> > change our code. Maybe I missed something, I am a scala newbie :) > > > >> > -Ryan > > > >> > On Oct 8, 3:57 pm, Timothy Perrett <timo...@getintheloop.eu> wrote: > > > >> > > Ok cool, I'll take a look at this tomrrow all being well. > > > >> > > Thanks for the feedback > > > >> > > Cheers, Tim > > > >> > > Sent from my iPhone > > > >> > > On 8 Oct 2009, at 20:43, Ryan Donahue <donahu...@gmail.com> wrote: > > > >> > > > I created the ticket:http://github.com/dpp/liftweb/issues/#issue/88 > > > >> > > > I do receive the payment_status field for PDT. I bet in practice > > >> > > > you will never receive a payment_status value other than Completed, > > > >> > > > because if the payment was not completed PayPal would not redirect > > >> > > > the user's browser back to your PDT URL. However, I have not > > >> > > > verified this and do check the payment status in my PDT code anyway > > > >> > > > (how could I verify that it will never happen?). So I would prefer > > > >> > > > that both were consistent, but just boxing the IPN payment status > > >> > > > will be fine too :) > > > >> > > > On Thu, Oct 8, 2009 at 2:49 PM, Timothy Perrett > > >> <timo...@getintheloop.eu > > >> > > > > wrote: > > >> > > > Im not married to the current API, so breaking changes are OK as > > >> > > > there are only a handful of people using this code right now. > > > >> > > > To be honest, this whole situation just underlines the need for > > >> > > > mocking in this module of lift... i've been meaning to do it since > > >> > > > the beginning but just never got round to it and lack of general > > >> > > > demand. > > > >> > > > Just about why they have a different signature... if memory serves > > >> > > > that would be because PaypalTransactionStatus is not supplied for > > >> > > > PDT. So whilst I see your point about them being consistent, im not > > > >> > > > sure there is value in having a Box parameter that would always be > > >> > > > Empty? For IPN however, boxing sounds good to me. > > > >> > > > Should just be a case of: > > > >> > > > Box !! info.paymentStatus > > > >> > > > Would you be happy with that? If you could raise a ticket for this > > >> > > > issue i'll patch it tomorrow and commit the code on a branch > > >> > > > related > > > >> > > > to that ticket. One the code goes through review then it will make > > >> > > > it into the master branch all being well. > > > >> > > > Cheers, Tim > > > >> > > > On 8 Oct 2009, at 18:52, Ryan Donahue wrote: > > > >> > > >> Yeah, I noticed my email got mangled. > > > >> > > >> It would make sense to me if PaypalIPN.actions and > > >> > > >> PaypalPDT.pdtResponse were consistent. > > > >> > > >> trait PaypalPDT { > > >> > > >> def pdtResponse: PartialFunction[(PayPalInfo, Req), LiftResponse] > > >> > > >> } > > > >> > > >> trait PaypalIPN { > > >> > > >> def actions: PartialFunction[(PayPalInfo, Req), Unit] > > >> > > >> } > > > >> > > >> If matching the status is just too convenient to lose, then how > > >> > > >> about this. > > > >> > > >> trait PaypalPDT { > > >> > > >> def pdtResponse: PartialFunction[(Box > > >> > > >> [PaypalTransactionStatus.Value], PayPalInfo, Req), LiftResponse] > > >> > > >> } > > > >> > > >> trait PaypalIPN { > > >> > > >> def actions: PartialFunction[(Box[PaypalTransactionStatus.Value], > > > >> > > >> PayPalInfo, Req), Unit] > > >> > > >> } > > > >> > > >> Either would be an API breaking change for the paypal module, but > > >> > > >> at least it'd be caught be the compiler. What do you think? > > > >> > > >> On Thu, Oct 8, 2009 at 1:33 PM, Timothy Perrett > > >> <timo...@getintheloop.eu > > >> > > >> > wrote: > > > >> > > >> The parameters were pretty mangled in your email... Which one > > >> > > >> would > > >> > > >> you propose is more generic that the current status one we have? > > >> > > >> Alternatively, it sounds to me like we might need to add some kind > > >> of > > >> > > >> special case match statement. > > > >> > > >> Thoughts? > > > >> > > >> Cheers, tim > > > >> > > >> On 8 Oct 2009, at 13:14, Ryan Donahue wrote: > > > >> > > >> > Hi Tim, > > > >> > > >> > Looking at "Table 2. Summary of subscription variables" of the > > >> page > > >> > > >> > you reference, payment_status is not included for messages with > > >> > > >> > a > > >> > > >> > txn_type of subscr_ signup, subscr_ cancel, subscr_ modify, or > > >> > > >> > subscr_eot. > > > >> > > >> > This matches the results I see in testing with PayPal Sandbox. > > >> > > >> > Neither subscr_signup or subscr_cancel include the > > >> payment_status. > > >> > > >> > Below are the parameters from two messages I have received (they > > >> > > >> > contain fake user info generated by PayPal Sandbox, so no > > >> security/ > > >> > > >> > privacy concerns). > > > >> > > >> > params from subscr_signup message: > > >> > > >> > btn_id -> List(1070963), test_ipn -> List(1), charset -> List > > >> > > >> > (windows-1252), address_country -> List(United States), > > >> > > >> > item_name > > > >> > > >> -> > > >> > > >> > List(Pro Account), recurring -> List(1), address_state -> > > >> List(CA), > > >> > > >> > amount3 -> List(9.99), address_street -> List(1 Main St), > > >> > > >> > verify_sign - > > >> > > >> >> List(Ab4im9HUsk1pOL1dlUXJxEoipQcaAJQqV047JE2KENGFX4meCETo.KLt), > > >> > > >> > address_status -> List(confirmed), period3 -> List(1 M), > > >> > > >> subscr_date - > > >> > > >> >> List(05:01:02 Oct 08, 2009 PDT), first_name -> List(Test), > > >> > > >> > address_country_code -> List(US), address_city -> List(San > > >> > > >> > Jose), > > >> > > >> > mc_currency -> List(USD), mc_amount3 -> List(9.99), > > >> > > >> > residence_country - > > >> > > >> >> List(US), payer_email -> List(buyer1_1254939630_...@gmail.com), > > >> > > >> > address_name -> List(Test User), txn_type -> > > >> > > >> > List(subscr_signup), > > >> > > >> > payer_status -> List(verified), subscr_id -> List > > >> > > >> > (S-8X87353621486401E), last_name -> List(User), payer_id -> List > > >> > > >> > (JMM2RTLNU5RPJ), reattempt -> List(1), notify_version -> > > >> List(2.8), > > >> > > >> > custom -> List(2), address_zip -> List(95131), receiver_email -> > > > >> > > >> List > > >> > > >> > (seller_1254749970_...@gmail.com), business -> List > > >> > > >> > (seller_1254749970_...@gmail.com) > > > >> > > >> > params from subscr_cancel message: > > >> > > >> > btn_id -> List(1070963), test_ipn -> List(1), charset -> List > > >> > > >> > (windows-1252), address_country -> List(United States), > > >> > > >> > item_name > > > >> > > >> -> > > >> > > >> > List(Pro Account), recurring -> List(1), address_state -> > > >> List(CA), > > >> > > >> > amount3 -> List(9.99), address_street -> List(1 Main St), > > >> > > >> > verify_sign - > > >> > > >> >> List(AvpXEetWAO5JsntihTfwE4HykmNlAsappICrYh6PaVgG3JJUYLtOnqVU), > > >> > > >> > address_status -> List(confirmed), period3 -> List(1 M), > > >> > > >> subscr_date - > > >> > > >> >> List(05:06:19 Oct 08, 2009 PDT), first_name -> List(Test), > > >> > > >> > address_country_code -> List(US), address_city -> List(San > > >> > > >> > Jose), > > >> > > >> > mc_currency -> List(USD), mc_amount3 -> List(9.99), > > >> > > >> > residence_country - > > >> > > >> >> List(US), payer_email -> List(buyer1_1254939630_...@gmail.com), > > >> > > >> > address_name -> List(Test User), txn_type -> > > ... > > read more » --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Lift" group. To post to this group, send email to liftweb@googlegroups.com To unsubscribe from this group, send email to liftweb+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/liftweb?hl=en -~----------~----~----~----~------~----~------~--~---
