Update project #215
Closed
Update project #215
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Updated deprecated "is_safe_url" function to "url_has_allowed_host_and_scheme". Added automatically database population, if DB is empty. Updated requirements to current version. Updated settings. Removed outdated version support. Updated gitignore.
Added type hints. Updated test cases. Removed last remaining snippets for outdated Django versions.
|
That's a huge contribution. I'll consider it ASAP, but it won't be instant, since I have to go through it thoroughly. Will be in touch here. Thanks. |
Removed __slots__ from dataclass, to match Python 3.8 support.
trco
reviewed
Apr 30, 2023
There was a problem hiding this comment.
This file should be removed, since it was meant only for compatibility with Django < 1.11.
# Import custom LoginView for Django versions < 1.11
if DJANGO_MAJOR_VERSION == '1' and '11' not in DJANGO_MINOR_VERSION:
from .compatibility import LoginView
else:
from django.contrib.auth.views import LoginView
|
|
||
| from .utils import is_ajax | ||
| AuthForm = TypeVar('AuthForm', bound=AuthenticationForm) |
There was a problem hiding this comment.
It would be better to define all the types in separate file.
There was a problem hiding this comment.
You need these types explicitly just in this module mixins.py. Are you sure to create another module and shifting these types into this new module, just to import them here anyways?
Merged
|
This PR was merged in #216. Thanks @aDramaQueen for your huge contribution. I didn't include types for now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi, this is a big one...
As I read through this project for my last PR #214, I was baffled that you're supporting Python 2...
A part of me admires this, that you want to support this very old versions. Backwards compatibility is always nice, but you have to do some cuts, otherwise, you will eventually go insane. I mean even the Python organization does not support Python 2 any longer: Python.org
Same goes for Django. Django 1.9 is waaaaayyy outdated. This is a serious security problem: security.snyk.io
Take a look at the roadmap from Django itself. You should orientate at the lates LTS version: djangoproject.com.
I would suggest you drop support for everthing older than Django 3.2.
If you do that, you may now even use the mighty type hint system.
This is basically what I did in this PR. I updated this Project to Django 3.2 with Python 3.8. This comes with some benefits:
I know, it seems, like I changed a lot. But this is just the surface. In fact I did NOT change the functionallity of the Prject at all. All I did was a refactoring.
OK not 100% true, I did change on line. In the
bootstrap_modal_forms.compatibility.LoginViewclass in methodget_redirect_url(...)I changed the deprecatedis_safe_url(...)method with the current oneurl_has_allowed_host_and_scheme(...). But that's it.I ran all your test, and they succeded 100%.
Long story short: Have a look at the changes & take what like, leave what you don't like. If there are any questions. Just ask.
Thanks