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: Souvent22 Status: patch (code needs review) Attachment: http://drupal.org/files/issues/clean_node.revisions.patch (1.39 KB) Got fuzz warning, cleaned up my patch. Samething, just patches easier. Souvent22 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.

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: Souvent22 Status: patch (code needs review) 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. Souvent22 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.

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: jvandyk Status: patch (code needs review) 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? jvandyk 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.

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: Souvent22 Status: patch (code needs review) 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. Souvent22 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.

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@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');