Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using attribute with RelatedList doesn't work #340

Open
AbdealiLoKo opened this issue Aug 26, 2020 · 3 comments
Open

Using attribute with RelatedList doesn't work #340

AbdealiLoKo opened this issue Aug 26, 2020 · 3 comments

Comments

@AbdealiLoKo
Copy link
Collaborator

AbdealiLoKo commented Aug 26, 2020

When I am using a RelatedList with an attribute= loading the relationship seems to cause an error about unknown attributes.

For example, if the Models are:

class School(Base):
    __tablename__ = "school"
    id = sa.Column("school_id", sa.Integer, primary_key=True)
    name = sa.Column(sa.String(255), nullable=False)

class Teacher(Base):
    __tablename__ = "teacher"
    id = sa.Column(sa.Integer, primary_key=True)

    current_school_id = sa.Column(
        sa.Integer, sa.ForeignKey(School.id), nullable=True
    )
    current_school = relationship(School, backref=backref("teachers"))

My schema is defined as:

class SchoolSchema(SQLAlchemySchema):
    class Meta:
        model = models.School
        sqla_session = session
        load_instance = True

    all_teachers = RelatedList(Related(), attribute="teachers")

Now when I try to use is:

schema = SchoolSchema()
out_school = schema.load({"all_teachers": [1]})

I get the error:

AttributeError: type object 'School' has no attribute 'all_teachers'
@AbdealiLoKo
Copy link
Collaborator Author

AbdealiLoKo commented Aug 26, 2020

After creating the PR - I saw that there is already a test test_model_schema_with_attribute

def test_model_schema_with_attribute(
self, models, schemas, school, student, session
):
class StudentSchema(Schema):
id = fields.Int()
class SchoolSchema(ModelSchema):
class Meta:
model = models.School
sqla_session = session
students = RelatedList(Related(attribute="students"), attribute="students")

where it shows that I would need to do:

all_teachers = RelatedList(Related(attribute="teachers"), attribute="teachers")

But I'm not sure if that is intuitive, because the attribute should be for the main field in the schema ?
Ideally the RelatedList should pass that onto the inner child during automatically ?

I see that this isn't a problem when using field_for() or auto_field() because it adds the same set of kwargs to both the inner field and RelatedField:

field_kwargs.update(kwargs)
ret = field_class(**field_kwargs)
if (
hasattr(prop, "direction")
and self.DIRECTION_MAPPING[prop.direction.name]
and prop.uselist is True
):
ret = RelatedList(ret, **kwargs)
return ret

@AbdealiLoKo
Copy link
Collaborator Author

AbdealiLoKo commented Aug 26, 2020

Seems like I have another issue with RelatedList and field_for() with RelatedList.
If I add a custom validator to a field like:

field_for(School, 'teachers', validate=lambda x: len(set(x)) == len(x))

It throws an error that Teacher is not itearable. Becuase this validator is also being applied to both: RelatedList and Related.

@sloria - Wanted to check if it makes sense and whether a fix to decouple the kwargs for RelatedList and Related would be accepted ?
I can attempt to create a PR

@sloria
Copy link
Member

sloria commented Aug 29, 2020

@AbdealiJK Yeah, that does look like a a bug--validate should apply to RelatedList and not Related. A PR would be great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants