Skip to content

Added reset by phone number functionality #74

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.

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

Closed

Conversation

peterolayinka
Copy link

Problem

This package is limited to recovering passwords by email only leaving out use case for where we have users signup with phone number.

solution

Added functionality to recover passwords by phone number.


def validate(self, attrs):
if not attrs.get("email") and not attrs.get("phone"):
raise serializers.ValidationError('Please provide Email or Phone Number')
Copy link

@pinheiro-bruno pinheiro-bruno Jan 15, 2020

Choose a reason for hiding this comment

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

This message doesn't make clear for user which fields needs to be provided. phone? phone_number?

-raise serializers.ValidationError('Please provide Email or Phone Number')
+raise serializers.ValidationError('Please provide email or phone field')

@Vayel
Copy link

Vayel commented Apr 9, 2020

Thanks for the PR! It would be convenient to pass the authentication method (email or phone) to the signal's receiver.

@enricostano
Copy link

What would be really nice is the ability to define a custom serializer extending the library. For instance in our case we need to identify the user by VAT number.

@enricostano
Copy link

Exactly this approach is what I was hoping for #93

@peterolayinka this would allow you to use phone number
@Vayel what do you think?

@Vayel
Copy link

Vayel commented Apr 28, 2020

@enricostano sounds good to me!

@ghost ghost requested a review from anx-cbenke July 27, 2020 10:25
Copy link
Contributor

@anx-cbenke anx-cbenke left a comment

Choose a reason for hiding this comment

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

A universal lookup field as proposed in #93 would also cover phonenumbers, so I would recommend to merge #93 and reject this PR.

@ghost ghost closed this Jul 30, 2020
@peterolayinka
Copy link
Author

Hi Guys,

Thanks for the review, I totally missed this conversation. I guess this is due to plenty of work-related emails from GitHub.

@enricostano
Copy link

@Morantron do we still need this?

@Morantron
Copy link

@Morantron do we still need this?

yep

@enricostano
Copy link

@anx-hnezbeda do you mind keeping this issue open? Thanks!

@peterolayinka
Copy link
Author

A universal lookup field as proposed in #93 would also cover phonenumbers, so I would recommend to merge #93 and reject this PR.

@Morantron @enricostano what approach would you prefer to have this merged?

Though, I made a fork already which I use in my codebase.

@peterolayinka peterolayinka deleted the reset_by_phonenumber branch June 6, 2024 17:50
This pull request was closed.
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.

6 participants