Unique Comments

Hi

Good coding practice and good work. I only have a few questions :

Manuel Review :

1)
if ($comments = db_select('field_data_comment_body', 'cb')
->fields('cb', array('entity_type', 'comment_body_value'))
->condition('entity_type', array('comment'))
->condition('comment_body_value', array($v[0]['value']))
->execute()
->fetchAll())
it is = or == ?

2)
$field = 'UPPER(cb.comment_body_value)';
this variable is not used / delete it

3)

->fields('c', array('cid', 'nid'))
do you need these fields ? You don't do anything with them.

Sorry for the delay, too many applications too few reviewers,
you can get a review bonus and we will come back to your application sooner.

adding in info for drupal versions

Manual Review:

1. hook_help, if you don't have content to put here then remove it.
2. Your Module is too short, try to extend it.
3. Your module only work for 'article' content type, extend it for other content types as well
4. Case 'Per Node' foreach ($result as $comments) { why you are looping here, even you can check it with if.
5. My suggestion for you to extend this module in a manner so that it will work with every content type and also extend the admin form for node types, so that user can select node type basis as well

I found couple of issues in your module :-

• unique_comments.admin.inc :- You are using system_settings_form, so there is no need to add a submit handler and set variables as it automatically saves the variables.
• All form item titles should be wrapped in t().

manual review:

• variable_get('unique_comments_allowed_nids', NULL): This will not scale. On a site with 100.000s of nodes you should not store this in a single variable. Create your own database table.
• "'#options' => $types,": #options is not automatically sanitized by the form API on the checkboxes form element. As node type labels are user provided input you need to sanitize them to avoid XSS vulnerabilities. See http://drupal.org/node/28984
• unique_comments_comment_form_validate(): chained invocations of the DB API should use 2 additional spaces of indentation on new lines.

Otherwise looks nearly ready to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Please leave the security tag, it's for statistical purposes.

Use l() instead of hard coding in hook_help() see l().

How are you to know a node nid ? this must be searched in the db.. try to combine this functionality with your table 'Exempt (weird choice of the word btw, needed to google it to understand what it meant) nodes'. Take a look at A K Chauhan’s Blog for an example.

Good module though

EDIT: this is not something I would define as 'need work' cause imo it can be implemented in a later stage.

manual review:

1. As I just learned a few days ago the form API indeed runs filter_xss_admin() on #options labels, so there was no security issue in your module (removing security tag), sorry about that. Anyway, the check_plain() does not hurt here.
2. unique_comments_node_list(): the theming function for tables accepts an "empty" setting for empty tables, so no need to check that yourself.
3. unique_comments_node_exemption_delete(): If a form has a submit function, then hidden form values are not needed. Instead, any values that you need to pass to $form_state['values'] can be declared in the $form array as #type => value.
4. "t('Deleting this will cause node :nid to be affected by Unique Comments again.', array(':nid' => $result[0]->nid)),": ":" is not a valid placeholder prefix, use either "@", "%" or "!". Also elsewhere.
5. Showing node IDs in the user interface is bad for humans, node titles are more familiar.

But that are just minor issues, otherwise I think this is RTBC. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects. Assigning to tim.plunkett as he might have time to finally approve this.

1. Your project page could contain a little more information and made more structured by some nice html tags ;) please follow the tips for a great project page.
2. I'd suggest you to make the "Add new Exemption." menu point a MENU_LOCAL_ACTION, would be more intuitive
3. I'd also suggest you to provide an autocomplete for the exemption textfield (have a look at the user autocomplete on how to do it)
4. Also your readme contains no installation instructions or information about dependencies, please take a moment to make your README.txt follow the guidelines for in-project documentation.
5. Please leave a new line between function short and long descriptions
   

   /**
    * Implements hook_node_update().
    * To save/remove the NID if Unique Comments is disabled/enabled
    * on a node upon node update.
    */

   ->

   /**
    * Implements hook_node_update().
    *
    * To save/remove the NID if Unique Comments is disabled/enabled
    * on a node upon node update.
    */

6. always use TRUE and FALSE instead of 1/0

but anyway, this module looks good enough to get promoted,

thanks for your contribution and welcome to the community of project contributors on drupal.org! :)

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

As you continue to work on your module, keep in mind: Commit messages - providing history and credit and Release naming conventions.

Thanks to the dedicated reviewer(s) as well.