Consolidate some precondition checks for repeats, fix a couple bits in the repeat...

Notice::saveNew() now does these checks directly when making a repeat:
* make sure the original is valid and existing
* stop you from repeating your own message
* stop you from repeating something you've previously repeated
* prevent repeats of any non-public messages
* explicit inScope() check to make sure you can read the original too (just in case there's a funky extension at play that changes scoping rules)

These error conditions throw exceptions, which the caller either uses as an error message or passes on up the stack, without having to duplicate the checks in each i/o channel.

diff --git a/actions/apistatusesretweet.php b/actions/apistatusesretweet.php
index 2bc9092ba6f42aa02313a03fbf904d52e5136019..4832da1823d6243d49a1122a144d4ad9607c5af1 100644 (file)
--- a/actions/apistatusesretweet.php
+++ b/actions/apistatusesretweet.php
@@ -78,42 +78,6 @@ class ApiStatusesRetweetAction extends ApiAuthAction
 
         $this->user = $this->auth_user;
 
-        if ($this->user->id == $this->original->profile_id) {
-            // TRANS: Client error displayed trying to repeat an own notice through the API.
-            $this->clientError(_('Cannot repeat your own notice.'),
-                               400, $this->format);
-            return false;
-        }
-
-        // Is it OK to repeat that notice (general enough scope)?
-
-        if ($this->original->scope != Notice::SITE_SCOPE &&
-            $this->original->scope != Notice::PUBLIC_SCOPE) {
-            $this->clientError(_('You may not repeat a private notice.'),
-                               403,
-                               $this->format);
-            return false;
-        }
-
-        $profile = $this->user->getProfile();
-
-        // Can the profile actually see that notice?
-
-        if (!$this->original->inScope($profile)) {
-            $this->clientError(_('No access to that notice.'),
-                               403,
-                               $this->format);
-            return false;
-        }
-
-        if ($profile->hasRepeated($id)) {
-            // TRANS: Client error displayed trying to re-repeat a notice through the API.
-            $this->clientError(_('Already repeated that notice.'),
-                               400, $this->format);
-            return false;
-        }
-
-
         return true;
     }
 

diff --git a/actions/repeat.php b/actions/repeat.php
index 4201a4ce955af96b69340d5e1e2e56e801979e59..44c57456fa3cb6f5ead2b24d0016ee778aba9e4a 100644 (file)
--- a/actions/repeat.php
+++ b/actions/repeat.php
@@ -73,20 +73,6 @@ class RepeatAction extends Action
             return false;
         }
 
-        // Is it OK to repeat that notice (general enough scope)?
-
-        if ($this->notice->scope != Notice::SITE_SCOPE &&
-            $this->notice->scope != Notice::PUBLIC_SCOPE) {
-            $this->clientError(_('You may not repeat a private notice.'),
-                               403);
-        }
-
-        if ($this->user->id == $this->notice->profile_id) {
-            // TRANS: Client error displayed when trying to repeat an own notice.
-            $this->clientError(_('You cannot repeat your own notice.'));
-            return false;
-        }
-
         $token  = $this->trimmed('token-'.$id);
 
         if (empty($token) || $token != common_session_token()) {
@@ -94,21 +80,6 @@ class RepeatAction extends Action
             return false;
         }
 
-        $profile = $this->user->getProfile();
-
-        // Can the profile actually see that notice?
-
-        if (!$this->notice->inScope($profile)) {
-            $this->clientError(_('No access to that notice.'), 403);
-        }
-
-
-        if ($profile->hasRepeated($id)) {
-            // TRANS: Client error displayed when trying to repeat an already repeated notice.
-            $this->clientError(_('You already repeated that notice.'));
-            return false;
-        }
-
         return true;
     }
 

diff --git a/classes/Notice.php b/classes/Notice.php
index b5eafb0ffa94fdba006f4c947e277705895547ca..81ccd8d74fa2b78ceb39246105a0f18586858325 100644 (file)
--- a/classes/Notice.php
+++ b/classes/Notice.php
@@ -350,13 +350,31 @@ class Notice extends Memcached_DataObject
 
             $repeat = Notice::staticGet('id', $repeat_of);
 
-            if (!empty($repeat) &&
-                $repeat->scope != Notice::SITE_SCOPE &&
+            if (empty($repeat)) {
+                throw new ClientException(_('Cannot repeat; original notice is missing or deleted.'));
+            }
+
+            if ($profile->id == $repeat->profile_id) {
+                // TRANS: Client error displayed when trying to repeat an own notice.
+                throw new ClientException(_('You cannot repeat your own notice.'));
+            }
+
+            if ($repeat->scope != Notice::SITE_SCOPE &&
                 $repeat->scope != Notice::PUBLIC_SCOPE) {
+                // TRANS: Client error displayed when trying to repeat a non-public notice.
                 throw new ClientException(_('Cannot repeat a private notice.'), 403);
             }
 
-            // XXX: Check for access...?
+            if (!$repeat->inScope($profile)) {
+                // The generic checks above should cover this, but let's be sure!
+                // TRANS: Client error displayed when trying to repeat a notice you cannot access.
+                throw new ClientException(_('Cannot repeat a notice you cannot read.'), 403);
+            }
+
+            if ($profile->hasRepeated($repeat->id)) {
+                // TRANS: Client error displayed when trying to repeat an already repeated notice.
+                throw new ClientException(_('You already repeated that notice.'));
+            }
 
             $notice->repeat_of = $repeat_of;
         } else {
@@ -1567,6 +1585,15 @@ class Notice extends Memcached_DataObject
         return $location;
     }
 
+    /**
+     * Convenience function for posting a repeat of an existing message.
+     *
+     * @param int $repeater_id: profile ID of user doing the repeat
+     * @param string $source: posting source key, eg 'web', 'api', etc
+     * @return Notice
+     *
+     * @throws Exception on failure or permission problems
+     */
     function repeat($repeater_id, $source)
     {
         $author = Profile::staticGet('id', $this->profile_id);

diff --git a/lib/command.php b/lib/command.php
index 35d070268414c57ab9ab3a776a409666f6dee4e8..aaad57776176a8e4cf43c072b211be22bd6681f1 100644 (file)
--- a/lib/command.php
+++ b/lib/command.php
@@ -537,44 +537,15 @@ class RepeatCommand extends Command
     {
         $notice = $this->getNotice($this->other);
 
-        if($this->user->id == $notice->profile_id)
-        {
-            // TRANS: Error text shown when trying to repeat an own notice.
-            $channel->error($this->user, _('Cannot repeat your own notice.'));
-            return;
-        }
-
-        // Is it OK to repeat that notice (general enough scope)?
-
-        if ($notice->scope != Notice::SITE_SCOPE &&
-            $notice->scope != Notice::PUBLIC_SCOPE) {
-            $channel->error($this->user, _('You may not repeat a private notice.'));
-        }
-
-        $profile = $this->user->getProfile();
-
-        // Can the profile actually see that notice?
-
-        if (!$notice->inScope($profile)) {
-            $channel->error($this->user, _('You have no access to that notice.'));
-        }
-
-        if ($profile->hasRepeated($notice->id)) {
-            // TRANS: Error text shown when trying to repeat an notice that was already repeated by the user.
-            $channel->error($this->user, _('Already repeated that notice.'));
-            return;
-        }
-
-        $repeat = $notice->repeat($this->user->id, $channel->source);
-
-        if ($repeat) {
+        try {
+            $repeat = $notice->repeat($this->user->id, $channel->source());
+            $recipient = $notice->getProfile();
 
             // TRANS: Message given having repeated a notice from another user.
             // TRANS: %s is the name of the user for which the notice was repeated.
             $channel->output($this->user, sprintf(_('Notice from %s repeated.'), $recipient->nickname));
-        } else {
-            // TRANS: Error text shown when repeating a notice fails with an unknown reason.
-            $channel->error($this->user, _('Error repeating notice.'));
+        } catch (Exception $e) {
+            $channel->error($this->user, $e->getMessage());
         }
     }
 }