Summary
The singular endpoint string is doing two unrelated jobs: it is the JSON payload wrapper key ({"sales_invoice": {...}}) and the stem of the URL path — with a trailing "s" appended by hand at 24 call sites (f"{self.endpoint}s/..."). This conflation is the root cause of several problems: Document cannot inherit the shared CRUD mixins and reimplements load/find_by_id/delete/save by hand, Synchronizable._sync_endpoint duplicates the snake_case derivation and reaches into cls.__private_attributes__, and every new call site must remember the pluralize-at-call-site convention.
Separating the two concepts deletes all of this rather than centralizing it.
Details
Current state:
MoneybirdModel.endpoint property (moneysnake/model.py:22-31) derives a singular name; all callers append "s" themselves (24 occurrences across model.py, sales_invoice.py, external_sales_invoice.py, financial_statement.py).
Synchronizable._sync_endpoint (moneysnake/model.py:130-138) re-derives the same name as a classmethod, pulling the _endpoint default out of cls.__private_attributes__ — a private pydantic registry — because the property needs an instance.
Document (moneysnake/document.py:91-129) needs the path documents/{resource}s, which the single-string model cannot express, so it reimplements load, find_by_id, delete, and the URL half of save instead of inheriting Loadable/Saveable/Deletable.
Suggested fix
Split the two concepts into two classmethods:
@classmethod
def _payload_key(cls) -> str:
"""JSON wrapper key for request bodies, e.g. "sales_invoice"."""
# single snake_case derivation, honoring an explicit override
@classmethod
def _collection_path(cls) -> str:
"""URL path of the collection, e.g. "sales_invoices"."""
return f"{cls._payload_key()}s"
Document overrides only _collection_path to return f"documents/{cls._payload_key()}s" and keeps _payload_key as the resource name — the payload key and the URL path are different values for documents, which is exactly why they must be separate methods.
- All 24
f"{self.endpoint}s..." call sites become f"{self._collection_path()}/..." (or a small _item_path(id) helper); the hand-appended "s" convention disappears entirely.
Synchronizable._sync_endpoint and the __private_attributes__ access are deleted; sync_list/sync_fetch use _collection_path.
Document.load / find_by_id / delete and its _base_path/_sync_endpoint overrides are deleted; it inherits the mixins like every other model.
- The instance
endpoint property can delegate to _payload_key() (or be removed if nothing external depends on it).
Related cleanups in the same file (fold in)
Loadable.load(self, id) takes an id parameter despite being an instance method — SalesInvoice(id=5).load(7) is legal and leaves the object with id=7. load() should take no argument and use self.id (raising if unset); find_by_id currently passes the id twice.
Deletable.delete_by_id returns the deleted entity with id=None — a husk object with no meaningful use. It should return None.
Both are small public-API changes; note them in the changelog.
Acceptance criteria
- The snake_case derivation exists once; no access to pydantic private-attribute internals remains.
- No call site appends
"s" to an endpoint string by hand.
Document and its subclasses inherit load/find_by_id/save/delete from the shared mixins, overriding only _collection_path (and its payload key via _resource).
- Request URLs and payload wrapper keys are byte-for-byte identical to today's for every entity (verify against the existing tests, which assert full paths).
- Existing tests pass, updated only where they exercise the removed
load(id) parameter or delete_by_id return value.
Summary
The singular
endpointstring is doing two unrelated jobs: it is the JSON payload wrapper key ({"sales_invoice": {...}}) and the stem of the URL path — with a trailing"s"appended by hand at 24 call sites (f"{self.endpoint}s/..."). This conflation is the root cause of several problems:Documentcannot inherit the shared CRUD mixins and reimplementsload/find_by_id/delete/saveby hand,Synchronizable._sync_endpointduplicates the snake_case derivation and reaches intocls.__private_attributes__, and every new call site must remember the pluralize-at-call-site convention.Separating the two concepts deletes all of this rather than centralizing it.
Details
Current state:
MoneybirdModel.endpointproperty (moneysnake/model.py:22-31) derives a singular name; all callers append"s"themselves (24 occurrences acrossmodel.py,sales_invoice.py,external_sales_invoice.py,financial_statement.py).Synchronizable._sync_endpoint(moneysnake/model.py:130-138) re-derives the same name as a classmethod, pulling the_endpointdefault out ofcls.__private_attributes__— a private pydantic registry — because the property needs an instance.Document(moneysnake/document.py:91-129) needs the pathdocuments/{resource}s, which the single-string model cannot express, so it reimplementsload,find_by_id,delete, and the URL half ofsaveinstead of inheritingLoadable/Saveable/Deletable.Suggested fix
Split the two concepts into two classmethods:
Documentoverrides only_collection_pathto returnf"documents/{cls._payload_key()}s"and keeps_payload_keyas the resource name — the payload key and the URL path are different values for documents, which is exactly why they must be separate methods.f"{self.endpoint}s..."call sites becomef"{self._collection_path()}/..."(or a small_item_path(id)helper); the hand-appended"s"convention disappears entirely.Synchronizable._sync_endpointand the__private_attributes__access are deleted;sync_list/sync_fetchuse_collection_path.Document.load/find_by_id/deleteand its_base_path/_sync_endpointoverrides are deleted; it inherits the mixins like every other model.endpointproperty can delegate to_payload_key()(or be removed if nothing external depends on it).Related cleanups in the same file (fold in)
Loadable.load(self, id)takes anidparameter despite being an instance method —SalesInvoice(id=5).load(7)is legal and leaves the object withid=7.load()should take no argument and useself.id(raising if unset);find_by_idcurrently passes the id twice.Deletable.delete_by_idreturns the deleted entity withid=None— a husk object with no meaningful use. It should returnNone.Both are small public-API changes; note them in the changelog.
Acceptance criteria
"s"to an endpoint string by hand.Documentand its subclasses inheritload/find_by_id/save/deletefrom the shared mixins, overriding only_collection_path(and its payload key via_resource).load(id)parameter ordelete_by_idreturn value.