[Looking for feedback] Make Admin raw_id_fields the default choice for FKs, or project-wide configurable

[Santiago Basulto]

Hey folks, I was about to submit a ticket but i thought it might be better to ask everybody for opinions on the matter first. I am running a couple of medium (not even large) Django websites (around +20K users) and we rely on the admin heavily. We have multiple models pointing to Users (or other derived models) and every time we create a new model, we MUST remember to include User in `raw_id_fields` for that model. If we forget to do so, the whole testing site crashes when the whole `SELECT * FROM User` query is run.

To add to the problem, some derived models (for example Customer) include in their `__str__` both the User + something else. Think about the model Customer in this way:

class Customer(models.Model):
    user = models.ForeignKey(User)
    plan = models.ForeignKey(Plan)


    def __str__(self):
        return f"{self.user} - {self.plan}"

(Not a real example, but to make the point)

Imagine any other model with an FK to Customer, an `Inquiry`, for example. If you open the `Inquiry` add/change page on the admin, the whole thing will blow up.

I know the related select field is an amazing feature, and looks slick on the admin when starting a new Django projects (specially for beginners), but it just doesn't scale for large websites.

My proposal

I think just a project-wide setting `settings.ADMIN_DEFAULT_FK_WIDGET = [raw|select]` would work and help us (running a medium/large site) a lot.

What do you think? 

Thanks for all the support, this community rocks 🤘!

PS: I can create a ticket if that's a better medium of discussion, just let me know?

[Santiago Basulto]

Btw, for reference, not the only one with this problem:

* https://twitter.com/mbrochh/status/1049209871583797248

* https://twitter.com/poswald/status/1072134086041300992

[Harro]

How would the raw_id_fields then work? Would that then turn them into selects again or would that be another setting?

If you want it in your whole project you could just extend the ModelAdmin and make the raw_id_fields a property that returns all the fields then use that as a base class for all your ModelAdmins, 

looking at the code the check if it is a foreignkey happens before that in all cases, so having non foreignkey fields in the list shouldn't matter.

[shiva kumar]

actually i am new to django i am not able to understand the problem which ur dealing with?

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/5dae8b31-356c-45f3-b707-83b5ef33f396%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Carlton Gibson]

This topic has come up before. 

e.g. https://groups.google.com/d/topic/django-developers/md39xtT_JEM/discussion 

(In which Aymeric suggests something similar to Harro's suggestion here.) 

[ludovic coues]

Django's admin default widget for foreign keys is a select. If you try to populate a select elements with 20,000 options, your page take at least a few seconds to load. Probably a minute or two, if it load at all.

By configuring your model admin to display the foreign key as a raw_id_fields, you get an simple input instead of a select. It's less nice to use but it's much faster.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAMsYeuFUNE3W8Cc2RGwn7L-LZ%2BhrNy9NPxiYVx_ZH7H4kUzR0Q%40mail.gmail.com.

[Collin Anderson]

I agree that default load-all-options is an annoying default. I just ran into this problem again myself in the last few weeks. One problem with any of the alternatives (besides making it readonly by default) is that it requires the other model to be registered in the admin, which could be also be annoying for some people.

I hope there's _something_ we can do to somehow improve the situation. Maybe we could at least improve the examples in the documentation? Maybe give an example in the docs of a ModelAdmin subclass that defaults to using raw_id?

--

You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/2ba44b9f-6526-4a6a-8311-632e83aeb4ba%40googlegroups.com.

[Carlton Gibson]

On Thursday, 17 January 2019 16:14:31 UTC+1, Collin Anderson wrote:

One problem with any of the alternatives (besides making it readonly by default) is that it requires the other model to be registered in the admin

Off-hand I don't follow you here. Can you explain. 

 

I hope there's _something_ we can do to somehow improve the situation. Maybe we could at least improve the examples in the documentation? Maybe give an example in the docs of a ModelAdmin subclass that defaults to using raw_id?

An example definitely.

Maybe we could add an attribute to ModelAdmin with a number: More than this use raw_id — but what would that look like? 

(Easy subclass rather than a setting...)

[Santiago Basulto]

I think the proposed solution of "you can just extend/subclass ModelAdmin" doesn't work, because the fields on different models can have different names. I can't just write one global ModelAdmin and then use it for all my models, because they'll have different names for their fields. Or if it works, it'll need A LOT of introspection (to dynamically check which fields are FKs and making them part of raw_id_fields).

Maybe I'm wrong and I'm missing the point, do you folks have an implementation of that ModelAdmin superclass to show?

[Santiago Basulto]

Ok, sorry for the Fake News, seems like it's not so complicated to make one ModelAdmin parent class that provides this behavior. Here's a working example:

from django.contrib import admin

from django.contrib.admin import widgets


class RawFieldModelAdmin(admin.ModelAdmin):
    def formfield_for_foreignkey(self, db_field, request, **kwargs):
        db = kwargs.get('using')
        if 'widget' not in kwargs:
            if db_field.name not in (self.get_autocomplete_fields(request), self.radio_fields):
                kwargs['widget'] = widgets.ForeignKeyRawIdWidget(db_field.remote_field, self.admin_site, using=db)


        return super().formfield_for_foreignkey(db_field, request, **kwargs)

Do you folks think we should add this to the docs? I still think that having a one-off setting for the "default foreign key" widget would be valuable, at least for me as a user.

What do you think?

[Josh Smeaton]

If we were happy with that particular implementation, then I'd prefer adding it as an official subclass thats importable for users rather than just dumping the code to the docs. But I guess the issue is a slippery slope - how many subclasses do we add for various ModelAdmin use cases. It's definitely an issue that bites many people, and I'd like to see some way forward.

[Harro]

The problem is that you can't just use it everywhere, like mentioned earlier it only works if the other side of the relation is also available in the admin (which might not be the case, or only for some of the fields.)

I would say add it to the good old djangosnippets website. 

But that's just my 2 cents.

[Collin Anderson]

> > One problem with any of the alternatives (besides making it readonly by default) is that it requires the other model to be registered in the admin

> Off-hand I don't follow you here. Can you explain. 

I stand corrected. raw_id_fields doesn't actually require the other model to be registered.

--

You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/f888916c-78b9-4a06-aa83-c880a114d034%40googlegroups.com.

[Carlton Gibson]

On 18 Jan 2019, at 17:20, Santiago Basulto <santiago...@gmail.com> wrote:

Seems like everybody agrees that for large sites, it's necessary.

Hang on, slow down. 🙂

Personally, I’m not sure it’s too onerous as-is. I’ve not yet seen exactly why subclassing ModelAdmin in one’s project isn’t good enough. It really depends on with the “it” is “it’s necessary” is taken to mean…

- Better examples in docs: Great, no problem. 

- **More** API on ModelAdmin: OK, maybe, but what does that look like exactly? 

- A setting: Really? Do we have to? Can we explore other options first?

Both the docs and adjustments to ModelAdmin options could be pushed forward with a PR. “Better handling of FK widgets for fields with large counts” — sounds great, but what does that involve?

Kind Regards,

Carlton

[Santiago Basulto]

Haha, sorry, got overly excited. What's the usual process to make these sort of decisions in these cases? Should we ask the users list?

The setting proposal is purely from my perspective as a user. For *my* (completely unilateral) point of view, this is very important and I'd love to have a place to just flip a switch and change them all; well, I'd even prefer for it to be the default behavior, but I know selects look cool too.

Now, from a "development" perspective, I do understand that it might be a big change.

[Adam Johnson]

For *my* (completely unilateral) point of view, this is very important and I'd love to have a place to just flip a switch and change them all; well, I'd even prefer for it to be the default behavior, but I know selects look cool too.

How many projects do you need this on? I think the example snippet is good enough for most cases, you could always wrap it up and put it on PyPI if you find yourself doing it across many projects (and maybe there's other stuff you do too)

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/e18c8811-35ae-4607-b9bb-7bb826e8389c%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Adam

[Carlton Gibson]

On 19 Jan 2019, at 11:48, Santiago Basulto <santiago...@gmail.com> wrote:

Haha, sorry, got overly excited. What's the usual process to make these sort of decisions in these cases? Should we ask the users list?

No problem. :)

We make decisions here. Say you’d have opened a Trac ticket for this, I’d have probably said `needsinfo`. So same as I’m saying here, we need to pin down what’s being proposed. 

The setting proposal is purely from my perspective as a user. For *my* (completely unilateral) point of view, this is very important and I'd love to have a place to just flip a switch and change them all; well, I'd even prefer for it to be the default behavior, but I know selects look cool too.

Now, from a "development" perspective, I do understand that it might be a big change.

Adding a setting may not be any harder than creating a subclass: the underlying implementation may look virtually identical. But in that case it’d be likely we’d favour the subclass, since a setting is a bit like a sledge-hammer — it’s probably overkill when you could just change an import and a superclass declaration. 

But, again, what does it look like? Is it better than (i.e. simple enough) providing a better example and letting people implement their own? 

FWIW, I think the default select is right for the vast majority of cases and is more useable by an order of magnitude. For me, it’s never really been an issue declaring raw_id_fields. (But yes, it does come up, so very happy to see suggestions.)

As Adam says, something like this could be put on PyPI, even if not in Django itself. Is there not something already I wonder? 

Don’t be discouraged. Please pursue the thought if you want it: implement something, open a PR, let’s discuss! 

Kind Regards,

Carlton