I have some nodes that get modified programatically. In same cases, I don't want the lock to be released when node_save is called. However, I do want to lock to be released in other cases (mainly when node_save is called from a form).
What I need is a way to disable the release of a lock in a certain context.
I looked at hook_content_lock_node_type_blacklist_alter, but that caches the result, so dynamic control of specific node ids would only work if the cache can be overridden. Also, using a callback would require global variables in the calling module to pass the state from the calling context to the callback.
So here is my solution: In the calling module, I set a new custom field in the node object to be saved:
$node->content_lock_preserve_lock = TRUE;
node_save($node);
unset($node->content_lock_preserve_lock);
In content_lock.module, I added a check for this variable to the top of _content_lock_is_lockable_node:
function _content_lock_is_lockable_node($node) {
static $lockable;
if ($node->content_lock_preserve_lock)
return FALSE;
//...
I don't know if it is allowed by Drupal standards to store custom variables in the node object, but it seems to have no side effects, and it seems to be the easiest solution I can think of.
Any thoughts?
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | content_lock-skip-release.patch | 1.95 KB | ohnobinki |
Comments
Comment #1
eugenmayer commentedThats an good idea, but i would change the implementation to be a "veto" hook. So if one of the hooks returns TRUE, lock is not remuffed ( ORed).
Comment #2
Bodo Maass commentedI already realised that I need this in other cases than node_save. So for me it would be good to keep the code as above, and simply rename the variable into $node->content_lock_veto_lock = TRUE.
Would that be an acceptable implementation, or do you think it should be a real hook that would be invoked at the top of function _content_lock_is_lockable_node?
The downside of a real hook is that it makes the implementation of the calling module more complicated, because it has to set a global state from within the context in which the lock is called so that the hook function knows about the state of the node in question. Storing the state in the node itself seems more elegant to me, but as I said, I don't know if that is even allowed.
Comment #3
eugenmayer commentedEventhough it is allowed, the hook implementation is much more flexible. If you have more then one module thinking / checking for a veto, values could be overriden in $node->content_lock_veto_lock without any control our knowledge. There is no way then to enforce "OR" logic. I would be glad if we could go the veto-way. You can see that in the code fo wysiwyg_imageupload uppon a file-deletion
Comment #4
Bodo Maass commentedI agree now that a hook is the proper way to go because I'm running into some scenarios where the simple contextual approach is not enough. I'll play with this more and send a patch in a while.
Comment #5
ohnobinki commentedHere's at least a patch to start from.
It adds hook_content_lock_skip_release($node) which is called only from content_lock_nodeapi(). Thus, it allows a module to arbitrarily disable the normal release of the lock upon node_save() or the user clicking the “Save” button on the form edit page.
It does not affect the ajax lock releases nor an admin or user clicking “release lock”. I think that it is fine to handle only the hook_nodeapi() case because this is targeted at preventing modules calling node_save() from messing unintentionally releasing locks. Would there be a reason to prevent lock releases via these other mechanisms?
Comment #6
eugenmayer commentedSounds just like perfect. thats was exectly what i had in mind. Very good work !
Comment #7
Bodo Maass commentedI would rather have the hook called from _content_lock_is_lockable_node and have it more generic.
So the hook could be called hook_content_lock_is_lockable, and it would prevent locking and unlocking of the node. At least that would fit with my current use case.
Comment #8
ohnobinki commentedComment #9
ohnobinki commented@Bobo Maass: Before going further, I can't help but think that node lockability and unlockability are two different things. Here is my issue with hook_content_lock_is_lockable() being able to prevent a node from being unlocked:
If I write a hook_content_lock_is_lockable(), the return value of that hook would control whether or not content_lock does anything at all with a node. If someone locks a node and then the node's lockability is changed so that the node is not lockable, then I would expect content_lock to completely ignore that a lock was ever made. I.e., if there is a node which is both locked and unlockable, then the lock will not be enforced because disabling lockability disables the logic which checks if a node is locked or not.
However, I do think that it is important to make _content_lock_is_lockable_node() extendable. So your proposed hook will be integrated.
But, I think either an additional hook or some change to this hook's API should be made if you want a module to be a consumer of node locks in the stead of a user.
Comment #10
ohnobinki commentedThe above hook is added in commit 4b62177.
Please create a new bug if you still need the functionality of both 1. preventing a node from being unlocked and 2. making that node act as if it is locked at the same time. With this commit, one can make a node both locked and not lockable at the same time, which means that users can freely edit the node and hack over eachother. Thus, this hook does not solve the originally posed issue even though it does solve the issue described bug's Summary.