django-rest-passwordreset
django-rest-passwordreset copied to clipboard
Wrong field lookup in views
Hi, thanks for this plugin! It looks like it'll save at least few hours of unnecessary work!
I just have installed the plugin and started getting internal server error from views.py
. The issue is the line 102
:
users = User.objects.filter(email__iexact=email)
In an app that I'm currently working on, there's no email
field, but it's actually a username
field that stores the email.
How about updating the plugin, so that it allows to parametrize the name of the field used throughout the data flow? This parameter could be use in views.py
, but also in serializers.py
(so there are no inconsistencies in parameter between them) and elsewhere.
Thanks!
That's a cool idea.
I guess that it wouldn't be too hard to implement the part with .filter()
, though I don't believe that making the serializer dynamic would be a good thing to do. In the end, it should always be an e-mail address to the serializer, it doesn't need to know more then that.
Unfortunately, I'm a little bit out of time at the moment, but I'm happy for someone to make a pull request.
This is great.
The Django user model by default does not have a 'unique' constraint on the email field, so I'd like to have users enter their username instead.
When can we expect this feature to be released?
@J-F-Far Just created a release candidate for it.
pip install "django-rest-passwordreset==1.1.0rc1"
Thanks for your quick response.
I'm having problems with this. I just installed the latest version 1.1.0rc1 into my project, and configured the settings value: 'DJANGO_REST_LOOKUP_FIELD = 'username'.
This hasn't made any difference. The view is still expecting an 'email'.
In the commit tagged '1.1.0rc1', the view 'ResetPasswordRequestToken' still uses the 'EmailSerializer', which is expecting the POSTed value to be a properly formatted EmailField, right?
Have you been able to verify that this works?
@anx-ckreuzberger I forked your repository and fixed this issue, by making the serializer used for the field configurable as well. Perhaps you'd like to review my branch named 'configurable_lookup'. Thanks.
Hi,
sorry for the long response time. Thanks for trying it out, and for reporting back, and also for the pull request :+1:
I will try to take a look at it this week.
I just took a look at it. It looks like in the original PR we did not consider that the lookup field can have a different type (just a different name). The implementation using the serializer in #47 seems to fix this, I've added some comments there.
Did you pull the code the branch master in my forked repository? As mentioned above, I had these changed on a branch called 'configurable_lookup' which I've since merged into master. I think you may need to build and pull these changes in again as I'm not seeing them in your tag 1.1.0rc1.
Thanks, sorry for the confusion.
Sorry but this discussion has exactly what I need it and get the user by username, any news in relation to this?
Thanks for the testing and the comments.
Unfortunately the Pull Request #47 by @J-F-Far has been closed. We did find several issues with the original implementation that need a rework before this feature is production ready. At this point also sorry to everyone that I didn't reply in such a long time.
At the moment the release candidate only allows to query users by email address by using an alternative field defined inDJANGO_REST_LOOKUP_FIELD
.
The feature Lookup another field that is not neccessarily an email address is currently not possible.
I will see if I can find spare time to add this feature.
@anx-ckreuzberger Did you find some problems with my implementation of this? As mentioned in a previous comment, I've merged this change into the master branch of my forked project. The original pull request was on a branch in my project called configurable_lookup, which has since been merged into master.
Forgive me is this has been confusing, I'm new to OSS contributing.
I've restored the previous branch that the pull request was opened on. Will this allow you to roll the feature into a release candidate?
No worries :)
I'll take a look at it, thank you!