Update Django to 1.11 LTS #1077
Conversation
Pacu2
force-pushed
the
feature/moving-to-django-1.11-lts
branch
from
40d4380
to
66aef99
Compare
Pacu2
force-pushed
the
feature/moving-to-django-1.11-lts
branch
from
7c7536a
to
86b4ea7
Compare
Codecov Report
@@ Coverage Diff @@
## master #1077 +/- ##
==========================================
+ Coverage 63.42% 63.65% +0.23%
==========================================
Files 105 106 +1
Lines 5818 5869 +51
Branches 721 729 +8
==========================================
+ Hits 3690 3736 +46
- Misses 1991 1994 +3
- Partials 137 139 +2
Continue to review full report at Codecov.
|
Pacu2
force-pushed
the
feature/moving-to-django-1.11-lts
branch
from
86b4ea7
to
8ccacb7
Compare
| {% if choice.data.value in choice.value %} | ||
| checked="checked" | ||
| {% endif %} | ||
| > |
There was a problem hiding this comment.
Sure, I just didn't like that over 200 chars long line
| @@ -222,6 +222,7 @@ def delete(self): | |||
|
|
|||
|
|
|||
| class ProductImageForm(forms.ModelForm): | |||
| use_required_attribute = False | |||
There was a problem hiding this comment.
Why did you set that attribute to False?
There was a problem hiding this comment.
There was a problem, with incorrect browser validation of required fields.
While on dashboard -> product -> images
I couldn't edit image's description, as it required me to upload a new photo instead.
|
Shouldn't we include https://github.com/anymail/django-anymail along with templated email? |
saleor/checkout/views/__init__.py
Outdated
| @@ -72,7 +72,7 @@ def login(request, checkout): | |||
| Allows user to choose if he wants to login before checkout or continue | |||
| as an anonymous user | |||
| """ | |||
| if request.user.is_authenticated: | |||
| if request.user.is_authenticated(): | |||
There was a problem hiding this comment.
This is backwards. Django seems to be moving away from this being a method (it's currently a property that returns a callable compatibility wrapper).
There was a problem hiding this comment.
On Django 1.8, where is_authenticated is still a method, this will always return True, it was incorrectly written at the first time. If we want to keep compatibility, it should stay this way.
It should be supported till Django 2.0.
There was a problem hiding this comment.
We don't need to support any version earlier than 1.11 as it's an LTS release.
| @@ -0,0 +1,84 @@ | |||
| from django import forms | |||
There was a problem hiding this comment.
Was this file brought in verbatim? If so, what is its license? Does it require attribution?
There was a problem hiding this comment.
Modified few lines, but most of it comes from its original source.
It's licensed under MIT conditions, can be found at:
https://github.com/florent1933/django-materializecss-form/blob/master/LICENSE
I've included the license in its folder.
| {{ payment_url }} | ||
|
|
||
| Sincerely, | ||
| Saleor Team |
There was a problem hiding this comment.
We should not hardcode "Saleor" here. Please use whatever we put in the site title.
requirements.txt
Outdated
| docutils==0.13.1 # via botocore | ||
| elasticsearch==2.4.1 | ||
| enum34==1.1.6 # via cryptography |
There was a problem hiding this comment.
This file was generated using Python 2. Please rebuild using Python 3 as that's the default version we're targeting.
|
Replying to #1077 (comment) by @patrys:
Do we want to put it as a default mailing service? |
|
I'm asking because some users will treat Saleor like an out of the box solution and expect email to Just Work™ (save for setting some environment variables). |
|
@patrys |
Pacu2
force-pushed
the
feature/moving-to-django-1.11-lts
branch
2 times, most recently
from
e07d1e2
to
34d60fd
Compare
Pacu2
force-pushed
the
feature/moving-to-django-1.11-lts
branch
from
34d60fd
to
1ead8c9
Compare
|
Please bump the Django requirement in |
|
@patrys Done, Django version updated |
|
🎊 |
Closes #810