fix django custom user mode + debug mode

[WillianPaiva]

with django in a custom user mode and debug mode the admin page was crashing so i just inserted a try except on validation avoiding the crashs on debug mode

[jschrewe]

Do you have a backtrace and a bit more explanation how you set it all up? While this works, the reason validate is there is because it's supposed to find problems faster. If you just catch the exception it might blow up later. And I would actually like to find the actual error instead of just masking it.

[Karmak23]

Hi, I'm working with Willian on the pull request.

Here is the stacktrace:

http://dev.1flow.net/development/1flow-dev-alternate/group/1177/

The problem happens because we use a custom user model, which is a standard feature of Django 1.5+.

If we don't bypass the validate() call in mongoadmin, then we need to patch Django in django/contrib/auth/admin.py, making the last line look like this:

if User._meta.swapped:
    admin.site.register(User, UserAdmin)

As patching Django is not an acceptable solution for us, we prefered patching mongoadmin.

We know that the solution is not perfect, but at least we still get the exception, without crashing the admin. It works perfectly, even with the patch we are proposing. But your advise is welcome :-)

regards,

[jschrewe]

Your user class is a standard Django model? Or coming from a mongoengine document? And do you still have problems if you don't use mongoadmin but only the standard admin? And I just looked into the docs for the custom user model and apparently you need to either register the old user admin with the new user model or both have to be adapted and registered under the the same name as the old one.

I'm probably not really telling you anything new here, just checking the obvious first.

And looking at the backtrace, what seems to happen is that mongoadmin identifies your user model (or some other model not really sure) as a django model and just passes it along to the standard Django admin validation. Which – for me – seems to point to a more general setup problem.

The function that blows up is:

def validate_fields_spec(cls, model, opts, flds, label):
    """
    Validate the fields specification in `flds` from a ModelAdmin subclass
    `cls` for the `model` model. `opts` is `model`'s Meta inner class.
    Use `label` for reporting problems to the user.

    The fields specification can be a ``fields`` option or a ``fields``
    sub-option from a ``fieldsets`` option component.
    """
    for fields in flds:
        # The entry in fields might be a tuple. If it is a standalone
        # field, make it into a tuple to make processing easier.
        if type(fields) != tuple:
            fields = (fields,)
        for field in fields:
            if field in cls.readonly_fields:
                # Stuff can be put in fields that isn't actually a
                # model field if it's in readonly_fields,
                # readonly_fields will handle the validation of such
                # things.
                continue
            check_formfield(cls, model, opts, label, field)
            try:
                f = opts.get_field(field)
            except models.FieldDoesNotExist:
                # If we can't find a field on the model that matches, it could be an
                # extra field on the form; nothing to check so move on to the next field.
                continue
            if isinstance(f, models.ManyToManyField) and not f.rel.through._meta.auto_created:
                raise ImproperlyConfigured("'%s.%s' "
                    "can't include the ManyToManyField field '%s' because "
                    "'%s' manually specifies a 'through' model." % (
                        cls.__name__, label, field, field))

More specifically this line:

if isinstance(f, models.ManyToManyField) and not f.rel.through._meta.auto_created:

And even more precise the through model seems to be missing. That is a model used for m2m relations and should either be auto created or defined by you (if you have extra data saved with the relationship).

In short: I don't think this is mongoadmin's fault, but would guess you really do have an issue with your setup. But of course I might be wrong.

[Karmak23]

Re,

in fact, the problem is triggered via (and only visible if) OVERRIDE_DJANGO_ADMIN=1: mongoadmin will loop onto 'django.contrib.auth' (which is legitimate) and load again its admin.py (after I've swapped out my User model). Django's admin code will in turn try to re-register (again) the "bare" Django User/UserAdmin models, which is not used (nor usable) anymore because it has been swapped.

I think the real bug is in the Django code: it should really register the UserAdmin class only if the Django user model is not swapped out. But given the number of pull requests they have and the time it takes to release a new version, I just don't wanna think about it. I was trying to workaround things in mongoadmin: having understood that the validate() occurs only if DEBUG, having it only LOG() instead of crash was enough for me (the crash doesn't occur in production where validate() is not called).

Besides this, I haven't any M2M keys in my User model:

class UserManager(BaseUserManager):
    """ This is a free adaptation of
        https://github.com/django/django/blob/master/django/contrib/auth/models.py  # NOQA
        as of 20130526. """

    def create_user(self, username, email, password=None, **extra_fields):
        """ Creates and saves a User with the given username,
            email and password. """

        now1 = now()

        if not email:
            raise ValueError('User must have an email')

        email = UserManager.normalize_email(email)

        user = self.model(username=username, email=email,
                          is_active=True, is_staff=False, is_superuser=False,
                          last_login=now1, date_joined=now1, **extra_fields)

        user.set_password(password)
        user.save(using=self._db)
        return user

    def create_superuser(self, username, email, password, **extra_fields):
        u = self.create_user(username, email, password, **extra_fields)
        u.is_staff = True
        u.is_active = True
        u.is_superuser = True
        u.save(using=self._db)
        return u

class User(AbstractBaseUser, PermissionsMixin, AbstractUserProfile):
    """ Username, password and email are required.
        Other fields are optional. """

    #NOTE: AbstractBaseUser brings `password` and `last_login` fields.

    username = models.CharField(_('User name'), max_length=254,
                                unique=True, db_index=True,
                                help_text=_('Required. letters, digits, '
                                            'and "@+-_".'))
    email = models.EmailField(_('email address'),  max_length=254,
                              unique=True, db_index=True,
                              help_text=_('Any valid email address.'))
    first_name = models.CharField(_('first name'), max_length=64, blank=True)
    last_name = models.CharField(_('last name'), max_length=64, blank=True)
    is_staff = models.BooleanField(_('staff status'), default=False,
                                   help_text=_('Designates whether the user '
                                               'can log into this admin '
                                               'site.'))
    is_active = models.BooleanField(_('active'), default=True,
                                    help_text=_('Designates whether this user '
                                                'should be treated as '
                                                'active. Unselect this instead '
                                                'of deleting accounts.'))
    date_joined = models.DateTimeField(_('date joined'), default=now)

    objects = UserManager()

    USERNAME_FIELD = 'username'
    REQUIRED_FIELDS = ('email', )

    class Meta:
        verbose_name = _('user')
        verbose_name_plural = _('users')

    def get_absolute_url(self):
        return _("/users/{username}/").format(urlquote(self.username))

    def get_full_name(self):
        """ Returns the first_name plus the last_name, with a space in between.
        """
        full_name = '%s %s' % (self.first_name, self.last_name)
        return full_name.strip()

    def get_short_name(self):
        "Returns the short name for the user."
        return self.username

Nor in the abstract profile model:

class AbstractUserProfile(models.Model):
    """ A mixin for any User class (even not real Django `User`)
        which adds primitives to get/set if a given email was sent
        to the user, and various other methods based on profile data.

        It's understood that the given user model which will use
        this mixin should either have a `.data` attribute of type
        ``JSONField``, or a `.profile.data` (JSONField too) attribute.

        Using this class allow many User classes to work in a similar
        way, be they having an dedicated profile, or not.
    """
    email_announcements = models.BooleanField(_('Email announcements'),
                                              default=True, blank=True)
    last_modified = models.DateTimeField(_('Last modified'), auto_now_add=True)

    register_data = JSONField(_('Register data, as JSON'),
                              default=lambda: {}, blank=True)
    hash_codes    = JSONField(_(u'Validation codes, as JSON'),
                              default=lambda: {'unsubscribe': uuid.uuid4().hex},
                              blank=True)
    sent_emails   = JSONField(_('sent emails names, as JSON'),
                              default=lambda: {}, blank=True)
    data          = JSONField(_('Other user data, as JSON'),
                              default=lambda: {}, blank=True)

    class Meta:
        abstract = True

    def email_user(self, subject, message, from_email=None):
        """ Sends an email to this User, [TODO: if not already done ?]. """

        send_mail(subject, message, from_email, [self.email])

    def has_email_sent(self, email_name):
        return self.sent_emails.get('email_sent_' + email_name, False)

    def log_email_sent(self, email_name):
        return self.sent_emails.setdefault('email_sent_' + email_name, True)

    def renew_hash_code(self, name, commit=True):
        self.hash_codes[name] = uuid.uuid4().hex
        if commit:
            self.save(update_fields=('hash_codes', ))

    def unsubscribe_url(self):
        return u'http://{0}{1}'.format(
            settings.SITE_DOMAIN, reverse('unsubscribe', kwargs={
                'email': base64.b64encode(self.email),
                'hash_code': self.hash_codes.get('unsubscribe')
            }))

I don't understand at all why this M2M test succeeds…

Regards, and thanks for your prompt answer !

[jschrewe]

Well at the moment mongoadmin doesn't even know that swappedexists (which isn't really surprising because it's based on Django 1.3 code). So I guess that is where the error comes from. It should be easy to fix by adding if not model._meta.swapped: before https://github.com/jschrewe/django-mongoadmin/blob/master/mongoadmin/sites.py#L91 Here's how Django does it: https://github.com/django/django/blob/1.5.1/django/contrib/admin/sites.py#L52

I'll try to do a proper fix today, just want to actually test a bit and check out where and how the swapped flag is set.

The PermissionsMixin uses m2m relations btw, though I doubt they're the error cause.

Thanks for the model definitions. They should help getting something to test faster.

[jschrewe]

It seems it was the missing check to see if the model was swapped. I can run the admin just fine with your definitions now. I'd appreciate it if you could also test it though. You need mongoadmin and mongodbforms from github for that. Let me know if there are more problems.

[Karmak23]

Nice ! We will test this tomorrow and will give you some feedback asap.

I didn't think about the PermissionsMixin… But yes it has some m2m obviously.

BTW I didn't tell you but PermissionMixin and AbstractBaseUser are coming verbatim from Django, we didn't modify them.

thanks for your fast answers & time,

[Karmak23]

Back for reporting: with latest mongodbforms/mongoadmin code, the admin "works", but:

• in our app which contains the user model, other models don't show (only user is visible), whereas with previous version of your code, all models were there.
• the user admin crashes (http://dev.1flow.net/development/1flow-dev/group/3341/). our user admin class:

import csv

from django.conf import settings
from django.http import HttpResponse
from django.template.defaultfilters import slugify
from django.contrib.admin.util import flatten_fieldsets
from django.contrib.auth.admin import UserAdmin
from django.utils.translation import ugettext_lazy as _
from django.contrib.auth.models import User as DjangoUser

from django.contrib import admin as django_admin
import mongoadmin as admin

from .models import EmailContent, User

from sparks.django.admin import languages, truncate_field

# [unused code removed…]

class CSVAdminMixin(django_admin.ModelAdmin):
    """
    Adds a CSV export action to an admin view.
    cf. http://djangosnippets.org/snippets/2908/
    """

    # This is the maximum number of records that will be written.
    # Exporting massive numbers of records should be done asynchronously.
    csv_record_limit = 10000
    extra_csv_fields = ()

    def get_actions(self, request):
        actions = self.actions if hasattr(self, 'actions') else []

        if request.user.is_superuser:
            actions.append('csv_export')

        actions = super(CSVAdminMixin, self).get_actions(request)
        return actions

    def get_extra_csv_fields(self, request):
        return self.extra_csv_fields

    def csv_export(self, request, qs=None, *args, **kwargs):
        response = HttpResponse(mimetype='text/csv')

        response['Content-Disposition'] = "attachment; filename={}.csv".format(
            slugify(self.model.__name__)
        )

        headers = list(self.list_display) + list(
            self.get_extra_csv_fields(request)
        )

        # BOM (Excel needs it to open UTF-8 file properly)
        response.write(u'\ufeff'.encode('utf8'))
        writer = csv.DictWriter(response, headers)

        # Write header.
        header_data = {}
        for name in headers:
            if hasattr(self, name) \
                    and hasattr(getattr(self, name), 'short_description'):
                header_data[name] = getattr(
                    getattr(self, name), 'short_description')

            else:
                field = self.model._meta.get_field_by_name(name)

                if field and field[0].verbose_name:
                    header_data[name] = field[0].verbose_name

                else:
                    header_data[name] = name

            header_data[name] = header_data[name].encode('utf-8', 'ignore')

        writer.writerow(header_data)

        # Write records.
        for row in qs[:self.csv_record_limit]:
            data = {}
            for name in headers:
                if hasattr(row, name):
                    data[name] = getattr(row, name)
                elif hasattr(self, name):
                    data[name] = getattr(self, name)(row)
                else:
                    raise Exception('Unknown field: %s' % (name,))

                if callable(data[name]):
                    data[name] = data[name]()

                if isinstance(data[name], unicode):
                    data[name] = data[name].encode('utf-8', 'ignore')
                else:
                    data[name] = unicode(data[name]).encode('utf-8', 'ignore')

            writer.writerow(data)

        return response

    csv_export.short_description = _(
        u'Export selected %(verbose_name_plural)s to CSV'
    )

class OneFlowUserAdmin(UserAdmin, CSVAdminMixin):

    list_display = ('id', 'username', 'email', 'full_name_display',
                    'date_joined', 'last_login',
                    'email_announcements',
                    'is_active', 'is_staff', 'is_superuser',
                    'groups_display', )
    list_display_links = ('username', 'email', 'full_name_display', )
    list_filter = ('email_announcements',
                   'is_active', 'is_staff', 'is_superuser', )
    ordering = ('-date_joined',)
    date_hierarchy = 'date_joined'
    search_fields = ('email', 'first_name', 'last_name', )
    change_list_template = "admin/change_list_filter_sidebar.html"
    change_list_filter_template = "admin/filter_listing.html"
    # Don't display the UserProfile inline, this will conflict with the
    # post_save() signal in profiles.models. There is a race condition…
    #inlines = [] if UserProfile is None else [UserProfileInline, ]

    def groups_display(self, obj):
        return u', '.join([g.name for g in obj.groups.all()]
                          ) if obj.groups.count() else u'—'

    groups_display.short_description = _(u'Groups')

    def full_name_display(self, obj):
        return obj.get_full_name()

    full_name_display.short_description = _(u'Full name')

admin.site.register(User, OneFlowUserAdmin)
admin.site.unregister(DjangoUser)

So for now we are sticking back to the previous version of code ;-)

regards,

[jschrewe]

Hrmpf, it would have been to good to be true if everything just worked...

[jschrewe]

I can't reproduce any of the described errors. Sorry.

If I add documents to the models file where the users are stored and add an admin for them, they show up as usual in the admin under the app label.

This line doesn't work for me however: admin.site.unregister(DjangoUser). And it shouldn't be necessary because the admin sorts that out itself.

And the crash does seem really, really weird. Because I have no idea how or why in this line actions = super(CSVAdminMixin, self).get_actions(request) CSVAdminMixin could be None (and that is the only place where something could expect something of type type).

Oh, not sure if you do any magic here: change_list_template = "admin/change_list_filter_sidebar.html" but I didn't have that template and didn't have from sparks.django.admin import languages, truncate_field.

[Karmak23]

Ah sorry.

The change_list_template comes from latest grappelli, and latest version of my sparks is on github.

DIsplaying hidden frames on the report show you were right, it's about actions = super(CSVAdminMixin, self).get_actions(request).

I will try/except the line asap and will keep you updated.

[Karmak23]

Just to let you know, your latest versions of mongoadmin/mongodbforms fix our initial problem for which this pull request was submitted. As far as we are concerned, you can thus ignore/close the current pull request which is not needed anymore.

The CSVAdminMixin related crash was a problem in our code and I fixed it.

Many thanks for your updates and followups on this particular issue.

best regards,

[jschrewe]

Awesome! Thanks for finding it.