Support overrides and historical values on customer, contact and attributes endpoints by tudorpavel · Pull Request #145 · chartmogul/chartmogul-python

tudorpavel · 2026-09-28T15:29:37Z

Adds support for the overrides and historical values API to the Python client, targeting the current API where write responses always include overrides.

Customer, Contact and Attributes register with_overrides in _bool_query_params so with_overrides=True serializes as true (same pattern as with_processing_status on DataSource); attributes_with_history passes through as a plain string query param.
Marshmallow schemas use unknown=EXCLUDE, so overrides and historical_values were dropped from responses; fields.Dict(load_default=None) entries on the Customer, Contact and Attributes schemas now keep them and make the attributes always present (None when the API omits the key), with attribute names round-tripping verbatim (Dict fields don't convert keys). The CustomAttributes schema keeps overrides, custom (absent when a DELETE removes the last attribute) and the DELETE message the same way; the add-by-email response loads through Customer._Schema and picks up overrides from there.
Request data dicts are dumped as-is, so overrides already reaches the API on every write; tests pin the outgoing bodies (last_request.json()) and query strings.
Tests: 210 passing (+14), flake8 clean; every write surface pins its outgoing body and the echoed overrides (distinguishing {} from None), mock statuses match the API (201 on creates, 202 on deletes), the by-email test exercises the real /customers/attributes/custom endpoint and asserts entry overrides, and the with-history tests use case-sensitive request mocks where attribute names are mixed-case (the API matches names case-sensitively).
README examples added for retrieves with the new params and writes with overrides, plus notes on flag semantics, which write calls accept overrides, the payload shape difference between the customer and custom attribute endpoints, the attributes_with_history format, that overrides live at the top level of the customer, and that historical value timestamps stay as strings.

keith-chartmogul

I'm not a Python developer, so Claude assisted with the heavy lifting on this review. There were a few points raised during the review, which all seem legitimate to my untrained eye, so I posted inline comments with those findings. Please feel free to ignore if they do not apply, but I want to make sure you see them and consider them before choosing to ignore or act on any of them. 👍

…ibutes resources

…sponse keys do not raise AttributeError

…story tests

…se AttributeError

…d retrieval

keith-chartmogul

Some more findings from another review round. All testing and documentation related, but I think these are all probably worth doing ahead of the release. 💪

keith-chartmogul · 2026-10-14T03:05:13Z

        self.assertEqual(mock_requests.call_count, 1, "expected call")
        self.assertEqual(mock_requests.last_request.qs, {})
        self.assertEqual(mock_requests.last_request.json(), sentCreateExpected)
+        self.assertIsNone(result.overrides)


Also applies to line 107 in test/api/test_custom_attrs.py

This now asserts overrides is None on a create response, but POST /customers always returns overrides (it renders with with_overrides: true). The same goes for test_custom_attrs.py:107: POST .../attributes/custom always includes overrides in its response. So both assertions pin a response shape the API can't produce, and they contradict the PR description ("write responses always include overrides").

Could we add overrides to the mocked responses for these two tests, and move the "missing key → None" check to a plain Customer.retrieve test? test_customer.py doesn't have one yet; only test_retrieve_with_overrides_and_history exists. Small related thing: test_create still mocks a 200, while the new test_create_with_overrides correctly uses 201.

Done 84d38fc 👍

keith-chartmogul · 2026-10-14T03:06:24Z

The PR description mentions that the add-by-email response picks up overrides through Customer._Schema, and the API does return overrides for each customer in that response. But this test's fixture (jsonResponse2) has no overrides and the test makes no assertions on the result, so nothing actually covers it.

Also, this calls CustomAttributes.add with uuid="CUSTOMER_UUID", so it hits the single-customer endpoint rather than the by-email one (/customers/attributes/custom). That isn't new to this PR, but since we're relying on this path now, could we call it without uuid (or via add_by_email), add overrides to the fixture entries, and assert result.entries[0].overrides?

Fixed 3c62539 👍 mocks being mocks..

keith-chartmogul · 2026-10-14T03:06:44Z

+        self.assertEqual(result.historical_values, historical_values)
+
+    @requests_mock.mock()
+    def test_create_with_overrides(self, mock_requests):


Nice. Could we add a matching test for Customer.modify with overrides? It's the main example in the README, and right now only create is tested.

The API also accepts overrides on two other writes that have no tests:

Contact.create / Customer.createContact

CustomAttributes.update (PUT)

They all serialize the body the same way, so the risk is low. A short body-pinning test for each would still back up the README, for example:

self.assertEqual(mock_requests.last_request.json(), jsonRequest) self.assertEqual(result.overrides, overrides)

Done b3daeb8 👍

keith-chartmogul · 2026-10-14T03:07:21Z

        self.assertEqual(mock_requests.last_request.qs, {})
        self.assertTrue(expected, {})
+
+    # case_sensitive pins that the SDK does not alter query param casing


Nit: title is all lowercase, so case_sensitive=True and the comment above it don't prove anything here; the test would pass either way. I'd drop the comment and use the default @requests_mock.mock(), unless we use a mixed-case value.

Right 09c715d — with all-lowercase contact field names it discriminates nothing, and a mixed-case value would pin a shape the contact endpoint rejects (field names match case-sensitively, so Title 422s). Reverted to the default mocker; the customer and attributes tests keep case_sensitive=True where salesRep makes it meaningful.

keith-chartmogul · 2026-10-14T03:07:49Z

 })
+chartmogul.Customer.modify(config, uuid='cus_5915ee5a-babd-406b-b8ce-d207133fb4cb', data={
+  "company": "Pinata Technologies",
+  "overrides": {


The examples show the payload shape, but not what the flags actually do. Could we add a sentence or two? Something like:

True pins the field's value after the write.

False unpins it and restores the most recent value from edit history.

While a field is pinned, API writes to it are reverted unless the request includes an override for that field.

CustomAttributes.remove skips pinned attributes; the response's message says which ones were kept.

Without this, someone updating a pinned field will see their write silently undone.

Added c259637 some short explanations 👍

Tweaked 69098ba

Correction to my earlier reply: you were right about the restore. UnpinService#unpin_field! ends with restore_most_recent_value! — blocked writes still land in edit history, so removing the override catches the field up to them. Our live check couldn't distinguish this because the pinned value was also the latest history entry. The README note is fixed accordingly.

keith-chartmogul · 2026-10-14T03:08:22Z

        custom = fields.Dict()
+        # load_default=None ensures these attributes are always present; the API
+        # omits the keys unless with_overrides / attributes_with_history is passed
+        overrides = fields.Dict(load_default=None)


Customer._Schema.attributes reuses this schema for its nested attributes field, so a Customer now always has customer.attributes.overrides and customer.attributes.historical_values, both None. On a customer, custom-attribute pins and history actually live in customer.overrides["attributes"] and customer.historical_values["attributes"], so these two will never be filled in.

I don't think it's worth a separate nested schema, but a README note would stop people reaching for customer.attributes.overrides.

Hope 33a102a is good enough. 👍

keith-chartmogul · 2026-10-14T03:08:33Z

 chartmogul.Customer.retrieve(config, uuid='cus_5915ee5a-babd-406b-b8ce-d207133fb4cb')
+chartmogul.Customer.retrieve(config, uuid='cus_5915ee5a-babd-406b-b8ce-d207133fb4cb',
+                             with_overrides=True, attributes_with_history='company,custom.channel')
+# with_overrides and attributes_with_history only work on retrieve; list and search endpoints ignore them


Could we document what format attributes_with_history takes?

It's a comma-separated string, and names are case-sensitive.

Custom attributes need the custom. prefix.

At most 100 names; unknown names return a 422.

Attributes.retrieve only accepts custom attributes, so attributes_with_history='company' there returns a 422.

I'd keep it a string rather than also accepting a list: the Ruby, Node, PHP and Go SDKs all take a comma-separated string.

Hope 925b8a3 makes sense, I kept the 100 cap hidden from dev docs so I'll do the same here.

keith-chartmogul · 2026-10-14T03:08:51Z

+  ],
+  'overrides': {'custom': {'channel': True}}
+})
 chartmogul.CustomAttributes.update(config, uuid='cus_5915ee5a-babd-406b-b8ce-d207133fb4cb', data={


The API also accepts overrides on these writes, but the README has no example for them:

Customer.create

Contact.create / Customer.createContact

CustomAttributes.update

CustomAttributes.add by email

Either an example for each, or a single line saying "all create/modify/update/add/remove calls accept an overrides dict", would cover it.

Went with the one-liner b05d8c2 👍

keith-chartmogul · 2026-10-14T03:08:59Z

 ```python
 chartmogul.Attributes.retrieve(config, uuid='cus_5915ee5a-babd-406b-b8ce-d207133fb4cb')
+chartmogul.Attributes.retrieve(config, uuid='cus_5915ee5a-babd-406b-b8ce-d207133fb4cb',
+                               with_overrides=True, attributes_with_history='custom.channel')


Historical value entries come back as plain dicts, so update_performed_at stays an ISO 8601 string and isn't converted to a datetime, unlike other datetime fields in the SDK. Since this section already notes that date attributes stay as strings, could we extend that note to cover historical_values?

Done 0e4a844

keith-chartmogul · 2026-10-14T03:09:57Z

        address = fields.Nested(Address._Schema, allow_none=True, unknown=EXCLUDE)
        website_url = fields.String(allow_none=True)
+        # load_default=None ensures these attributes are always present; the API
+        # omits the keys unless with_overrides / attributes_with_history is passed


Also applies to line 38 in chartmogul/api/contact.py

Small accuracy fix: write responses (create/modify) always include overrides, so "omits the keys unless with_overrides / attributes_with_history is passed" only holds for retrieve. Maybe "on retrieve, the API omits these keys unless…"? The same wording is in contact.py:38. It's fine as is in attributes.py, since that endpoint is GET-only.

Clarified 289508a

… to a plain retrieve test

…overrides

…names are lowercase

…bute endpoints

…n the README

tudorpavel requested review from jessicahearn, keith-chartmogul and ya-petrov-evgeniy September 29, 2026 04:45

jessicahearn approved these changes Sep 29, 2026

View reviewed changes

keith-chartmogul reviewed Sep 30, 2026

View reviewed changes

Comment thread test/api/test_attributes.py Outdated

Comment thread chartmogul/api/customer.py Outdated

Comment thread test/api/test_custom_attrs.py

Comment thread chartmogul/api/contact.py

Comment thread chartmogul/api/customer.py

tudorpavel added 6 commits October 13, 2026 12:58

Support overrides and historical values in customer, contact and attr…

a64fb20

…ibutes resources

Default overrides, historical_values and message to None so absent re…

9235255

…sponse keys do not raise AttributeError

Use case-sensitive request mocks and real attribute casing in with-hi…

d3e2e91

…story tests

Default custom to None so a DELETE of the last attribute does not rai…

4c2354b

…se AttributeError

Note in README that overrides query params only apply to single-recor…

50db223

…d retrieval

Mock 201 for the create customer endpoint to match the API

31952ea

tudorpavel force-pushed the tudorpavel/overrides-and-historical-values branch from 7cafdef to 31952ea Compare October 13, 2026 10:07

keith-chartmogul reviewed Oct 14, 2026

View reviewed changes

tudorpavel added 14 commits October 14, 2026 20:30

Mock overrides on write responses and move the absent-key None checks…

84d38fc

… to a plain retrieve test

Exercise the by-email endpoint in test_add_to_email and assert entry …

3c62539

…overrides

Pin request bodies and echoed overrides on the remaining write surfaces

b3daeb8

Reword test comments to avoid pin terminology

845d2b4

Drop the case-sensitive mocker from the contact test since its field …

09c715d

…names are lowercase

Explain override flag semantics in the README examples

c259637

Note the overrides shape difference between customer and custom attri…

0cd94cb

…bute endpoints

Note that customer overrides live at the top level, not under attributes

33a102a

Document override flag semantics and attributes_with_history format i…

925b8a3

…n the README

Note that all write calls accept overrides in the README

b05d8c2

Note that historical value timestamps stay as strings in the README

0e4a844

Scope the omitted-keys schema comment to retrieve responses

289508a

Use API instead of an email in historical value fixtures

3997fda

Correct the false flag semantics in the README

69098ba

tudorpavel merged commit 2f97f44 into main Oct 15, 2026
9 checks passed

tudorpavel deleted the tudorpavel/overrides-and-historical-values branch October 15, 2026 09:27

Conversation

tudorpavel commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

keith-chartmogul left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

keith-chartmogul left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tudorpavel Oct 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tudorpavel commented Sep 28, 2026 •

edited

Loading

tudorpavel Oct 15, 2026 •

edited

Loading