From c4683bb3babe088b9a95df34c5f8a950e7dc9431 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Tue, 2 Jun 2015 13:30:29 +0200 Subject: [PATCH 1/3] $notice->id is always set here! Added assert() call to possibly review bugs in the future. --- QvitterPlugin.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/QvitterPlugin.php b/QvitterPlugin.php index 72ed426..daaab26 100644 --- a/QvitterPlugin.php +++ b/QvitterPlugin.php @@ -663,6 +663,7 @@ class QvitterPlugin extends Plugin { * @return boolean hook flag */ function onStartNoticeDistribute($notice) { + assert($notice->id > 0); // since we removed tests below // don't add notifications for activity type notices if($notice->object_type == 'activity') { @@ -699,7 +700,7 @@ class QvitterPlugin extends Plugin { if($notice->reply_to) { $replyparent = $notice->getParent(); $replyauthor = $replyparent->getProfile(); - if ($replyauthor instanceof Profile && !empty($notice->id)) { + if ($replyauthor instanceof Profile) { $reply_notification_to = $replyauthor->id; $this->insertNotification($replyauthor->id, $notice->profile_id, 'reply', $notice->id); } @@ -719,7 +720,7 @@ class QvitterPlugin extends Plugin { $all_mentioned_user_ids[] = $mentioned->id; // only notify if mentioned user is not already notified for reply - if($reply_notification_to != $mentioned->id && !empty($notice->id)) { + if($reply_notification_to != $mentioned->id) { $this->insertNotification($mentioned->id, $notice->profile_id, 'mention', $notice->id); } } From d69453f846ef01c50a398d9d4cad572986b3f51f Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Tue, 2 Jun 2015 14:04:22 +0200 Subject: [PATCH 2/3] Exception handling for notice parent fetching --- QvitterPlugin.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/QvitterPlugin.php b/QvitterPlugin.php index daaab26..750f293 100644 --- a/QvitterPlugin.php +++ b/QvitterPlugin.php @@ -698,13 +698,17 @@ class QvitterPlugin extends Plugin { $reply_notification_to = false; // check for reply to insert in notifications if($notice->reply_to) { - $replyparent = $notice->getParent(); - $replyauthor = $replyparent->getProfile(); - if ($replyauthor instanceof Profile) { + try { + $replyauthor = $notice->getParent()->getProfile(); $reply_notification_to = $replyauthor->id; $this->insertNotification($replyauthor->id, $notice->profile_id, 'reply', $notice->id); - } + //} catch (NoParentNoticeException $e) { // TODO: catch this when everyone runs latest GNU social! + // This is not a reply to something (has no parent) + } catch (NoResultException $e) { + // Parent notice or its author's profile not found! Complain louder? + common_log(LOG_ERR, 'NoResultException: '.$e->getMessage()); } + } // check for mentions to insert in notifications $mentions = common_find_mentions($notice->content, $notice); From 2c24b705de8ea309aa4246fae02c88d8ecfb7d1b Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Tue, 2 Jun 2015 14:44:44 +0200 Subject: [PATCH 3/3] Correct error log message --- QvitterPlugin.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/QvitterPlugin.php b/QvitterPlugin.php index 750f293..8e0cc14 100644 --- a/QvitterPlugin.php +++ b/QvitterPlugin.php @@ -705,8 +705,8 @@ class QvitterPlugin extends Plugin { //} catch (NoParentNoticeException $e) { // TODO: catch this when everyone runs latest GNU social! // This is not a reply to something (has no parent) } catch (NoResultException $e) { - // Parent notice or its author's profile not found! Complain louder? - common_log(LOG_ERR, 'NoResultException: '.$e->getMessage()); + // Parent author's profile not found! Complain louder? + common_log(LOG_ERR, "Parent notice's author not found: ".$e->getMessage()); } }