Skip to content

Commit

Permalink
Properly handle non-nullable relationships (#645)
Browse files Browse the repository at this point in the history
* Properly handle non-nullable relationships

* Add test case for nullable foreign key
  • Loading branch information
sloria authored Jan 11, 2025
1 parent 52741e6 commit 0312a50
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 4 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ Bug fixes:
* Fix behavior of ``include_fk = False`` in options when parent
schema sets ``include_fk = True`` (:issue:`440`).
Thanks :user:`uhnomoli` for reporting.
* Fields generated from non-nullable `sqlalchemy.orm.relationship`
correctly set ``required=True`` and ``allow_none=False`` (:issue:`336`).
Thanks :user:`AbdealiLoKo` for reporting.

Other changes:

Expand Down
5 changes: 4 additions & 1 deletion src/marshmallow_sqlalchemy/convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,10 @@ def _add_relationship_kwargs(
nullable = True
for pair in prop.local_remote_pairs:
if not pair[0].nullable:
if prop.uselist is True:
if (
prop.uselist is True
or self.DIRECTION_MAPPING[prop.direction.name] is False
):
nullable = False
break
kwargs.update({"allow_none": nullable, "required": not nullable})
Expand Down
24 changes: 21 additions & 3 deletions tests/test_conversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,18 +86,36 @@ def test_enum_with_class_converted_to_enum_field(self, models):

def test_many_to_many_relationship(self, models):
student_fields = fields_for_model(models.Student, include_relationships=True)
assert type(student_fields["courses"]) is RelatedList
courses_field = student_fields["courses"]
assert type(courses_field) is RelatedList
assert courses_field.required is False

course_fields = fields_for_model(models.Course, include_relationships=True)
assert type(course_fields["students"]) is RelatedList
students_field = course_fields["students"]
assert type(students_field) is RelatedList
assert students_field.required is False

def test_many_to_one_relationship(self, models):
student_fields = fields_for_model(models.Student, include_relationships=True)
assert type(student_fields["current_school"]) is Related
current_school_field = student_fields["current_school"]
assert type(current_school_field) is Related
assert current_school_field.allow_none is False
assert current_school_field.required is True

school_fields = fields_for_model(models.School, include_relationships=True)
assert type(school_fields["students"]) is RelatedList

teacher_fields = fields_for_model(models.Teacher, include_relationships=True)
current_school_field = teacher_fields["current_school"]
assert type(current_school_field) is Related
assert current_school_field.required is False

def test_many_to_many_uselist_false_relationship(self, models):
teacher_fields = fields_for_model(models.Teacher, include_relationships=True)
substitute_field = teacher_fields["substitute"]
assert type(substitute_field) is Related
assert substitute_field.required is False

def test_include_fk(self, models):
student_fields = fields_for_model(models.Student, include_fk=False)
assert "current_school_id" not in student_fields
Expand Down

0 comments on commit 0312a50

Please sign in to comment.