Code

Opened 7 years ago

Closed 7 years ago

#3247 closed defect (fixed)

Disable field creation in forms for hidden model fields

Reported by: mssnlayam@… Owned by: adrian
Component: Forms Version: master
Severity: normal Keywords:
Cc: philipp.keller@…, bs1984@…, christian@…, yatiohi@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

django.newforms.models.form_for_model() should not create a form field for model fields declared with editable=False .

Attachments (3)

disable_formfield_for_hidden_fields.diff (1.7 KB) - added by mssnlayam@… 7 years ago.
disable_formfield_for_hidden_fields_1.diff (1.8 KB) - added by mssnlayam@… 7 years ago.
Patch from the root directory of django distribution, please ignore the previous patch.
disable_formfield_for_hidden_fields_trunk.diff (2.9 KB) - added by Philipp Keller <philipp.keller@…> 7 years ago.
Patch matching 4451, included patch as well

Download all attachments as: .zip

Change History (18)

Changed 7 years ago by mssnlayam@…

Changed 7 years ago by mssnlayam@…

Patch from the root directory of django distribution, please ignore the previous patch.

comment:1 Changed 7 years ago by adrian

I'm not sure whether this should be the correct behavior. Is editable=False intended to be only for the admin site, or does it mean that field shouldn't be edited by *any* form?

comment:2 Changed 7 years ago by adrian

  • Summary changed from [patch] Disable field creation in forms for hidden model fields to [patch] [design-q] Disable field creation in forms for hidden model fields

I'm putting [design-q] in the subject to designate "a design decision needs to be made here." It's just as a little experiment to see whether this extra tagging is useful.

comment:3 Changed 7 years ago by adrian

  • Summary changed from [patch] [design-q] Disable field creation in forms for hidden model fields to [design-q] [patch] Disable field creation in forms for hidden model fields

Changed summary to put [design-q] first, to be consistent.

comment:4 Changed 7 years ago by Philipp Keller <philipp.keller@…>

I've read the following
http://www.b-list.org/weblog/2006/11/02/django-tips-auto-populated-fields:

The created and updated fields now have editable=False. This means that they will never show up in an automatic manipulator or in the admin. They’re still required, they just won’t be displayed anywhere. 

So I supposed this behaviour is meant for newforms too and just never got implemented.

But for the discussion: shouldn't be the other way round? I'd expect a editable=false field not to appear in a automatic generated form. I'd expect the form field list for admin in a configuration in the admin subclass.

comment:5 Changed 7 years ago by adrian

  • Summary changed from [design-q] [patch] Disable field creation in forms for hidden model fields to Disable field creation in forms for hidden model fields
  • Triage Stage changed from Unreviewed to Design decision needed

comment:7 follow-up: Changed 7 years ago by Philipp Keller <philipp.keller@…>

  • Triage Stage changed from Design decision needed to Accepted

Since the discussion didn't bring up any objections against this solution, I switch triage to "accepted"

comment:8 in reply to: ↑ 7 Changed 7 years ago by Michael Radziej <mir@…>

  • Needs documentation set
  • Needs tests set
  • Patch needs improvement set

Hi Philip, you're welcome to help out and triage tickets! But, please, go through everything as described in triage docs.

The patch in this ticket does not apply anymore (so I can't test it). Can you please update it? Tests and docs will be needed before it can be checked id, too.

Changed 7 years ago by Philipp Keller <philipp.keller@…>

Patch matching 4451, included patch as well

comment:9 Changed 7 years ago by Philipp Keller <philipp.keller@…>

  • Needs tests unset
  • Owner changed from adrian to anonymous
  • Patch needs improvement unset
  • Status changed from new to assigned
  • Version set to SVN

Hi Michael, sorry, I forgot to add needs_docs and needs_tests to this ticket.
I did a new patch that matches the current trunk. I've also included a test in model_forms/models.py
It's quite hard to add documentation since the whole models.py isn't documented yet. Some time ago I wrote a tutorial on my blog (http://code.pui.ch/2007/01/07/using-djangos-newforms/) that could serve as a start point for adding documentation for the form_for_model and form_for_instance functions.
What do you think?

comment:10 Changed 7 years ago by mir@…

  • Owner changed from anonymous to adrian
  • Status changed from assigned to new

Thanks for the tests. The assigned field is to match tickets to core developers, please leave it as it is unless you know exactly what you're doing ;-)

comment:11 Changed 7 years ago by Philipp Keller <philipp.keller@…>

  • Cc philipp.keller@… added

comment:12 Changed 7 years ago by anonymous

  • Cc bs1984@… added

comment:13 Changed 7 years ago by christian@…

  • Cc christian@… added

I have tested the patch (disable_formfield_for_hidden_fields_trunk.diff), and it seems to work fine.

comment:14 Changed 7 years ago by anonymous

  • Cc yatiohi@… added

comment:15 Changed 7 years ago by adrian

  • Resolution set to fixed
  • Status changed from new to closed

(In [4548]) Fixed #3247 -- newforms form_for_model() and form_for_instance() no longer create form fields for database fields with editable=False. Thanks for the patch, mssnlayam@… and Philipp Keller

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.