Merge pull request #2863 from bagerard/improve_error_message_unsaved_object

Error improvement for referenced Documents not saved yet

Author: bagerard

docs/changelog.rst

@@ -22,6 +22,7 @@ Development
 - Add support for collation/hint/comment to delete/update and aggregate #2842
 - BREAKING CHANGE: Remove LongField as it's equivalent to IntField since we drop support to Python2 long time ago (User should simply switch to IntField) #2309
 - BugFix - Calling .clear on a ListField wasn't being marked as changed (and flushed to db upon .save()) #2858
+- Improve error message in case a document assigned to a ReferenceField wasn't saved yet #1955
 - BugFix - Take `where()` into account when using `.modify()`, as in MyDocument.objects().where("this[field] >= this[otherfield]").modify(field='new') #2044
 
 Changes in 0.29.0

mongoengine/fields.py

@@ -111,6 +111,14 @@
 RECURSIVE_REFERENCE_CONSTANT = "self"
 
 
+def _unsaved_object_error(document):
+    return (
+        f"The instance of the document '{document}' you are "
+        "trying to reference has an empty 'id'. You can only reference "
+        "documents once they have been saved to the database"
+    )
+
+
 class StringField(BaseField):
     """A unicode string field."""
 
@@ -1215,10 +1223,7 @@ def to_mongo(self, document):
 
             # XXX ValidationError raised outside of the "validate" method.
             if id_ is None:
-                self.error(
-                    "You can only reference documents once they have"
-                    " been saved to the database"
-                )
+                self.error(_unsaved_object_error(document.__class__.__name__))
 
             # Use the attributes from the document instance, so that they
             # override the attributes of this field's document type
@@ -1262,10 +1267,7 @@ def validate(self, value):
             )
 
         if isinstance(value, Document) and value.id is None:
-            self.error(
-                "You can only reference documents once they have been "
-                "saved to the database"
-            )
+            self.error(_unsaved_object_error(value.__class__.__name__))
 
     def lookup_member(self, member_name):
         return self.document_type._fields.get(member_name)
@@ -1370,10 +1372,7 @@ def to_mongo(self, document, use_db_field=True, fields=None):
             # We need the id from the saved object to create the DBRef
             id_ = document.pk
             if id_ is None:
-                self.error(
-                    "You can only reference documents once they have"
-                    " been saved to the database"
-                )
+                self.error(_unsaved_object_error(document.__class__.__name__))
         else:
             self.error("Only accept a document object")
 
@@ -1394,10 +1393,7 @@ def prepare_query_value(self, op, value):
         # XXX ValidationError raised outside of the "validate" method.
         if isinstance(value, Document):
             if value.pk is None:
-                self.error(
-                    "You can only reference documents once they have"
-                    " been saved to the database"
-                )
+                self.error(_unsaved_object_error(value.__class__.__name__))
             value_dict = {"_id": value.pk}
             for field in self.fields:
                 value_dict.update({field: value[field]})
@@ -1411,10 +1407,7 @@ def validate(self, value):
             self.error("A CachedReferenceField only accepts documents")
 
         if isinstance(value, Document) and value.id is None:
-            self.error(
-                "You can only reference documents once they have been "
-                "saved to the database"
-            )
+            self.error(_unsaved_object_error(value.__class__.__name__))
 
     def lookup_member(self, member_name):
         return self.document_type._fields.get(member_name)
@@ -1513,10 +1506,7 @@ def validate(self, value):
 
         # We need the id from the saved object to create the DBRef
         elif isinstance(value, Document) and value.id is None:
-            self.error(
-                "You can only reference documents once they have been"
-                " saved to the database"
-            )
+            self.error(_unsaved_object_error(value.__class__.__name__))
 
     def to_mongo(self, document):
         if document is None:
@@ -1533,10 +1523,7 @@ def to_mongo(self, document):
             id_ = document.id
             if id_ is None:
                 # XXX ValidationError raised outside of the "validate" method.
-                self.error(
-                    "You can only reference documents once they have"
-                    " been saved to the database"
-                )
+                self.error(_unsaved_object_error(document.__class__.__name__))
         else:
             id_ = document
 
@@ -2535,10 +2522,7 @@ def validate(self, value):
                 )
 
         if pk is None:
-            self.error(
-                "You can only reference documents once they have been "
-                "saved to the database"
-            )
+            self.error(_unsaved_object_error(self.document_type.__name__))
 
     def prepare_query_value(self, op, value):
         if value is None:
@@ -2607,8 +2591,9 @@ def __get__(self, instance, owner):
     def validate(self, value):
         if isinstance(value, LazyReference) and value.pk is None:
             self.error(
-                "You can only reference documents once they have been"
-                " saved to the database"
+                _unsaved_object_error(
+                    self.__class__.__name__
+                )  # Actual class is difficult to predict here
             )
         return super().validate(value)
 

tests/fields/test_reference_field.py

@@ -20,7 +20,7 @@ class Test(Document):
         class NonDocumentSubClass:
             pass
 
-        # fails if given an non Document subclass
+        # fails if given a non Document subclass
         with pytest.raises(ValidationError, match=ERROR_MSG):
 
             class Test(Document):  # noqa: F811
@@ -46,12 +46,17 @@ class BlogPost(Document):
         with pytest.raises(ValidationError):
             ReferenceField(EmbeddedDocument)
 
-        user = User(name="Test User")
+        unsaved_user = User(name="Test User")
 
         # Ensure that the referenced object must have been saved
         post1 = BlogPost(content="Chips and gravy taste good.")
-        post1.author = user
-        with pytest.raises(ValidationError):
+        post1.author = unsaved_user
+        expected_error = (
+            "The instance of the document 'User' you are "
+            "trying to reference has an empty 'id'. You can only reference "
+            "documents once they have been saved to the database"
+        )
+        with pytest.raises(ValidationError, match=expected_error):
             post1.save()
 
         # Check that an invalid object type cannot be used
@@ -61,6 +66,7 @@ class BlogPost(Document):
             post1.validate()
 
         # Ensure ObjectID's are accepted as references
+        user = User(name="Test User")
         user_object_id = user.pk
         post3 = BlogPost(content="Chips and curry sauce taste good.")
         post3.author = user_object_id