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 -> List(subscr_cancel),
> >> > > >> > 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)
>
> >> > > >> > Thanks,
>
> >> > > >> > Ryan
>
> >> > > >> > On Oct 7, 6:18 pm, Timothy Perrett <timo...@getintheloop.eu>
> >> wrote:
> >> > > >> >> Ryan,
>
> >> > > >> >> The PayPal stuff is pretty much my baby - its awesome that your
>
> >> > > >> using
> >> > > >> >> it in production!
>
> >> > > >> >> See what your saying - its been a while since I did anything
> >> with
> >> > > >> >> PayPal; what fields *are*
>
> ...
>
> read more »
>
> Paypal.diff
> 2KViewDownload
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
