Skip to content

Conversation

@olzhasar
Copy link
Contributor

@olzhasar olzhasar commented Oct 9, 2020

Using manager's get_by_natural_key should be preferred for getting users as oftentimes this method is customized by developers

@bluetech
Copy link
Member

bluetech commented Oct 9, 2020

Hi @olzhasar,

get_by_natural_key is used in Django serialization (e.g. fixtures), but here we just want to fetch a user by username. The natural key might be something else entirely. So using it here seems wrong to me.

@olzhasar
Copy link
Contributor Author

olzhasar commented Oct 9, 2020

Hi @olzhasar,

get_by_natural_key is used in Django serialization (e.g. fixtures), but here we just want to fetch a user by username. The natural key might be something else entirely. So using it here seems wrong to me.

Hello @bluetech,

I disagree. get_by_natural_key is used not only during serialization, but also in authentication backends: https://github.com/django/django/blob/master/django/contrib/auth/backends.py
If you have a customized auth backend, you will oftentimes customize get_by_natural_key method for your UserModel's default manager. I believe pytest-django should fetch users the same way as authentication backend

@bluetech
Copy link
Member

bluetech commented Oct 9, 2020

get_by_natural_key is used not only during serialization, but also in authentication backends

Aah didn't know that. I guess they are using it as some shortcut.

Can you describe how you customize get_by_natural_key()? Is it different from the default implementation (getting by USERNAME_FIELD)?

@olzhasar
Copy link
Contributor Author

olzhasar commented Oct 9, 2020

Can you describe how you customize get_by_natural_key()? Is it different from the default implementation (getting by USERNAME_FIELD)?

@bluetech
Sure, consider our example:
We use custom encrypted field for USERNAME_FIELD. As a result, we cannot search by it directly.
Our get_by_natural_key method calculates hash for the input and looks for a user by hash (which is stored in db).
We actively use pytest-django in our project, but we cannot use admin_client fixture for now due to the above reason

Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

OK, so this sounds fine to me. I added a suggestion for a comment to explain the situation a bit.

Co-authored-by: Ran Benita <ran@unusedvar.com>
@bluetech bluetech merged commit bb9e86e into pytest-dev:master Oct 9, 2020
@bluetech
Copy link
Member

bluetech commented Oct 9, 2020

Thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants