Opened 3 years ago

Last modified 3 years ago

#25139 new New feature

ModelFormSet: allow swapping unique values

Reported by: Jon Dufresne Owned by: nobody
Component: Forms Version: master
Severity: Normal Keywords:
Cc: johannes.linke@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

As a user, I would expect to be able to swap unique values in an HTML form backed by a ModelFormSet. Turns out, this is not the case. The user must do a two step process of first setting the values to temporary value, then swapping the values. I have created a test case in the Django project that demonstrates my expectation in tests/model_formsets/tests.py:

    def test_swap_unique_values(self):
        p0 = Product.objects.create(slug='car-red')
        p1 = Product.objects.create(slug='car-blue')
        FormSet = modelformset_factory(Product, fields="__all__")
        data = {
            'form-TOTAL_FORMS': '2',
            'form-INITIAL_FORMS': '2',
            # Swap slug values.
            'form-0-id': p0.pk,
            'form-0-slug': 'car-blue',
            'form-1-id': p1.pk,
            'form-1-slug': 'car-red',
        }
        formset = FormSet(data)
        self.assertTrue(formset.is_valid())
        formset.save()

        p0 = Product.objects.get(pk=p0.pk)
        self.assertEquals(p0.slug, 'car-blue')
        p1 = Product.objects.get(pk=p1.pk)
        self.assertEquals(p1.slug, 'car-red')

However, the form fails to validate on the is_valid() line with the message "Product with this Slug already exists.".

When thinking about the details, I understand why this happens. But I don't think it should error in this way. Swapping values is a convenience that a program can handle without the intervention of a human.

Knowing that in the end all values will be unique, I think the Django form engine should go ahead with the steps necessary to get there. I think it is reasonable for a user to expect that they could swap unique values through an HTML form in this way.

As a point of reference, the following test passes. It demonstrates a user using temporary values.

    def test_swap_unique_values_two_step(self):
        p0 = Product.objects.create(slug='car-red')
        p1 = Product.objects.create(slug='car-blue')
        FormSet = modelformset_factory(Product, fields="__all__")
        data = {
            'form-TOTAL_FORMS': '2',
            'form-INITIAL_FORMS': '2',
            'form-0-id': p0.pk,
            'form-0-slug': 'temp0',
            'form-1-id': p1.pk,
            'form-1-slug': 'temp1',
        }
        formset = FormSet(data)
        self.assertTrue(formset.is_valid())
        formset.save()

        data = {
            'form-TOTAL_FORMS': '2',
            'form-INITIAL_FORMS': '2',
            # Swap slug values.
            'form-0-id': p0.pk,
            'form-0-slug': 'car-blue',
            'form-1-id': p1.pk,
            'form-1-slug': 'car-red',
        }
        formset = FormSet(data)
        self.assertTrue(formset.is_valid())
        formset.save()

        p0 = Product.objects.get(pk=p0.pk)
        self.assertEquals(p0.slug, 'car-blue')
        p1 = Product.objects.get(pk=p1.pk)
        self.assertEquals(p1.slug, 'car-red')

Change History (3)

comment:1 Changed 3 years ago by Claude Paroz

Triage Stage: UnreviewedAccepted

Yes, ideally this should work. Now the implementation might not be trivial, as unique checking exists also at the database level.

comment:2 Changed 3 years ago by karyon

i hit the same bug, and i wasn't able to come up with a workaround that covers all the edge cases and that's not manually fiddling with that data dictionary and parsing the strings in there.

any fix or pointers how to fix would be appreciated, as this is a user-facing bug that we have no acceptable workaround for right now.

comment:3 Changed 3 years ago by karyon

Cc: johannes.linke@… added
Note: See TracTickets for help on using tickets.
Back to Top