Opened 8 years ago
Closed 8 years ago
#27295 closed Cleanup/optimization (fixed)
Add a system check to prohibit models that start with an underscore
Reported by: | Victor Porton | Owned by: | Quentin Fulsher |
---|---|---|---|
Component: | Core (System checks) | Version: | 1.10 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
Running manage.py test
for the attached Django project causes a meaningless exception.
It is probably because _UsersGroup class name starts with underscore.
$ python3 manage.py test Creating test database for alias 'default'... E ====================================================================== ERROR: test_users (auth_app.tests.UsersGroupTestCase) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/porton/Projects/gnosis-error/auth_app/tests.py", line 13, in setUp self.A.subgroups.add(self.B) File "/usr/lib/python3/dist-packages/django/db/models/fields/related_descriptors.py", line 881, in add self._add_items(self.source_field_name, self.target_field_name, *objs) File "/usr/lib/python3/dist-packages/django/db/models/fields/related_descriptors.py", line 1025, in _add_items .values_list(target_field_name, flat=True) File "/usr/lib/python3/dist-packages/django/db/models/query.py", line 731, in values_list clone = self._values(*fields) File "/usr/lib/python3/dist-packages/django/db/models/query.py", line 714, in _values query.add_fields(field_names, True) File "/usr/lib/python3/dist-packages/django/db/models/sql/query.py", line 1630, in add_fields name.split(LOOKUP_SEP), opts, alias, allow_many=allow_m2m) File "/usr/lib/python3/dist-packages/django/db/models/sql/query.py", line 1402, in setup_joins names, opts, allow_many, fail_on_missing=True) File "/usr/lib/python3/dist-packages/django/db/models/sql/query.py", line 1327, in names_to_path "Choices are: %s" % (name, ", ".join(available))) django.core.exceptions.FieldError: Cannot resolve keyword 'to' into field. Choices are: from__usersgroup, from__usersgroup_id, id, to__usersgroup, to__usersgroup_id ---------------------------------------------------------------------- Ran 1 test in 0.002s FAILED (errors=1) Destroying test database for alias 'default'...
Attachments (1)
Change History (10)
by , 8 years ago
Attachment: | error.tar.gz added |
---|
comment:1 by , 8 years ago
Component: | Core (Other) → Database layer (models, ORM) |
---|---|
Summary: | An exception on correct code → Models with underscores break query lookups |
My first instinct is to add a system check to prohibit models with underscores in the name. Maybe we should first ask if anyone is successfully using such models.
comment:2 by , 8 years ago
I posted on django-developers to ask if anyone is using underscores in model names.
comment:3 by , 8 years ago
Component: | Database layer (models, ORM) → Core (System checks) |
---|---|
Easy pickings: | set |
Summary: | Models with underscores break query lookups → Add a system check to prohibit models that start with an underscore |
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → Cleanup/optimization |
The mailing list indicates that underscores are used in model names, so it's probably enough to prevent underscores at the start of a name.
comment:4 by , 8 years ago
If no one minds, I would like to work on this ticket. It seems like this project would just require adding a system check for the underscore and then adding tests to check the code is validating the names correctly.
comment:5 by , 8 years ago
Yes, you may claim the ticket, no need to ask.
As Michal suggested on the mailing list, 'I think it makes sense to also check for and flag as errors model names starting or ending with underscores, or containing double underscores, since those can lead to invalid field names."
comment:6 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:7 by , 8 years ago
Has patch: | set |
---|---|
Patch needs improvement: | set |
I submitted a pull request for this ticket here:
https://github.com/django/django/pull/7370
I am having some issues getting the test cases to run correctly. Its probably just a misunderstanding of how the tests actually work. I go over it in more detail in the pull request.
comment:8 by , 8 years ago
Patch needs improvement: | unset |
---|
A project which triggers a bug