jschrewe / django-mongoadmin

Integrates mongodb into django's admin
http://www.schafproductions.com/projects/mongo-admin/
BSD 3-Clause "New" or "Revised" License
112 stars 38 forks source link

`raw_id_fields` crashing #22

Closed Karmak23 closed 10 years ago

Karmak23 commented 11 years ago

Hi,

I tried to use raw_id_fields in the admin classes, but it failed (see the stacktrace).

Reading your code (in validation.py), what I don't understand is that raw_id_fields is tested against Django's ManyToMayField and ForeignKey, which will obviously fail because in mongoengine we don't have these fields.

Is it just a matter of replacing them with ReferenceField and GenericReferenceField or is it much more complicated than that ?

regards,

jschrewe commented 11 years ago

To be honest the code you saw is most likely still original Django code, maybe with some small adjustments (there is quite a bit of that still around). And I admit I've never even heard of raw_id_fields. What does that do?

But my guess is that just replacing the field names is not enough. At least not for ManyToManyRelations. The closest mongoengine has to them are ListField(ReferenceFields)) and they rather suck if you try to mirror Django functionality to them.

I need to take a closer look at this but it seems that the formfields for raw_id_fields is done in formfield_for_manytomany (see here) and that Django uses a special widget for them.

And just so I know: Are you trying to use them with the standard mongoengine ids? Or do you have something like an IntFielddeclared as primary key?

Karmak23 commented 11 years ago

raw_id_fields allows to search in foreign keys without loading the related objects (see there). In my own case, I have a database with 3 millions article (and growing fast), each of them can be marked as a duplicate of another. Without raw_id_fields, loading the change view of 1 article makes Django process the 3M others to display them in a list, which makes at best the browser crash, at worst nginx timeout because django never responds. The custom widget is the point: it's smart and fast for this kind of big data.

Any ForeignKey/ManyToMany (or MongoDB equivalent) with large data in the base really needs this to make the django admin staying correctly usable.

For now, I'm trying to use them only with ReferenceField, which is the MongoEngine equivalent of Django's ForeignKey.

I will perhaps take time to see if I can hack the code to make it barely work, but currently I'm so overloaded with other tasks that I don't know when at all…

jschrewe commented 11 years ago

Nah, it should be fine if you give me some documents and maybe an admin class to play with.

Just so I get the idea correctly and sorry for the questions. I'm a bit busy too. What is supposed to happen if you click on the magnifying glass?

jschrewe commented 11 years ago

So, after looking a bit into this: Do you need raw_id_fields to work with GenericReferenceFields? Because at the moment I don't see how that could be made to work, but maybe I'm just missing something.

Karmak23 commented 11 years ago

No, GenericRefereceField is not worth the investigation I think. I use it in some places, but tend to replace it with RerefenceField, which allows implementing reverse_delete_rule and seems much more robust.

For as much as I remember, in a standard Django admin, cliking on the glass brings up a popup allowing to search by id or by name (don't remember how to specify the fields to search onto). In theory, it should work for listfield too (this is the ManyToMany equivalent). Currently, the listfield works correctly with the filter_horizontal django admin option. But again, this is only valid on my development machine where I have a small subset of tags. In production with 300k+ of them in many languages, this is unusable.

For now, I have replaced raw_id_fields with exclude, which obviously doesn't do the job at all ;-) But at least I can "see" the documents, even if incomplete, and can edit the non-referencefield-related parts.

If you manage to make it work with ReferenceField, i will owe you a beer. Perhaps more a pack of beers ;-) Tell me if you have a wishlist on amazon.

My code for Document:


import os
import re
import ast
import sys
import uuid
import errno
import logging
import requests
import strainer
import html2text
import feedparser

from statsd import statsd
from random import randint, randrange
from constance import config

from bs4 import BeautifulSoup
#from xml.sax import SAXParseException

from celery import task, chain as tasks_chain

from celery.exceptions import SoftTimeLimitExceeded

from pymongo.errors import DuplicateKeyError

from mongoengine import (Document, EmbeddedDocument,
                         ValidationError, NULLIFY, PULL, CASCADE)
from mongoengine.errors import NotUniqueError
from mongoengine.fields import (IntField, FloatField, BooleanField,
                                DateTimeField,
                                ListField, StringField,
                                URLField,
                                ReferenceField, GenericReferenceField,
                                EmbeddedDocumentField)

from django.conf import settings
from django.utils.translation import ugettext_lazy as _
from django.utils.text import slugify
from django.contrib.auth import get_user_model

@task(name='Tag.replace_duplicate_everywhere')
def tag_replace_duplicate_everywhere(tag_id, dupe_id, *args, **kwargs):

    tag  = Tag.objects.get(id=tag_id)
    dupe = Tag.objects.get(id=dupe_id)
    return tag.replace_duplicate_everywhere(dupe, *args, **kwargs)

class Tag(Document, DocumentHelperMixin):
    name     = StringField(verbose_name=_(u'name'), unique=True)
    slug     = StringField(verbose_name=_(u'slug'))
    language = StringField(verbose_name=_(u'language'), default='')
    parents  = ListField(ReferenceField('self',
                         reverse_delete_rule=PULL), default=list)
    children = ListField(ReferenceField('self',
                         reverse_delete_rule=PULL), default=list)
    duplicate_of = ReferenceField('Tag', reverse_delete_rule=NULLIFY,
                                  verbose_name=_(u'Duplicate of'),
                                  help_text=_(u'Put a "master" tag here to '
                                              u'help avoiding too much '
                                              u'different tags (eg. singular '
                                              u'and plurals) with the same '
                                              u'meaning and loss of '
                                              u'information.'))

    meta = {
        'indexes': ['name', ]
    }

And the admin class:


import dateutil.parser

from humanize.i18n import django_language

from constance import config

from django.conf import settings

from django.utils.translation import ugettext_lazy as _
from django.contrib.auth import get_user_model
from django.contrib.auth.admin import UserAdmin
from django.core.urlresolvers import reverse

from .models.nonrel import Tag, Feed, Article, CONTENT_TYPE_MARKDOWN
from .models.reldb import HelpContent

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

from sparks.django.admin import languages, truncate_field

class TagAdmin(admin.DocumentAdmin):

    list_display = ('id', 'name', 'language', 'duplicate_of',
                    'parents_display', 'children_display')
    list_display_links = ('id', 'name', )
    #list_editable = ('language', )
    search_fields = ('name', 'slug', )
    change_list_filter_template = "admin/filter_listing.html"
    filter_horizontal = ('parents', 'children', )

    def change_view(self, request, object_id, extra_context=None):
        self.exclude = ('origin', )  # 'parents', 'children', )

        return super(TagAdmin, self).change_view(request, object_id,
                                                 extra_context=None)

    def parents_display(self, obj):

        if obj.parents:
            return u', '.join(u'<a href="/admin/models/tag/{0}" '
                              u'target="_blank">{1}</a>'.format(
                                  parent.id, parent.name)
                              for parent in obj.parents)

        return u'—'

    parents_display.short_description = _(u'Parents')
    parents_display.allow_tags = True

    def children_display(self, obj):

        if obj.children:
            return u', '.join(u'<a href="/admin/models/tag/{0}" '
                              u'target="_blank">{1}</a>'.format(
                                  child.id, child.name)
                              for child in obj.children)

        return u'—'

    children_display.short_description = _(u'Children')
    children_display.allow_tags = True

Sorry for the massive imports, I didn't mind cleaning them and just wanted you not to miss anything. In case you don't have sparks and ever need it, it's here.

jschrewe commented 11 years ago

It works now for me with ReferenceFields and a very limited amount of data. ListField(ReferenceField()) should follow soon and be quite straight forward now.

Let me know if there are any problems. As far as I can tell the page that appears after you click on the glass is the normal list display.

jschrewe commented 11 years ago

Oh, you need the newest mongodbforms too.

jschrewe commented 11 years ago

So, that should make ListField(ReferenceFields()) work too.

As (almost) always you need to update mongodbforms to the newest commit for this to work. I'm leaving the issue open in case there are some issues with more documents or the list index.

Oh, and I have a small and eclectic wishlist: http://www.amazon.de/registry/wishlist/F0V6T5HPY15G and not much updated after I bought a Kindle...

Karmak23 commented 11 years ago

Thanks for the updates and patches.

I will test your code as soon as time permits it.

BTW, as #21 is still a problem for us, I will probably have to merge patch manually to test (I'm currently running at commit 2d3d32a9e666d14f0b2fd0a8178d4bd75e084b14 because the User Model related one makes mongoadmin don't work in our setup).

OMG, it's been time since I didn't speak german. Hope I made it correctly on amazon.de and that you will receive the gift ;-)

regards,

jschrewe commented 11 years ago

There was a small change for #21 in fdcedbd66b8120c2081b5565808c60c06c652ebd

But I guess the best way to avoid any of the issues you have is to make the test completely optional with a setting. I'll hopefully get around to that tonight.

The international support from Amazon is sadly horrible, but cheers for the gift regardless. I'm sure it'll work fine.

Karmak23 commented 11 years ago

I tested your code on a simple ReferenceField() and it doesn't crash anymore.

But clicking on the magnifier just bring me back to the model changelist. Could it be I have something wrong with Django's static settings and I miss some JS files ?

BTW, using it on a more complex model gives me this stacktrace, eg. NameError: global name 'document' is not defined in mongodbforms/documentoptions.py line 33 (don't forget to click on “Show 54 hidden frames” to get the last interesting one; I always forget…). I tried to replace document with self and got this crash. Didn't have time to investigate further.

regards,

jschrewe commented 11 years ago

Yes, well the crash is one of the late night copy & paste oversights. One more point for learning how to write tests I guess. It should work with the newest commit to mongodforms though.

jschrewe commented 11 years ago

Oh, the changelist. Yes, this seems to be what Django does there. That part simply uses the Django code via inheritance. For single references (aka ReferenceFields()) Django adds a search filter on the reverse field. Which is what you were hoping for?

That is one of the annoying parts of not having access to the reverse of relations, I can either a lot of pseudo smart code to figure them out at run time or more information must be provided on the admin.

Karmak23 commented 11 years ago

With the latest mongodbforms it doesn't crash anymore, thanks.

But a raw_id_fields on a ListField(ReferenceField()) doesn't create the empty input if I already have one in the database. Eg. it doesn't replicate the documented behavior which works for ListField(StringField()), which consists of always having one more empty input widget.

Any hint on the magnifying glass problem ?

regards,

jschrewe commented 11 years ago

"Eg. it doesn't replicate the documented behavior which works for ListField(StringField()), which consists of always having one more empty input widget."

Oh. That is harder. Django uses by default just a comma separated list (and the widgets handles the transformation to a list). That works just fine and you can click in the changelist view on as many objects as you want and get a lot of object ids separated by commas. Some CSS to make the input larger was left as an exercise to the reader ;)

I could probably adjust the widget mongodbforms uses for ListField(ReferenceFields()) the main problem with that is that it still isn't dynamic and would allow to only add one reference per edit.

jschrewe commented 11 years ago

screen shot 2013-09-14 at 12 21 52 am

So this is what I get when I use raw_id_fields with absolutely no other options to a standard relational Django model and click on the glass. Which looks pretty much like what you get with mongoadmin.

If you have a screenshot of what you expect or a bit more information I'm happy to look into it a bit more.

Karmak23 commented 11 years ago

In fact, at least what you describe would be perfectly OK to me.

I don't mind if the widget uses only IDs separated by commas and handles the split before saving to MongoEngine as a list. If it's the way it works, no problem.

If you got it to work with mongoengine, then I must have a problem with my javascript. Currently, when I click on the magnifier, no popup shows. I'm brought back to the model's changelist and I loose whatever data I've edited in the individual change view of the instance I was editing.

jschrewe commented 11 years ago

Okay, the fast fix seems to be to turn off grappelli and everything works as intended.

I just tried it with grappelli and run into the same problem. Not sure yet why, but I'll hopefully find out soon.

jschrewe commented 11 years ago

Or maybe I just didn't set up grappelli the right way. To check if the javascript loading is the problem: STATIC + admin/js/admin/RelatedObjectLookups.js must be loaded.

If the asset loading works I can't really find any errors.

Karmak23 commented 11 years ago

OK I will check grappelli, thanks for this hint.

Perhaps I have another problem on this side because we have django-compressor and django.js in our settings, to pre-cache/pre-compile/pre-{whatever} every JS/coffee and SASS/CSS file. Setups are different on my dev machine and in production.

Short, this makes the whole CSS/JS setup rather complicated. I'll tell you.

jschrewe commented 11 years ago

I've never used django.js, but afaik django-cpmpressor does nothing without a templatetag, which isn't in the admin templates.

I also made the inputs for the raw_id_fields widget larger yesterday, although with a style attribute on the input tag. Not sure if that is a great idea, but at least the widget is useable.

Oh, and the crappy test for #21 now looks at the database setting and only does anything if the db engine is the dummy backend. That should solve the syncdb problems.

Karmak23 commented 10 years ago

Sorry to not have updated this issue for too long. I still have problems with mongoadmin in the django admin, and raw_id_fields still doesn't work, though I think this particular issue is not related to your code.

I'm planning to switch back to PostgreSQL, there are too many underlying problems with MongoDB (not related to mongoadmin, in fact).