PYMODM-95 Validating EmbeddedMongoModel in EmbeddedDocument type fields #55
Conversation
shreybatra
force-pushed
the
fix/embedded-model
branch
from
3c92c6a
to
f36c649
Compare
|
Also corrected the test case where Travis was failing. The testcase mentioned MongoModel in an EmbeddedDocumentListField. It has been corrected. @ShaneHarvey @prashantmital please review. |
There was a problem hiding this comment.
This is a great start. I think it can be improved in two ways:
-
Create a
RelatedEmbeddedModelFieldsBaseclass (in pymodm/base/fields.py) that inherits fromRelatedModelFieldsBase. The new class should perform the embedded model validation in__init__as well as override therelated_modelproperty. This way the embedded model is validated even if it's a string. Also if we ever need to add a new field class that holds an embedded model we can just inherit fromRelatedEmbeddedModelFieldsBase. -
Add tests in the relevant
test/field_types/files for validation of embedded model classes and strings.
shreybatra
force-pushed
the
fix/embedded-model
branch
2 times, most recently
from
686c4b4
to
a62a428
Compare
|
@ShaneHarvey done the changes. Please review. :) |
shreybatra
force-pushed
the
fix/embedded-model
branch
from
a62a428
to
72fa8be
Compare
pymodm/fields.py
Outdated
| from pymodm.errors import ValidationError, ConfigurationError | ||
| from pymodm.base.fields import MongoBaseField | ||
| from pymodm.files import File, GridFSStorage, FieldFile, ImageFieldFile | ||
| from pymodm.vendor import parse_datetime |
There was a problem hiding this comment.
Please undo these import rearrangements since it isn't relevant to this change. We like to keep them ordered the way they are: standard library, third party, and finally imports from the current library (pymodm).
There was a problem hiding this comment.
@ShaneHarvey done this.
pymodm/base/fields.py
Outdated
| @@ -267,6 +267,37 @@ def _model_to_document(self, value): | |||
| '%s is not a valid %s' % (value, self.related_model.__name__)) | |||
|
|
|||
|
|
|||
| class RelatedEmbeddedModelFieldsBase(RelatedModelFieldsBase): | |||
| '''Base class for EmbeddedDocument and EmbeddedDocumentListField.''' | |||
There was a problem hiding this comment.
Can you change to double quotes ("""...""")?
There was a problem hiding this comment.
@ShaneHarvey done this.
| @@ -194,3 +194,22 @@ def test_coerce_reference_type(self): | |||
| comment = Comment(body='this is a comment', post=post_id).save() | |||
| comment.refresh_from_db() | |||
| self.assertEqual('this is a post', comment.post.body) | |||
|
|
|||
pymodm/base/fields.py
Outdated
| '''Base class for EmbeddedDocument and EmbeddedDocumentListField.''' | ||
|
|
||
| def __init__(self, model, verbose_name=None, mongo_name=None, **kwargs): | ||
| super(RelatedEmbeddedModelFieldsBase, self).__init__(model=model, |
There was a problem hiding this comment.
Please wrap all lines at 80 characters.
There was a problem hiding this comment.
@ShaneHarvey done this.
shreybatra
force-pushed
the
fix/embedded-model
branch
from
72fa8be
to
1964460
Compare
|
@ShaneHarvey done. Please review. |
|
Thanks @shreybatra! |
|
@ShaneHarvey ur welcome! |
Checked and validated type of related_field in EmbeddedDocumentField and EmbeddedDocumentListField. Raised Value error the same was as RelatedModelFieldsBase.