add field name validation to EmbeddedModelField lookups #224
Conversation
WaVEV
force-pushed
the
EMF-field-name-validation
branch
3 times, most recently
from
483c876 to
f4f229a
Compare
| @@ -104,6 +104,16 @@ def test_nested(self): | |||
| ) | |||
| self.assertCountEqual(Book.objects.filter(author__address__city="NYC"), [obj]) | |||
|
|
|||
| def test_nested_not_exists(self): | |||
| msg = "Address.city has no field named 'president'" | |||
There was a problem hiding this comment.
This error doesn't look correct. city is a CharField which doesn't have subfields (rather than address being model in the other test) . Instead what would come after it would be a lookup. I'd expect an error like: FieldError: Unsupported lookup 'president' for CharField 'city'. If that's feasible, can we also use the "perhaps you meant" logic from https://github.com/django/django/blob/c28f821c9067050ba0d099349a4dfea2b29faf99/django/db/models/sql/query.py#L1436-L1452?
There was a problem hiding this comment.
🤔 I think is possible, will try.
There was a problem hiding this comment.
Did you try adding the "perhaps you meant" logic to suggest what lookup the user may have mistyped?
There was a problem hiding this comment.
I didn't, but I think it is possible, just have to check the choices like Django does (don't remember the method that does this)
There was a problem hiding this comment.
Done 😄 , I never read more than 3 lines.
timgraham
changed the title
timgraham
force-pushed
the
EMF-field-name-validation
branch
from
5189e55 to
a1c6b39
Compare
timgraham
force-pushed
the
EMF-field-name-validation
branch
from
a1c6b39 to
37af20e
Compare
| def test_invalid_lookup_with_suggestions(self): | ||
| msg = ( | ||
| "Unsupported lookup '{lookup}' for CharField 'name', " | ||
| "perhaps you meant {suggested_lookups}?" | ||
| ) | ||
| with self.assertRaisesMessage( | ||
| FieldDoesNotExist, msg.format(lookup="exactly", suggested_lookups="exact or iexact") | ||
| ): | ||
| Book.objects.filter(author__name__exactly="NYC") | ||
| with self.assertRaisesMessage( | ||
| FieldDoesNotExist, msg.format(lookup="gti", suggested_lookups="gt or gte") | ||
| ): | ||
| Book.objects.filter(author__name__gti="NYC") | ||
| with self.assertRaisesMessage( | ||
| FieldDoesNotExist, msg.format(lookup="is_null", suggested_lookups="isnull") | ||
| ): | ||
| Book.objects.filter(author__name__is_null="NYC") |
There was a problem hiding this comment.
Can we add a test that also checks the case where no suggestion will be provided?
with self.assertRaisesMessage(
FieldDoesNotExist, "Unsupported lookup '{lookup}' for CharField 'name'."
):
Book.objects.filter(author__name__zzzz="NYC")There was a problem hiding this comment.
The test is on the line 161, test_invalid_lookup
fixes #217