Skip to content

Deduplicate the copy-pasted update() method on the DetailsAttribute classes #31

Description

@TimEvci

Summary

The update() method is copy-pasted onto each *DetailsAttribute class, and it is identical to MoneybirdModel.update(). That's four copies of the same non-obvious logic.

Details

The same body:

def update(self, data: dict[str, Any]) -> None:
    validated = self.model_validate({**self.model_dump(), **data})
    for key in self.__class__.model_fields:
        object.__setattr__(self, key, getattr(validated, key))

appears in:

  • MoneybirdModel.update (moneysnake/model.py:36)
  • SalesInvoiceDetailsAttribute.update (moneysnake/sales_invoice.py:26)
  • ExternalSalesInvoiceDetailsAttribute.update (moneysnake/external_sales_invoice.py:24)
  • DocumentDetailsAttribute.update (moneysnake/document.py:32)

The logic is subtle (it uses model_validate plus object.__setattr__ to run field validators without flattening nested models to dicts), so having four copies is a real maintenance risk.

Suggested fix

Extract a thin base holding update() (and to_dict()), and have both MoneybirdModel and the *DetailsAttribute classes inherit it, e.g.:

class UpdatableModel(BaseModel):
    def to_dict(self) -> dict[str, Any]:
        return self.model_dump(exclude_none=True)

    def update(self, data: dict[str, Any]) -> None:
        ...

Acceptance criteria

  • update() is defined once.
  • The *DetailsAttribute classes behave identically to today.
  • Existing detail-update 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