Figure out why we need to delete related comments from NodeStorage::delete(). The core NodeStorage handler doesn't seem to need this (it's probably done via some hook somewhere). But for some reason that hook (or whatever other mechanism) doesn't seem to fire when using Multiversion's NodeStorage handler.

  /**
   * {@inheritdoc}
   *
   * @todo: {@link https://www.drupal.org/node/2597534 Figure out why we need
   * this}, core seems to solve it some other way.
   */
  public function delete(array $entities) {
    // Delete all comments before deleting the nodes.
    $comment_storage = \Drupal::entityManager()->getStorage('comment');
    foreach ($entities as $entity) {
      if ($entity->comment) {
        $comments = $comment_storage->loadThread($entity, 'comment', 1);
        $comment_storage->delete($comments);
      }
    }
    $this->deleteEntities($entities);
  }

Once this is figured out we should be able to remove the NodeStorage handler for good, since this is the last method we override in NodeStorage.

CommentFileSizeAuthor
#10 2597534-10.patch4.02 KBbenjy
#6 2597534-6.patch1.8 KBbenjy

Comments

stevector created an issue. See original summary.

dixon_’s picture

Priority: Normal » Major
dixon_’s picture

Title: Remove NodeStorage class » Figure out why we need to delete related comments from NodeStorage::delete
Issue summary: View changes
dixon_’s picture

Priority: Major » Normal
benjy’s picture

I think this happens in comment_entity_predelete which is never fired because EntityStorageBase::delete() is never called.

I wonder if it would be better for the storage to override doDelete so all the hooks still fire?

benjy’s picture

Status: Active » Needs review
StatusFileSize
new1.8 KB

Heres a patch, see what the tests say.

Status: Needs review » Needs work

The last submitted patch, 6: 2597534-6.patch, failed testing.

The last submitted patch, 6: 2597534-6.patch, failed testing.

The last submitted patch, 6: 2597534-6.patch, failed testing.

benjy’s picture

Status: Needs work » Needs review
StatusFileSize
new4.02 KB

Status: Needs review » Needs work

The last submitted patch, 10: 2597534-10.patch, failed testing.

The last submitted patch, 10: 2597534-10.patch, failed testing.

The last submitted patch, 10: 2597534-10.patch, failed testing.

The last submitted patch, 10: 2597534-10.patch, failed testing.

The last submitted patch, 10: 2597534-10.patch, failed testing.

The last submitted patch, 10: 2597534-10.patch, failed testing.

dpovshed’s picture