Make checking for node edit forms easier

eaton - July 22, 2007 - 22:00
Project:Drupal
Version:7.x-dev
Component:node system
Category:feature request
Priority:minor
Assigned:eaton
Status:needs work
Issue tags:Needs Documentation
Description

When hook_form_altering the node editing form, there's crazy goofy things that need to be done -- pulling a form element out of the form, checking for a special key, appending it to the form_id, and so on...

This just sets $form['#node_edit_form'] = TRUE. Check that, and you know you're on the node edit form. ;) It's not a killer patch or anything, but it would definitely make things simpler.

AttachmentSizeStatusTest resultOperations
nodetype_form.patch3.77 KBIgnoredNoneNone

#1

moshe weitzman - July 26, 2007 - 04:33

oh yes, this is far too messy right now. subscribe.

#2

catch - October 26, 2007 - 23:29
Status:needs review» needs work

no longer applies. Probably small enough it could squeeze into this version though.

#3

moshe weitzman - November 8, 2007 - 19:26

anyone available to reroll and rtbc this?

#4

Pancho - February 8, 2008 - 00:05
Version:6.x-dev» 7.x-dev

Moving feature requests to D7 queue.

#5

moshe weitzman - September 24, 2008 - 20:27
Status:needs work» needs review

Back from the dead, and ready for review. The whole test suite is passing for me.

AttachmentSizeStatusTest resultOperations
mw.patch5.09 KBIgnoredNoneNone

#6

moshe weitzman - September 24, 2008 - 20:52

Now, with a period at end of a comment, as suggested by Catch.

AttachmentSizeStatusTest resultOperations
mw.patch5.09 KBIgnoredNoneNone

#7

webchick - September 24, 2008 - 22:03

+100 for something nicer than

-  if (isset($form['type']) && isset($form['#node']) && $form['type']['#value'] . '_node_form' == $form_id) {

This patch is a really nice DX improvement.

However, why:

+  $form['#node_type'] = $node->type;

Since $form['#node'] is set on any form where $form['#node_edit_form'], you can just use $form['#node']->type same as you use $form['#node']->nid or any other node property. Then that makes one less FAPI property for people to know about.

#8

eaton - September 24, 2008 - 23:03

Yeah, I concur that the #node_type property is overkill. checking for #node to ensure that it's a node edit form is risky, but once we know it's a node edit form, we can very easily check its type. Thanks to PHP eval order, the following works nicely:

if (!empty($form['#node_edit_form']) && $form['#node']->type == 'blog') {...}

#9

webchick - September 24, 2008 - 23:08
Status:needs review» needs work

Cool. Let's remove that (doesn't seem to be used anywhere in the patch) and get correct the "// Set the id of the top-level form tag" comment so it correctly reflects what the code below is doing now.

#10

webchick - September 24, 2008 - 23:10

My mistake, I was looking at an earlier patch "+ // Set the id and other useful elements." capitalize ID please.

Also, I don't see a hunk for book.module? Are there other ones we're missing too?

#11

moshe weitzman - September 26, 2008 - 01:54
Status:needs work» needs review

Removed that extra line which set the type. I did *not* change id to ID because I refer to the HTML id attribute which is almost always lowercase.

I added the book.module hunk. I searched for more instances of this node form check but found none more in core.

AttachmentSizeStatusTest resultOperations
mw.patch5.63 KBIgnoredNoneNone

#12

sun - September 27, 2008 - 04:02

Re-rolled with appropriate menu.module changes.

AttachmentSizeStatusTest resultOperations
drupal.node-form-checking.patch6.29 KBIgnoredNoneNone

#13

Dries - September 27, 2008 - 20:37
Status:needs review» fixed

Looks like a small but valuable improvement and all tests continue to pass. Committed to CVS HEAD.

#14

webchick - September 27, 2008 - 21:56
Status:fixed» needs work

Docs, please! :D

* Need an update to the upgrading docs
* Need an update to the FAPI reference for HEAD.

#15

moshe weitzman - September 28, 2008 - 02:28

Added to upgrade guide http://drupal.org/node/224333#node_form ... I'm not touching that FAPI reference doc though. I proposed an alternate solution for that doc but it has not been embraced yet - #100680: Forms API & other big array documentation

#16

sun - September 10, 2009 - 17:29

.

 
 

Drupal is a registered trademark of Dries Buytaert.