Inconsistent handling of &$node in hook_nodeapi on $op

Good move. Making data added by hook_node_load() available to hook_node_(insert|update)() makes total sense.

About the patch, the code should probably go in node_form_submit_build_node(), not in node_submit(), because that's really an operation we need to do when building the $node object from the form values, which is the job of node_form_submit_build_node().

In my opinion, this patch is nothing more than an ugly bandaid on a much larger fundamental problem. The bandaid might work but I think it'll serve to help obscure the fundamental problem, which is that our workflow is broken.

The part that's broken in our workflow is that our 'submit' function assumes it will be writing to the database, and that it creates a node object from $form_state['values'] via a cast (object) rather than properly modifying an existing node. This breaks our CRUD (node_save() expects form input, not a true $node object) and it also makes the preview process *hell* because during preview, the $node object you receive isn't a real node, but a converted form array. If you want to see how hellish that is, go check out code in taxonomy.module that exists solely to detect whether the node is being displayed in preview or not because in the normal case, $node->taxonomy is an array of taxonomy term objects; but in the broken preview case, it is an array of taxonomy tids from form input.

IMO Eclipse can fix his problem with a little form_alter, which puts the fields he needs into the $form with '#type' => 'value' -- that requires no core patch at all and 'update' will now get his values just like they were supposed to.

The real fix for this, IMO, is to tell node_form_submit() that its responsibility is NOT to save the form, but instead to build up a node object, and the $form_state can be examined by the caller of the node_form to see what action to take: i.e, display a preview and rerender the form, or save the newly built $node object. node_save already has nodeapi hooks and as a bonus, we can fix our CRUD by removing reliance upon the form for saving data, meaning we might truly be able to build and modify $node objects programmatically without worrying that some module assumes a form only.

Please note that one of the purposes of my modifications to FAPI was to allow the possibility of moving node_save() out of the _submit so that our preview path could become sane again.

I'd say the patch in #8 makes sense. This way $node elements not present in the form keep their existing values, instead of currently requiring ugly '#type' => 'value' form elements. Field API works around that by requiring field storage engine not to alter existing values for fields not present in the incoming 'node'.

Also, while you can only agree with merlinofchaos about the awfulness of the dumb 'form values to object' cast, I don't think the patch is closely related. It does its thing aside of the awfulness, but does not add to it, nor takes us in a direction that would make the awfulness harder to clean. If "The real fix for this, IMO, is to tell node_form_submit() that its responsibility is NOT to save the form, but instead to build up a node object", then this task still would have to apply incoming form values onto the existing node to nuild up the new one.

subscribing.

The patch, as is, cleans up a nasty inconsistency with node_submit() that requires contrib modules to use node_load to verify certain aspects of the data. This is a pretty clean solution that adds consistency to the API.

I would, however, like to see what merlinofchaos has in mind, in the form of a patch, so we could compare the two "fixes."

Fixed in #592572: Always pass nodes as objects.