macropin / django-registration

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

Django 3.0 Support #371

Closed clarkmoody closed 4 years ago

clarkmoody commented 4 years ago

Django 3.0 removes django.utils.encoding.python_2_unicode_compatible(), which is used as a decorator on the RegistrationProfile model.

Removing this decorator is required for Django 3.0 support.

macropin commented 4 years ago

Removing this would require dispensing with python 2.x support. Perhaps a PR with a conditional import and decorator would allow us to maintain backwards compatibility?

perceptiveminds commented 4 years ago

I'm not sure what is the most elegant way to solve this, but I did it by importing python_2_unicode_compatible from six instead of from django.utils.encoding:

https://github.com/perceptiveminds/django-registration/commit/7cc20f474fe3e0f00fb7d284accb52e74467452a

clarkmoody commented 4 years ago

@perceptiveminds That looks like the least painful solution 😅

clarkmoody commented 4 years ago

Went ahead a made a PR from @perceptiveminds fork.

perceptiveminds commented 4 years ago

I fixed requirements.txt and the tests for Django 2+3 compatibility. Now the tests should pass. Could you try a new PR on my fork at perceptiveminds/django-registration ?

clarkmoody commented 4 years ago

@perceptiveminds Your two latest commits have automatically been added to the PR, so no need to open another one. Glancing at the test report, it looks like the linter is complaining about import sort order.

macropin commented 4 years ago

I don't think we want to introduce a dependency on six. My understanding is that decorator is only required with python 2.x and early Django versions. Can you update the PR to perform a conditional import and noop the decorator when not required?

perceptiveminds commented 4 years ago

After some fixes, only the tests need the six import now. I tried to fix the lint / tox / test errors. Only one standing: In Django 3, the UserCreationForm contains the error message

The two password fields didn’t match

(with a non-ASCII ’ character)

To check for that message, the ’ character needs to be in tests/forms.py. But then tox / lint fails with a non-ASCII error for Python 2, even when using # coding = utf-8.

Do you know a way to fix the code to pass that test for Python 2? (It should not matter at all, since Django 3 doesn't even run on Python 2)