Upgrade Django to 1.6

[aronasorman]

Support for Django 1.5 has been dropped with the recent release of Django 1.7. That as well a slew of goodies should make a compelling case for upgrading to Django 1.6.

Filing this for 0.13, but will most probably get in by 0.14.

TODO

• [ ] bin/kalite manage shows kalitectl.py as the command name
• [ ] Models calling "validate" should use "clean" (securesync)

[djallado]

@aronasorman.

I ran some of the management command on django 1.6: attached result on management command

I also ran test on some apps: attached result on test

[aronasorman]

This looks fun :)

[djallado]

I attached a checklist on packages for current and upgrade. attached checklist

[djallado]

I update django to 1.6.8 and South to 0.8.4

Error when I use setup_mac.command management command:

[djallado]

I upgrade django_extensions to 1.4.0 and delete 0002_add_english_languagepack.py in i18n migrations then setup_mac.command work without an error. Manage.py kaserve also works fine.

[aronasorman]

Alright, cool! Any updates on the tests?

[benjaoming]

Now comes a very long comment. Some of the headlines will get their own issues, I will update it accordingly tomorrow. But please be notified of all these thoughts :) I'll gladly help out on some of these!

[benjaoming]

KA Lite currently ships with a modified version of Django. Not only is it bad practice, but the modifications seem quite unnecessary. So instead of spending time applying the same modifications to Django 1.6, let's remove the motivation / necessitation for these changes!

Changes are in the KA Lite Django 1.5 are listed here

Suggestions on how to get rid of them.......

Before you get started, have a look at this tool:

http://beyondgrep.com/why-ack/

On Ubuntu: apt-get install ack-grep

python-packages/django/core/serializers/base.py

There is no longer an differences here! So just ignore this.

Modification doesn't exist anymore (MDEA)

python-packages/django/core/serializers/python.py

Freebie! MDEA

python-packages/django/templates/base.py

The origin of the problem is the use of the repeatblock tag which as per this issue requires a change in Django to work:

https://code.djangoproject.com/ticket/4529

What to do

1. Find all occurrences of {% load repeatblock %}...

   ack-grep --html repeatblock 
   kalite/coachreports/templates/coachreports/landing_page.html
   3:{% load repeatblock %}
   29:                        <a class="changeable-link" href="{% url 'scatter_view' xaxis='pct_mastery' yaxis='effort' %}?{% repeatblock topic_paths %}">
   37:                        <a class="changeable-link" href="{% url 'timeline_view' xaxis='ex:completed_timestamp' yaxis='pct_mastery' %}?{% repeatblock topic_paths %}">
   
   kalite/coachreports/templates/coachreports/timeline_view.html
   3:{% load repeatblock %}
   
   kalite/coachreports/templates/coachreports/scatter_view.html
   3:{% load repeatblock %}
   39:        {% repeatblock data_options %}
   

2. Replace all this with an include pattern, for instance {% include "coachreports/includes/topic_paths" %}. In the case of timeline_view.html, it might just go because its not even used :)

3. Remove fle_utils.templatetags.repeatblock.

python-packages/django/contrib/auth/forms.py

MDEA

python-packages/django/contrib/auth/models.py

The change is regarding this part:

diff -r ./contrib/auth/models.py /home/benjamin/django1.5/django/./contrib/auth/models.py
377,379c377,378
<     #Changed to modify username length from 30 to 75 (matching email addresses)
<     username = models.CharField(_('username'), max_length=75, unique=True, #  KA-LITE-MOD (use email for username)
<         help_text=_('Required. 75 characters or fewer. Letters, numbers and '
---
>     username = models.CharField(_('username'), max_length=30, unique=True,
>         help_text=_('Required. 30 characters or fewer. Letters, numbers and '

Firstly, we need to obtain an insight in how users are being created and whether any emails are stored in the username field. This would be an easy way to ensure that the above code was actually never used.

facility.FacilityUser does not make use of any of this, it has its own username field which has max_length=30.

I tried searching for the pattern "User.+create", which is insufficient. But I get the idea that there are no views that create users in kalite from the following comment (in facility.views)

Different codepaths for the following:
* Django admin/teacher creates student (add_facility_student)
* Django admin creates teacher
* Django admin/edits a user, self, or student edits self (edit_facility_user)
* Student creates self (facility_user_signup)

python-packages/django/utils/cache.py

This change seems to relate to a perception that request.LANGUAGE_CODE is somehow wrong. However, this is a misunderstanding regarding that request.LANGUAGE_CODE is set by Django's builtin middleware LocaleMiddleware is mistakenly not in the project by default, but only set in i18n/settings.py

See separate issue to verify that the middleware can be added without problems:

utils/translations/trans_real.py

Undocumented change for running ugettext(None)

Before:

eol_message = message.replace(str('\r\n'), str('\n')).replace(str('\r'), str('\n'))

After:

# KA-LITE-MOD (bcipolli)
#  Django chokes when translation string is None, which is hard to detect upstream.
#  Django - be more robust!!
#  Change: translated None is ... None!

# str() is allowing a bytestring message to remain bytestring on Python 2
eol_message = message and message.replace(str('\r\n'), str('\n')).replace(str('\r'), str('\n')) or None

I think this change is uncalled for. Apparently trying to translate a None type doesn't happen anywhere on the interwebs...

https://encrypted.google.com/search?num=30&hl=en&q=django+eol_message+AttributeError%3A+%27NoneType%27+object+has+no+attribute+%27replace%27

In case we are trying to translate a None type object, something is wrong and it would be nice to have this fixed, rather than changing Django to accommodate for odd cases.

Change related to cache keys

This change reflects an assumption that settings.LANGUAGE_CODE is an unsupported sublanguage of settings.LANGUAGES, however this should be quite absurd and rather treated as an exception! Why would someone use settings.LANGUAGE_CODE = 'fr-ca' if this is not in settings.LANGUAGES ? That would be deliberately stupid :)

Before:

 return settings.LANGUAGE_CODE

After:

# KA-LITE-MOD (bcipolli):
# run settings.LANGUAGE_CODE through same gauntlet as above
# Otherwise, if we try to default to a sub-language (en-us), but only
# the base language is installed (en), we'll get different behavior
# based on client (some of which hit this fallback) IF the sub-language
# is not installed.
#
# Since apps can't necessarily control that, we want the same fall-back
# on LANGUAGE_CODE as for everything else.
lang_code = settings.LANGUAGE_CODE

if lang_code and lang_code not in supported:
    lang_code = lang_code.split('-')[0] # e.g. if fr-ca is not supported fallback to fr

if lang_code and lang_code in supported and check_for_language(lang_code):
    return lang_code
else:
    #raise Exception("No language code could be determined; even fall-back on settings.LANGUAGE_CODE (%s) failed!" % settings.LANGUAGE_CODE)
    # return the base, base default.
    return "en"

Undocumented changes regarding handlebars / i18nize_templates

I came across this when I was using the diff command :) I know that you guys or @cpauya made this change, so I trust you can figure out a way for the i18nize_templates command to work without patching Django itself :) I've looked at i18nize_templates (the app) and it looks really interesting!!

So I'm wondering if can be more encapsulated? If -- as I understand it -- KA Lite is using it to once in a while mark unmarked strings for translation, then it shouldn't be part of the code that's distributed, anyways?

core/management/commands/test.py

HEADLESS

@aronasorman

The change ensures that a couple of test-specific changes are added, especially one which is intended for Travis (I would suppose). The way of mending this and normal practice in Django applications is to manage settings relevant for testing and travis separately and pass the settings module to the test suite.

There is a line in .travis.yml that states how tests should be run:

python kalite/manage.py test

This line could look like this instead:

python kalite/manage.py test --settings=settings_travis

Inside settings_travis.py (nevermind the name for now, settings have to be refactored later), we just do:

from settings import *
HEADLESS = True

if settings.BUILT

MDEA

[djallado]

I ran tests on my current changes on the upgrade and it raise a minimal error compared to the previous tests.

Test on some apps

I reuse python-packages/django/utils/translation/trans_real.py from old version. Remove validate_unique argument of full_clean method.

So far, I can add student and student can watch video. Student can't take test which I think an issue in develop.

[benjaoming]

@djallado I think it's a bad idea to reuse trans_real -- this is the diff between 1.5 and 1.6, hence why we need to get rid of all our own modifications in Django so we don't have to figure out what this means.........

10d9
< import warnings
13d11
< from django.utils.datastructures import SortedDict
15d12
< from django.utils.functional import memoize
20d16
< from django.utils.translation import TranslatorCommentWarning
34d29
< _checked_languages = {}
49d43
< 
82a77
>         self.django_output_charset = 'utf-8'
145c140
<         if base_lang(lang) in [base_lang(trans) for trans in list(_translations)]:
---
>         if base_lang(lang) in [base_lang(trans) for trans in _translations]:
288,289c283
<         # force unicode, because lazy version expects unicode
<         result = force_text(message)
---
>         result = message
361,388d354
< check_for_language = memoize(check_for_language, _checked_languages, 1)
< 
< def get_supported_language_variant(lang_code, supported=None, strict=False):
<     """
<     Returns the language-code that's listed in supported languages, possibly
<     selecting a more generic variant. Raises LookupError if nothing found.
< 
<     If `strict` is False (the default), the function will look for an alternative
<     country-specific variant when the currently checked is not found.
<     """
<     if supported is None:
<         from django.conf import settings
<         supported = SortedDict(settings.LANGUAGES)
<     if lang_code:
<         # if fr-CA is not supported, try fr-ca; if that fails, fallback to fr.
<         generic_lang_code = lang_code.split('-')[0]
<         variants = (lang_code, lang_code.lower(), generic_lang_code,
<                     generic_lang_code.lower())
<         for code in variants:
<             if code in supported and check_for_language(code):
<                 return code
<         if not strict:
<             # if fr-fr is not supported, try fr-ca.
<             for supported_code in supported:
<                 if supported_code.startswith((generic_lang_code + '-',
<                                               generic_lang_code.lower() + '-')):
<                     return supported_code
<     raise LookupError(lang_code)
390c356
< def get_language_from_path(path, supported=None, strict=False):
---
> def get_language_from_path(path, supported=None):
394,396d359
< 
<     If `strict` is False (the default), the function will look for an alternative
<     country-specific variant when the currently checked is not found.
400c363
<         supported = SortedDict(settings.LANGUAGES)
---
>         supported = dict(settings.LANGUAGES)
402,408c365,368
<     if not regex_match:
<         return None
<     lang_code = regex_match.group(1)
<     try:
<         return get_supported_language_variant(lang_code, supported, strict=strict)
<     except LookupError:
<         return None
---
>     if regex_match:
>         lang_code = regex_match.group(1)
>         if lang_code in supported and check_for_language(lang_code):
>             return lang_code
422c382
<     supported = SortedDict(settings.LANGUAGES)
---
>     supported = dict(settings.LANGUAGES)
436,439c396,400
<     try:
<         return get_supported_language_variant(lang_code, supported)
<     except LookupError:
<         pass
---
>     if lang_code and lang_code not in supported:
>         lang_code = lang_code.split('-')[0] # e.g. if fr-ca is not supported fallback to fr
> 
>     if lang_code and lang_code in supported and check_for_language(lang_code):
>         return lang_code
445a407,411
>         # We have a very restricted form for our language files (no encoding
>         # specifier, since they all must be UTF-8 and only one possible
>         # language each time. So we avoid the overhead of gettext.find() and
>         # work out the MO file manually.
> 
459,465c425,432
<         try:
<             accept_lang = get_supported_language_variant(accept_lang, supported)
<         except LookupError:
<             continue
<         else:
<             _accepted[normalized] = accept_lang
<             return accept_lang
---
>         for lang, dirname in ((accept_lang, normalized),
>                 (accept_lang.split('-')[0], normalized.split('_')[0])):
>             if lang.lower() not in supported:
>                 continue
>             for path in all_locale_paths():
>                 if os.path.exists(os.path.join(path, dirname, 'LC_MESSAGES', 'django.mo')):
>                     _accepted[normalized] = lang
>                     return lang
467,470c434
<     try:
<         return get_supported_language_variant(settings.LANGUAGE_CODE, supported)
<     except LookupError:
<         return settings.LANGUAGE_CODE
---
>     return settings.LANGUAGE_CODE
507,509d470
<     lineno_comment_map = {}
<     comment_lineno_cache = None
< 
571d531
< 
573,591d532
<             # Handle comment tokens (`{# ... #}`) plus other constructs on
<             # the same line:
<             if comment_lineno_cache is not None:
<                 cur_lineno = t.lineno + t.contents.count('\n')
<                 if comment_lineno_cache == cur_lineno:
<                     if t.token_type != TOKEN_COMMENT:
<                         for c in lineno_comment_map[comment_lineno_cache]:
<                             filemsg = ''
<                             if origin:
<                                 filemsg = 'file %s, ' % origin
<                             warn_msg = ("The translator-targeted comment '%s' "
<                                 "(%sline %d) was ignored, because it wasn't the last item "
<                                 "on the line.") % (c, filemsg, comment_lineno_cache)
<                             warnings.warn(warn_msg, TranslatorCommentWarning)
<                         lineno_comment_map[comment_lineno_cache] = []
<                 else:
<                     out.write('# %s' % ' | '.join(lineno_comment_map[comment_lineno_cache]))
<                 comment_lineno_cache = None
< 
648,651c589
<                 if t.contents.lstrip().startswith(TRANSLATOR_COMMENT_MARK):
<                     lineno_comment_map.setdefault(t.lineno,
<                                                   []).append(t.contents)
<                     comment_lineno_cache = t.lineno
---
>                 out.write(' # %s' % t.contents)
671,674c609
<         if priority:
<             priority = float(priority)
<         if not priority:        # if priority is 0.0 at this point make it 1.0
<              priority = 1.0
---
>         priority = priority and float(priority) or 1.0

[djallado]

@aronasorman.

I will use django 1.6.10 instead of 1.6.8, because this release address several security issues.

Steps:

1. apply unmodified django 1.6.10
2. check package dependencies
3. go thru ben's fixes one by one
4. check tests
5. check functionalities

[djallado]

I rename request.raw_post_data to request.body. (request.raw_post_data on Django < 1.4).

[djallado]

Removed {% load repeatblock %} in some html file and replaced it to include pattern is already been done. Basically, functionality of affected changes work the same because we only replaced repeatblock with include pattern.
The step 3 above, remove fle_utils.templatetags.repeatblock was not perform because I din't find it.

Screenshot below show javascript error when taking test. before this will not raise javascript error, but when I pull and merge develop to my branch this error occur.

[aronasorman]

Almost a year now since this was opened :)

[benjaoming]

0.16 seems good for 1.6... then 0.17 can be for django 1.7 :)