node_delete doesn't pass params to _delete calls.

Right, this patch is so the modification I made to event.module works.

I added functionality for repeating events to event.module, and in order for the delete to work I had to make this small change outside of event.module (believe me I really tried not to). What this patch does is pass variables originally passed to the delete function to whats called after the user clicks confirm.

In other words when a user clicks delete the first thing they get is a prompt to confirm deletion, after the user clicks yes, the delete function is called, but the only variables that are passed are nid and confirmed, all other variables are lost, which is usually ok because no other vars needed for deletion, however in my case I needed another variable.

This patch has no effect on all current modules, the reason i made this a bug report is because it seems to me like variables originally passed to node_delete should still exist after the user clicks confirm, even though current modules dont use other vars.

I made a reference to this bug from this bug: http://drupal.org/node/view/4122

If the changes I made to event.module ever get put into the current cvs version, then this patch also needs to get put in.

-Darrian

$node is passed, but after the user clicks confirm, $node only has two fields, nid and confirm. I need an extra field, which is whether I'm supposed to delete only this event, or all events in the group. This is for the reocurring events feature I added. My modification adds the extra fields originally in $node back to $node after the user clicks confirm.

-Darrian

In order for my repeating/reocurring event module patch to get submitted to cvs, this very small patch needs to make it into core. It does not effect the functionality of anything current.

Here's the details of what happens, >>> is what happens when my node patch is there:

1. User is looking at the form to delete an existing event, there are a bunch of fields like date/time title, etc. This holds true for any node, but I am going to reference an event node.
2. User clicks delete to delete the node being looked at. (User checks to delete all events in this group - from my event patch)
3. All the fields for that node are a form element at this point, some hidden, like nid, and a variable I set telling the event_delete function to delete all events in this group.
4. The code makes its way to the node_delete($edit) function. At this point all the form elements from above are members of the $edit variable, including my variable mentioned above.
5. The code checks to see if the user already clicked confirm, at this point they haven't.
6. Now we're in the else branch that creates only two form elements, the nid, and confirm. This has been ok in the past, because the only thing that you needed to delete a node was its nid. But in my case I want to delete multiple nodes, so this doesnt work.
>>> My node patch creates hidden form elements of all the previous variables, not just nid and confirm.
7. The code makes it way back to node_delete, this time however there are only two members of $edit, nid and confirm.
8. The code checks to see if the user has clicked confirm, and they have.
9. Now we're in the true branch which deletes the core node components, then calls the node specific delete function, in our case the event_delete function, passing only the nid and confirm variables.
>>> My node patch makes it so all variables, including my varible to check whether we're deleting all nodes or not, are passed back in.
10. Now we're in the event_delete function. The event delete function checks for the variable to see if we're deleting all events in group, it doesnt exist, so it acts as if the user didn't check to delete all events, and deletes just this event.
>>> With my patch, all previous variables still exist, so event_delete knows that it needs to delete multiple nodes.

Hopefully this patch gets into core before feature freeze for the sake of the event module. If it doesnt, then my patch for repeating/reocurring events also cannot make it into cvs.

Thanks, Darrian

Does the code freeze mean that this patch wont make it into 4.5?

-Darrian

Remember, for node_delete, only nid and confirm are passed.

But I already do as you suggest, to be more specific:

node_load is called, those variables, along with extra fields that are not loaded with node_load (such as my variable for deleting group events), are passed to subsequent _delete calls.

Just to re-iterate, without this patch the ONLY variables that make it to the final delete call are nid and confirm.

-Darrian

Right, the point is to get variables from the form passed to the delete function, namely 1 variable, which tells me if I should delete 1 event, or all grouped reoccuring events, that I need for the modification I made to the event module.

The way I made the modification is very UN-obtrusive. No variables get squashed, and everything current ignores the new variables.

Also, the variable that I want exists in the node_delete function the first time around, its only after the user clicks confirm that all variables except the nid are lost. So I could delete things with my modified event module if i skip the confirm step, but I'm sure that would bite someone down the road and I really dont want to do it that way.

-Darrian

Here's the UI i'm referring to. This patch to node.module is extremely small and unobtrusive... Why so much resistance to it?

Here's an updated patch for drupal 4.5.1

-Darrian

A group is in fact multiple nodes. I did it this way for a few reasons, one of them being other modules (such as remindme) work with it better this way.

I am no longer working on development of this feature, so I am not going to redesign the way its done. I only try and keep it updated so other people that need this feature can use it, seeing as there is no other alternative.

As for this particular patch to node.module I still think it is a bug because variables available the first time any _delete function is called dissappear after the confirm step. Those variables should be available to module developers regardless of how they are used.

-Darrian

I switched this to bug report so maybe someone would look at it again.

Forgetting about the changes i've made to the event module, can someone look at this patch to see if it's suitable to be put into core?

I've looked at this (node.module, node_delete) code, and code that uses this code and I cannot see anything wrong with my patch. It doesnt affect the way anything works, it doesn't affect security, its very small.

The only thing this patch does is increase the flexibilty of drupal and gives more options to module developers.

Thanks,
Darrian

Here is an updated patch for drupal version 4.6.1

-Darrian