[Django] #25139: ModelFormSet: allow swapping unique values

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[Django] #25139: ModelFormSet: allow swapping unique values

Django
#25139: ModelFormSet: allow swapping unique values
-----------------------------+--------------------
     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.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/052.88bce714b3bc49c5cdd2d12506428980%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #25139: ModelFormSet: allow swapping unique values

Django
#25139: ModelFormSet: allow swapping unique values
-----------------------------+------------------------------------
     Reporter:  jdufresne    |                    Owner:  nobody
         Type:  New feature  |                   Status:  new
    Component:  Forms        |                  Version:  master
     Severity:  Normal       |               Resolution:
     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 <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.2aa8ac4ab9d9d70b663c7c5374fc1e48%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #25139: ModelFormSet: allow swapping unique values

Django
In reply to this post by Django
#25139: ModelFormSet: allow swapping unique values
-----------------------------+------------------------------------
     Reporter:  jdufresne    |                    Owner:  nobody
         Type:  New feature  |                   Status:  new
    Component:  Forms        |                  Version:  master
     Severity:  Normal       |               Resolution:
     Keywords:               |             Triage Stage:  Accepted
    Has patch:  0            |      Needs documentation:  0
  Needs tests:  0            |  Patch needs improvement:  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 <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.b6d7e15e79c6b2931c607dc5af547709%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #25139: ModelFormSet: allow swapping unique values

Django
In reply to this post by Django
#25139: ModelFormSet: allow swapping unique values
-----------------------------+------------------------------------
     Reporter:  jdufresne    |                    Owner:  nobody
         Type:  New feature  |                   Status:  new
    Component:  Forms        |                  Version:  master
     Severity:  Normal       |               Resolution:
     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 karyon):

 * cc: johannes.linke@… (added)


--
Ticket URL: <https://code.djangoproject.com/ticket/25139#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.d693af4ab2b59fa23d53a1dac6c8ae3c%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #25139: ModelFormSet: allow swapping unique values

Django
In reply to this post by Django
#25139: ModelFormSet: allow swapping unique values
------------------------------+---------------------------------------
     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
  Needs tests:  0             |  Patch needs improvement:  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 <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.6172cb46e419c56a12e069c669345516%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #25139: ModelFormSet: allow swapping unique values

Django
In reply to this post by Django
#25139: ModelFormSet: allow swapping unique values
------------------------------+-----------------------------------------
     Reporter:  Jon Dufresne  |                    Owner:  Parth Patil
         Type:  New feature   |                   Status:  assigned
    Component:  Forms         |                  Version:  master
     Severity:  Normal        |               Resolution:
     Keywords:                |             Triage Stage:  Someday/Maybe
    Has patch:  0             |      Needs documentation:  0
  Needs tests:  0             |  Patch needs improvement:  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 <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.f0a909cc77e7e2c9c869288845f089b8%40djangoproject.com.