robmcvey / cakephp-paypal

CakePHP 2.0 plugin for PayPal Website Payment Pro
60 stars 44 forks source link

Added card store via vault #12

Closed zot24 closed 10 years ago

zot24 commented 10 years ago

Added support to StoreCreditCard using the new RestFull API this should cover this enhancement https://github.com/robmcvey/cakephp-paypal/issues/9

robmcvey commented 10 years ago

Israel, you the man! Mega!

Can you do me a huge favour and update your if/else blocks as per CakePHP coding standards? I'm really trying to stick to this for these plugins (and my code in general!).

Many thanks!

zot24 commented 10 years ago

Hi Rob!

I'll!

Thanks man,

zot24 commented 10 years ago

Hi mate! sorry took me awhile do the modification hope now all be good! ;)

robmcvey commented 10 years ago

I've merged your new methods and added some more test cases in branch card-store.

If you rebase (or just pull this branch in to yours) can you update the following test method before pushing back to this PR? : https://github.com/robmcvey/cakephp-paypal/commit/fe694d572cede6cd786548ed69cbcb13c427b2ba

I'm not sure what the format of $creditCard should be?

zot24 commented 10 years ago

I update the README file on this commit explaining it, check it and let me know if you found any problem 7c38540c7da2ddb6549c726db5716e337599da00

zot24 commented 10 years ago

ops sorry I see what you mean!

Yep I'll update the test don't worry.

robmcvey commented 10 years ago

Hi Israel - is this now closed, or? Let me know, will try and review this at some point before the weekend :smile:

zot24 commented 10 years ago

Hi Rob!

I have time yet to split the logic into a files, but I'll do, hope during this week.

robmcvey commented 10 years ago

Ah OK no rush! I was actually looking at this today - it would be great to use traits:

https://github.com/robmcvey/cakephp-paypal/tree/split-logic/Lib

But this breaks anything < PHP 5.4 which until CakePHP 3.0 is released is not something I'm happy with really - and maintaining two versions until that time doesn't make sense.

How were you thinking of approaching this?

zot24 commented 10 years ago

I think could be a good idea but we can achieve the same goal (at the moment because the code is not to big) using inheritance as I did on my branch split-logic, have a look.

However what do you reckon could be the best approach? My team need split up the logic because we are using this library in a project and right now is a little bit messy, so maybe I'll start working on the same approach that we talked at the begining https://github.com/robmcvey/cakephp-paypal/issues/15 and maybe later we can review it an adopt traits if we consider it necessary and a better option.

What are your thoughts?

BTW maybe will be better continues this conversation on this issue https://github.com/robmcvey/cakephp-paypal/issues/15 instead this one