macropin / django-registration

Django-registration (redux) provides user registration functionality for Django websites.
http://django-registration-redux.readthedocs.org
Other
975 stars 350 forks source link

show a form error when the user name or email has an illegal character #423

Closed jmordkoff closed 1 year ago

jmordkoff commented 2 years ago

Hi, forgive me if I'm not following the process. I'm new to github. This small change avoids a server hiccup is a user's name or email contains an illegal character (according to your DB).

joshblum commented 1 year ago

@JeremyMordkoff can you rebase this branch to pickup the CI fixes I added in https://github.com/macropin/django-registration/pull/432?

jmordkoff commented 1 year ago

Yes, and I have another fix for running tests that use atomic transactions and need mail.Outbox

I've been using git for a few years but almost exclusively in a corporate environment so if I screw up, feel free to criticize

On Sun, Feb 5, 2023, 4:55 PM Joshua Blum @.***> wrote:

@JeremyMordkoff https://github.com/JeremyMordkoff can you rebase this branch to pickup the CI fixes I added in #432 https://github.com/macropin/django-registration/pull/432?

— Reply to this email directly, view it on GitHub https://github.com/macropin/django-registration/pull/423#issuecomment-1418274755, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAY3RWMYQLJUMVDZLTAPBYTWWAOWFANCNFSM5TL7HG6Q . You are receiving this because you authored the thread.Message ID: @.***>

jmordkoff commented 1 year ago

These changes are a mess. Let me try to untangle them

First, I am getting a failure in my unit tests when I try to use mail.outbox. I see it works for you, but for some reason it does not work for me. I don't know why.

Second, my fix for bad characters was actually triggered by bad characters in the cell-phone field (I am using a custom registration form) so I think I am better off using a better field validator instead.

Bottom line ... I wouldn't wait for my changes. They will probably turn out to be nothing.

On Sun, May 8, 2022 at 9:19 PM Joshua Blum @.***> wrote:

@.**** commented on this pull request.

Thanks! Can you additionally add a unit test to cover this change?

In registration/views.py https://github.com/macropin/django-registration/pull/423#discussion_r867572979 :

@@ -53,7 +55,11 @@ def dispatch(self, request, *args, **kwargs):

     return super().dispatch(request, *args, **kwargs)

 def form_valid(self, form):
  • new_user = self.register(form)

  • try:

  • new_user = self.register(form)

  • except OperationalError:

  • form.adderror(None, ("invalid characters destected in name or email"))

⬇️ Suggested change

  • form.adderror(None, ("invalid characters destected in name or email"))

  • form.adderror(None, ("invalid characters detected in name or email"))

— Reply to this email directly, view it on GitHub https://github.com/macropin/django-registration/pull/423#pullrequestreview-965512504, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAY3RWM7X37IU4PTZX2V2WLVJBR2VANCNFSM5TL7HG6Q . You are receiving this because you authored the thread.Message ID: @.***>

--

Jeremy Mordkoff

"Darkness cannot drive out darkness, only light can do that. Hate cannot drive out hate, only love can do that." —Martin Luther King Jr., Strength to Love, 1963

"some dance to remember, some dance to forget" -- Eagles, Hotel California

joshblum commented 1 year ago

@jmordkoff no problem, thanks for opening a PR. I'm going to close this for now, if you have a general fix you'd like to add, feel free to open another PR in the future.