Remove setConfig()

[shendy-a8c]

Fixes #1330

Changes proposed in this Pull Request

Since wcSettings.setSetting is deprecrated and no longer available, remove setConfig() completely.
There is only one place where setConfig() is used where it was intended to set nonce from server to a global variable.
Fortunately, in that only 1 case, we can just set nonce to a variable as it's still with in the same context where nonce is needed in ajax call.

Testing instructions

• Install WooCommerce Blocks.
• Create a new page with the checkout block. I have 2 separate checkout pages: one for shortcode checkout and one for checkout block.

• Add a product to cart and checkout with checkout block.
• Use SCA test credit card 4000002500003155.
• With base branch, expect an error: wc.wcSettings.setSetting is not a function.
• With this branch, expect success.
• Test the same flow with shortcode checkout because the code change affects shortcode checkout as well.
• Test guest checkout (in incognito mode maybe in case you're logged in as admin) and choose to create an account during checkout. Make sure Allow customers to create an account during checkout is enabled in wp-admin > WooCommerce > Settings > Accounts & Privacy.
  This can only be tested with shortcode checkout because I don't see checkout block lets you create an account.
  We need to test this because of this comment even though I find that the code will always pass nonce, not only for that specific flow.

---

• Added changelog entry (or does not apply)
• Covered with tests (or have a good reason not to test in description ☝️)
• Tested on mobile (or does not apply)

[vbelolapotkov]

Hey @shendy-a8c ! Thanks for the quick PR.

Since wcSettings.setSetting is deprecrated and no longer available

I was thinking about that part, do you know the version when this was deprecated. Asking in the context of compatibility and support of older versions where it was available.

[vbelolapotkov]

I think there were a reason for doing it so complex. Do you know why it was implemented like that before? I'm afraid that nonce value might be unavailable in some cases unless stored in wcSettings.

I'm also curious if deprecation of setSettings method was done without recommended replacement?

[shendy-a8c]

Do you know why it was implemented like that before?

My best guess is that it was designed to always have nonces available in global variable and when nonce for updating the order status needs to be updated, in this case, only when 3ds credit card is used and when user is a guest creating an account (because nonce depends on user id and guest and logged in user id have different user id), it's cleaner to just update the global variable with the new nonce and read the nonce value from global variable when updating the order status.

We can stick with that design but need to be a bit hacky by modifying the value of the global variable wcSettings.woocommerce_payments_data.updateOrderStatusNonce directly. It's mentioned here that though wcSettings should be immutable but immutability can't be enforced.

I'm afraid that nonce value might be unavailable in some cases unless stored in wcSettings.

I'm sure there won't be any case where nonce is unavailable because window.wcSettings.woocommerce_payments_data is available on page load. Try to load a checkout block page and see

<script id='wc-settings-js-before'>
var wcSettings = wcSettings || JSON.parse( decodeURIComponent( ... ) );
</script>

I'm also curious if deprecation of setSettings method was done without recommended replacement?

No replacement that I know of. The idea is to make global variable immutable. I find @nerrad's comment here:
Any code wanting to work with server side setting values that could be mutated over the life of a session should be using ajax or REST API endpoints to update and get current values.
I understand the "get current values" part but not sure how to update.

[vbelolapotkov]

I read the point about immutability and get it. Then there should be a way to add the nonce value to the wcSettings on the site via filter or something else, no? If so, I'd take this approach and keep reading nonce from the settings rather than relying on js to set nonce.

My best guess is that it was designed to always have nonces available in global variable and when nonce for updating the order status needs to be updated, in this case, only when 3ds credit card is used and when user is a guest creating an account (because nonce depends on user id and guest and logged in user id have different user id), it's cleaner to just update the global variable with the new nonce and read the nonce value from global variable when updating the order status.

It's a good guess, but I'd ask authors. @RadoslavGeorgiev , @DanReyLop and @dwainm could please add more info about nonces in checkout blocks? Why did we set it via JS rather than PHP?

[RadoslavGeorgiev]

@shendy-a8c is on the right track.

1. updateOrderStatusNonce is defined in PHP here.
2. It will be updated here in case the server sends a new one after a redirect URL has been generated (remember, they carry order IDs, client secret, etc.). At the point of writing the code, I have seemingly thought that this step is optional, but it isn't: it's always happening.
3. And it's finally used here.

Those are all the 3 places where updateOrderStatusNonce appears.

Looking at the original PR, the flow is already the same as described in the list above. At this point I'm inclined to think that this is just a mistake from my side, and that including the nonce in get_payment_fields_js_config is not needed at all, since it's received along with the intent data.

---

This can only be tested with shortcode checkout because I don't see checkout block lets you create an account.

@shendy-a8c please take a look at this comment: https://github.com/Automattic/woocommerce-payments/pull/723/files#r494208543
You can test this with the checkout block as well, although being in the api/index.js file, the change will probably not break the block.

[shendy-a8c]

Thank you @RadoslavGeorgiev for clarifying. My next step would be to remove updateOrderStatusNonce from get_payment_fields_js_config and test guest checkout creating an account in checkout blocks and shortcode checkout.

I saw that PR already merged but I didn't see the option to create an account in checkout block. I now realize that the toggle to turn that on is on the block's settings, not in Woo's settings.

[shendy-a8c]

I was thinking about that part, do you know the version when this was deprecated.

Per woocommerce/woocommerce-blocks#3019, in WooCommerce Blocks 3.2.0.

[shendy-a8c]

Removed updateOrderStatusNonce in 60e1848.

Tested guest checkout creating a new account in checkout block and in shortcode checkout.
Need to enable this option so checkout block allows account creation for guest checkout:

[shendy-a8c]

Self note: follow up next step to update https://github.com/Automattic/woocommerce-payments/wiki/Critical-flows

User type: Shopper
Area: WooCommerce Blocks
Flow Name: Checkout with SCA card
Testing instructions: wc.wcSettings.setSetting is not a function

[dmallory42]

Works well upon testing, and code all LGTM. Thanks for making this update, @shendy-a8c!

[shendy-a8c]

I believe @kalessil already cut a branch for release, so I need to update changelog and readme in this PR. Correct @kalessil ?

[vbelolapotkov]

I believe @kalessil already cut a branch for release, so I need to update changelog and readme in this PR. Correct @kalessil ?

Yes, correct.

[kalessil]

Picked for v2.3.1