Thanks, David Glick from onenw also gave me some code review comments, so I'm going to work through his suggestions today. (I'll attach them). I'll also remove the log line. Currently I don't have plans to use it in production, but hopefully ONE/Northwest will have a client use case someday. I have sent transactions from this to the paypal test site, and I have used payflowpro in a past job, but that was with a java wrapper around the http post api.
David's Notes: Docs - Include a couple lines about what PayFlowPro is and when you would want to use it vs. PayPal's other offerings - Maybe improve installation instructions (what to change in buildout, how to configure and activate the plugin after starting up Plone) - Note the missing implementation of refunds - Note dependency on the rest of GetPaid, just to be clear :) - Limit line length to 80 chars if possible Packaging - Not much point in including a buildout (bootstrap.py and buildout.cfg) in the package, as it won't be functional without the rest of GetPaid. - A couple incorrect copyright notices (top of setup.py) - I would change the version to 1.0a1 (alpha) or 1.0b1 (beta) before releasing to pypi. Easy_install (and buildout, which uses it) treat packages ending in "dev" specially and they may not be found in some configurations. Code - Add i18n message ids to "Sandbox" and "Production" strings in interfaces.py (hmm, but this might cause problems with the endpoint URL lookup in paypal.py ...maybe it's not worth it for now) - Not much point to having IPaypalPayFlowProProcessor if it doesn't declare anything new. Just make the processor declare it implements IPaymentProcessor instead. - Is there a more global GetPaid currency setting that we should use instead of setting it just on the processor? (I don't know.) - I notice the lack of tests, though it is hard to test something this dependent on an external service. - I would make the refund method raise 'Not Implemented' as an exception rather than just returning a string. (Unless the caller is checking for that string explicitly.) -Rob On Mon, Jan 26, 2009 at 3:12 PM, Lucie Lejard <[email protected]> wrote: > > Hi Rob, > > I looked quick at the new egg you created. The code seems correct to > me. I just wouldn't put a log containing the last four digit of the > credit card. > I added this to getpaid so that the package gets loaded if it is installed: > <include zcml:condition="installed getpaid.payflowpro" > package="getpaid.payflowpro" /> > > I almost did a payment but I don't have an account with payflow pro :-) > > are you going to use it on production at some point? > > Thanks > Lucie > -- > S i x F e e t U p , I n c . | http://www.sixfeetup.com > Phone: +1 (317) 861-5948 x605 > ANNOUNCING the first Plone Immersive Training Experience | Sept. 10-11-12, > 2009 > http://www.sixfeetup.com/immerse > > > > On Wed, Jan 21, 2009 at 4:48 PM, Rob LaRubbio <[email protected]> wrote: > > I've just added a processor for PayPal's payflowpro product to svn and > > pypi. I've tested it against paypal's sandbox manager, but have not run > any > > live charges through it (I don't have a merchant account). It should be > > complete functional for charges, refunds have not been implemented. It > also > > could probably use some password obfuscation for PCI compliance. > > > > If you have any question let me know, you can play around with it by > > uncommenting it's egg in the normal svn checkout building. Or adding it > as > > an addpackages with getpaid.recipe. Thanks. > > > > -Rob > > > > > > > > > > > --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "getpaid-dev" 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/getpaid-dev?hl=en -~----------~----~----~----~------~----~------~--~---
