Fixes #339 and add support for object specific filtering #341

bpotard · 2023-09-14T15:37:27Z

Hello,

This PR adds support for all "optimised" object-specific filters supported by the xero accounting API (as documented on https://developer.xero.com/documentation/api/accounting/overview) as of August 2023.

For example:

>>> xero.contacts.filter(searchTerm='Bob',summaryOnly=True)
>>> xero.invoices.filter(Statuses=['PAID', 'VOIDED'])
>>> xero.purchaseorders.filter(DateFrom=datetime.date(2023,9,14))

It also add some typing and basic input validation on filter fields, e.g.

>>> xero.invoices.filter(IDs=['1', '2'])
...
ValueError: badly formed hexadecimal UUID string

Finally, it also resolves issue #339.

…akboy3742#339 - undocumented xero error messaging for list filters

…case, but basemanager assumes capitalized, so force capitalization in initialisation

…untyped fields

bpotard · 2023-09-29T17:04:27Z

src/xero/manager.py

@@ -8,7 +8,7 @@ def __init__(self, name, credentials, unit_price_4dps=False, user_agent=None):
        from xero import __version__ as VERSION  # noqa

        self.credentials = credentials
-        self.name = name.capitalize()
+        self.name = name.capitalize() if name.islower() else name


I had added this as a hack to make the tests behave "as expected". But unfortunately that breaks the behaviour for all objects that have more than one upper case letter, e.g. "PurchaseOrders", as it enforce only the first letter is capitalized.

Now that I think about it, it should really be the tests that should be updated to use the CapitalizedNames of the objects rather than a lower case version...

Agreed. The problem is with the test, not the manager.

bpotard · 2023-09-29T17:07:11Z

tests/test_manager.py

@@ -226,13 +226,18 @@ def test_filter_ids(self):
        manager = Manager("contacts", credentials)

        uri, params, method, body, headers, singleobject = manager._filter(
-            IDs=["1", "2", "3", "4", "5"]


These are actually invalid IDs, as only UUIDs are accepted. Sending this as input to the xero API results in a rather strange error message in an undocumented format. I suggest to run the tests on valid input, i.e. UUIDs.

bpotard · 2023-09-29T17:08:57Z

src/xero/exceptions.py

-            self.problem = self.errors[0]
-            super().__init__(response, payload["oauth_problem_advice"][0])
-
+            if payload:


Handles the undocumented format of error messages from xero when sending malformed UUIDs for a list filter request.

bpotard · 2023-09-29T17:10:51Z

src/xero/basemanager.py

+    OBJECT_FILTER_FIELDS = {
+        "Invoices": {
+            "createdByMyApp": bool,
+            "summaryOnly": bool,
+            "IDs": list,
+            "InvoiceNumbers": list,
+            "ContactIDs": list,
+            "Statuses": list,
+        },
+        "PurchaseOrders": {"DateFrom": date, "DateTo": date, "Status": str},
+        "Quotes": {
+            "ContactID": UUID,
+            "ExpiryDateFrom": date,
+            "ExpiryDateTo": date,
+            "DateFrom": date,
+            "DateTo": date,
+            "Status": str,
+            "QuoteNumber": str,
+        },
+        "Journals": {"paymentsOnly": bool},
+        "Budgets": {"DateFrom": date, "DateTo": date},
+        "Contacts": {
+            "IDs": list,
+            "includeArchived": bool,
+            "summaryOnly": bool,
+            "searchTerm": str,
+        },
+        "TrackingCategories": {"includeArchived": bool},
+    }


Document valid filtering fields for Objects along with some basic typing

bpotard · 2023-09-29T17:12:25Z

src/xero/basemanager.py

+            def get_filter_value(key, value, value_type=None):
+                if key in self.BOOLEAN_FIELDS or value_type == bool:
+                    return "true" if value else "false"
+                elif key in self.DATE_FIELDS or value_type == date:
+                    return f"{value.year}-{value.month}-{value.day}"
+                elif key in self.DATETIME_FIELDS or value_type == datetime:
+                    return value.isoformat()
+                elif key.endswith("ID") or value_type == UUID:
+                    return "%s" % (
+                        value.hex if type(value) == UUID else UUID(value).hex
+                    )
+                else:
+                    return value


Helper function to convert filter fields to the format expected by xero

bpotard · 2023-09-29T17:13:37Z

src/xero/basemanager.py

+            KNOWN_PARAMETERS = ["order", "offset", "page"]
+            object_params = self.OBJECT_FILTER_FIELDS.get(self.name, {})
+            LIST_PARAMETERS = list(
+                filter(lambda x: object_params[x] == list, object_params)
+            )
+            EXTRA_PARAMETERS = list(
+                filter(lambda x: object_params[x] != list, object_params)
+            )
+


Differentiated processing for basic fields and "list" fields

bpotard · 2023-09-29T17:15:09Z

src/xero/basemanager.py

+            for param in KNOWN_PARAMETERS + EXTRA_PARAMETERS:
+                if param in kwargs:
+                    params[param] = get_filter_value(
+                        param, kwargs.pop(param), object_params.get(param, None)
+                    )


Processing of simple filter fields, with some basic typed conversion of input data

bpotard · 2023-09-29T17:17:16Z

src/xero/basemanager.py

+            for param in LIST_PARAMETERS:
                if param in kwargs:
-                    params[param] = kwargs.pop(param)
+                    if param.endswith("IDs"):
+                        params[param] = ",".join(
+                            map(lambda x: UUID(x).hex, kwargs.pop(param))
+                        )
+                    else:
+                        params[param] = ",".join(kwargs.pop(param))


Processing of list filter fields, with no input verification except for IDs that are interpreted as UUIDs.

freakboy3742

I've made the modification to the test that you've identified - but otherwise, this looks good to merge. Thanks for the contribution!

freakboy3742 · 2023-09-30T01:49:51Z

src/xero/manager.py

@@ -8,7 +8,7 @@ def __init__(self, name, credentials, unit_price_4dps=False, user_agent=None):
        from xero import __version__ as VERSION  # noqa

        self.credentials = credentials
-        self.name = name.capitalize()
+        self.name = name.capitalize() if name.islower() else name


Agreed. The problem is with the test, not the manager.

blaise and others added 5 commits September 14, 2023 16:10

Add support for object specific filter parameters; resolves issue fre…

57dcd7a

…akboy3742#339 - undocumented xero error messaging for list filters

Merge branch 'freakboy3742:main' into master

a09d691

Clean up code with pre-commit

7ddbcb4

It seems tests assume the name of the manager can be passed in lower …

501d600

…case, but basemanager assumes capitalized, so force capitalization in initialisation

Adapt test as IDs are now validated; do not force string casting for …

39905aa

…untyped fields

freakboy3742 mentioned this pull request Sep 27, 2023

pypi version severely outdated #342

Closed

Remove regression which breaks behaviour of PurchaseOrders, etc.

ec6bdd1

bpotard commented Sep 29, 2023

View reviewed changes

freakboy3742 added 2 commits September 30, 2023 09:51

Correct manager usage in tests.

891a05d

Minor formatting cleanup.

e661f46

freakboy3742 approved these changes Sep 30, 2023

View reviewed changes

freakboy3742 merged commit b617239 into freakboy3742:main Sep 30, 2023
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #339 and add support for object specific filtering #341

Fixes #339 and add support for object specific filtering #341

bpotard commented Sep 14, 2023

bpotard Sep 29, 2023

freakboy3742 Sep 30, 2023

bpotard Sep 29, 2023

bpotard Sep 29, 2023 •

edited

Loading

bpotard Sep 29, 2023

bpotard Sep 29, 2023

bpotard Sep 29, 2023

bpotard Sep 29, 2023

bpotard Sep 29, 2023

freakboy3742 left a comment

freakboy3742 Sep 30, 2023

Fixes #339 and add support for object specific filtering #341

Fixes #339 and add support for object specific filtering #341

Conversation

bpotard commented Sep 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bpotard Sep 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

freakboy3742 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bpotard Sep 29, 2023 •

edited

Loading