node_save() doesn't set $node->comment
Wesley Tanaka - December 9, 2005 - 17:30
| Project: | Drupal |
| Version: | x.y.z |
| Component: | node.module |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed |
Description
going through my set of patches off of drupal cvs that i needed to make to get things working.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| set_node_comment.patch | 445 bytes | Ignored | None | None |
#1
by the way, i think the idea here: http://drupal.org/node/40256
would cause these kinds of problems to be flushed out very quickly.
#2
please commit me!
Also, for those searching for bugs using the search:
comment off default disabled disable new node forum topic comments ignore admin setting
#3
This fixes the forum bug for 4.7 too. I think drupal sets comments disabled by default, so this would be better.
if (!isset($node->comment)) {$node->comment = variable_get("comment_$node->type", COMMENT_NODE_DISABLED);
}
#4
I'm confused on this one - check comment_nodeapi, 'validate'. The $node->comment is being set there, to the default READ_WRITE (which I think is the right setting, not DISABLED) - why isn't it becoming a part of the $node? Isn't validate supposed to be called before save?
#5
in validate you can't change anything. validate validates.
#6
#7
This is the same patch as wtanaka's, but removes the useless set in comment_nodeapi, validate.
#8
+1 to Morbus's patch. And definitely keep it READ_WRITE. (This also fixes http://drupal.org/node/41152 )
#9
Sorry, forgot to set the status -- this does the job (Morbus's patch)
#10
Morbus's one looks correct...
#11
Ugly patch. Why do we set that field in node_save()? We don't do that for any of the other 'default values'?
#12
You're right. Attached a new one.
#13
I've applied http://drupal.org/files/issues/_40563_comments_0.patch
against 4.7.0-test2, and it works for me.
#14
#15
#16
Tested, works as expected.
#17
Why are we providing a form value for a comment option in node.module? This does not make sense.
#18
Previously, we were doing this in comment_nodeapi 'validate' - that code still remains, actually, and is removed as part of the original patch. Chx tells me that validate no longer (or maybe, never did - I dunno) modifies the form variables, so it can't happen there. The only other place it *could* happen would be in comment_nodeapi 'insert' and 'update', right? In which case, that'd be an entirely new query, just to set the node.comment? Need direction for proper handling of this.
#19
This is all, IMO, on the wrong track.
For one, we shouldn't be reverting to the default in any case. If an admin set it, just because a user edits it doesn't mean it should revert. And it doesn't have to.
formsapi gives us:
<?php$form['user_comments']['comment'] = array(
'#type' => 'value',
'#value' => $selected,
);
?>
#20
Merlin, you are on the right track. Let's see this version.
#21
I moved the prepare op so that patch is smaller. To my eyes, this seems correct. The prepare bit happens when the node is new, otherwise node_load loads $node->comment. The form part is correct: admins can change this, users are not presented with this change but in place there is a value typed field (which is immutable) with same value.
#22
I thoroughly tested this patch, creating different users (non admin) before and after applying the patch. Before applying it, SELECT comment from node where nid=NODE# returned 0, and on the site, no comments being added was possible. After applying, nid=NODE# returned 2, and commenting was possible.
#23
Tested http://drupal.org/files/issues/node-comment_1.patch just posted by chx. Works perfect. +1
#24
What happens if an unprivileged user POSTS data on their own?
#25
Nothing. When the form is built, the node->comment gets a value and that won't be overwritten in form builder.
#26
Committed to HEAD. Thanks.
#27