Change admin search_fields to favor `USERNAME_FIELD` instead of "email".

[charettes]

Description of the Change

Per this request.

---

First nothing guarantees that the user model has a field named "email" as it can be set to a different name using EMAIL_FIELD. At the very least the get_email_field_name method should have been used.

Secondly nothing guarantees that EMAIL_FIELD is going to be indexed and thus suitable for search purposes. On the other hand USERNAME_FIELD must be unique and thus indexed in order to enforce this constraint.

For these reasons USERNAME_FIELD represents a better choice to allow the different toolkit models to be searched by through the admin.

Checklist

• [x] PR only contains one change (considered splitting up PR)
• [x] unit-test added (not sure if needed given #1041 didn't include any and Django enforce admin checks)
• [x] documentation updated (I assume nothing is needed here either)
• [ ] CHANGELOG.md updated (only for user relevant changes)
• [x] author name in AUTHORS

[charettes]

(which in all fairness is how it is documented in the contrib admin models documentation!)

That's a valid point that might be worth raising upstream. I'll see what I can do.

I'm happy to submit a PR for the first point as it's unambiguously a good change but I'm not convinced about the other ones.

There doesn't exist a setting today and Django already provides a way to override registered Django admins

class ApplicationAdmin(admin.ApplicationAdmin):
    search_fields = ("name", "user__email")
    search_help_text = "Search applications by name and user email"

admin.site.register(application_model, ApplicationAdmin)

In fact that's the approach we used at $JOB when we had to make sure user__email was not used as it was causing a lot of contention on one of our databases.

IMO settings like APPLICATION_ADMIN_CLASS are an anti-pattern as Django already has a pattern to override per-model admin classes and adding extra settings to target specific model admin options is a slippery slope so I don't think 2. and 3. are worth doing.

To summarize I think we should either proceed with this PR as it is as user__username constitute a better default than user__email or close this PR and only do 1.

[jaap3]

In my case the User model has an email field, but doesn't have a username field. This issue is still relevant to me, because this field uses a case-insensitive non-deterministic collation. The way Django Admin implements search_fields doesn't work with this type of field. (See also: https://adamj.eu/tech/2023/02/23/migrate-django-postgresql-ci-fields-case-insensitive-collation/#adjust-queries).

It's possible to work around this by using admin.unregister and registering a customised version of DOT admin classes.

However, maybe DOT should just drop the ability to search by email. Instead it can document how to customize the admin classes instead, using search_fields as an example. DOT doesn't really need to make any assumptions about the user model.

It would be a breaking change, but it is not a "mission critical" feature. Those that need it can bring it back using their own code.

The PR that first tried to introduce this had a fitting comment by @IvanAnishchuk :

Not sure if assuming things regarding user model is a good approach here

https://github.com/jazzband/django-oauth-toolkit/pull/723#pullrequestreview-304238599

P.S. @charettes I didn't even think about the APPLICATION_*_CLASS settings, surely those have to be deprecated at some point? It does imply that subclassing the admin classes is an expected use-case.