Automattic / woocommerce-payments

Accept payments via credit card. Manage transactions within WordPress.
https://wordpress.org/plugins/woocommerce-payments/
Other
174 stars 69 forks source link

Use `get_rest_url()` instead of hardcoded `/wp-json/` in REST API endpoints #6057

Open ricardo opened 1 year ago

ricardo commented 1 year ago

Description

Original context: https://github.com/Automattic/woocommerce-payments-private/pull/34#discussion_r1160249523

In some places in the code we're assuming a few REST API endpoints contain the /wp-json/ prefix. While this works by default, it can cause issues if the endpoint is changed. E.g with the rest_url_prefix filter.

To prevent potential issues, we should update the code to use the get_rest_url() function to retrieve the REST API endpoint URL dynamically, rather than relying on a hardcoded string.

Acceptance criteria

leonardola commented 1 year ago

I did some digging into this and I only found these places where it's used and they are either tests or setup scripts

https://github.com/Automattic/woocommerce-payments/blob/e8068180d9814daf3a58d19ac94d4c5e3dcfba9e/tests/js/jest-test-file-setup.js#L90

https://github.com/Automattic/woocommerce-payments/blob/9ea339b751e5c9d90e8d90a10510b4d0ce464c07/tests/unit/woopay/test-class-woopay-session.php#L35

https://github.com/Automattic/woocommerce-payments/blob/bd01196d2c289d8e06f833a605380ee05758b14a/tests/e2e/env/setup.sh#L259

I also found this code that uses WooPay's wp-json endpoint

https://github.com/Automattic/woocommerce-payments/blob/c8f19eca3515fd6fa591952f13bc7c95f5f8ac2f/client/checkout/woopay/email-input-iframe.js#L383

Using the get_rest_url() function in my setup returns index.php?rest_route= instead of /wp-json. To get /wp-json I had to use rest_get_url_prefix() which is used in other parts of the code.

For the js tests and setup scripts I don't think we have a simple way to retrieve the rest api prefix.

ricardo commented 1 year ago

IMO we shouldn't mind about the tests or setup scripts. 🙂

There were a few more instances when this issue was created. Looks like those were fixed. ```bash git rev-list -n 1 --before="2023-04-17" develop # 43d77ea3b3413ff7ecc875836a33d5d33bcf54a7 git checkout 43d77ea3b3413ff7ecc875836a33d5d33bcf54a7 ```

@leonardola Have you tried get_rest_url() with a $path parameter like this?

leonardola commented 1 year ago

@leonardola Have you tried get_rest_url() with a $path parameter like this?

Yep it just changes the value of the rest_route param

vbelolapotkov commented 8 months ago

@c-shultz this one is obviously related to WooPay but doesn't show up on your board, could you please check?