1

Summary

If we filter the queryset for a choice field on a Django admin form, it seems we have to be very careful not to exclude existing values for that field (on the model instance associated with the form).

If we do exclude them, the existing values will be considered invalid, which can lead to nasty surprises later on.

Is there a generic way to prevent this from happening?

More detail can be found below.

Background

The desire to restrict the available choices for (the representation of) a ForeignKey or ManyToManyField on a Django admin form appears to be fairly common:

Judging from the discussions above, there are several different ways to restrict these choices, but most of them boil down to filtering the queryset for a ModelChoiceField (or ModelMultipleChoiceField) on the admin form.

General problem

Here is what the docs have to say about the ModelChoiceField.queryset:

A QuerySet of model objects from which the choices for the field are derived and which is used to validate the user’s selection. It’s evaluated when the form is rendered.

If you think about this, it is actually quite easy to shoot yourself in the foot with a filter, without even being aware of it at first.

For example, if the current value for a ForeignKey field (or ManyToManyField) on an existing model instance is accidentally excluded from the queryset, that would invalidate the current value.

Depending on what you want to achieve, this could be desired behavior, but it could also lead to nasty surprises.

Example

To illustrate this, please consider the following example of a filtered queryset, straight from the Django documentation for ModelAdmin.formfield_for_foreignkey():

class MyModelAdmin(admin.ModelAdmin):
    def formfield_for_foreignkey(self, db_field, request, **kwargs):
        if db_field.name == "car":
            kwargs["queryset"] = Car.objects.filter(owner=request.user)
        return super().formfield_for_foreignkey(db_field, request, **kwargs)

Based on this example, let us assume this is part of my_app with the following models:

class Car(models.Model):
    owner = models.ForeignKey(to=settings.AUTH_USER_MODEL, null=True, blank=True, 
        on_delete=models.CASCADE)


class MyModel(models.Model):
    car = models.ForeignKey(to=Car, null=True, blank=True, on_delete=models.CASCADE)

If you play around with this on the admin site, everything seems to work well, at first.

However, what happens, for example, if there are two (staff) users who have access to the same instance of MyModel?

Scenario

Suppose we have user_a who owns car_x, and user_b who owns car_y.

First, user_a creates a MyModel instance (viz. my_model) via the admin site, and sets my_model.car = car_x.

Next, user_b wants to change that same my_model via the admin site.

Due to our queryset filter, the current value for my_model.car, i.e. car_x, is not valid for user_b, so they can only choose from the empty value (---) or car_y.

Moreover, user_b will not even be aware that my_model.car had a different value, originally.

As a result, the value of my_model.car will be changed after user_b saves the form, without anyone being aware of it.

Disclaimer: I am aware that this specific scenario could be fixed easily, e.g. by preventing different users from accessing the same MyModel instance. However, the specific scenario is not important here. The point is, there are infinitely many ways to accidentally exclude existing values from the formfield queryset, and in many cases the admin user will not be aware when this happens.

A similar problem arises if your queryset filter excludes Car instances that are newly created via the add another button on the MyModel admin page. However, in that case, the user would at least see the validation error.

Questions

It seems to me that many people must have encountered this type of issue, but I haven't been able to find any similar questions on SO.

This makes me wonder: Am I approaching this the wrong way? Is it a design flaw on my part, if I run into this issue?

If not, is there a generic way to protect against this kind of issue?

djvg
  • 11,722
  • 5
  • 72
  • 103

1 Answers1

0

Hoping for better answers, here's my own attempt:

There is one way to work around this, that I know of, but I'm not sure if this would be the best approach.

Basically, if the problem is that the existing value might be excluded from the queryset, then what we need to do is make sure any existing values are explicitly included.

Here are two (similar) implementations, based on the example in the original post (tested in Django 2.2):

Implementation A uses a slightly customized ModelForm, which provides a natural way to get access to the current MyModel instance, but this breaks up the code:

class MyModelForm(ModelForm):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        # add the current car to the queryset (or empty queryset)
        self.fields["car"].queryset |= Car.objects.filter(
            id=self.instance.car_id)


class MyModelAdmin(admin.ModelAdmin):
    form = MyModelForm
    
    def formfield_for_foreignkey(self, db_field, request, **kwargs):
        if db_field.name == "car":
            kwargs["queryset"] = Car.objects.filter(owner=request.user)
        return super().formfield_for_foreignkey(db_field, request, **kwargs)

Implementation B uses a slightly hacky way to get access to the current MyModel instance from within formfield_for_foreignkey(), but keeps everything in one place:

class MyModelAdmin(admin.ModelAdmin):
    def formfield_for_foreignkey(self, db_field, request, **kwargs):
        if db_field.name == "car":
            # get the current value for the car field (if any)
            current_car_id = None
            current_my_model_id = request.resolver_match.kwargs.get('object_id')
            if current_my_model_id:
                my_model = MyModel.objects.get(id=current_my_model_id)
                current_car_id = my_model.car.id
            # combine the querysets
            qs_current_car = Car.objects.filter(id=current_car_id)
            qs_user_cars = Car.objects.filter(owner=request.user)
            kwargs['queryset'] = qs_user_cars | qs_current_car
        return super().formfield_for_foreignkey(db_field, request, **kwargs)

Perhaps we could also do all the queryset filtering inside MyModelForm.__init__(), but then we would need to hack the request into __init__(), e.g. as described here.

djvg
  • 11,722
  • 5
  • 72
  • 103