[D6] prohibit double edit

* git release branch is missing, see http://drupal.org/node/1015226
* README.txt is missing
* info file: version, core, datestamp at the bottom should be removed, this is added by drupal.org packaging automatically
* prohibit_double_edit_install(): doc block needs formatting, see http://drupal.org/node/1354#hookimpl
* "// rull all update functions" what does that mean?
* Make sure all comment sentences start capitalized and end with "."
* "foreach($functs['user'] AS $func)" code style errors, see http://drupal.org/node/318#controlstruct
* module file: always use 2 spaces indentation
* "@param boolean $json echo json code" parameter description should be on a new line, see http://drupal.org/node/1354#functions

Review of the 6.x-1.x branch:

• Run coder to check your style, some issues were found (please check the Drupal coding standards):
  

  Severity minor, Drupal Commenting Standards, Internationalization, Drupal Security Checks, Drupal SQL Standards, Drupal Coding Standards
  
  sites/all/modules/pareview_temp/test_candidate/prohibit_double_edit.module:
   +28: [critical] table names should be enclosed in {curly_brackets}
   +88: [normal] Use uppercase for PHP constants, e.g. NULL, TRUE, FALSE
   +104: [normal] Use uppercase for PHP constants, e.g. NULL, TRUE, FALSE
   +110: [normal] Use uppercase for PHP constants, e.g. NULL, TRUE, FALSE
   +113: [normal] Use uppercase for PHP constants, e.g. NULL, TRUE, FALSE
   +118: [normal] Use uppercase for PHP constants, e.g. NULL, TRUE, FALSE
   +121: [normal] Use uppercase for PHP constants, e.g. NULL, TRUE, FALSE
   +129: [normal] Use uppercase for PHP constants, e.g. NULL, TRUE, FALSE
  
  sites/all/modules/pareview_temp/test_candidate/prohibit_double_edit.install:
   +-1: [normal] @file block missing
   +12: [minor] in most cases, replace the string function with the drupal_ equivalent string functions
   +25: [minor] Missing parenthesis after function name
  
  Status Messages:
   Coder found 1 projects, 3 files, 1 critical warnings, 8 normal warnings, 2 minor warnings, 0 warnings were flagged to be ignored
  

• Lines in README.txt should not exceed 80 characters, see the guidelines for in-project documentation.
• @file doc block is missing in the module file, see http://drupal.org/node/1354#files .
• @file doc block is missing in the install file, see http://drupal.org/node/1354#files .
• Assignments should hava a space before and after the operator, see http://drupal.org/node/318#operators
  

  ./prohibit_double_edit.js:1:var prohibit_double_edit_interval=null;
  

This automated report was generated with PAReview.sh, your friendly project application review script. Please report any bugs to klausi.

Review of the 6.x-1.x branch:

• Run coder to check your style, some issues were found (please check the Drupal coding standards):
  

  Severity minor, Drupal Commenting Standards, Internationalization, Drupal Security Checks, Drupal SQL Standards, Drupal Coding Standards
  
  sites/all/modules/pareview_temp/test_candidate/prohibit_double_edit.module:
   +9: [minor] There should be no trailing spaces
   +35: [critical] table names should be enclosed in {curly_brackets}
  
  sites/all/modules/pareview_temp/test_candidate/prohibit_double_edit.install:
   +6: [minor] There should be no trailing spaces
   +16: [minor] in most cases, replace the string function with the drupal_ equivalent string functions
   +29: [minor] Missing parenthesis after function name
  
  Status Messages:
   Coder found 1 projects, 3 files, 1 critical warnings, 4 minor warnings, 0 warnings were flagged to be ignored
  

• Comments should be on a speparate before the code line, see http://drupal.org/node/1354#inline
  

  ./prohibit_double_edit.module:12:define('PROHIBIT_DOUBLE_EDIT_FREE_AFTER', (60 * 10)); // the update is all 3mins this means 3lost connections for be shure (10 minutes)
  

• ./prohibit_double_edit.module: The description for the @param/@return documentation is either missing or not formatted correctly. See http://drupal.org/node/1354#functions
  

  19- */
  87- */
  110- */
  134- */
  

This automated report was generated with PAReview.sh, your friendly project application review script. Please report any bugs to klausi.

manual review:

• prohibit_double_edit_form_alter(): this is a hook implementation, please document it as such. See http://drupal.org/node/1354#hookimpl . Also for others.
• "WHERE nid=%d;": there should be a space before and after the "=", see http://drupal.org/node/2497 . Also elsewhere.
• prohibit_double_edit_menu(): indentatione errors, always use 2 spaces per level.
• "$nid = (INT) $nid;": "INT" should be "int", see http://drupal.org/node/318#cast
• prohibit_double_edit.js: indentation errors, always use 2 spaces, also in javascript files. opening "{" of functions should be on the same line, "else {" should be on a new line etc.
• "Root module class holding basic information": there is no class in this module.
• prohibit_double_edit_menu(): security problem: anyone with the access content permission can got to that paths and trigger an update in the node table. Make sure to check if the user is even allowed to update the node. Furthermore this is a CSRF vulnerability, as I could trick you into clicking a link http://example.com/prohibit_double_edit/ping/1 which would update the record for node 1 in the node table on your behalf. Make sure to read http://drupal.org/writing-secure-code and all subpages, especially http://drupal.org/node/178896

Review of the 6.x-1.x branch:

• Run coder to check your style, some issues were found (please check the Drupal coding standards):
  

  Severity minor, Drupal Commenting Standards, Internationalization, Drupal Security Checks, Drupal SQL Standards, Drupal Coding Standards
  
  sites/all/modules/pareview_temp/test_candidate/prohibit_double_edit.install:
   +6: [minor] There should be no trailing spaces
  
  Status Messages:
   Coder found 1 projects, 3 files, 1 critical warnings, 3 minor warnings, 0 warnings were flagged to be ignored
  

• ./prohibit_double_edit.module: comment lines should break at 80 characters, see http://drupal.org/node/1354#general
  

  // the update is all 3mins this means 3lost connections for be shure (10 minutes)
  

• ./prohibit_double_edit.module: The description on the line after the @param/@return documentation is either missing or not formatted correctly. See http://drupal.org/node/1354#functions
  

  118- *
  155- */
  

This automated report was generated with PAReview.sh, your friendly project application review script. Please report any bugs to klausi.

manual review:

• prohibit_double_edit_install(): do not run update functions in your install function! The install function should always contain the most recent code that your module needs. Update functions are only needed for upgrading older versions of your module. As this is not a full project yet you don't need an update function now.
• prohibit_double_edit_update_6101(): indentation errors, always use 2 spaces to indent array items, see http://drupal.org/node/318#array
• prohibit_double_edit.js: code style errors, make sure that use the same style as in PHP code. "{" should be on the same line as function signatures. Indentation should be 2 spaces.
• "db_query('UPDATE {node} SET edit_uid=%d, edit_last_seen=NOW() WHERE nid=%d;', $user->uid, (int) $nid);": there should be a space before and after "=", see http://drupal.org/node/2497 . Also elsewhere.
• prohibit_double_edit_update_6101(): why do you use datetime as database field type and not the Drupal int standard for dates? Then you would not need the different queries for mysql/pgsql where you convert to unix timestamps anyway.
• why do you need 2 menu paths? my approach would be to set the timestamp on hook_view(), use the javascript for pinging in and use hook_update() to reset the timestamp.

Review of the 6.x-1.x branch:

• Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards):
  

  
  FILE: ...s/all/modules/pareview_temp/test_candidate/prohibit_double_edit.install
  --------------------------------------------------------------------------------
  FOUND 0 ERROR(S) AND 2 WARNING(S) AFFECTING 2 LINE(S)
  --------------------------------------------------------------------------------
    8 | WARNING | Format should be * Implements hook_foo().
   33 | WARNING | Format should be * Implements hook_foo().
  --------------------------------------------------------------------------------
  
  
  FILE: ...es/all/modules/pareview_temp/test_candidate/prohibit_double_edit.module
  --------------------------------------------------------------------------------
  FOUND 3 ERROR(S) AND 1 WARNING(S) AFFECTING 3 LINE(S)
  --------------------------------------------------------------------------------
   11 | ERROR   | Inline comments must start with a capital letter
   11 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
      |         | question marks
   33 | ERROR   | The first index in a multi-value array must be on a new line
   39 | WARNING | A comma should follow the last multiline array item. Found: 0
  --------------------------------------------------------------------------------
  

• All text files should end in a single newline (\n). See http://drupal.org/node/318#indenting
  

  ./prohibit_double_edit.js
  

This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.

Source: http://ventral.org/pareview - PAReview.sh online service

please do not change it to RTBC your self, this has to be done by a reviewer who made a manual in-depht review and thinks that this is ready to go

There are a few coding style issues found by using the tool at:

http://ventral.org/pareview

The current results are at:

http://ventral.org/pareview/httpgitdrupalorgsandboxgreenrover331270858git

After some discussions on the reviewers group is looks like this on-line test is the best one to use currently to check your code against code standards. I.e. Once you've tided up those points we can move on to reviewing the code.

I've pulled down a copy of the code and will test it out tomorrow.

Hi,

I've not used javascript much but am an experienced programmer so should be able to look into your module and hopefully review it.

Since you are using D6 as well would it be possible for you to have a look at my project. The project page is at:

http://drupal.org/sandbox/bailey86/1278830

and the review issue is at:

http://drupal.org/node/1330454

The new module application queue is quite busy at the moment so I think the best way is for us to help each other out by reviewing each others modules.

Thanks,

Kevin

Points list
-----------

1. I've tidied up the English in the project description.

This module prohibits a second person editing the same node which a first person is already editing.

It displays a warning message if two people try to edit a node at the same time.

If the first person starts editing it will set a bit and ping it by ajax every 3 minutes. This bit is then released on save or page blur.

If the browser crahes (not able to send onBlur release), the lock will time out after ten mintues.

2. Change the cloning instructions

The cloning commands should be:

$ git clone --branch 6.x-1.x http://git.drupal.org/sandbox/greenrover33/1270858.git prohibit_double_edit

or for older git clients which don't have the --branch option use:

$ git clone http://git.drupal.org/sandbox/greenrover33/1270858.git prohibit_double_edit
$ cd prohibit_double_edit
$ git checkout --track origin/6.x-1.x -b 6.x-1.x

3. Is this function finished?

RE This function doesn't look quite right - xxxx/6102 - can you explain how this is expected to work? The code is:

/**
* Implements hook_update_xxxx().
* add is edit by bit field to {node} table
*/
function prohibit_double_edit_update_6102() {
$ret = array();

$field = array(
'type' => 'int',
'size' => 'normal',
'description' => 'UnixTimestamp of last seen, ajax, of last edit this node',
);

db_change_field($ret, 'node', 'edit_last_seen', 'edit_last_seen', $field);

return $ret;
}

4. Function comment block

For the function prohibit_double_edit_form_alter(&$form, $form_state, $form_id) the comment block above it does not define the parameters and return value.

This should have a single line on the first line. Look at:

http://drupal.org/node/1354#functions

5. Functionality issue.

I've installed the module and it doesn't seem to do anything.

In separate browsers I've logged into the site twice - both as the user admin. But I am able to edit the same page on both browsers. Are you expecting the users to log in with different names?

Actually I've logged in as different users as well and still can't see any functionality. What am I missing?

Updated issue summary.

Hi,

I did try to run it with two different users logged in via two different browsers - but nothing seemed to happen when I tried to edit the same node in both browsers at the same time.

Am I missing something? Or could you list out a test run through of what steps I should take and what the expected outcome should be showing me and how it stops the double editing.

Cheers,

Kevin

I was wondering how this module is diferent from the Content Lock module.

There are still some coding errors as reported by pareview.

Review of the 6.x-1.x branch:

Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards). See attachment.
This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Get a review bonus and we will come back to your application sooner.

FILE: ...es/all/modules/pareview_temp/test_candidate/prohibit_double_edit.module
--------------------------------------------------------------------------------
FOUND 7 ERROR(S) AFFECTING 7 LINE(S)
--------------------------------------------------------------------------------
45 | ERROR | Array indentation error, expected 6 spaces but found 8
46 | ERROR | Array indentation error, expected 6 spaces but found 8
47 | ERROR | Array indentation error, expected 6 spaces but found 8
48 | ERROR | Array indentation error, expected 6 spaces but found 8
49 | ERROR | Array indentation error, expected 6 spaces but found 8
52 | ERROR | Array indentation error, expected 6 spaces but found 18
53 | ERROR | Array closing indentation error, expected 4 spaces but found 8
--------------------------------------------------------------------------------

Doing a manual review of the code showed that the above is true. Check out http://drupal.org/coding-standards for drupal coding standards.

@frob if you copy from ventral.org/pareview, use the source tab, so all code will be encapsuled in code tags and all links will be linked ;-)

Review of the 6.x-1.x branch:

• Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Get a review bonus and we will come back to your application sooner.

FILE: ...es/all/modules/pareview_temp/test_candidate/prohibit_double_edit.module
--------------------------------------------------------------------------------
FOUND 7 ERROR(S) AFFECTING 7 LINE(S)
--------------------------------------------------------------------------------
 45 | ERROR | Array indentation error, expected 6 spaces but found 8
 46 | ERROR | Array indentation error, expected 6 spaces but found 8
 47 | ERROR | Array indentation error, expected 6 spaces but found 8
 48 | ERROR | Array indentation error, expected 6 spaces but found 8
 49 | ERROR | Array indentation error, expected 6 spaces but found 8
 52 | ERROR | Array indentation error, expected 6 spaces but found 18
 53 | ERROR | Array closing indentation error, expected 4 spaces but found 8
--------------------------------------------------------------------------------

Source: http://ventral.org/pareview - PAReview.sh online service

Just minor coding standard errors, that should not hold up manual reviews.

Get a review bonus and we will come back to your application sooner.

Maybe you could look at the Content Lock module and see if you can add an option to not lock notes for too long a time? If it's a good option then it would be good to have it on a single module.

What I recommend is, post an issue over at the Content Lock module and see if there is any way to collaborate on this problem. Maybe sign-on as a contributor to that module and start a new major branch that includes both ideas.

Post that issue in the Content Lock issue que and then mark this as active with a link to that issue.

Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: http://drupal.org/node/1270872
Project 2: http://drupal.org/node/1598666

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

change git clone instructions