From 660e8c6efcee8278aee23f378cb9eab426837ec7 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Tue, 3 Feb 2015 16:28:33 +0100 Subject: [PATCH] Fave::addNew now calls Notice::saveActivity as a bonus we've fixed several FIXME issues for favorite email notification and updated parts of the codebase for these activities to a more modern style. --- lib/mail.php | 67 ------------- plugins/AnonymousFave/actions/anonfavor.php | 9 +- plugins/Favorite/EVENTS.txt | 5 - plugins/Favorite/FavoritePlugin.php | 95 ++++++++++++++++--- .../Favorite/actions/apifavoritecreate.php | 87 +++-------------- .../Favorite/actions/atompubshowfavorite.php | 12 +-- plugins/Favorite/actions/favor.php | 31 +----- plugins/Favorite/classes/Fave.php | 56 ++++++----- plugins/Favorite/lib/favcommand.php | 13 --- plugins/YammerImport/lib/yammerimporter.php | 7 +- 10 files changed, 141 insertions(+), 241 deletions(-) diff --git a/lib/mail.php b/lib/mail.php index 68ac8a74cd..188792d02a 100644 --- a/lib/mail.php +++ b/lib/mail.php @@ -660,73 +660,6 @@ function mail_notify_message($message, $from=null, $to=null) return mail_to_user($to, $subject, $body, $headers); } -/** - * Notify a user that one of their notices has been chosen as a 'fave' - * - * Doesn't check that the user has an email address nor if they - * want to receive notification of faves. Maybe this happens higher - * up the stack...? - * - * @param User $rcpt The user whose notice was faved - * @param Profile $sender The user who faved the notice - * @param Notice $notice The notice that was faved - * - * @return void - */ -function mail_notify_fave(User $rcpt, Profile $sender, Notice $notice) -{ - // This test is actually "if the sender is sandboxed" - if (!$sender->hasRight(Right::EMAILONFAVE)) { - return; - } - - if ($rcpt->hasBlocked($sender)) { - // If the author has blocked us, don't spam them with a notification. - return; - } - - if (!$rcpt->getPref('email', 'notify_fave', 1)) { - return; - } - - $bestname = $sender->getBestName(); - - common_switch_locale($rcpt->language); - - // TRANS: Subject for favorite notification e-mail. - // TRANS: %1$s is the adding user's long name, %2$s is the adding user's nickname. - $subject = sprintf(_('%1$s (@%2$s) added your notice as a favorite'), $bestname, $sender->getNickname()); - - // TRANS: Body for favorite notification e-mail. - // TRANS: %1$s is the adding user's long name, $2$s is the date the notice was created, - // TRANS: %3$s is a URL to the faved notice, %4$s is the faved notice text, - // TRANS: %5$s is a URL to all faves of the adding user, %6$s is the StatusNet sitename, - // TRANS: %7$s is the adding user's nickname. - $body = sprintf(_("%1\$s (@%7\$s) just added your notice from %2\$s". - " as one of their favorites.\n\n" . - "The URL of your notice is:\n\n" . - "%3\$s\n\n" . - "The text of your notice is:\n\n" . - "%4\$s\n\n" . - "You can see the list of %1\$s's favorites here:\n\n" . - "%5\$s"), - $bestname, - common_exact_date($notice->created), - common_local_url('shownotice', - array('notice' => $notice->id)), - $notice->content, - common_local_url('showfavorites', - array('nickname' => $sender->getNickname())), - common_config('site', 'name'), - $sender->getNickname()) . - mail_footer_block(); - - $headers = _mail_prepare_headers('fave', $rcpt->getNickname(), $sender->getNickname()); - - common_switch_locale(); - mail_to_user($rcpt, $subject, $body, $headers); -} - /** * Notify a user that they have received an "attn:" message AKA "@-reply" * diff --git a/plugins/AnonymousFave/actions/anonfavor.php b/plugins/AnonymousFave/actions/anonfavor.php index c9282f15f6..4029154d72 100644 --- a/plugins/AnonymousFave/actions/anonfavor.php +++ b/plugins/AnonymousFave/actions/anonfavor.php @@ -66,14 +66,9 @@ class AnonFavorAction extends RedirectingAction // TRANS: Client error. throw new AlreadyFulfilledException(_m('This notice is already a favorite!')); } - $fave = Fave::addNew($profile, $notice); - if (!$fave instanceof Fave) { - // TRANS: Server error. - $this->serverError(_m('Could not create favorite.')); - } - - Fave::blowCacheForProfileId($profile->id); + // Throws exception + $stored = Fave::addNew($profile, $notice); if ($this->boolean('ajax')) { $this->startHTML('text/xml;charset=utf-8'); diff --git a/plugins/Favorite/EVENTS.txt b/plugins/Favorite/EVENTS.txt index c15839c8a0..115324b58b 100644 --- a/plugins/Favorite/EVENTS.txt +++ b/plugins/Favorite/EVENTS.txt @@ -1,11 +1,6 @@ Events that come with the Favorite plugin ========================================= -StartFavorNotice: Saving a notice as a favorite -- $profile: profile of the person faving (can be remote!) -- $notice: notice being faved -- &$fave: Favor object; null to start off with, but feel free to override. - EndFavorNotice: After saving a notice as a favorite - $profile: profile of the person faving (can be remote!) - $notice: notice being faved diff --git a/plugins/Favorite/FavoritePlugin.php b/plugins/Favorite/FavoritePlugin.php index 77f0531609..dabc6c5e81 100644 --- a/plugins/Favorite/FavoritePlugin.php +++ b/plugins/Favorite/FavoritePlugin.php @@ -226,19 +226,6 @@ class FavoritePlugin extends ActivityHandlerPlugin } } - protected function notifyMentioned(Notice $stored, array &$mentioned_ids) - { - require_once INSTALLDIR.'/lib/mail.php'; - - foreach ($mentioned_ids as $id) { - $mentioned = User::getKV('id', $id); - if ($mentioned instanceof User && $mentioned->id != $stored->profile_id - && $mentioned->email && $mentioned->getPref('email', 'notify_fave', $this->email_notify_fave)) { // do we have an email, and does user want it? - mail_notify_fave($mentioned, $stored->getProfile(), $stored->getParent()); - } - } - } - // API stuff /** @@ -380,6 +367,22 @@ class FavoritePlugin extends ActivityHandlerPlugin return true; } + public function onEndFavorNotice(Profile $actor, Notice $target) + { + try { + $notice_author = $target->getProfile(); + // Don't notify ourselves of our own favorite on our own notice, + // or if it's a remote user (since we don't know their email addresses etc.) + if ($notice_author->id == $actor->id || !$notice_author->isLocal()) { + return true; + } + $local_user = $notice_author->getUser(); + mail_notify_fave($local_user, $actor, $target); + } catch (Exception $e) { + // Mm'kay, probably not a local user. Let's skip this favor notification. + } + } + /** * EndInterpretCommand for FavoritePlugin will handle the 'fav' command * using the class FavCommand. @@ -510,3 +513,69 @@ class FavoritePlugin extends ActivityHandlerPlugin return true; } } + +/** + * Notify a user that one of their notices has been chosen as a 'fave' + * + * @param User $rcpt The user whose notice was faved + * @param Profile $sender The user who faved the notice + * @param Notice $notice The notice that was faved + * + * @return void + */ +function mail_notify_fave(User $rcpt, Profile $sender, Notice $notice) +{ + if (!$rcpt->receivesEmailNotifications() || !$rcpt->getPref('email', 'notify_fave', $this->email_notify_fave)) { + return; + } + + // This test is actually "if the sender is sandboxed" + if (!$sender->hasRight(Right::EMAILONFAVE)) { + return; + } + + if ($rcpt->hasBlocked($sender)) { + // If the author has blocked us, don't spam them with a notification. + return; + } + + // We need the global mail.php for various mail related functions below. + require_once INSTALLDIR.'/lib/mail.php'; + + $bestname = $sender->getBestName(); + + common_switch_locale($rcpt->language); + + // TRANS: Subject for favorite notification e-mail. + // TRANS: %1$s is the adding user's long name, %2$s is the adding user's nickname. + $subject = sprintf(_('%1$s (@%2$s) added your notice as a favorite'), $bestname, $sender->getNickname()); + + // TRANS: Body for favorite notification e-mail. + // TRANS: %1$s is the adding user's long name, $2$s is the date the notice was created, + // TRANS: %3$s is a URL to the faved notice, %4$s is the faved notice text, + // TRANS: %5$s is a URL to all faves of the adding user, %6$s is the StatusNet sitename, + // TRANS: %7$s is the adding user's nickname. + $body = sprintf(_("%1\$s (@%7\$s) just added your notice from %2\$s". + " as one of their favorites.\n\n" . + "The URL of your notice is:\n\n" . + "%3\$s\n\n" . + "The text of your notice is:\n\n" . + "%4\$s\n\n" . + "You can see the list of %1\$s's favorites here:\n\n" . + "%5\$s"), + $bestname, + common_exact_date($notice->created), + common_local_url('shownotice', + array('notice' => $notice->id)), + $notice->content, + common_local_url('showfavorites', + array('nickname' => $sender->getNickname())), + common_config('site', 'name'), + $sender->getNickname()) . + mail_footer_block(); + + $headers = _mail_prepare_headers('fave', $rcpt->getNickname(), $sender->getNickname()); + + common_switch_locale(); + mail_to_user($rcpt, $subject, $body, $headers); +} diff --git a/plugins/Favorite/actions/apifavoritecreate.php b/plugins/Favorite/actions/apifavoritecreate.php index e0c8c0cd83..736bd7b0a1 100644 --- a/plugins/Favorite/actions/apifavoritecreate.php +++ b/plugins/Favorite/actions/apifavoritecreate.php @@ -24,15 +24,14 @@ * @author Craig Andrews * @author Evan Prodromou * @author Zach Copley + * @author Mikael Nordfeldth * @copyright 2009 StatusNet, Inc. * @copyright 2009 Free Software Foundation, Inc http://www.fsf.org * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html GNU Affero General Public License version 3.0 - * @link http://status.net/ + * @link http://gnu.io/ */ -if (!defined('STATUSNET')) { - exit(1); -} +if (!defined('GNUSOCIAL')) { exit(1); } /** * Favorites the status specified in the ID parameter as the authenticating user. @@ -43,27 +42,22 @@ if (!defined('STATUSNET')) { * @author Craig Andrews * @author Evan Prodromou * @author Zach Copley + * @author Mikael Nordfeldth * @license http://www.fsf.org/licensing/licenses/agpl-3.0.html GNU Affero General Public License version 3.0 - * @link http://status.net/ + * @link http://gnu.io/ */ class ApiFavoriteCreateAction extends ApiAuthAction { var $notice = null; - /** - * Take arguments for running - * - * @param array $args $_REQUEST args - * - * @return boolean success flag - */ - function prepare($args) + protected $needPost = true; + + protected function prepare(array $args=array()) { parent::prepare($args); - $this->user = $this->auth_user; $this->notice = Notice::getKV($this->arg('id')); - if ($this->notice->repeat_of != '' ) { + if (!empty($this->notice->repeat_of)) { common_log(LOG_DEBUG, 'Trying to Fave '.$this->notice->id.', repeat of '.$this->notice->repeat_of); common_log(LOG_DEBUG, 'Will Fave '.$this->notice->repeat_of.' instead'); $real_notice_id = $this->notice->repeat_of; @@ -73,28 +67,9 @@ class ApiFavoriteCreateAction extends ApiAuthAction return true; } - /** - * Handle the request - * - * Check the format and show the user info - * - * @param array $args $_REQUEST data (unused) - * - * @return void - */ - function handle($args) + protected function handle() { - parent::handle($args); - - if ($_SERVER['REQUEST_METHOD'] != 'POST') { - $this->clientError( - // TRANS: Client error. POST is a HTTP command. It should not be translated. - _('This method requires a POST.'), - 400, - $this->format - ); - return; - } + parent::handle(); if (!in_array($this->format, array('xml', 'json'))) { $this->clientError( @@ -103,7 +78,6 @@ class ApiFavoriteCreateAction extends ApiAuthAction 404, $this->format ); - return; } if (empty($this->notice)) { @@ -113,7 +87,6 @@ class ApiFavoriteCreateAction extends ApiAuthAction 404, $this->format ); - return; } // Note: Twitter lets you fave things repeatedly via API. @@ -125,23 +98,10 @@ class ApiFavoriteCreateAction extends ApiAuthAction 403, $this->format ); - return; } - $fave = Fave::addNew($this->user->getProfile(), $this->notice); - - if (empty($fave)) { - $this->clientError( - // TRANS: Client error displayed when marking a notice as favourite fails. - _('Could not create favorite.'), - 403, - $this->format - ); - return; - } - - $this->notify($fave, $this->notice, $this->user); - Fave::blowCacheForProfileId($this->user->id); + // throws exception on failure + $stored = Fave::addNew($this->scoped, $this->notice); if ($this->format == 'xml') { $this->showSingleXmlStatus($this->notice); @@ -149,25 +109,4 @@ class ApiFavoriteCreateAction extends ApiAuthAction $this->show_single_json_status($this->notice); } } - - /** - * Notify the author of the favorite that the user likes their notice - * - * @param Favorite $fave the favorite in question - * @param Notice $notice the notice that's been faved - * @param User $user the user doing the favoriting - * - * @return void - */ - function notify($fave, $notice, $user) - { - $other = User::getKV('id', $notice->profile_id); - if ($other && $other->id != $user->id && !empty($other->email)) { - require_once INSTALLDIR.'/lib/mail.php'; - - mail_notify_fave($other, $user->getProfile(), $notice); - // XXX: notify by IM - // XXX: notify by SMS - } - } } diff --git a/plugins/Favorite/actions/atompubshowfavorite.php b/plugins/Favorite/actions/atompubshowfavorite.php index c71f1a11b3..2b8c9704aa 100644 --- a/plugins/Favorite/actions/atompubshowfavorite.php +++ b/plugins/Favorite/actions/atompubshowfavorite.php @@ -55,13 +55,13 @@ class AtompubshowfavoriteAction extends ApiAuthAction /** * For initializing members of the class. * - * @param array $argarray misc. arguments + * @param array $args misc. arguments * * @return boolean true */ - function prepare($argarray) + protected function prepare(array $args=array()) { - parent::prepare($argarray); + parent::prepare($args); $profileId = $this->trimmed('profile'); $noticeId = $this->trimmed('notice'); @@ -94,13 +94,11 @@ class AtompubshowfavoriteAction extends ApiAuthAction /** * Handler method * - * @param array $argarray is ignored since it's now passed in in prepare() - * * @return void */ - function handle($argarray=null) + protected function handle() { - parent::handle($argarray); + parent::handle(); switch ($_SERVER['REQUEST_METHOD']) { case 'GET': diff --git a/plugins/Favorite/actions/favor.php b/plugins/Favorite/actions/favor.php index 37668c989c..d1c9df34f5 100644 --- a/plugins/Favorite/actions/favor.php +++ b/plugins/Favorite/actions/favor.php @@ -74,14 +74,8 @@ class FavorAction extends FormAction throw new AlreadyFulfilledException(_('You have already favorited this!')); } - $fave = Fave::addNew($this->scoped, $this->target); - - if (empty($fave)) { - $this->serverError(_('Could not create favorite.')); - } - - $this->notify($fave, $this->target, $this->scoped); - Fave::blowCacheForProfileId($this->scoped->id); + // throws exception on failure + $stored = Fave::addNew($this->scoped, $this->target); return _('Favorited the notice'); } @@ -93,25 +87,4 @@ class FavorAction extends FormAction $disfavor->show(); } } - - /** - * Notify the author of the favorite that the user likes their notice - * - * @param Favorite $fave the favorite in question - * @param Notice $notice the notice that's been faved - * @param User $user the user doing the favoriting - * - * @return void - */ - function notify($fave, $notice, $user) - { - $other = User::getKV('id', $notice->profile_id); - if ($other && $other->id != $user->id && !empty($other->email)) { - require_once INSTALLDIR.'/lib/mail.php'; - - mail_notify_fave($other, $user->getProfile(), $notice); - // XXX: notify by IM - // XXX: notify by SMS - } - } } diff --git a/plugins/Favorite/classes/Fave.php b/plugins/Favorite/classes/Fave.php index 3f2ebe4727..9ab9f9337b 100644 --- a/plugins/Favorite/classes/Fave.php +++ b/plugins/Favorite/classes/Fave.php @@ -48,32 +48,29 @@ class Fave extends Managed_DataObject * @throws Exception on failure */ static function addNew(Profile $actor, Notice $target) { + $act = new Activity(); + $act->verb = ActivityVerb::FAVORITE; + $act->time = time(); + $act->id = self::newUri($actor, $target, common_sql_date($act->time)); + $act->title = _("Favor"); + // TRANS: Message that is the "content" of a favorite (%1$s is the actor's nickname, %2$ is the favorited + // notice's nickname and %3$s is the content of the favorited notice.) + $act->content = sprintf(_('%1$s favorited something by %2$s: %3$s'), + $actor->getNickname(), $target->getProfile()->getNickname(), + $target->rendered ?: $target->content); + $act->actor = $actor->asActivityObject(); + $act->target = $target->asActivityObject(); + $act->objects = array(clone($act->target)); - $fave = null; + $url = common_local_url('AtomPubShowFavorite', array('profile'=>$actor->id, 'notice'=>$target->id)); + $act->selfLink = $url; + $act->editLink = $url; - if (Event::handle('StartFavorNotice', array($actor, $target, &$fave))) { + // saveActivity will in turn also call Fave::saveActivityObject which does + // what this function used to do before this commit. + $stored = Notice::saveActivity($act, $actor); - $fave = new Fave(); - - $fave->user_id = $actor->id; - $fave->notice_id = $target->id; - $fave->created = common_sql_now(); - $fave->modified = common_sql_now(); - $fave->uri = self::newUri($actor, - $target, - $fave->created); - - // throws exception (Fave specific until migrated into Managed_DataObject - $fave->insert(); - - self::blowCacheForProfileId($fave->user_id); - self::blowCacheForNoticeId($fave->notice_id); - self::blow('popular'); - - Event::handle('EndFavorNotice', array($actor, $target)); - } - - return $fave; + return $stored; } // exception throwing takeover! @@ -141,7 +138,11 @@ class Fave extends Managed_DataObject $act->time = strtotime($this->created); // TRANS: Activity title when marking a notice as favorite. $act->title = _("Favor"); - $act->content = $target->rendered ?: $target->content; + // TRANS: Message that is the "content" of a favorite (%1$s is the actor's nickname, %2$ is the favorited + // notice's nickname and %3$s is the content of the favorited notice.) + $act->content = sprintf(_('%1$s favorited something by %2$s: %3$s'), + $actor->getNickname(), $target->getProfile()->getNickname(), + $target->rendered ?: $target->content); $act->actor = $actor->asActivityObject(); $act->target = $target->asActivityObject(); @@ -345,7 +346,12 @@ class Fave extends Managed_DataObject static function saveActivityObject(ActivityObject $actobj, Notice $stored) { $object = self::parseActivityObject($actobj, $stored); - $object->insert(); // exception throwing! + $object->insert(); // exception throwing in Fave's case! + + self::blowCacheForProfileId($fave->user_id); + self::blowCacheForNoticeId($fave->notice_id); + self::blow('popular'); + Event::handle('EndFavorNotice', array($stored->getProfile(), $object->getTarget())); return $object; } diff --git a/plugins/Favorite/lib/favcommand.php b/plugins/Favorite/lib/favcommand.php index d89b7f3421..cf5ca79218 100644 --- a/plugins/Favorite/lib/favcommand.php +++ b/plugins/Favorite/lib/favcommand.php @@ -32,19 +32,6 @@ class FavCommand extends Command return; } - // @fixme favorite notification should be triggered - // at a lower level - - $other = User::getKV('id', $notice->profile_id); - - if ($other && $other->id != $this->user->id && !empty($other->email)) { - require_once INSTALLDIR.'/lib/mail.php'; - - mail_notify_fave($other, $this->user->getProfile(), $notice); - } - - Fave::blowCacheForProfileId($this->user->id); - // TRANS: Text shown when a notice has been marked as favourite successfully. $channel->output($this->user, _('Notice marked as fave.')); } diff --git a/plugins/YammerImport/lib/yammerimporter.php b/plugins/YammerImport/lib/yammerimporter.php index 4cc5cb2d4a..0992882670 100644 --- a/plugins/YammerImport/lib/yammerimporter.php +++ b/plugins/YammerImport/lib/yammerimporter.php @@ -160,7 +160,12 @@ class YammerImporter foreach ($data['faves'] as $nickname) { $user = User::getKV('nickname', $nickname); if ($user) { - Fave::addNew($user->getProfile(), $notice); + try { + Fave::addNew($user->getProfile(), $notice); + } catch (Exception $e) { + // failed, let's move to the next + common_debug('YammerImport failed favoriting a notice: '.$e->getMessage()); + } } }