Admin list_editable failed to edit

[Djing]

1. I have two Manager for my Model, one shows only valid items and another one shows all items, and then I change the model's default manager to the previous one which shows only valid items.
2. I inherit the ModelAdmin and rewrite get_queryset method to make the queryset use the manager which shows all items.
3. Add valid field to the list_editable.
4. Here comes the bug. When all items are valid, it's ok and I can change item to invalid(valid=False) and save. But once there is an invalid item, you can't edit it anymore. It just shows "Please correct the error below" whithout any other message.

[Djing]

1. all valid items

[Djing]

2.it's ok to edit item to invalid

[Djing]

3. can't edit anymore once invalid item appear

[Djing]

The exception is DoesNotExist, and it is thrown by the to_python method of django.forms.models.ModelChoiceField.

    def to_python(self, value):
        if value in self.empty_values:
            return None
        try:
            key = self.to_field_name or 'pk'
            if isinstance(value, self.queryset.model):
                value = getattr(value, key)
            value = self.queryset.get(**{key: value})
        except (ValueError, TypeError, self.queryset.model.DoesNotExist):
            raise ValidationError(self.error_messages['invalid_choice'], code='invalid_choice')
        return value

The ModelChoiceField and it's queryset come from the add_fields method of django.forms.models.BaseModelFormSet. This method builds field for pk according to the comment.

    # Omit the preceding part...
    if isinstance(pk, (ForeignKey, OneToOneField)):
        qs = pk.remote_field.model._default_manager.get_queryset()
    else:
        qs = self.model._default_manager.get_queryset()
    qs = qs.using(form.instance._state.db)
    if form._meta.widgets:
        widget = form._meta.widgets.get(self._pk_field.name, HiddenInput)
    else:
        widget = HiddenInput
    form.fields[self._pk_field.name] = ModelChoiceField(qs, initial=pk_value, required=False, widget=widget)
    # Omit the latter part...

The qs here use the default manager of Model which only shows valid items, so invalid item can't be found.

And I noticed that BaseModelFormSet has the correct queryset which shows all items when it is created by ModelAdmin.

[Josh Michael Karamuth]

Came across the same problem and you're right.

qs = self.model._default_manager.get_queryset()

Should be

qs = self.get_queryset()

Thanks for debugging! Will you submit a PR for this?

[Djing]

Replying to Josh Michael Karamuth:

Came across the same problem and you're right.

qs = self.model._default_manager.get_queryset()

Should be

qs = self.get_queryset()

Thanks for debugging! Will you submit a PR for this?

Yes.

[David Sanders]

Please note the documentation warns about these situations where a default queryset hide/restricts the data it retrieves: ​https://docs.djangoproject.com/en/4.1/topics/db/managers/#default-managers

it’s a good idea to be careful in your choice of default manager in order to avoid a situation where overriding get_queryset() results in an inability to retrieve objects you’d like to work with.

It may be better to have the valid manager as the non-default and get the admin to use this instead.

[Djing]

Replying to Josh Michael Karamuth:

This is my first time writing PR for django, and I follow the doc step by step. Now I'm having some problems with the test.
After modifying the code as mentioned before, there are 2 existing tests which located in model_formsets.tests.ModelFormsetTest failed.

    def test_prevent_change_outer_model_and_create_invalid_data(self):
        author = Author.objects.create(name="Charles")
        other_author = Author.objects.create(name="Walt")
        AuthorFormSet = modelformset_factory(Author, fields="__all__")
        data = {
            "form-TOTAL_FORMS": "2",
            "form-INITIAL_FORMS": "2",
            "form-MAX_NUM_FORMS": "",
            "form-0-id": str(author.id),
            "form-0-name": "Charles",
            "form-1-id": str(other_author.id),  # A model not in the formset's queryset.
            "form-1-name": "Changed name",
        }
        # This formset is only for Walt Whitman and shouldn't accept data for
        # other_author.
        formset = AuthorFormSet(
            data=data, queryset=Author.objects.filter(id__in=(author.id,))
        )
        self.assertTrue(formset.is_valid())
        formset.save()
        # The name of other_author shouldn't be changed and new models aren't
        # created.
        self.assertSequenceEqual(Author.objects.all(), [author, other_author])

    def test_edit_only_object_outside_of_queryset(self):
        charles = Author.objects.create(name="Charles Baudelaire")
        walt = Author.objects.create(name="Walt Whitman")
        data = {
            "form-TOTAL_FORMS": "1",
            "form-INITIAL_FORMS": "1",
            "form-0-id": walt.pk,
            "form-0-name": "Parth Patil",
        }
        AuthorFormSet = modelformset_factory(Author, fields="__all__", edit_only=True)
        formset = AuthorFormSet(data, queryset=Author.objects.filter(pk=charles.pk))
        self.assertIs(formset.is_valid(), True)
        formset.save()
        self.assertCountEqual(Author.objects.all(), [charles, walt])

They both give the formset a non-default queryset and pass data not in the queryset, and see if it just work for data in the queryset.(It seems just the opposite of mine).
So, only one of two situations can pass the test for self.assertTrue(formset.is_valid()), either theirs or mine.

I don't know how to deal with this situation.

Or just as David Sanders says:

It may be better to have the valid manager as the non-default and get the admin to use this instead

[Carlton Gibson]

This is a duplicate of #33375: if you really need this pattern define a form class with the id field, using the desired queryset, so that BaseModelFormSet.add_fields() doesn't add the field with the default manager.

David is on track: you need to be extra careful with setting default managers that filter the QuerySet.