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

tim.plunkett’s picture

Assigned:Unassigned» tim.plunkett

Going to try this.

tim.plunkett’s picture

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

Status:Active» Needs review
Crell’s picture

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

tim.plunkett’s picture

Status:Needs review» Postponed
tim.plunkett’s picture

Status:Postponed» Needs work

Forgot to unpostpone this! Still on this.

tim.plunkett’s picture

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.

jibran’s picture

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

tim.plunkett’s picture

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.

jibran’s picture

Status:Needs review» Reviewed & tested by the community

:)

pwolanin’s picture

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.

Dries’s picture

Status:Reviewed & tested by the community» Fixed

Committed to 8.x. Thanks!

tstoeckler’s picture

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

Crell’s picture

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

tstoeckler’s picture

tim.plunkett’s picture

Assigned:tim.plunkett» Unassigned

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