Skip to content

Conversation

@m000
Copy link

@m000 m000 commented Nov 25, 2025

Description

TokenAdmin assumed a fixed username field for the model specified by settings.AUTH_USER_MODEL. However, django allows using some other field as the username, and this is specified by overriding the USERNAME_FIELD property. See [1].

Also, switched default ordering to also use the username, and added filter for creation date.

[1] https://docs.djangoproject.com/en/5.2/topics/auth/customizing/#django.contrib.auth.models.CustomUser.USERNAME_FIELD

@m000 m000 force-pushed the fix/username-lookup branch 2 times, most recently from a0161c4 to 5ff42dc Compare November 25, 2025 18:07
@m000 m000 force-pushed the fix/username-lookup branch from 5ff42dc to 941cc57 Compare November 25, 2025 23:25
@m000 m000 force-pushed the fix/username-lookup branch from 941cc57 to fb633f4 Compare November 26, 2025 11:29
@auvipy auvipy requested review from auvipy and Copilot November 26, 2025 16:55
Copilot finished reviewing on behalf of auvipy November 26, 2025 16:56
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the TokenAdmin class to properly support custom user models with different username fields. Django allows projects to customize the username field via USERNAME_FIELD, and the admin interface previously hardcoded 'username' instead of respecting this setting.

Key changes:

  • Updated search_fields and ordering to dynamically use User.USERNAME_FIELD instead of hardcoded 'username'
  • Added list_filter for the 'created' field
  • Changed default ordering from descending creation date to ascending username field

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +28 to +30
search_fields = ('user__%s' % User.USERNAME_FIELD,)
search_help_text = _('Username')
ordering = ('-created',)
ordering = ('user__%s' % User.USERNAME_FIELD,)
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

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

The changes to use USERNAME_FIELD instead of hardcoded 'username' lack test coverage. Consider adding tests that verify the admin configuration works correctly with a custom user model that uses a different USERNAME_FIELD (e.g., 'email'), ensuring search and ordering function as expected.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

@m000 it would be great to add test coverage for the changes.

Copy link
Author

Choose a reason for hiding this comment

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

Added. It took a while to figure it out. Admin tests are a major PITA because modules include code that runs on load.

@m000 m000 requested a review from p-r-a-v-i-n November 26, 2025 17:52
@m000 m000 force-pushed the fix/username-lookup branch from fb633f4 to dd3e82d Compare November 27, 2025 14:17
Copy link
Contributor

@p-r-a-v-i-n p-r-a-v-i-n left a comment

Choose a reason for hiding this comment

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

Thanks for adding test coverage.

Admin tests are a major PITA because modules include code that runs on load.

😄 I'm happy you did it. Nice work :) . well this part of test and code also new for me.

other than this everything looks good to me , I think the precommit hook is failing fix it and lets wait for Bruno and Auvi for feedback.

@m000 m000 force-pushed the fix/username-lookup branch from dd3e82d to d78b161 Compare November 27, 2025 15:17
- Respect USERNAME_FIELD of the user model.
- Default ordering by username.
- Filter by creation date.
@m000
Copy link
Author

m000 commented Nov 27, 2025

Thanks for adding test coverage.

Admin tests are a major PITA because modules include code that runs on load.

😄 I'm happy you did it. Nice work :) . well this part of test and code also new for me.

other than this everything looks good to me , I think the precommit hook is failing fix it and lets wait for Bruno and Auvi for feedback.

Thanks for the hint. I had missed the existence of the pre-commit hooks.

I had chomped an extra line and isort complained. It should be good now.

@m000 m000 force-pushed the fix/username-lookup branch from d78b161 to 77be42e Compare November 27, 2025 16:47
@browniebroke browniebroke added this to the 3.17 milestone Dec 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants