Validate Kwargs before and possibly after passing them to Shopify #500

Overview

Validate kwargs before passing them to shopify and either raise an exception or warn if the kwarg is likely to produce no effect (the current behavior). I'd propose implementing this as a session-level flag to change behavior which would likely be implemented in the __encoded_params_for_signature method of the Session class.

something like:

with shopify.Session.temp(shop_url, api_version, token, validate_kwargs='fail'):
   orders = shopify.Order.find(ids="1234")

where:
default - current behavior
warn - current behavior, with warning that kwargs will not work as expected
fail - raise exception before generating query string.

With this new behavior:

shopify.Order.find(ids="1234")
##################
InvalidKwargError: The ids kwarg should be a comma-separated list of ids.

...

Type

• New feature
• Changes to existing features

Motivation

What inspired this feature request? What problems were you facing?

I recently had the experience of trying to fetch two orders while testing some code:

shopify.Order.find(ids="1234,5678")
Next I wanted to check for a single order so I deleted the second order:
shopify.Order.find(ids="1234")

Those with a keen eye could spot the bug: ID's needs a comma-separated list of order IDs. according to the docs. Without the comma, the kwarg is basically disregarded and all (non-archived) orders are returned. This can lead to unexpectedly long running queries that is hard to debug when this code is generated dynamically or buried under several layers of other code.

Here are a few examples of the proposed validation:

• IDS : if "ids" in kwargs, validate that there is at least one "," and that the values in the "ids" string are integers. The response could also be validated as we'd expect to receive <= number of objects in the response. eg: if the "ids" filter has two elements, we can expect <=2 objects in the response.
• status: check that the status kwarg has one of the valid values ['open', 'closed', 'cancelled', 'any']
• created_at_min: check that the min-date is a valid iso-formatted date

With the warn/fail arguments I proposed above, if the kwarg didn't pass the test, the code would loudly warn or fail and make debugging much easier.

An alternative, and possibly easier to implement solution, would be to add configurable logging. The ability to easily see the full query string as well as the raw response body could make the same debugging a lot easier.

I'm happy to open a PR for either proposed solution, but would appreciate some guidance from the maintainers before doing so.
...

Area

• Add any relevant Area: <area> labels to this issue

Checklist

• I have described this feature request in a way that is actionable (if possible)