Opened 10 years ago
Closed 10 years ago
#26470 closed Cleanup/optimization (fixed)
Checks performed during Permission creation should use the check framework.
| Reported by: | Simon Charette | Owned by: | nobody |
|---|---|---|---|
| Component: | contrib.auth | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
The django.contrib.auth.management.check_permission function that is connected to the post_migrate signal performs static model analysis checks that should rely on the check framework. At the moment a ValidationError or a CommandError is raised when a check fails.
The checks performed are the following:
- Check that custom permissions don't clash with built-ins and are not duplicated;
- Check that models
verbose_namedoesn't exceed a certain length based on thePermission.name.max_length; - Check that permissions
namedoesn't exceed a certain length based on thePermission.name.max_length.
For 2. and 3. I suggest we make them issue a check.Warning with a message noting that the name will be truncated to N characters instead of crashing with an IntegrityError if the check is ignored/disabled.
Change History (7)
comment:1 by , 10 years ago
| Has patch: | set |
|---|
comment:2 by , 10 years ago
| Triage Stage: | Unreviewed → Ready for checkin |
|---|
Those checks were added before the permission name max_length was increased from 50 to 255 (#8162). Therefore I expect few if any users to exceed that limit these days. I don't see much benefit to adding truncation logic as proposed.
comment:3 by , 10 years ago
| Patch needs improvement: | set |
|---|---|
| Triage Stage: | Ready for checkin → Accepted |
Alright, I'll add them as errors later today in this case.
comment:5 by , 10 years ago
| Patch needs improvement: | unset |
|---|
comment:6 by , 10 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
https://github.com/django/django/pull/6421