list_editable will update wrong rows on a multiuser system if pagination is enabled
|Reported by:||emilianodelvalle@…||Owned by:||nobody|
|Severity:||Normal||Keywords:||list_editable list multiuser|
|Has patch:||no||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
If there is a model with fields which are "list editable" and the number of rows for that model enables pagination, an update on those fields could not work as expected (other rows are updated).
- Lets say that the user updates fields on the first page on the change list view.
- While saving those changes, the information on the database changes (another user or another process added rows that should be shown before the rows updated by the user, shifting the rows modified by the user to a second page).
- Django will update the editable fields on the newest rows with the data entered for other rows.
I debugged the code and I think that the bug is on the BaseModelFormSet. For some reason, if it can't retrieve a instance model using the pk, it will use the index to retrieve it. However, the queryset that it's using has been limited by pagination. Therefore if the row updated has been moved to another page, it won't be found.
This belongs to BaseModelFormSet, take a look at the final if on the _construct_form method.
def _existing_object(self, pk): if not hasattr(self, '_object_dict'): self._object_dict = dict([(o.pk, o) for o in self.get_queryset()]) return self._object_dict.get(pk) def _construct_form(self, i, **kwargs): if self.is_bound and i < self.initial_form_count(): # Import goes here instead of module-level because importing # django.db has side effects. from django.db import connections pk_key = "%s-%s" % (self.add_prefix(i), self.model._meta.pk.name) pk = self.data[pk_key] pk_field = self.model._meta.pk pk = pk_field.get_db_prep_lookup('exact', pk, connection=connections[self.get_queryset().db]) if isinstance(pk, list): pk = pk kwargs['instance'] = self._existing_object(pk) if i < self.initial_form_count() and not kwargs.get('instance'): kwargs['instance'] = self.get_queryset()[i] return super(BaseModelFormSet, self)._construct_form(i, **kwargs)
Change History (4)
comment:1 Changed 3 years ago by julien
- Needs documentation unset
- Needs tests unset
- Patch needs improvement unset
comment:3 Changed 3 years ago by julien
- Resolution set to duplicate
- Status changed from new to closed