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

Additional CancelOrder params being wrapped in "order" breaks functionality #66

Open
tseven opened this issue Jan 9, 2018 · 3 comments

Comments

@tseven
Copy link

tseven commented Jan 9, 2018

ServiceDescription for 'CancelOrder':

        'CancelOrder' => [
            'httpMethod'       => 'POST',
            'uri'              => 'admin/orders/{id}/cancel.json',
            'responseModel'    => 'GenericModel',
            'summary'          => 'Cancel a given order',
            'data'             => ['root_key' => 'order'],
            'parameters'       => [
                'id' => [
                    'description' => 'Order ID',
                    'location'    => 'uri',
                    'type'        => 'integer',
                    'required'    => true
                ]
            ],
            'additionalParameters' => [
                'location' => 'json',
            ],
        ],

Since the root_key is set to 'order', the additional parameters passed to the cancelOrder method gets wrapped and end up looking like:

['order' => [ 'reason' => 'fraud' ]

Which does't comply with the API spec:
https://help.shopify.com/api/reference/order#cancel

I've been able to "fix" the issue by setting the root_key to NULL. This of course bypasses the unwrapping of the Shopify result as well.

What is the proper fix here?

Thanks

@bakura10
Copy link
Member

bakura10 commented Jan 9, 2018

Hi,

Indeed, Shopify seems very inconsistent here. I assumed they were using a wrapping key everywhere but it seems to be an exception for this one.

@danizord are you making use of cancelOrder anywhere in our app? I don't think so.

In all cases it seems the best fix is indeed to remove the wrap key and adjust the service client for wrapping and unwrapping. Could you please do a PR ? :)

@tseven
Copy link
Author

tseven commented May 9, 2018

Is there an update on this?

@bakura10
Copy link
Member

bakura10 commented May 9, 2018

Hi,

I do not any longer do any back-end work, sorry, so I will likely do not maintain it anymore. But feel free to submit a PR and I'll be happy to review and merge.

As Shopify is inconsistent here by having something different in request and reponse, the "cleanest" way will be to keep the "order", and add an exception on the unwrapping here: https://github.com/zf-fr/zfr-shopify/blob/master/src/ShopifyClient.php#L554

You have all the info needed (the endpoint that was called...) so you can change the root key depending on the API call made.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants