Skip to content

Conversation

@cofin
Copy link
Member

@cofin cofin commented Dec 15, 2025

Summary

Fixes #556 - Regression where service.create() failed when passing nested dictionaries for relationship data.

Changes

  • Add _convert_relationship_value() helper function to handle relationship conversion
  • Modify model_from_dict() to detect RelationshipProperty attributes using SQLAlchemy's class_mapper
  • Recursively convert nested dicts for one-to-one and many-to-one relationships
  • Convert list of dicts for one-to-many and many-to-many relationships
  • Handle None, empty lists, and mixed lists (dicts + model instances) correctly
  • Preserve backward compatibility for non-nested usage

Example Usage

data = {
    "name": "John Doe",
    "profile": {"bio": "Developer"},  # Converted to Profile instance
    "addresses": [
        {"street": "123 Main St"},
        {"street": "456 Oak Ave"}
    ]  # Each dict converted to Address instance
}
user = model_from_dict(User, **data)

Tests Added

  • 17 comprehensive unit tests covering:
    • Single nested dict (one-to-one)
    • List of nested dicts (one-to-many)
    • Deeply nested structures (3+ levels)
    • None and empty list handling
    • Mixed lists (dicts + instances)
    • Many-to-many relationships
    • Performance benchmarks

Test plan

  • All 184 unit tests pass
  • Type checking passes (mypy + pyright)
  • Linting passes (ruff check + format)
  • Pre-commit hooks pass
  • No manual edits to _sync.py files

@codecov-commenter
Copy link

codecov-commenter commented Dec 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.49%. Comparing base (2bf5157) to head (5cd8cec).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #637      +/-   ##
==========================================
+ Coverage   80.43%   80.49%   +0.06%     
==========================================
  Files          87       87              
  Lines        6451     6471      +20     
  Branches      838      845       +7     
==========================================
+ Hits         5189     5209      +20     
  Misses        998      998              
  Partials      264      264              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cofin cofin changed the title fix(#556): recursively convert nested dicts in model_from_dict fix: recursively convert nested dicts in model_from_dict Dec 15, 2025
@cofin cofin force-pushed the feat/nested-dict-handling branch from 3e0666d to 769377a Compare December 15, 2025 17:03
cofin and others added 2 commits December 20, 2025 11:36
…_dict

Fixed regression where service.create() failed when passing nested
dictionaries for relationship data. The model_from_dict() function
now uses SQLAlchemy's class_mapper to detect relationships and
recursively converts nested dicts to model instances.

Changes:
- Add _convert_relationship_value() helper function
- Modify model_from_dict() to detect RelationshipProperty attributes
- Handle one-to-one, one-to-many, and many-to-many relationships
- Preserve existing model instances in mixed lists
- Add 17 comprehensive unit tests for nested dict handling
Signed-off-by: Cody Fincher <[email protected]>
@cofin cofin force-pushed the feat/nested-dict-handling branch from 769377a to 5cd8cec Compare December 20, 2025 17:36
converted_data[attr_name] = _convert_relationship_value(
value=value,
related_model=related_model,
is_collection=attr.uselist or False,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couldnt you just do attr.uselis since itd always be a bool

Suggested change
is_collection=attr.uselist or False,
is_collection=attr.uselist,

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Unable to create model from dict with nested dicts

4 participants