Skip to content

Added requirement for UserAdmin.search_fields to 3.10 release notes. #6811

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Added requirement for UserAdmin.search_fields to 3.10 release notes. #6811

wants to merge 1 commit into from

Conversation

carltongibson
Copy link
Collaborator

Closes #6808.

@carltongibson carltongibson mentioned this pull request Jul 16, 2019
Merged
rpkilby
rpkilby reviewed Jul 17, 2019
View reviewed changes
docs/community/release-notes.md

**Note**: If you're using a custom user model, ensure your user admin
declared the `search_fields` property, in order to work with `TokenAdmin`
autocomplete fields.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be helpful to mention the (admin.E040) so the error code is searchable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, maybe link to: https://docs.djangoproject.com/en/stable/ref/contrib/admin/#django.contrib.admin.ModelAdmin.autocomplete_fields

@rpkilby
Copy link
Contributor

rpkilby commented Jul 17, 2019

I think the documentation approach makes sense, as this should only affect projects that define a custom user model/admin that doesn't set search_fields. If a project uses the builtin user model/admin or inherits those, they should be fine.

@rpkilby
Copy link
Contributor

rpkilby commented Jul 17, 2019

Of course, the other option is to just revert #6762 and let users modify the admin when there is a sufficiently large number of users in their database.

@carltongibson carltongibson mentioned this pull request Jul 17, 2019
Closed
6 tasks
@lovelydinosaur
Copy link
Contributor

lovelydinosaur commented Jul 17, 2019 •
edited
Loading

I think we should probably prefer #6817 - given that Django docs heavily nudge folks towards custom user models it's likely that the change in behavior would affect a significant number of upgrades, and I don't think it's a clear enough improvement for us to do that.

Let's help folks in the right direction for customizing the TokenAuth admin themselves instead.

@lovelydinosaur lovelydinosaur mentioned this pull request Jul 17, 2019
Merged
@carltongibson
Copy link
Collaborator Author

OK, if you prefer. Seems a bit of a shame but...

@lovelydinosaur
Copy link
Contributor

I'm not super wild about either option TBH.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@rpkilby rpkilby rpkilby left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rest_framework 3.10.0 - crash with runserver
3 participants
@carltongibson @rpkilby @lovelydinosaur