added transaction fee support to sale class #232

chaitanyakuber · 2015-02-03T01:27:12Z

I am raising this PR mostly for discussion while I wait for PayPal support engineers to respond.

Last week we discovered that a new "transaction_fee" element had been added to the response for a POST request to /v1/payments/payment. This causes an issue with this SDK as reflection is used to map the JSON to PHP Objects.

Problem JSON

{
  "id": "PAY-2DD06538S7025641FKTHAEII",
  "create_time": "2015-02-01T02:38:26.026Z",
  "update_time": "2015-02-01T02:38:26.026Z",
  "state": "approved",
  "intent": "sale",
  "payer": {
    "payment_method": "paypal",
    "payer_info": {
      "email": "chaitanya@**********",
      "first_name": "Chaitanya",
      "last_name": "Kuber",
      "payer_id": "6RT3WYKUX89LN"
    }
  },
  "transactions": [
    {
      "amount": {
        "total": "5.00",
        "currency": "AUD",
        "details": {}
      },
      "description": "BTQ Order",
      "related_resources": [
        {
          "sale": {
            "id": "93P456876H0968255",
            "create_time": "2015-02-01T02:38:26.026Z",
            "update_time": "2015-02-01T02:38:26.026Z",
            "state": "completed",
            "amount": {
              "total": "5.00",
              "currency": "AUD"
            },
            "protection_eligibility": "ELIGIBLE",
            "protection_eligibility_type": "ITEM_NOT_RECEIVED_ELIGIBLE, UNAUTHORIZED_PAYMENT_ELIGIBLE",
            "transaction_fee": {
              "value": "0.10",
              "currency": "AUD"
            },
            "parent_payment": "PAY-2DD06538S7025641FKTHAEII",
            "links": [
              {
                "href": "https://api.paypal.com/v1/payments/sale/93P456876H0968255",
                "rel": "self",
                "method": "GET"
              },
              {
                "href": "https://api.paypal.com/v1/payments/sale/93P456876H0968255/REFUND",
                "rel": "refund",
                "method": "POST"
              },
              {
                "href": "https://api.paypal.com/v1/payments/payment/PAY-2DD06538S7025641FKTHAEII",
                "rel": "parent_payment",
                "method": "GET"
              }
            ]
          }
        }
      ]
    }
  ],
  "links": [
    {
      "href": "https://api.paypal.com/v1/payments/payment/PAY-2DD06538S7025641FKTHAEII",
      "rel": "self",
      "method": "GET"
    }
  ]
}

Reflection Code is https://github.com/chaitanyakuber/PayPal-PHP-SDK/blob/master/lib/PayPal/Common/PayPalModel.php#L184
When the above JSON is processed by this code we see "Method PayPal\Api\Sale::getTransactionFee() does not exist" in our logs.

Questions

Are others seeing this issue ?
Is this the right fix ?

coveralls · 2015-02-03T02:13:53Z

Coverage increased (+0.08%) to 72.54% when pulling 58be8bd on chaitanyakuber:transaction-fee-patch into ac69aa6 on paypal:master.

jaypatel512 · 2015-02-03T03:01:17Z

Thank you so much for your Pull Request. I will go through this tomorrow morning, and get this merged in as soon as possible.

Thanks.

- Updates to #232

jaypatel512 · 2015-02-03T20:35:15Z

Hey @chaitanyakuber ! I have updated the master branch with the field added. I had to make changes to your pull request. Actually, we didnt need to create a new TransactionFee class, it is actually a currency object.

You could create an instance of that as shown here.

Let me know if you run into any such issues. Thank you so much for your pull request.

chaitanyakuber · 2015-02-03T22:02:16Z

Thanks @jaypatel512
I will test this out today.

Antengo · 2015-02-17T00:17:30Z

@chaitanyakuber @jaypatel512 we are having the same issues pulling in 1.1.* did this get push to composer yet?

jaypatel512 · 2015-02-17T00:36:03Z

Hey @Antengo ! v1.2.0 has the Order API support.

It is available in packagist

Antengo · 2015-02-17T00:43:01Z

Hey @jaypatel512 Thanks! Can you go into your process for the minor versioning? 1.0 to 1.1 had sweeping breaking API changes, but a bug fix in 1.1.1 was pushed to a new minor 1.2.0? We had locked down on 1.1.* to avoid breaking changes in minors.

jaypatel512 · 2015-02-17T01:16:08Z

We do try our level best to semver standards as much as possible. When a new public API is introduced, we make a new MINOR version update. However, if there is only a small bug fix, where we fix something that is broken in current version goes into PATCH update. MAJOR updates are only done when we introduce a big breaking change.

The changes from 1.0 to 1.1 was:

just adding new features, and fixing internal issues.
Enabled Cancel API Support for Unclaimed Payouts (New APIs introduced)
Encrypting Access Token in Cached Storage (Internal Change, No external dev handling required, backward compatible).
Updated Billing Agreement Search Transaction code to pass start_date and end_date (Broken API fixed. Bug fix as older was never working anyway.)
Updated OAuthToken to throw proper error on not receiving access token (New Error Message Text).
Minor Bug Fixes and Documentation Updates (No Breaking Changes)

It should not have broken any API change. Please let me know if I have accidentally made any breaking changes in 1.1 from 1.0.

The changes from 1.1 to 1.2 includes:

Order API Support (New APIs Introduced. No breaking changes)
Introduced DEBUG mode in Logging. Deprecated FINE. (Deprecations. Update from developers required).
Ability to not Log on DEBUG, in live mode (Security Change. Very Important)
Vault APIs Update API Support (New API Support)
Transaction Fee Added in Sale Object (Adding new Getter Setters, no breaking changes).

Again, If I have accidentally made a backward compatible change without making a major version change, please let me know and I will definitely make sure about that in future.

Antengo · 2015-02-17T01:22:35Z

More so that the addition of transaction_fee to the API caused a breaking error in 1.1.1 that the fix for (this pull) was put into 1.2.0 forcing us to upgrade.

I'll pay more attention to the repro going forward as the SDK matures.

Thanks!

On Feb 16, 2015, at 8:16 PM, Jay notifications@github.com wrote:

We do try our level best to semver standards as much as possible. When a new public API is introduced, we make a new MINOR version update. However, if there is only a small bug fix, where we fix something that is broken in current version goes into PATCH update. MAJOR updates are only done when we introduce a big breaking change.

The changes from 1.0 to 1.1 was:

just adding new features, and fixing internal issues.
Enabled Cancel API Support for Unclaimed Payouts (New APIs introduced)
Encrypting Access Token in Cached Storage (Internal Change, No external dev handling required, backward compatible).
Updated Billing Agreement Search Transaction code to pass start_date and end_date (Broken API fixed. Bug fix as older was never working anyway.)
Updated OAuthToken to throw proper error on not receiving access token (New Error Message Text).
Minor Bug Fixes and Documentation Updates (No Breaking Changes)

It should not have broken any API change. Please let me know if I have accidentally made any breaking changes in 1.1 from 1.0.

The changes from 1.1 to 1.2 includes:

Order API Support (New APIs Introduced. No breaking changes)
Introduced DEBUG mode in Logging. Deprecated FINE. (Deprecations. Update from developers required).
Ability to not Log on DEBUG, in live mode (Security Change. Very Important)
Vault APIs Update API Support (New API Support)
Transaction Fee Added in Sale Object (Adding new Getter Setters, no breaking changes).
Again, If I have accidentally made a backward compatible change without making a major version change, please let me know and I will definitely make sure about that in future.

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

jaypatel512 · 2015-02-17T03:06:54Z

Agreed. We do discourage branching each minor version, and doing fix versions on older minor versions. It keeps external developers encouraged to use latest SDK. However, even so, upgrading from 1.1.1 to 1.2.0 should not have any breaking changes at all.

But I would definitely keep a sharp eye on doing upgrades for things like this, with as much ideal way possible.

added transaction fee support to sale class

58be8bd

jaypatel512 merged commit 58be8bd into paypal:master Feb 3, 2015

jaypatel512 pushed a commit that referenced this pull request Feb 3, 2015

Adding Transaction Fee in Sale Object

ec46c39

- Updates to #232

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added transaction fee support to sale class #232

added transaction fee support to sale class #232

chaitanyakuber commented Feb 3, 2015

coveralls commented Feb 3, 2015

jaypatel512 commented Feb 3, 2015

jaypatel512 commented Feb 3, 2015

chaitanyakuber commented Feb 3, 2015

Antengo commented Feb 17, 2015

jaypatel512 commented Feb 17, 2015

Antengo commented Feb 17, 2015

jaypatel512 commented Feb 17, 2015

Antengo commented Feb 17, 2015

jaypatel512 commented Feb 17, 2015

added transaction fee support to sale class #232

added transaction fee support to sale class #232

Conversation

chaitanyakuber commented Feb 3, 2015

coveralls commented Feb 3, 2015

jaypatel512 commented Feb 3, 2015

jaypatel512 commented Feb 3, 2015

chaitanyakuber commented Feb 3, 2015

Antengo commented Feb 17, 2015

jaypatel512 commented Feb 17, 2015

Antengo commented Feb 17, 2015

jaypatel512 commented Feb 17, 2015

Antengo commented Feb 17, 2015

jaypatel512 commented Feb 17, 2015