The example code shown in http://api.drupal.org/api/function/hook_node_type/6 needs to be updated; the code shown for the operation update is already executed by Drupal core code, and it doesn't need to be done from third-party modules.

CommentFileSizeAuthor
#9 docs-881112.patch2.35 KBavpaderno

Comments

avpaderno’s picture

See http://api.drupal.org/api/function/node_type_form_submit/6, where Drupal variables used from form fields present in the content type edit form are renamed.
Variables are not automatically deleted from Drupal core when a content type is deleted.

jhodgdon’s picture

Good catch. This does need an update.

avpaderno’s picture

I can change the example implementation myself; which implementation should I use as example? comment_node_type()?

jhodgdon’s picture

Yeah, I guess so, although it only illustrates the 'delete' $op. If we could find an example that did all the ops, that would be better, but I didn't see one in core... The other option would be to make a mish-mash like:

// Example of 'delete' operation from comment.module:
 (code)
// Example of 'insert' operation from ...
etc.
avpaderno’s picture

book_node_type() has code for the update operation. I was going to avoid to show the code for that operation to make clear that it is not anymore needed when Drupal variables are used in the content type form.

It would be possible to add a sentence to explain that.

jhodgdon’s picture

I think it makes sense to include a sample of update to illustrate why you might remotely need it, along with a comment explaining that you don't usually (as you have described above).

avpaderno’s picture

if it would not be for the comment to add, I would do the change myself; comments in the code are not my strength. :-)

For the rest, I would take the code from two different modules, and add a comment before each snippets saying from where the code comes from.

If I correctly recall, the example code for hook_theme() was changed by me to include code coming from a core module different from the module used for the existing code to show all the theme declaration cases.

jhodgdon’s picture

Here's what I would put in the comment (or something similar):

Most modules will not need the update or insert operations, because form fields you add to the node type editing form will be automatically saved as variables. For instance, if you add a field called 'my_module_foo' to content type 'machine_name', its value will be saved as variable 'my_module_foo_machine_name' (and if the machine name is updated, the variables will be updated appropriately). Here is an example where you do need an update operation (from the book module), because the simple default case doesn't cover what needs to be done.

avpaderno’s picture

Status: Active » Needs review
StatusFileSize
new2.35 KB
jhodgdon’s picture

Status: Needs review » Needs work

I think I would take the part starting with "Here is an example" and make that into a code comment rather than keeping it in the docblock header. I would put it right after the comment you have in the code that says // book.module now.

Also, as a style point, there should be a blank line before the first @param.

dleatham’s picture

A suggestion in relation to #8 above... I'm trying to implement a system where, when an instance of content_type_A is created, instances of content_type_B and content_type_C are created and the two are linked back to content_type_A.

I've seen a fair number of posts in the forums where others are struggling with this type of use-case. I'd like to see an example of how the 'insert' $op can be used to catch the insertion of one type of content so that instances of other content types can be created/linked.

Appreciate your efforts at making Drupal better.

jhodgdon’s picture

Status: Needs work » Fixed

I've applied the above patch, slightly modified, to the contrib repository.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.