[drupal-devel] [bug] Node.module does not fire event when a revision is deleted.

[Junyor]

Issue status update for 
http://drupal.org/node/31323
Post a follow up: 
http://drupal.org/project/comments/add/31323

 Project:      Drupal
 Version:      cvs
 Component:    node.module
 Category:     bug reports
 Priority:     critical
 Assigned to:  Anonymous
 Reported by:  Souvent22
 Updated by:   Junyor
-Status:       patch (code needs review)
+Status:       patch (code needs work)

When I delete a revision, I get the following message:


"Deleted revision ()."


and in watchdog:


": deleted revision ()."


>From a quick look at the code, it seems that the node is deleted before
the messages are created.


Also, there's no 'delete-revision' in upload_nodeapi, so files
associated with the deleted revision are never deleted.




Junyor



Previous comments:
------------------------------------------------------------------------

Thu, 15 Sep 2005 17:36:16 +0000 : Souvent22

Attachment: http://drupal.org/files/issues/node_revisions.patch (1.39 KB)

When you delete a revision, nothing is fired to tell other modules that
a revision of that node has been deleted. This patch fires a
_invoke_nodeapi" so that other modules will know, and can catch the
event that a revision of a node has been deleted, and then take the
correct action. This is needed for other modules such as
'upload.module', which keeps files for each revision. This patch also
creates a watchdog entry when a revision is deleted.


I'm sure the wording of the watch dog entry, if it should be watched,
_invoke vs another paramenter, etc. will be argued, but I wanted to get
this up, so I can hear the arguments, and get this quared away and
patched it. I think this is really important. Thanks.




------------------------------------------------------------------------

Thu, 15 Sep 2005 17:43:01 +0000 : Souvent22

Attachment: http://drupal.org/files/issues/clean_node.revisions.patch (1.39 KB)

Got fuzz warning, cleaned up my patch. Samething, just patches easier.




------------------------------------------------------------------------

Thu, 15 Sep 2005 17:55:28 +0000 : Souvent22

Attachment: http://drupal.org/files/issues/clean_node.revisions_0.patch (1.32 KB)

Changed working for the watchdog and for the messege that reported after
deletion.




------------------------------------------------------------------------

Thu, 15 Sep 2005 18:14:40 +0000 : Souvent22

Attachment: http://drupal.org/files/issues/clean_node.revisions_1.patch (1.44 KB)

Some more wording issues, but i think this is ready to go.




------------------------------------------------------------------------

Thu, 15 Sep 2005 18:25:21 +0000 : m3avrck

Attachment: http://drupal.org/files/issues/clean_node.revisions_1_0.patch (1.44 KB)

Better patch, gets rid of extraneous parentheses.




------------------------------------------------------------------------

Thu, 15 Sep 2005 18:32:52 +0000 : jvandyk

Some quick observations:


- move the node_load to just in front of the node_invoke_nodapi call,
as it's unnecessary unless the if statement succeeds
- no need to comment that you are going to call node_invoke_nodeapi;
just call it


The error about deleting the current revision seems odd. If I ask to
delete the current revision, I should maybe be redirected to the node
deleting page?




------------------------------------------------------------------------

Thu, 15 Sep 2005 18:38:18 +0000 : m3avrck

jvandyk, I'm not sure I understand your last comment about the delete
part. It seemed intuitive to me how the patch worked, did you actually
try it out yet? It worked as I expected it to work. I agree with your
other comments though.




------------------------------------------------------------------------

Thu, 15 Sep 2005 18:40:27 +0000 : Souvent22

Attachment: http://drupal.org/files/issues/clean_node.revisions_2.patch (1.43 KB)

thanks for the comments. I'd think after deleting a node, you'd go back
to the revisions listing for that node.




------------------------------------------------------------------------

Fri, 16 Sep 2005 09:12:40 +0000 : killes at www.drop.org

This is a lose end of the revisions patch that needs fixing. i am ok
with the proposed patch. I think that Dries will object to
node_invoke_nodeapi($node, 'delete-revision');
and would maybe prefer
node_invoke_nodeapi($node, 'revision delete');