CVS edit link for patrickmj

The CommentPress plugin for WordPress has been extremely popular among teachers and scholars, but there is currently not a corresponding module for Drupal. The Annotation module appears to come close, but has limitations that prevent its being widely used. And the paragraph-by-paragraph functionality is a key concern to teachers and scholars. (See the discussion in the Contributed Modules Group: http://groups.drupal.org/node/72748 ).

The need for such a module came to my attention at a THATCamp unconference, when a participant told me that she is mashing together a Drupal site for its social features with WordPress to use CommentPress for her project to publish her book online. And so, I am applying for a CVS account to work on such a module.

A working example of the code can be found on my blog post about beginning the module. The responses there have been encouraging that the module would be useful and of interest to many teachers and scholars.

CommentFileSizeAuthor
#1 open_review-6.x-0.1-dev.tar_.gz298.39 KBpatrickmj

Comments

patrickmj’s picture

patrickmj commented
Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new open_review-6.x-0.1-dev.tar_.gz298.39 KB

Many thanks,

Here's my submission. The Code Review module produced some dings on it regarding tabs and spaces. I attempted to fix them, but I think I must not know the quirks of my editor well enough to eradicated them completely.

Thanks for looking at this,
Patrick

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Assigned: Unassigned » avpaderno
Status: Needs review » Needs work
  • The points reported in this review are not in order or importance / relevance.
  • Most of the times I report the code that present an issue. In such cases, the same error can be present in other parts of the code; the fact I don't report the same issue more than once doesn't mean the same issue is not present in different places.
  • Not all the reported points are application blockers; some of the points I report are simple suggestions to who applies for a CVS account. For a list of what is considered a blocker for the application approval, see CVS applications review, what to expect. Keep in mind the list is still under construction, and can be changed to adapt it to what has been found out during code review, or to make the list clearer to who applies for a CVS account.
  1. Files available from third-party sites should not be committed in drupal.org repository.
  2. The module doesn't implement hook_uninstall().
  3. The module depends on PHP5 but it doesn't declare its dependency from PHP5. 
  4. function open_review_footer() {  
  $node = node_load(arg(1));
  if (variable_get('open_review_' . $node->type, 0)) {    
    $output = "<div id='open-review-footer'>";
    $output .= "<div id='open-review-comments-dialog'></div>";
    $output .= "<div id='open-review-comment-form-dialog'></div>";  
    $output .= "</div>";
    $output .= "<script type='text/javascript'>";
    if (variable_get('open_review_form_in_popup_'. $node->type, 0)) {
      $output .= "OpenReview.commentsInPopup = true;";	
    } 
    else {
  	  $output .= "if (OpenReview) { OpenReview.commentsInPopup = false; }";
    }
    $output .= "</script>";  
    return $output;
  }
}

    The module doesn't verify if the user has access to the node being show.
    There is a Drupal function that is used to include JavaScript code in a Drupal page. 

  5.     $query = "SELECT COUNT(c.cid) FROM {node} n INNER JOIN {comments} c ON n.nid = c.nid INNER JOIN {open_review} o ON c.cid = o.cid WHERE n.nid = %d AND timestamp > %d AND c.status = %d AND o.p_id = '%s' " ;
    $result = db_result(db_query('SELECT COUNT(c.cid) FROM {node} n INNER JOIN {comments} c ON n.nid = c.nid INNER JOIN {open_review} o ON c.cid = o.cid WHERE n.nid = %d AND timestamp > %d AND c.status = %d AND o.p_id = "%s" ', $nid, $timestamp, COMMENT_PUBLISHED, $paraId));

    The code should probably use db_rewrite_sql().

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs work » Closed (won't fix)