Implement GetExpressCheckoutDetails and MassPay operations

[alfaproject]

In our business we need to do extra risk and security checks, and thus I had the necessity to implement this express operation.

I have created unit tests for it, and also made the AbstractRequest a little bit better, imo.

There are no BC breaks, so you can just up the middle version number if you want. (;

[alfaproject]

I took the opportunity to add MassPay to this PR.
I only realised I was working on the develop branch now, and I'm sorry for that, but I can start over and do proper PRs for separate features if you want.

Either way, it shouldn't matter. These features don't break existing code, and they don't conflict between each other. (;

[amacneil]

That's ok, it's just that large pull requests take me longer to review. I'll try to add some comments, but it might be easier if the masspay stuff is a separate PR.

[amacneil]

I'm explicitly avoiding adding any @author type tags or copyright notices in omnipay, since it can get ridiculous trying to decide what is a "significant" enough contribution to warrant being mentioned here. With git & github it's trivial to see who authored what.

[alfaproject]

Fair enough, I'll remove those in the next update.

[amacneil]

Wait, how is GetExpressCheckoutDetails different from GetTransactionDetails added in #3?

[alfaproject]

GetTransactionDetails is used to get information about a specific transaction, and thus requires a transaction id.
https://developer.paypal.com/webapps/developer/docs/classic/api/merchant/GetTransactionDetails_API_Operation_NVP/

GetExpressCheckoutDetails is used to get information about a specific checkout, and thus requires the checkout token. (the transaction wasn't made yet)
https://developer.paypal.com/webapps/developer/docs/classic/api/merchant/GetExpressCheckoutDetails_API_Operation_NVP/

[amacneil]

I see. So you are expected to call GetExpressCheckoutDetails after the user returns from PayPal, but before you confirm the transaction (so you have a chance to confirm once again the details with them)? That makes more sense.

In that case, along with the Omnipay conventions perhaps we can call this fetchCheckoutDetails()? (so far we have fetchTransaction(), fetchCreditCard() etc for remote operations to distinguish them from simple parameter getters/setters).

[amacneil]

I recently moved some of the files around (in line with PSR-4), so you will probably need to update your PR to match. I'd also much prefer to review the MassPay stuff separately, since this is already quite a large pull request. Let me know when you've addressed the issues above, and I'll take a look at it :)

[alfaproject]

I'll create 2 new PRs for both methods using your newest code. It's probably better then creating a commit to remove MassPay on this branch. I don't think I can do it today, because I'm mega busy at work finishing off one of our newer APIs, but I'll defo do it this week.

[amacneil]

Cool, no hurry.

FYI you can easily rebase/squash your commits and force push to this branch to update the PR.

On 8 Apr 2014, at 4:32 pm, João Dias notifications@github.com wrote:

I'll create 2 new PRs for both methods using your newest code. It's probably better then creating a commit to remove MassPay on this branch. I don't think I can do it today, because I'm mega busy at work finishing off one of our newer APIs, but I'll defo do it this week.

—
Reply to this email directly or view it on GitHub.

[alfaproject]

Ah yeah, your right. Forgot about that. Cheers. (;

[tomsowerby]

I'd like to see this implemented too. Is it still under development?

[da40]

I too would appreciate it if this fork could be merged, as PayPal Express is unusable without GetExpressCheckoutDetails. Don't need the MassPay part.

[amacneil]

Someone care to make a new PR with just the GetExpressCheckoutDetails part? I think the method in omnipay should probably be named $gateway->fetchCheckout(), but open to suggestions.

[tomsowerby]

On it.

[tomsowerby]

Added PR #18

[amacneil]

Fixed by #18