Files: 
CommentFileSizeAuthor
#7 node-1946434-7.patch32.07 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 57,093 pass(es).
[ View ]
#2 node-confirm-form-1946434-2.patch9.94 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 53,269 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Comments

Assigned:Unassigned» tim.plunkett

Going to try this.

StatusFileSize
new9.94 KB
FAILED: [[SimpleTest]]: [MySQL] 53,269 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Status:Active» Needs review

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.)

Status:Needs review» Postponed

Status:Postponed» Needs work

Forgot to unpostpone this! Still on this.

Status:Needs work» Needs review
StatusFileSize
new32.07 KB
PASSED: [[SimpleTest]]: [MySQL] 57,093 pass(es).
[ View ]

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.

Status:Needs review» Reviewed & tested by the community

:)

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.

Status:Reviewed & tested by the community» Fixed

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.

Assigned:tim.plunkett» Unassigned

Automatically closed -- issue fixed for 2 weeks with no activity.