Ryan, I have pushed the code to master so give it a couple of hours and Hudson should automatically start pulling those changes into 1.1- SNAPSHOT JARs for you. Alternatively do a pull and build locally.
Cheers, Tim On 10 Oct 2009, at 01:38, Timothy Perrett wrote: > > 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> >>> 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 >>>> <[email protected]>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 <[email protected]> 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 <[email protected]> >>>>>> 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 <[email protected]> >>>>>>> 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 >>>>> <[email protected] >>>>>>>>> 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 >>>>> <[email protected] >>>>>>>>>> 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 >>>>>>>>>>> ([email protected]), >>>>>>>>>> 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 >>>>>>>>>> ([email protected]), business -> List >>>>>>>>>> ([email protected]) >> >>>>>>>>>> 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 >>>>>>>>>>> ([email protected]), >>>>>>>>>> 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 [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/liftweb?hl=en -~----------~----~----~----~------~----~------~--~---
