There is no hook_comment_validate(),

hook_comment_presave(): "The comment passed validation and is about to be saved"

There isn't any details in http://drupal.org/node/224333 on how hook_comment has change in D7.

??

Comments

joachim’s picture

Title: What is hook_comment($op == 'validate') in D7? » hook_comment($op == 'validate') has gone
Category: support » bug
Priority: Normal » Major

Sounds like a regression from D6 to me. Changing to bug report.

mattyoung’s picture

Issue tags: +API change

Add an issue tag so we don't go to RC1 and forget about this

mattyoung’s picture

StatusFileSize
new861 bytes

would something like this work?

joachim’s picture

Don't have time to test sorry, but mind the whitespace, and maybe also add a comment like 'Invoke hook_comment_validate blah blah'.

joachim’s picture

Status: Active » Needs work

Oh and something in comment.api.php too.

bfroehle’s picture

comment_validate() has been removed:
#335034: refactor comment validate/save logic: Use comment_form_validate() for all form-related validation of comments.

Did I miss something? I suggest closing the (non)-issue.

mattyoung’s picture

Status: Needs work » Needs review
StatusFileSize
new1.87 KB

>Use comment_form_validate() for all form-related validation of comments

comment_form_validate() is the validation function for the "comment_form" in comment.module. There is no "hook" to give other modules a chance validate.

Also the patch in #335034: refactor comment validate/save logic doesn't seem to match what's in comment.module currently. For example, there is no function comment_invoke_comment() in comment.module right now but it's in the patch.

I don't understand this at all. As is, I think we need to have a hook_comment_validate().

Here is a new patch. This one makes hook_comment_validat() work like D6's hook_comment($op == 'validate')

bfroehle’s picture

Status: Needs review » Needs work

Look to see how the mollom module does it. Use hook_form_alter and add your validation to $form['#validate'].

mattyoung’s picture

Mollom is not a good example because it needs to insert its validation into lots of forms so it's appropriate for it to use form_alter() to do its things uniformly there. What we have here is specifically letting modules validate comment on submit/preview. hook_comment($op == 'validate') exists in D6, there is no reason to remove it . If removing that make sense, then why not also remove hook_node_validate() and go use hook_form_alter() instead?

mattyoung’s picture

Status: Needs work » Needs review

Hello, can we get this in?

marcingy’s picture

Status: Needs review » Needs work

Patch has white spaces and a typo. Plus why are we doing this

$function($form_state['values']);

rather than

$function($form, $form_state);
mattyoung’s picture

Status: Needs work » Needs review
StatusFileSize
new1.97 KB

Changed and fixed typo. But I don't know what the white spaces are.

joachim’s picture

Status: Needs review » Needs work

Whitespace means empty spaces at the end of lines of code.

+++ modules/comment/comment.api.php	23 Nov 2010 03:32:51 -0000
@@ -12,6 +12,21 @@
+ * should be set with form_set_error().
+ * ¶
+ * @param $form

There's a space at the end of the middle line here.

+++ modules/comment/comment.api.php	23 Nov 2010 03:32:51 -0000
@@ -12,6 +12,21 @@
+  // validate the comment body

Sentence case here.

+++ modules/comment/comment.api.php	23 Nov 2010 03:32:51 -0000
@@ -12,6 +12,21 @@
+  validate_comment($form_state['values']['comment_body']['und'][0]['value']);

This function doesn't actually exist, does it? I don't think it's a good idea to put fictional functions into the API examples.

+++ modules/comment/comment.module	23 Nov 2010 03:32:52 -0000
@@ -2112,6 +2112,12 @@ function comment_form_validate($form, &$
+  // invoke hook_comment_validate

Sentence case again.

+++ modules/comment/comment.module	23 Nov 2010 03:32:52 -0000
@@ -2112,6 +2112,12 @@ function comment_form_validate($form, &$
+  	$function = $name . '_comment_validate';

Indentation wrong here.

I'm sure this seems like nitpicking to you -- it used to me too ;) -- but this level of attention to detail means that the whole of the code in Drupal core is consistent, which in turn makes it much clearer to read and work with.

Powered by Dreditor.

bfroehle’s picture

If you really want to pursue this, I think you'd have to also use LANGUAGE_NONE instead of 'und' for consistency and clarity.

However, please see #512492: Remove hook_comment_validate(). I don't think this patch fits with the current direction Drupal is heading.

damien tournoud’s picture

Status: Needs work » Closed (works as designed)

This is definitely by design. The hook was purposely removed by #512492: Remove hook_comment_validate().

mattyoung’s picture

By design? So to validate comment, you have to write two functions. One of them is boiler plate mumbo jumbo. This doesn't make sense to me. I don't think having a dedicated comment_validate hook is confusing. To the contrary, it make thing clearer and easier for the reader of the code to understand. This also makes comment hooks consistent with node hooks: there is a hook_node_validate().