Dear All,

There is an issue with comment delete in this module. The problem is with the code in the notify.module.

Line number: 172

function notify_comment_delete($comment) {
  \Drupal::database()->delete('notify_unpublished_queue')
    ->condition('cid', $comment->cid)
    ->execute();
}

The $comment->cid should be $comment->id(); there is no variable cid in $comment.

I created a patch for the same

Thanks
Sumith Babu

CommentFileSizeAuthor
#7 notify-comment-delete-3067931-7.patch380 bytesgisle
#4 Notify hunk.png30.52 KBkrina.addweb
#4 comment-delete-4.patch404 byteskrina.addweb
comment-delete.patch372 bytessumithb
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sumithb created an issue. See original summary.

sumithb’s picture

sumithb CreditAttribution: sumithb commented
Issue summary: View changes
gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
CreditAttribution: gisle at Hannemyr Nye Medier AS commented
Status: Active » Needs review

Thanks for the patch!
Changing status.

krina.addweb’s picture

krina.addweb CreditAttribution: krina.addweb at AddWeb Solution Pvt. Ltd. commented
FileSize
comment-delete-4.patch404 bytes
Notify hunk.png30.52 KB

@sumithb, As the attached patch throwing hunk error while checking in my local system, I created the new patch for the same & it is working fine for me.

sumithb’s picture

sumithb CreditAttribution: sumithb commented

@krina.addweb thanks for updating the patch. I think I missed the last bracket while creating the patch file.

baikho’s picture

baikho
Primary language Dutch
CreditAttribution: baikho as a volunteer and at Access commented
Status: Needs review » Reviewed & tested by the community

#4 fixes the issue. Marking RTBC

gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
CreditAttribution: gisle at Hannemyr Nye Medier AS commented
FileSize
notify-comment-delete-3067931-7.patch380 bytes

The reason the patch attached to the issue summary fails is because it does not use the conventional a/ and b/ prefixes for the name of the file. I.e. it starts out with:

diff --git notify.module notify.module
index 1643a74..b6fbe76 100644
--- notify.module
+++ notify.module

instead of:

diff --git a/notify.module b/notify.module
index 1643a74..b6fbe76 100644
--- a/notify.module
+++ b/notify.module

This makes git apply throw the following error:

fatal: git diff header lacks filename information when removing 1 leading pathname component (line 5)

The patch in #4 applies cleanly, but breaks the indenting code standard. Attached is a corrected version of the initial patch by sumithb.

  • gisle committed 7b60ad2 on 8.x-1.x authored by sumithb 
    Issue #3067931 by sumithb, krina.addweb, baikho: Fixed error with...
gisle’s picture

gisle
he/him
Primary language Norwegian Bokmål
Location Norway
CreditAttribution: gisle at Hannemyr Nye Medier AS commented
Status: Reviewed & tested by the community » Fixed

Pushed to the latest snapshot of 8.x-1.x-dev.

Status: Fixed » Closed (fixed)

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