Merge lp:~widelands-dev/widelands-website/auto_one_to_one_ractivating into lp:widelands-website

[kaputtnik]

Another issue from the update to django 1.8:

Formerly to django 1.8 entrys to the database of wlprofile and wlggz has been created on demand.

For some reason i didn't checked this when working on the django 1.8 branch and thought the database entries got created during registering.

The last django errors from today told me that there is something wrong: A user who registered in year 2010 tried to access his profile the first time, and django through the error. For this user i created the wlprofile and wlggz database entries by hand over the admin site. Comparing the amount of users and wlprofiles i found that we have over 4000 users while there are only a bit more than 2000 wlprofiles :-S

This branch brings back the old behavior. I didn't know much about the code change, just get it from here:

https://github.com/skorokithakis/django-annoying/issues/36

So the code may need a review from a more experienced user.

This branch is tested here at home including normal registration over the registration app. I decided to move the relevant code to wl_utils because this code was used two times (wlprofile and wlggz). Additionally removed the code introduced by my misunderstanding.

We should get this in as soon as possible.

[lp:~widelands-dev/widelands-website/auto_one_to_one_ractivating]

431. By kaputtnik on 2016-11-15

resolved merge conflict because of settings.py

[GunChleoc (gunchleoc)]

django-annoying released that fix over a year ago, so it should be quite safe.

We should compare with their current code though:

https://github.com/skorokithakis/django-annoying/blob/master/annoying/fields.py#L37

I noticed that this has an extra @atomic in it.

[kaputtnik (franku)]

The @atomic decorator is only used with transactions, which depends on

- the version of database
- the capability of the Database
- table engine of the Database
- the settings of a database.

As far as i know we doesn't use transactions in our code and trust to the django default Autocommit mode (which is also default in MySql)

Nevertheless by reading through https://docs.djangoproject.com/en/1.8/topics/db/transactions/ i guess it is not bad to set this decorator. But i am no database specialist and my guess is maybe wrong.

I think we will never use transactions, so why should we (i) add this decorator if the consequences are unknown...

This Database stuff is really complicated... lets keep it as simple as it could be, imho.

[lp:~widelands-dev/widelands-website/auto_one_to_one_ractivating]

432. By kaputtnik on 2016-11-15

merged with trunk

[SirVer (sirver)]

I cannot really comment on the AutoSingleRelatedObjectDescriptor - it seems to work around various non-backwards compatible changes in django overtime. For example this comment about caching and not returning the created objects sounds frightening....

Let's get it in and hope for the best. Sometimes hope is a strategy :)

review: Approve

[lp:~widelands-dev/widelands-website/auto_one_to_one_ractivating]

433. By kaputtnik on 2016-11-15

make it responsible with future Django versions

[kaputtnik (franku)]

Related to your code comment: Yes it was a failure during the conflict resolving.

Merged and deployed. Hopefully all is working now...

=== modified file 'mainpage/views.py'
--- mainpage/views.py	2016-10-20 20:30:14 +0000
+++ mainpage/views.py	2016-11-15 22:47:42 +0000
@@ -55,32 +55,6 @@
 def legal_notice_thanks(request):
     return render(request, 'mainpage/legal_notice_thanks.html')
 
-from wlprofile.models import Profile
-from registration.backends.hmac.views import RegistrationView
-from django.contrib.auth.models import User
-from wlggz.models import GGZAuth
-
-class OwnRegistrationView(RegistrationView):
-    """Overwriting the default function to save also the extended User model
-    (wlprofile)"""
-
-    def create_inactive_user(self, form):
-        """Additionally save the custom enxtended user data."""
-        new_user = form.save(commit=False)
-        new_user.is_active = False
-        new_user.save()
-        reg_user = User.objects.get(username=new_user)
-        # Creating a wlprofile
-        ext_profile = Profile(user=reg_user)
-        ext_profile.save()
-        # Creating a ggzprofile
-        ggz_profile = GGZAuth(user=reg_user)
-        ggz_profile.save()
-
-        self.send_activation_email(new_user)
-
-        return new_user
-
 
 def developers(request):
     """This reads out some json files in the SVN directory, and returns it as a

=== modified file 'settings.py'
--- settings.py	2016-10-29 07:00:53 +0000
+++ settings.py	2016-11-15 22:47:42 +0000
@@ -3,8 +3,9 @@
 # Build paths inside the project like this: os.path.join(BASE_DIR, ...)
 import os
 
-BASE_DIR = os.path.dirname(os.path.abspath(__file__))#+ '/widelands'
+BASE_DIR = os.path.dirname(os.path.abspath(__file__))
 DEBUG = True
+
 ADMINS = (
     # ('Your Name', '[e-mail]'),
 )
@@ -75,7 +76,7 @@
     'django.contrib.staticfiles',
     'django.contrib.sites',
     'django.contrib.humanize',
-    #'django_comments',
+    'django_comments',
     'nocaptcha_recaptcha',
     # Thirdparty apps, but need preload
     'tracking', # included as wlapp

=== modified file 'urls.py'
--- urls.py	2016-08-02 19:22:42 +0000
+++ urls.py	2016-11-15 22:47:42 +0000
@@ -9,7 +9,7 @@
 from news.feeds import NewsPostsFeed
 from django.views.generic.base import RedirectView
 from django.contrib.syndication.views import Feed
-from mainpage.views import OwnRegistrationView
+from registration.backends.hmac.views import RegistrationView
 from mainpage.forms import RegistrationWithCaptchaForm
 
 urlpatterns = [
@@ -18,7 +18,7 @@
 
     # Django builtin / Registration
     # overwrite registration with own implementation
-    url (r'^accounts/register/$', OwnRegistrationView.as_view(form_class=RegistrationWithCaptchaForm), name='registration_register'),
+    url (r'^accounts/register/$', RegistrationView.as_view(form_class=RegistrationWithCaptchaForm), name='registration_register'),
     url(r'^accounts/', include('registration.backends.hmac.urls')),
     url('^', include('django.contrib.auth.urls')),
     

=== modified file 'wl_utils.py'
--- wl_utils.py	2016-10-17 20:24:04 +0000
+++ wl_utils.py	2016-11-15 22:47:42 +0000
@@ -6,3 +6,54 @@
             return request.META[key]
     # No match -> Return a fictional IP to have the model fields not empty
     return '192.168.255.255'
+
+
+# AutoOneToOneField
+# =================
+# Initial implemenation details about AutoOneToOneField:
+#   http://softwaremaniacs.org/blog/2007/03/07/auto-one-to-one-field/
+#
+# This doesn't worked anymore with django 1.8
+# changed according to:
+#   https://github.com/skorokithakis/django-annoying/issues/36
+
+
+# SingleRelatedObjectDescriptor gets renamed with Django 1.9
+try:
+    from django.db.models.fields.related import SingleRelatedObjectDescriptor
+except ImportError:
+    from django.db.models.fields.related_descriptors import ReverseOneToOneDescriptor as SingleRelatedObjectDescriptor
+
+from django.db.models import OneToOneField
+from django.db.models.fields.related import SingleRelatedObjectDescriptor
+from django.db import models
+
+class AutoSingleRelatedObjectDescriptor(SingleRelatedObjectDescriptor):
+    """
+    The descriptor that handles the object creation for an AutoOneToOneField.
+    """
+    def __get__(self, instance, instance_type=None):
+        model = getattr(self.related, 'related_model', self.related.model)
+
+        try:
+            return (super(AutoSingleRelatedObjectDescriptor, self)
+                    .__get__(instance, instance_type))
+        except model.DoesNotExist:
+            obj = model(**{self.related.field.name: instance})
+
+            obj.save()
+
+            # Don't return obj directly, otherwise it won't be added
+            # to Django's cache, and the first 2 calls to obj.relobj
+            # will return 2 different in-memory objects
+            return (super(AutoSingleRelatedObjectDescriptor, self)
+                    .__get__(instance, instance_type))
+
+class AutoOneToOneField(OneToOneField):
+    """
+    OneToOneField creates dependent object on first request from parent object
+    if dependent oject has not created yet.
+    """
+
+    def contribute_to_related_class(self, cls, related):
+        setattr(cls, related.get_accessor_name(), AutoSingleRelatedObjectDescriptor(related))

=== removed file 'wlggz/fields.py'
--- wlggz/fields.py	2016-08-08 18:18:04 +0000
+++ wlggz/fields.py	1970-01-01 00:00:00 +0000
@@ -1,33 +0,0 @@
-"""
-Details about AutoOneToOneField:
-    http://softwaremaniacs.org/blog/2007/03/07/auto-one-to-one-field/
-"""
-
-from StringIO import StringIO
-import logging
-
-from django.db.models import OneToOneField
-from django.db.models.fields.related import SingleRelatedObjectDescriptor
-from django.db import models
-
-
-class AutoSingleRelatedObjectDescriptor(SingleRelatedObjectDescriptor):
-    def __get__(self, instance, instance_type=None):
-        try:
-            return super(AutoSingleRelatedObjectDescriptor, self).__get__(instance, instance_type)
-        except self.related.model.DoesNotExist:
-            obj = self.related.model(**{self.related.field.name: instance})
-            obj.save()
-            return obj
-
-
-class AutoOneToOneField(OneToOneField):
-    """
-    OneToOneField creates dependent object on first request from parent object
-    if dependent oject has not created yet.
-    """
-
-    def contribute_to_related_class(self, cls, related):
-        setattr(cls, related.get_accessor_name(), AutoSingleRelatedObjectDescriptor(related))
-        #if not cls._meta.one_to_one_field:
-            #cls._meta.one_to_one_field = self

=== modified file 'wlggz/migrations/0001_initial.py'
--- wlggz/migrations/0001_initial.py	2016-04-18 13:29:23 +0000
+++ wlggz/migrations/0001_initial.py	2016-11-15 22:47:42 +0000
@@ -3,8 +3,7 @@
 
 from django.db import models, migrations
 from django.conf import settings
-import wlggz.fields
-
+import wl_utils
 
 class Migration(migrations.Migration):
 
@@ -21,7 +20,7 @@
                 ('lastlogin', models.DateTimeField(null=True, verbose_name='ggz lastlogin')),
                 ('permissions', models.IntegerField(default=7, verbose_name='ggz permissions')),
                 ('confirmed', models.IntegerField(default=1, verbose_name='confirmed', editable=False)),
-                ('user', wlggz.fields.AutoOneToOneField(related_name='wlggz', verbose_name='User', to=settings.AUTH_USER_MODEL)),
+                ('user', wl_utils.AutoOneToOneField(related_name='wlggz', verbose_name='User', to=settings.AUTH_USER_MODEL)),
             ],
             options={
                 'verbose_name': 'ggz',

=== modified file 'wlggz/models.py'
--- wlggz/models.py	2016-08-05 18:20:29 +0000
+++ wlggz/models.py	2016-11-15 22:47:42 +0000
@@ -9,7 +9,7 @@
 
 from django.db import models
 from django.contrib.auth.models import User
-from fields import AutoOneToOneField
+from wl_utils import AutoOneToOneField
 from django.utils.translation import ugettext_lazy as _
 from pybb.models import Post
 

=== modified file 'wlprofile/fields.py'
--- wlprofile/fields.py	2016-06-05 11:00:17 +0000
+++ wlprofile/fields.py	2016-11-15 22:47:42 +0000
@@ -1,38 +1,7 @@
-"""
-Details about AutoOneToOneField:
-    http://softwaremaniacs.org/blog/2007/03/07/auto-one-to-one-field/
-"""
-
 from StringIO import StringIO
+from django.db import models
 import logging
 
-from django.db.models import OneToOneField
-from django.db.models.fields.related import SingleRelatedObjectDescriptor
-from django.db import models
-from django.core.files.uploadedfile import SimpleUploadedFile
-
-
-class AutoSingleRelatedObjectDescriptor(SingleRelatedObjectDescriptor):
-    def __get__(self, instance, instance_type=None):
-        try:
-            return super(AutoSingleRelatedObjectDescriptor, self).__get__(instance, instance_type)
-        except self.related.model.DoesNotExist:
-            obj = self.related.model(**{self.related.field.name: instance})
-            obj.save()
-            return obj
-
-
-class AutoOneToOneField(OneToOneField):
-    """
-    OneToOneField creates dependent object on first request from parent object
-    if dependent oject has not created yet.
-    """
-
-    def contribute_to_related_class(self, cls, related):
-        setattr(cls, related.get_accessor_name(), AutoSingleRelatedObjectDescriptor(related))
-        #if not cls._meta.one_to_one_field:
-            #cls._meta.one_to_one_field = self
-
 
 class ExtendedImageField(models.ImageField):
     """

=== modified file 'wlprofile/migrations/0001_initial.py'
--- wlprofile/migrations/0001_initial.py	2016-04-18 13:29:23 +0000
+++ wlprofile/migrations/0001_initial.py	2016-11-15 22:47:42 +0000
@@ -3,9 +3,9 @@
 
 from django.db import models, migrations
 from django.conf import settings
+import wl_utils
 import wlprofile.fields
 
-
 class Migration(migrations.Migration):
 
     dependencies = [
@@ -29,7 +29,7 @@
                 ('signature', models.TextField(default=b'', max_length=255, verbose_name='Signature', blank=True)),
                 ('avatar', wlprofile.fields.ExtendedImageField(default=b'wlprofile/anonymous.png', upload_to=b'wlprofile/avatars/', verbose_name='Avatar', blank=True)),
                 ('show_signatures', models.BooleanField(default=True, verbose_name='Show signatures')),
-                ('user', wlprofile.fields.AutoOneToOneField(related_name='wlprofile', verbose_name='User', to=settings.AUTH_USER_MODEL)),
+                ('user', wl_utils.AutoOneToOneField(related_name='wlprofile', verbose_name='User', to=settings.AUTH_USER_MODEL)),
             ],
             options={
                 'verbose_name': 'Profile',

=== modified file 'wlprofile/models.py'
--- wlprofile/models.py	2016-07-31 08:44:48 +0000
+++ wlprofile/models.py	2016-11-15 22:47:42 +0000
@@ -1,6 +1,7 @@
 from django.db import models
 from django.contrib.auth.models import User
-from fields import AutoOneToOneField, ExtendedImageField
+from fields import ExtendedImageField
+from wl_utils import AutoOneToOneField
 from django.utils.translation import ugettext_lazy as _
 from pybb.models import Post