[Django] #25139: ModelFormSet: allow swapping unique values
Django
-----------------------------+--------------------
Reporter: jdufresne | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+--------------------
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')
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25139>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: jdufresne | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* needs_tests: => 0
* needs_docs: => 0
Comment:
Yes, ideally this should work. Now the implementation might not be
trivial, as unique checking exists also at the database level.
--
Ticket URL: <https://code.djangoproject.com/ticket/25139#comment:1>
Django
Reporter: jdufresne | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25139#comment:2>
Django
Reporter: jdufresne | Owner: nobody
Type: New feature | Status: new
Component: Forms | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by karyon):
* cc: johannes.linke@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/25139#comment:3>
Django
Reporter: Jon Dufresne | Owner: Parth Patil
Type: New feature | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Parth Patil):
* owner: nobody => Parth Patil
* status: new => assigned
Comment:
I don't think so this is feasible, this will require n(n-1)/2 comparisons
to determine whether any two of the models are swapped.
This looks easy in the above case but won't be a good idea for a general
case.
Correct me if I'm wrong or if anyone has a better approach for
implementation of this feature. I would like to work on it.
But as of now, I feel this ticket should be closed
--
Ticket URL: <https://code.djangoproject.com/ticket/25139#comment:4>
Django
Reporter: Jon Dufresne | Owner: Parth Patil
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Someday/Maybe
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Accepted => Someday/Maybe
Comment:
[https://groups.google.com/d/topic/django-
developers/jGk5FvwBy5c/discussion Comment on the mailing list]:
> What would it take: fetching the set of to_be_unique values and
comparing it to the set of values submitted and then assigning both within
a transaction... — meh, possible but I'm not sure it'd be clean, or
something we'd want to bundle-in, even if we had the solution available,
vs, putting it in a third-party package...
I'll bump this to Someday/Maybe for now. Seems like a nice-to-have but
easier to think about with a proposal in hand.
--
Ticket URL: <https://code.djangoproject.com/ticket/25139#comment:5>
Django
Reporter: Jon Dufresne | Owner: Parth Patil
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Someday/Maybe
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Sergey Fedoseev):
* cc: Sergey Fedoseev (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/25139#comment:6>
Django
Reporter: Jon Dufresne | Owner: Parth Patil
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Someday/Maybe
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by felixxm):
#31932 is a related ticket for excluding objects marked for deletion from
unique checks. It looks more feasible.
--
Ticket URL: <https://code.djangoproject.com/ticket/25139#comment:7>
Django
Reporter: Jon Dufresne | Owner: Parth Patil
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Someday/Maybe
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Marco Beri):
* cc: Marco Beri (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/25139#comment:8>