Skip to content

Model Moneybird's nested-attributes convention declaratively (follow-up to detail/payment de-duplication) #37

Description

@TimEvci

Summary

Follow-up to the detail/payment de-duplication issue (mixin extraction). The deeper pattern behind that duplication is Moneybird's Rails-style "nested attributes" convention, which is currently reimplemented in four save() methods: pop a child collection, re-key it to *_attributes, and append {"id": ..., "_destroy": True} for removed items. This could be declared once per model and applied by a single shared save().

Note: this is design-level work with judgment calls; it depends on the mixin extraction landing first and is not a mechanical task.

Occurrences

  • details_attributessales_invoice.py, external_sales_invoice.py, document.py
  • custom_fields_attributessales_invoice.py
  • financial_mutations_attributesfinancial_statement.py (no _destroy tracking today)

Suggested direction

Declare the mapping "child field → *_attributes key, with destroy-tracking" per model, and let a base save() apply it:

class NestedAttribute:
    def __init__(self, field: str, key: str, destroyable: bool = True): ...

# e.g. on SalesInvoice:
#   _nested = [NestedAttribute("details", "details_attributes"),
#              NestedAttribute("custom_fields", "custom_fields_attributes", destroyable=False)]

The base save() iterates the declared nested attributes, serializes each (including _destroy entries for removed children), drops the raw field from the payload, and performs the POST-or-PATCH — so per-entity save() overrides disappear.

Design questions to settle during implementation

  • Where the declaration lives (class attribute vs. pydantic model config) and how it interacts with the HasDetails mixin.
  • Whether FinancialStatement.financial_mutations should gain _destroy support (it lacks it today — keep behavior identical unless deliberately extended).
  • Document's save posts under {self._resource: body} while the others use {self.endpoint: body} — the shared save() needs one way to name the payload root.

Acceptance criteria

  • Per-entity save() overrides are removed; one shared save() handles all nested-attribute serialization.
  • _destroy handling and *_attributes re-keying live in exactly one place.
  • Existing save/detail/destroy tests pass unchanged.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions