Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLI router broken for some commands #14750

Closed
electrokit opened this issue Apr 27, 2017 · 4 comments
Closed

CLI router broken for some commands #14750

electrokit opened this issue Apr 27, 2017 · 4 comments
Labels
type: bug The issue is a confirmed bug.

Comments

@electrokit
Copy link

CLI won't handle these commands:
wp wc --user=test shipping_zone_method list
wp wc --user=test shipping_zone_method get 0
wp wc --user=test shipping_zone_location list
wp wc --user=test shipping_zone get 0

They all echo:
Error: No route was found matching the URL and request method {"status":404}

WooCommerce 3.0.4
WordPress 4.7.4

@mikejolley
Copy link
Member

@justinshreve any idea if this should be working? I cannot get it working either.

@mikejolley mikejolley added the type: bug The issue is a confirmed bug. label Apr 27, 2017
@electrokit
Copy link
Author

I think I found it. The commands expect a zone_id, but it isn't in the $supported_ids array (and therefore won't show in the usage text)

It will work if we add the row
'zone_id' => __( 'Zone ID.', 'woocommerce' ),
to the $supported_ids array in class-wc-cli-runner.php:register_route_commands()

and fix the broken route texts by replacing (?P<zone_id>[\d-]+) by (?P<zone_id>[\d]+) (removing the minus sign) in all places:
tests/unit-tests/api/shipping-zones.php:66
tests/unit-tests/api/shipping-zones.php:67
tests/unit-tests/api/shipping-zones.php:68
includes/api/class-wc-rest-shipping-zone-methods-controller.php:29
includes/api/class-wc-rest-shipping-zone-methods-controller.php:56
includes/api/class-wc-rest-shipping-zone-locations-controller.php:29
includes/api/class-wc-rest-shipping-zones-controller.php:50

Note: I haven't tested normal REST functionality, so I don't know if this breaks something

@justinshreve
Copy link
Contributor

I haven't dug further but yeah it looks like that ID/route format needs added to the CLI runner. I'm not sure if we need to change the route on the API side -- it would be nicer to fix in the CLI runner code.

mikejolley added a commit that referenced this issue Apr 28, 2017
@mikejolley
Copy link
Member

See #14787

claudiosanches pushed a commit that referenced this issue Apr 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug The issue is a confirmed bug.
Projects
None yet
Development

No branches or pull requests

3 participants