Remove node_save() and node_delete()/node_delete_multiple() in favor of $node->save()/$node->delete()

Thanks for working on this.

Yes, the functions should be removed.

You should be able to convert those in that test as well. If you can call node_save($whatever['thing']), then $whatever['think'] is a node object, however it might be named, so you can call ->save() on that and it should work.

Weird. I can't find a request that actually uses that in the tests, only one that loads it. Try to change it to menu_test_callback and then let's see if anything breaks.

index 0000000..5080fa9
--- /dev/null

--- /dev/null
+++ b/.gitignoreundefined

+++ b/.gitignoreundefined
@@ -0,0 +1,2 @@

@@ -0,0 +1,2 @@
+sites
+sites/*
\ No newline at end of file

Looks like you accidently added your .gitignore file to the patch. I suggest you add .gitignore to .gitignore, then it will ignore itself or add these rules to .git/info/exclude

+++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentLinksTest.phpundefined
@@ -168,7 +168,8 @@ function setEnvironment(array $info) {
-      node_save($this->node);
+      $node = $this->node;
+      $node->save();

You don't need to assign it to $node, you can simply call $this->node->save().

+++ b/core/modules/node/lib/Drupal/node/Tests/NodeRevisionPermissionsTest.phpundefined
@@ -51,7 +51,8 @@ function setUp() {
-        node_save($revision);
+        $node = node_load($revision->nid);
+        $node->save();

Same here. Everything that's passed to node_save() must be a node object, even if it's not named like that, so $revision->save() should work fine.

+++ b/core/modules/node/node.api.phpundefined
@@ -39,7 +39,7 @@
- * - Creating a new node (calling node_save() on a new node):
+ * - Creating a new node (calling $node->save() on a new node):

This looks quite readable but I'm not sure if this is valid, will have to check with someone that knows these documentation rules better.

+++ b/core/modules/node/node.api.phpundefined
@@ -518,7 +518,7 @@ function hook_node_revision_delete(\Drupal\Core\Entity\EntityInterface $node) {
- * This hook is invoked from node_save() after the database query that will
+ * This hook is invoked from $node->save() after the database query that will

This is actually not really true anymore. It should simply say something like "from the storage controller after the node was saved". If you're not sure, leave this part for now and I'll try to re-work it later.
Same for the other hook documentations in node.api.php

Looks good otherwise, I guess you still need to convert the node_save()'s in that search comment test. If you click on view details and then expand the detail log at the end, you can see that it dies with a fatal error trying to call that function somewhere.

Thanks for working on this, appreciated!

Note that you can upload the node_delete() changes as part of the same patch. Except we want to define that we should open a separate issue for it. Which might not be the dumbest idea (the only problem is that it could result in unnecessary conflicts between the two patches).

So if you want to, feel free to edit the original issue title and description, remove delete and create a separate issue for it. Your decision :)

+++ b/core/modules/node/node.moduleundefined
@@ -1001,21 +1001,9 @@ function node_save(EntityInterface $node) {
 function node_delete($nid) {
-  node_delete_multiple(array($nid));
+  entity_delete_multiple('node', array($nid));

You can remove this as well and call $node->delete() instead. As this does just receive the node id, you might have to add some node_load()'s but only where you don't have a node object already.

Exactly, $node->delete() instead of node_delete($node->nid).

Looks good, a few (like 3 I think) cases that can be simplified, see #13.

Will try to find someone tomorrow for the documentation thing, I'm not sure myself. The problem with $node->save(), although it makes kinda sense is that it's not referencable to anything. But maybe that's fine, or maybe we just want to make some proper english out of it, e.g. "while saving a node". But there are some references that are supposed to reference where this actually happens, but they stopped making sense a long time ago anyway (node_save() isn't where the fun stuff happens, that's in the storag controller but that's hard to reference).

#28: 1999322-28-node_save-and-node_delete-conversions.patch queued for re-testing.

#28: 1999322-28-node_save-and-node_delete-conversions.patch queued for re-testing.

Ok, this looks good. Discussed this with a few people and this is good to go.

Improving the hook documentation there to something that makes sense should happen in #1824184: Remove and replace entity type specific API hook documentation with generic entity API hook documentation

We already have https://drupal.org/node/1400186, added a list of related removed functions there.

Adding every issue that's related to that is going to be too much...

Removing tag.