Nodelocks

Welcome with your application.

The project could be too small to get your full project access, but you could get it out as single project, see the discussion over here: http://groups.drupal.org/node/195848

• Some of you text strings are missing t().
• You have files in your MASTER branch, you should not have that, only a README.txt pointing to the correct branch
• Please remove the LICENSE.txt file. Drupal will add the appropriate version automatically during packaging so your repository should not include it.

Also you have some coding standards issue to take care about:
http://ventral.org/pareview/httpgitdrupalorgsandboxr2integrated1401726git

The module itself is a little bit simple (but nothing bad with that), with standard hooks, and does not tell so much about your coding skills, but maybe you pass anyhow, but first of all, take care of the issues, and add comments to the code.

Hi @r2integrated,

I've just stumbled onto your sandbox looking for a quick way to lock nodes. Your module is definitely a good idea and you've done a great job with the code.

I have a couple suggestions I'd like to share with you. These may help you get the access you deserve and for the module to be more universally appealing.

1. The module should be able to lock nodes other than Pages.
2. The solution you provided keeps honest users from deleting a node. If any user goes to node//delete they can subvert your locks and delete the node anyway. As a result, malicious users with delete permissions can still cause problems.

I won't adjust the status of this request since I don't have authority to say yes or no to your application. Hope you get through!

Hi. Firstly, I think this module is a great idea and I can already think of a few applications for it.

A couple of little issues here and there:

• I'm not sure if it's related to your module or another one (I disabled all contrib modules), but I get the following error message what I visit admin/config/development/nodelocks: Notice: Undefined offset: 5 in _menu_translate() (line 762 of /home/sam/Sites/drupal7/includes/menu.inc).
• I'm not sure a textarea is the best way to represent a bunch of nodes separated by a comma. It can lead to cases where I have 1,1,1,1,1,1,1,1 as valid input etc.
• I'm not quite sure what the tabs on admin/config/development/nodelocks are meant to do, they both seem to simply reload the page. Maybe a more clear explanation of those would be valuable.
• You also have a console.log in your JS file. Last time I checked, this can break earlier versions of IE, and in fact IE9 I believe (unless dev tools is open).

Keep up the good work though, I can see real value in this module. :)

Hello

1. Consider something with hook_help - I would have thought it useful, and relatively simple :)
2. Agree with @dhsamb that the string of comma separated node ids isn't great - perhaps look into create a table of locked node ids? node_install is relatively simple.
   Maybe (see attached image) add a tab to the content panel for locked nodes? This could be done via a simple DB query and allow for bulk unlocking?
3. Not sure on some of the menu arrays - tried to delete using 'admin/config/development/nodelocks/unlock/%' (swapping the % for a node id and it died - the node was unlocked - but after a lot of error messages

Really like the module though.

Pat

Hello,

A field is an excellent idea - especially re views (the module "views" just in case we are getting words mixed up).
I haven't used fields as an alternative to my own tables (never came up).
I would still look for a tab to display "all locked nodes" just so managing a lot of nodes could be made a little easier - that is user preference though.

I was testing a module last night and it had a handy message to say if a term had a property - perhaps a locked node could display a message when an administrator views it?

Great ideas though - just making sure you see this as help and not criticism :)

If people are also saying your module isn't complicated enough (point number 1) maybe you could extend the module with some hooks for other developers - so a node_locked, node_unlocked?

Pat

Just regarding your issue of displaying locked nodes. I had a similar issue of displaying nodes that had an exemption to my module.
http://drupal.org/node/1462094
I ended up displaying them in a table with some other information and a link to delete them. I'm not saying you should use it, but since your module gave me some ideas for implementing things I thought I'd share my way of getting round the issue you're having!

PAReview points out that you do not need to declare the nodelocks.module file[] in your .info file but that severely minor. It also says you need a newline, but if one is added it will probably tell you you have too many...

I like the nodelocks checkbox on the node creation page. It says "Prevent node from being deleted" but does that include editing? You may want to change the field description.

Is it possible to have two permissions? Prevent editing and prevent deletion?

Perhaps the nodelocks options could be added as publishing status'. That and find a way to allow use of the content overview page to find/lock multiple nodes at once. Just adding nodelock as a publishing status may accomplish the former.

I tried out the module, but it doesn't seem to do what it was supposed to. First I added a bolean field, and chose the nodelocks widget. You should possibly create a field in your .install so there is one ready to go when the module is installed. I gave two roles full permissions on article content, but only gave one role permissions on nodelock. Created an article and locked it. Switched to the non-nodelocks user and was able to delete the node without a problem.

Personally, I don't think this should rely on adding a field to a node. Personally, I think this would all be best accomplished with hook_form_node_type_form_alter() in the same way comment and path do. Then adding a new page at admin/content/nodelocks to manage all the locks on a site. See comment_menu() to see how the comment module added their page at admin/content/comment.

Also, you should implement hook_help(). When I installed the module I went to admin/help for instructions but there was none for nodelocks.

Adding a ready made field should be as easy as using field_create_field() in your .install file.

All I did was add a new boolean field and selected the nodelocks widget. I always saw the delete button whether the nodelock boolean was checked or unchecked.

I can check again once you add your hook_help and field_create_field().

Took a look at module you have created and here are a few things I would like to point out:
1. You should update readme file with the instructions on how to get the locking working;
2. Just one tiny error in PAReview;
You can fix it like this:

/**
 * Removes the nodelocks storage field.
 *
 * This will not delete the underlying field, it simply indicates that the
 * field should not be used by Nodelocks.
 *
 * @param string $bundle
 *   The bundle name.
 */

3. Looks like function nodelocks_form_node_form_submit in nodelocks.module is not used anywhere, if so then you should remove it from the code;
Some things that you could consider implementing in the future.
4. I would suggest putting just a checkbox in node type form to enable/disable node locking and depending on that automatically create or remove nodelock field. That would require less actions from the user and so the module would be more user friendly;
5. Some faster way to change the locked status of a node rather than opening the node edit form would be nice to have.

Apart from above things the intended functionality was working for me.
Good luck!

Closing due to lack of activity. Feel free to reopen if you are still working on this application.