-
-
Notifications
You must be signed in to change notification settings - Fork 87
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
Allow relation _id fields to use a sequence #253
Changes from 4 commits
0b4625a
7543a3c
37385eb
0a3cb72
3f3ddc3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -363,10 +363,20 @@ def _make( | |
] | ||
else: | ||
self.m2m_dict[field.name] = self.model_attrs.pop(field.name) | ||
# is an _id relation that has a sequence defined | ||
elif ( | ||
(isinstance(field, OneToOneField) or isinstance(field, ForeignKey)) | ||
and hasattr(field, "attname") | ||
and field.attname in self.iterator_attrs | ||
): | ||
self.model_attrs[field.attname] = next( | ||
self.iterator_attrs[field.attname] | ||
) | ||
elif field.name not in self.model_attrs: | ||
if ( | ||
not isinstance(field, ForeignKey) | ||
or "{0}_id".format(field.name) not in self.model_attrs | ||
or hasattr(field, "attname") | ||
and field.attname not in self.model_attrs | ||
): | ||
self.model_attrs[field.name] = self.generate_value( | ||
field, commit_related | ||
|
@@ -476,6 +486,14 @@ def _skip_field(self, field: Field) -> bool: | |
if isinstance(field, FileField) and not self.create_files: | ||
return True | ||
|
||
# Don't Skip related _id fields defined in the iterator attributes | ||
if ( | ||
(isinstance(field, OneToOneField) or isinstance(field, ForeignKey)) | ||
and hasattr(field, "attname") | ||
and field.attname in self.iterator_attrs | ||
): | ||
return False | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a fan of returning a False here rather than running through the conditions and falling back to the False at the end. The last 2 conditions catch this scenario and it was getting complicated to isolate the right case. Suggestions welcome. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I totally agree with you here @ashiazed, but I think this is a result of a major problem that is the fact that there's practically no abstractions under this major I mean, this is done like that since the very beginning of the project and never was refactored. But today, it's more clear to me that we should have a piece of code to receive a field and generate a value/skip it. Baker class would only trust that everything happened as expected. If we had such object, I feel that these validations wouldn't be all grouped together like that. |
||
|
||
# Skip links to parent so parent is not created twice. | ||
if isinstance(field, OneToOneField) and self._remote_field(field).parent_link: | ||
return True | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I swapped this to checking "attname" as it mirrors the same technique that Django uses internally.