Convert all of confirm_form() in node.admin.inc and node.pages.inc to the new form interface

Going to try this.

Started here: http://drupalcode.org/sandbox/tim.plunkett/1698392.git/shortlog/refs/hea...

We'll need to have #1947432: Add a generic EntityAccessCheck to replace entity_page_access() and #111715: Convert node/content types into configuration first. I'll postpone this after the bot comes back.

For the bot.

+++ b/core/modules/node/lib/Drupal/node/Form/ContentTypeDeleteForm.php
@@ -0,0 +1,87 @@
+/**
+ * @todo.

It's not that hard, is it? ;-) Yes, needs a docblock.

+++ b/core/modules/node/lib/Drupal/node/Form/ContentTypeDeleteForm.php
@@ -0,0 +1,87 @@
+  /**
+   * The content type being deleted.
+   *
+   * @var \stdClass
+   */
+  protected $type;

$type is still a stdClass? We have those? Why? :-) (Not something to fix here; just noting it.)

+++ b/core/modules/node/lib/Drupal/node/Form/ContentTypeDeleteForm.php
@@ -0,0 +1,87 @@
+    $num_nodes = db_query("SELECT COUNT(*) FROM {node} WHERE type = :type", array(':type' => $this->type->type))->fetchField();

This implies we need an injected database object instead.

Although perhaps this suggests we need a higher level method on another service instead, since injecting the DB into the form here seems wrong.

+++ b/core/modules/node/lib/Drupal/node/Form/ContentTypeDeleteForm.php
@@ -0,0 +1,87 @@
+    // Delete the content type.
+    node_type_delete($this->type->type);

Is this not a service yet?

+++ b/core/modules/node/lib/Drupal/node/Form/ContentTypeDeleteForm.php
@@ -0,0 +1,87 @@
+    menu_router_rebuild();

I don't think menu_router_rebuild() addresses the new routing system, so we need to do that here, too. (That is a service.)

I think every single thing except my lazy @todo that @Crell points out is part of #512026: Move $form_state storage from cache to new state/key-value system #111715: Convert node/content types into configuration.

Forgot to unpostpone this! Still on this.

Completely re-written from scratch, since all of the confirm forms for node types were handled in the ConfigEntity conversion.

This looks awesome RTBC for me but can you explain these maybe I am wrong.

+++ b/core/modules/node/lib/Drupal/node/Access/NodeRevisionAccessCheck.phpundefined
@@ -0,0 +1,158 @@
+   * @var \Drupal\Core\Entity\EntityStorageControllerInterface

I think it should be NodeStorageController

+++ b/core/modules/node/lib/Drupal/node/Access/NodeRevisionAccessCheck.phpundefined
@@ -0,0 +1,158 @@
+   * @var \Drupal\Core\Entity\EntityAccessControllerInterface

And I think it should be NodeAccessController

No, we don't typehint with class, only with interfaces.
There is nothing special about NodeStorageController, which is why it doesn't have its own interface.

:)

Having discussion with effulgentia, wim, and jbeach - we need a follow-up to remove the duplication here and generate the entity routes (or at least path patterns) based on the entity annotation.

Committed to 8.x. Thanks!

+++ b/core/modules/node/lib/Drupal/node/Access/NodeRevisionAccessCheck.php
@@ -0,0 +1,158 @@
+      if ($node->isDefaultRevision() && ($this->connection->query('SELECT COUNT(*) FROM {node_field_revision} WHERE nid = :nid AND default_langcode = 1', array(':nid' => $node->id()))->fetchField() == 1 || $op == 'update' || $op == 'delete')) {

Similar to #4: Shouldn't this query be in a method on the storage controller? If so, I'll open a follow-up issue, just wanted to check first.

Actually, it looks like it should be a method on NodeInterface.

Opened #2040545: Move database query out of \Drupal\node\Access\NodeRevisionAccessCheck for that.