Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#18872 closed New feature (fixed)

Add prefix attribute to FormMixin

Reported by: dragonsnaker@… Owned by: anonymous
Component: Generic views Version: dev
Severity: Normal Keywords: FormMixin prefix
Cc: ibustamante Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: yes UI/UX: no

Description

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.

Change History (19)

comment:1 by ibustamante, 12 years ago

Cc: ibustamante added

comment:2 by ibustamante, 12 years ago

Owner: changed from nobody to ibustamante
Status: newassigned

comment:4 by ibustamante, 12 years ago

Version: 1.4master

comment:5 by ibustamante, 12 years ago

Summary: Adding prefix attribute to FormMixinAdd prefix attribute to FormMixin

comment:6 by Łukasz Rekucki, 11 years ago

Needs tests: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Please submit patches as either diff attachments or links to *pull request*, which makes them easy to review.

in reply to:  6 comment:7 by ibustamante, 11 years ago

Created a pull request to add the patch as requested: https://github.com/django/django/pull/477

comment:8 by Zbigniew Siciarz, 11 years ago

Owner: changed from ibustamante to Zbigniew Siciarz

comment:9 by Zbigniew Siciarz, 11 years ago

Needs documentation: set

comment:10 by Zbigniew Siciarz, 11 years ago

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?

comment:11 by konradhalas, 11 years ago

Yes, but you can always create two (and more) different forms within one page and with prefix field you can avoid inputs name collision.

comment:12 by ibustamante, 11 years ago

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.

comment:13 by druidjaidan, 11 years ago

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).

comment:14 by Daniele Procida, 11 years ago

Owner: changed from Zbigniew Siciarz to Daniele Procida

I have tentatively reserved this ticket for first-time committers who take part in the Don't be afraid to commit workshop at the 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...

comment:15 by alasdair, 11 years ago

Owner: changed from Daniele Procida to anonymous

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?

    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.

comment:16 by Gilberto Gonçalves <lursty@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In ef37b23050637da643b47b1ee744702d4d603f4c:

Fixed #18872 -- Added prefix to FormMixin

Thanks @ibustama for the initial patch and dragonsnaker for opening the
report.

comment:17 by Marc Tamlyn <marc.tamlyn@…>, 11 years ago

In 257a137c430cd325f3deeda8daffbf03a3cb20fd:

Merge pull request #1294 from LuRsT/added_prefix_to_form_mixin

Fixed #18872 -- Added prefix to FormMixin

comment:18 by Tim Graham, 11 years ago

In the release notes, this was added under "Backwards incompatible changes". I believe it should be under "Minor features" instead?

comment:19 by Tim Graham <timograham@…>, 11 years ago

In e10757ff4dbbc1aedd09df6c542948409c49d75f:

Doc cleanup for FormMixin.prefix; refs #18872.

Note: See TracTickets for help on using tickets.
Back to Top