B/C upgrade instructions 1.1.0 > 2.1.0?

[Ruud68]

So using version 1.1.0 I thought it was time to upgrade to 2.1.0
but. my code broke e.g. on the removed function storeInVault() on the PaypalWalletVaultInstructionBuilder.

I understand that things break between major versions, but where can I find:

• a changelog of all things that have B/C issue and how to resolve these?
• instructions on how to use this now (e.g. how do I replace the storeInVault() functionality with something that works and does the same?

I must be looking in all the wrong places but I cannot find anything online that is of any help?

So help wanted: I'm totally in the blind here...

[Dani-Kirby]

Hey @Ruud68 thanks for reaching out. You can check our CHANGELOG.md to see what changes have taken effect. Notably, we've included two new APIs (Transaction Search and Subscriptions Billing) and fixed existing bugs that were affecting transactions. We are in the process of communicating with the right team to update the docs and demo ASAP, but I'm afraid it will take some time. We will update you once they do.

In regards to your inquiry about the storeInVault() issue, I'm investigating and will be reaching out once I have additional information to provide. Thanks again for bringing this to our attention.

[Ruud68]

Hi, @Dani-Kirby thanks for your answer. To be honest: the change log is useless as it doesn't reflect what the actual changes are that have been made. It feels more like a 'we need a changelog but don't have time to do it' one.
So next step is then comparing what changes have been made between releases: again useless, this is from 1.1.0 to 2.0.0 > "Showing 630 changed files with 33,442 additions and 3,924 deletions." Good luck finding anything in there! So the only way now is upgrade to the new version and wait for the errors and then backtrace them to the file they came from and do a diff on these files... very 'costly' to say the least.

As a word of advise: only accept changes when it is accompanied with the change in the documentation. This helps the developers making the changes to also take the extra step and provide an actual change that others can work with.

Also, but this is my opinion: I welcome that this SDK is created and maintained but IMO it is build way over complicated. Never worked with an SDK where a very simple API call that requires one variable needs so much (undocumented) code to get it working. it's like needing higher math when going shopping and need to get the price for an article.

I also work with stripe and Mollie sdk's for (recurring) payments and I do not know if you ever checked these out: but these are so much easier and as such so much better in maintaining.

[gerardojbaez]

@Ruud68 Don't spend too much time giving them feedback. They don't take it. These SDKs are not a priority for them (see #55) and IMO are there to minimally meet some common needs. I've found that the best approach when working with PayPal is to simply work with the restful API.

[Ruud68]

@gerardojbaez feedback is mainly for myself: for getting it of my chest :) and agree: way easier to just use the restful API, but then again: same issue here with documentation.

[naolson-paypal]

Hey @gerardojbaez - We can appreciate your frustrations, but please do not actively try to dissuade the community from providing feedback, it is detrimental to improving the SDKs. I realize your experience with the product and the team is unique, but we are working hard to figure out how to improve the SDKs to make integrating with PayPal easier.

@Ruud68 - Thank you again for the feedback 👍

[Dani-Kirby]

Hey @Ruud68 thanks for the follow-up here. I'll admit that this change from 1.1.0 > 2.1.0 is much more sizeable than we had initially hoped for which definitely complicates things a fair deal. We have identified the source of the missing storeInVault() functionality within our API schemas we base the SDK on and are working to get it resolved. I'll reach out here when that change is fixed and deployed.

I also completely understand the frustration in regards to documentation and complexity of the SDK. We take feedback seriously and are actively working to make both our product and documentation better. In terms of documentation, unfortunately there are a number of moving parts and the publishing of the documentation to our developer portal can hit some unexpected roadblocks with another internal team. Rest assured we are working closely with them to get things updated as soon as we can.

We value you bringing this all up to us and are working to see what improvements we can make in the coming releases. Please don't hesitate to reach out with any additional feedback.

[gerardojbaez]

Hey @naolson-paypal - Yes, you guys are truly unique, I'll admit that.

please do not actively try to dissuade the community from providing feedback

Please don't put words on my mouth. I'm not "actively dissuading" the community from providing feedback; my advice was clearly about not spending too much time (and effort) on the kind of feedback submitted (regarding DX). And on top of that, this is the first instance where I comment on a fellow developer with such advice; therefore you saying "actively dissuading" is plain wrong.

I truly wish for PayPal to invest more in DX, which includes proper allocation of resources with the goal of improving all these technical aspects.

[Ruud68]

@Dani-Kirby thanks for reply.
this is actually the first project where a stable release (I say stable, but the way it is handled imo itmore or less feels like alpha) removes methods without giving a timely heads-up in the form of a deprecation notice. So what I (and probably every other developer) would expect from a stable release is:
in version 2.0.0 set the to be removed method as deprecated with clear replacement instructions (e.g. use instead) and a removal version number.

So e.g:

    /**
     * Sets store in vault field.
     *
     * @param string|null $value
     * 
     * @deprecated  2.0.0 will be removed in 3.0 use ...... instead
     */
    public function storeInVault(?string $value): self
    {
        $this->instance->setStoreInVault($value);
        return $this;
    }

That way we get notice via our IDE and are able to do out job in a planned manner rather then be smacked in the face with trying to find out what changed why and how to fix it.

but back to the issue at hand. I am now in need of a fix and the documentation is a real no go. Can somebody correct the following code snippet so at least the storeInVault needed for recurring payments is working with the 2.x version of the SDK?

           $purchaseUnits = [
                PurchaseUnitRequestBuilder::init(
                    AmountWithBreakdownBuilder::init(
                        $this->config->paypal_currency,
                        $orderParameters['order_amount']
                    )->build()
                )
                    ->referenceId($orderParameters['order_code'])
                    ->description(\trim($this->config->order_description . ' ' . $orderParameters['order_code']))
                    ->build()
            ];

            $orderRequest = OrderRequestBuilder::init(
                CheckoutPaymentIntent::CAPTURE,
                $purchaseUnits
            );

            $paymentSourceBuilder = PaymentSourceBuilder::init();

                if ($orderParameters['recurring']) {
                    // We need to add the vault request as attribute
                    $vault = PaypalWalletVaultInstructionBuilder::init(PaypalPaymentTokenUsageType::MERCHANT)
                        ->storeInVault(StoreInVaultInstruction::ON_SUCCESS);
                    $attributes = PaypalWalletAttributesBuilder::init();
                    $attributes->vault($vault->build());
                    $paypalWallet->attributes($attributes->build());
                }

                $paymentSourceBuilder->paypal($paypalWallet->build());

                $orderRequest->paymentSource($paymentSourceBuilder->build());

            $collect = [
                'body'   => $orderRequest->build(),
                'prefer' => 'return=representation'
            ];

            $ordersController = $this->paypal->getOrdersController();
            $apiResponse      = $ordersController->createOrder($collect);
            $order            = $apiResponse->getResult();

[wikando-mu]

I also stumbled over the issue that the storeInVault Method was removed and our Tests and Static Code Analysis was failing.
It's also a little bit interesting that you added all attributes from vault - but not the store_in_vault attribute, confusing.

I tried to create a little patch/test to show you where i expect the "store in vault" Method to use it and get it in the request.

Restore_storeInVault_functionality.patch

[Ruud68]

@wikando-mu thanks for your response. This patch basically reverses the part where the method storeinVault was removed, correct?
Not sure if that is what the maintainers of this repository are aiming at (there is a reason why it was removed, which is beyond me).

If they could provide the 'new' how-to for the removed method in the code i provided above, then I think we are good to go... (at least if that is the only thing braking things in the 2.1 version)

[wikando-mu]

@Ruud68 yes, it's only a "revert" of the code which was removed.
IMHO, it only make sense to set the "storeInVault" Attribute there, where it was.
Why? Because the class have all "vault" attributes, which are also documented in the PayPal documentation (see here ) but not the "store_in_vault" ?

But that is only my opinion/idea, and my idea is they should make a little fix and create the version 2.1.1 to solve this problem.

I also need the "store_in_vault" attribute and i haven't found a way to make it work again after they removed it in the new release, because we couldn't set these removed "store_in_vault" manually 🤷

My actually Solution is to use the 1.1.0 Version, till they fix or tell us how we could use/set the "store in vault" attribute.

[Ruud68]

@wikando-mu I agree, and also agree on staying on v1. But thing is: been down that rabbit hole once with the predecessor which became unsupported and then archived. Took a big investment to get to this SDK and when the stable version came out it was time to take the plunge and make it 'permanent', just to learn now that it is not stable at all when you look at the way things go in and out: this is alpha material.

So yes, staying on V1 for now, but from what I have seen that is not maintained.

So the dilemma I as a developer am facing is: do i stay on an unmaintained version or do i go with an alpha release.

So looking at @naolson-paypal here (sorry, you where the one in this thread :)) on what to do here.

How do we use the new vault (failing code attached), or will that function be restored in a next release.? Who needs to be involved to get this library managed in a way it is usable for production?

[wikando-mu]

@Ruud68 i fully agree what you said.
i like this SDK and we want to support it and give feedback.

Nobody is perfect when we develop things and it's okay to make failures, no pressure, fix it, release it, fine.

The problem is, that vaulting, with the version 2.1.0, is actually not usable, because if you don't set the "store_in_vault" attribute, you get an error for the create-order request.

if there is no solution for this problem, we can't update the version.

@naolson-paypal hopefully you could help us and could agree what we said/reported here.

[Dani-Kirby]

Hey @wikando-mu and @Ruud68 this is great feedback for us thank you! The missing storeInVault() method was an unintentional break that we will be correcting in the next patch or minor bump that we are working on now and hope to have released soon, but I don't have an exact timeline as of now. For now, I would recommend staying on V1 for this functionality until we are able to get the bug fix released. I'll be reaching out here as soon as it is live.

I've also noted all the feedback and our team's program manager is aware and ensuring that our roadmap accounts for feedback such as this as we continue to move forward and develop this SDK. It definitely wasn't our intention to break the SDK in this way and as such that's why you don't see anything referencing a replacement method or deprecation notice. Rest assured we are actively working on solving the root of this error so this doesn't happen in future releases.

[Ruud68]

Hey @wikando-mu and @Ruud68 this is great feedback for us thank you! The missing storeInVault() method was an unintentional break that we will be correcting in the next patch or minor bump that we are working on now and hope to have released soon, but I don't have an exact timeline as of now. For now, I would recommend staying on V1 for this functionality until we are able to get the bug fix released. I'll be reaching out here as soon as it is live.

I've also noted all the feedback and our team's program manager is aware and ensuring that our roadmap accounts for feedback such as this as we continue to move forward and develop this SDK. It definitely wasn't our intention to break the SDK in this way and as such that's why you don't see anything referencing a replacement method or deprecation notice. Rest assured we are actively working on solving the root of this error so this doesn't happen in future releases.

That said, saw release 2.2.0 yesterday and there was no communication regarding this issue / feedback. Am I safe to assume that version 2.2.0 is still a NOGO when needing vaulting?