[Django] #18872: Adding prefix attribute to FormMixin
Django
--------------------------------+------------------------------
Reporter: dragonsnaker@… | Owner: nobody
Type: New feature | Status: new
Component: Generic views | Version: 1.4
Severity: Normal | Keywords: FormMixin prefix
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------+------------------------------
I just want to suggest adding a "prefix" attribute, and its corresponding
get_prefix() method, to the django.views.generix.edit.FormMixin, in order
to make it possible to add a prefix to the form.
I'll attach a patch later, in case my suggestion is considered.
--
Ticket URL: <https://code.djangoproject.com/ticket/18872>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Type: New feature | Status: new
Component: Generic views | Version: 1.4
Needs tests: 0 | Patch needs improvement: 0
Changes (by ibustamante):
* cc: ibustamante (added)
* needs_docs: => 0
* needs_tests: => 0
* needs_better_patch: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/18872#comment:1>
Django
Reporter: dragonsnaker@… | Owner: ibustamante
Type: New feature | Status: assigned
Changes (by ibustamante):
* owner: nobody => ibustamante
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/18872#comment:2>
Django
Reporter: dragonsnaker@… | Owner: ibustamante
Type: New feature | Status: assigned
Needs tests: 0 | Patch needs improvement: 0
Changes (by ibustamante):
* has_patch: 0 => 1
Comment:
Patch added: https://github.com/ibustama/django/tree/ticket_18872
--
Ticket URL: <https://code.djangoproject.com/ticket/18872#comment:3>
Django
Reporter: dragonsnaker@… | Owner: ibustamante
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Changes (by ibustamante):
* version: 1.4 => master
--
Ticket URL: <https://code.djangoproject.com/ticket/18872#comment:4>
Django
----------------------------------+---------------------------------------
Reporter: dragonsnaker@… | Owner: ibustamante
Type: New feature | Status: assigned
Component: Generic views | Version: master
Severity: Normal | Resolution:
--
Ticket URL: <https://code.djangoproject.com/ticket/18872#comment:5>
Django
----------------------------------+---------------------------------------
Reporter: dragonsnaker@… | Owner: ibustamante
Type: New feature | Status: assigned
Component: Generic views | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by lrekucki):
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
* stage: Unreviewed => Accepted
Comment:
Please submit patches as either diff attachments or links to *pull
request*, which makes them easy to review.
--
Ticket URL: <https://code.djangoproject.com/ticket/18872#comment:6>
Django
----------------------------------+---------------------------------------
Reporter: dragonsnaker@… | Owner: ibustamante
Type: New feature | Status: assigned
Component: Generic views | Version: master
Severity: Normal | Resolution:
Keywords: FormMixin prefix | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Comment (by ibustamante):
Created a pull request to add the patch as requested:
https://github.com/django/django/pull/477
--
Ticket URL: <https://code.djangoproject.com/ticket/18872#comment:7>
Django
----------------------------------+------------------------------------
Type: New feature | Status: assigned
Component: Generic views | Version: master
Severity: Normal | Resolution:
Keywords: FormMixin prefix | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Changes (by zsiciarz):
* owner: ibustamante => zsiciarz
--
Ticket URL: <https://code.djangoproject.com/ticket/18872#comment:8>
Django
----------------------------------+------------------------------------
Reporter: dragonsnaker@… | Owner: zsiciarz
Type: New feature | Status: assigned
Component: Generic views | Version: master
Severity: Normal | Resolution:
Keywords: FormMixin prefix | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 1
Changes (by zsiciarz):
* needs_docs: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/18872#comment:9>
Django
----------------------------------+------------------------------------
Reporter: dragonsnaker@… | Owner: zsiciarz
Type: New feature | Status: assigned
Component: Generic views | Version: master
Severity: Normal | Resolution:
Keywords: FormMixin prefix | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Comment (by zsiciarz):
Well, just a thought: prefixes are used to discriminate between multiple
forms within the same view. Since the FormMixin handles only one form,
what's the use case for setting a custom prefix for that form?
--
Ticket URL: <https://code.djangoproject.com/ticket/18872#comment:10>
Django
----------------------------------+------------------------------------
Reporter: dragonsnaker@… | Owner: zsiciarz
Type: New feature | Status: assigned
Component: Generic views | Version: master
Severity: Normal | Resolution:
Keywords: FormMixin prefix | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Comment (by konradhalas):
Yes, but you can always create two (and more) '''different''' forms within
one page and with prefix field you can avoid inputs name collision.
--
Ticket URL: <https://code.djangoproject.com/ticket/18872#comment:11>
Django
----------------------------------+------------------------------------
Reporter: dragonsnaker@… | Owner: zsiciarz
Type: New feature | Status: assigned
Component: Generic views | Version: master
Severity: Normal | Resolution:
Keywords: FormMixin prefix | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Comment (by ibustamante):
I want to reinforce what @konradhalas said. It's possible to have multiple
forms on the same page when you're using AJAX, for example. In this case
you would need to have a prefix set, so as to avoid input id collision.
--
Ticket URL: <https://code.djangoproject.com/ticket/18872#comment:12>
Django
----------------------------------+------------------------------------
Reporter: dragonsnaker@… | Owner: zsiciarz
Type: New feature | Status: assigned
Component: Generic views | Version: master
Severity: Normal | Resolution:
Keywords: FormMixin prefix | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Comment (by druidjaidan):
Have to agree. There are lots of reasons you might need to use a prefix
that don't involve processing more than one form. I ran into this because
we had a form on the page that acted as a context switcher, changing it's
value caused the form to be processed, changed a session value, and
reloaded the page given the new context. This caused an id collision
anytime the user was on a page that happened to have a form that had the
same field name (really common considering the context was a FK).
--
Ticket URL: <https://code.djangoproject.com/ticket/18872#comment:13>
Django
----------------------------------+------------------------------------
Type: New feature | Status: assigned
Component: Generic views | Version: master
Severity: Normal | Resolution:
Keywords: FormMixin prefix | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Changes (by EvilDMP):
* owner: zsiciarz => EvilDMP
Comment:
I have tentatively reserved this ticket for first-time committers who take
part in the [https://dont-be-afraid-to-commit.readthedocs.org/ Don't be
afraid to commit workshop] at the [http://2013.djangocon.eu/ DjangoCon
Europe 2013] sprints on 18th and 19th May.
If you want to tackle this ticket before then, please '''don't''' let the
fact that it's assigned to me stop you. Feel free to re-assign it to
yourself and do whatever you like to it.
'''Note''' - the pull request https://github.com/django/django/pull/477
for this one has been closed because "it needs docs and tests". You know
what to do...
--
Ticket URL: <https://code.djangoproject.com/ticket/18872#comment:14>
Django
Reporter: dragonsnaker@… | Owner: anonymous
Type: New feature | Status: assigned
Component: Generic views | Version: master
Severity: Normal | Resolution:
Keywords: FormMixin prefix | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Changes (by alasdair):
* owner: EvilDMP => anonymous
Comment:
Grabbing the ticket. I aim to add docs and tests by the end of the weekend
June 1-2.
Is there any reason why we shouldn't always include the prefix in the form
kwargs?
{{{
#!py
def get_form_kwargs(self):
"""
Returns the keyword arguments for instantiating the form.
"""
kwargs = {'initial': self.get_initial(),
'prefix': self.get_prefix(),
}
}}}
There shouldn't be any problems doing this, because the default
`prefix=None` matches the default `prefix=None` in the form class. Always
including prefix makes it easier to describe in the get_form_kwargs docs
("the prefix argument is set to get_prefix" than in the initial pull
request, where prefix is only included if it evaluates to True.
--
Ticket URL: <https://code.djangoproject.com/ticket/18872#comment:15>
Django
----------------------------------+-------------------------------------
Reporter: dragonsnaker@… | Owner: anonymous
Component: Generic views | Version: master
Keywords: FormMixin prefix | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Changes (by Gilberto Gonçalves <lursty@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"ef37b23050637da643b47b1ee744702d4d603f4c"]:
{{{
#!CommitTicketReference repository=""
revision="ef37b23050637da643b47b1ee744702d4d603f4c"
Fixed #18872 -- Added prefix to FormMixin
Thanks @ibustama for the initial patch and dragonsnaker for opening the
report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18872#comment:16>
Django
----------------------------------+-------------------------------------
Reporter: dragonsnaker@… | Owner: anonymous
Type: New feature | Status: closed
Component: Generic views | Version: master
Severity: Normal | Resolution: fixed
Keywords: FormMixin prefix | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Comment (by Marc Tamlyn <marc.tamlyn@…>):
In [changeset:"257a137c430cd325f3deeda8daffbf03a3cb20fd"]:
{{{
#!CommitTicketReference repository=""
revision="257a137c430cd325f3deeda8daffbf03a3cb20fd"
Merge pull request #1294 from LuRsT/added_prefix_to_form_mixin
Fixed #18872 -- Added prefix to FormMixin
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18872#comment:17>
Django
----------------------------------+-------------------------------------
Reporter: dragonsnaker@… | Owner: anonymous
Type: New feature | Status: closed
Component: Generic views | Version: master
Severity: Normal | Resolution: fixed
Keywords: FormMixin prefix | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Comment (by timo):
In the release notes, this was added under "Backwards incompatible
changes". I believe it should be under "Minor features" instead?
--
Ticket URL: <https://code.djangoproject.com/ticket/18872#comment:18>
Django
----------------------------------+-------------------------------------
Reporter: dragonsnaker@… | Owner: anonymous
Type: New feature | Status: closed
Component: Generic views | Version: master
Severity: Normal | Resolution: fixed
Keywords: FormMixin prefix | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Comment (by Tim Graham <timograham@…>):
In [changeset:"e10757ff4dbbc1aedd09df6c542948409c49d75f"]:
{{{
#!CommitTicketReference repository=""
revision="e10757ff4dbbc1aedd09df6c542948409c49d75f"
Doc cleanup for FormMixin.prefix; refs #18872.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/18872#comment:19>