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

@@ -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"
  *

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) {
+        // Throws exception
-            // TRANS: Server error.
+        $stored = Fave::addNew($profile, $notice);
-            $this->serverError(_m('Could not create favorite.'));
-        }
-
-        Fave::blowCacheForProfileId($profile->id);
 
         if ($this->boolean('ajax')) {
             $this->startHTML('text/xml;charset=utf-8');

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

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);
+}

plugins/Favorite/actions/apifavoritecreate.php

@@ -24,15 +24,14 @@
  * @author    Craig Andrews <candrews@integralblue.com>
  * @author    Evan Prodromou <evan@status.net>
  * @author    Zach Copley <zach@status.net>
+ * @author   Mikael Nordfeldth <mmn@hethane.se>
  * @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')) {
+if (!defined('GNUSOCIAL')) { exit(1); }
-    exit(1);
-}
 
 /**
  * Favorites the status specified in the ID parameter as the authenticating user.
@@ -43,27 +42,22 @@ if (!defined('STATUSNET')) {
  * @author   Craig Andrews <candrews@integralblue.com>
  * @author   Evan Prodromou <evan@status.net>
  * @author   Zach Copley <zach@status.net>
+ * @author   Mikael Nordfeldth <mmn@hethane.se>
  * @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;
 
-    /**
+    protected $needPost = true;
-     * Take arguments for running
+
-     *
+    protected function prepare(array $args=array())
-     * @param array $args $_REQUEST args
-     *
-     * @return boolean success flag
-     */
-    function prepare($args)
     {
         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;
     }
 
-    /**
+    protected function handle()
-     * Handle the request
-     *
-     * Check the format and show the user info
-     *
-     * @param array $args $_REQUEST data (unused)
-     *
-     * @return void
-     */
-    function handle($args)
     {
-        parent::handle($args);
+        parent::handle();
-
-        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;
-        }
 
         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);
+        // throws exception on failure
-
+        $stored = Fave::addNew($this->scoped, $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);
 
         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
-        }
-    }
 }

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':

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);
+        // throws exception on failure
-
+        $stored = 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);
 
         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
-        }
-    }    
 }

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();
+        return $stored;
-
-            $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;
     }
 
     // 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;
     }

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.')); 
     } 

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());
+                    }
                 }
             }