newmessage (and Message class) fixed for FormAction

Also added a needLogin function to the Action class, which will do
redirect to login page with proper returnto setting.

diff --git a/actions/apidirectmessagenew.php b/actions/apidirectmessagenew.php
index e33077cdac7d5277a310ce7b5efe71c62b3bc41d..d642252b4dc1864eb8052458b4999b0b344474e7 100644 (file)
--- a/actions/apidirectmessagenew.php
+++ b/actions/apidirectmessagenew.php
@@ -163,11 +163,6 @@ class ApiDirectMessageNewAction extends ApiAuthAction
             $this->source
         );
 
-        if (is_string($message)) {
-            $this->serverError($message);
-            return;
-        }
-
         $message->notify();
 
         if ($this->format == 'xml') {

diff --git a/actions/newmessage.php b/actions/newmessage.php
index 6882da4f180d46b465b4266ab66512201d5879b4..4ff9d8e9e93f627b96e060026f914095b97d1f63 100644 (file)
--- a/actions/newmessage.php
+++ b/actions/newmessage.php
@@ -45,15 +45,8 @@ if (!defined('STATUSNET') && !defined('LACONICA')) {
  * @link     http://status.net/
  */
 
-class NewmessageAction extends Action
+class NewmessageAction extends FormAction
 {
-
-    /**
-     * Error message, if any
-     */
-
-    var $msg = null;
-
     var $content = null;
     var $to = null;
     var $other = null;
@@ -80,32 +73,15 @@ class NewmessageAction extends Action
      * @return void
      */
 
-    function handle($args)
+    protected function prepare($args)
     {
-        parent::handle($args);
+        parent::prepare($args);
 
         if (!common_logged_in()) {
-            // TRANS: Error message displayed when trying to perform an action that requires a logged in user.
-            $this->clientError(_('Not logged in.'), 403);
-        } else if ($_SERVER['REQUEST_METHOD'] == 'POST') {
-            $this->saveNewMessage();
-        } else {
-            $this->showForm();
+            $this->needLogin();
         }
-    }
-
-    function prepare($args)
-    {
-        parent::prepare($args);
-
-        $user = common_current_user();
 
-        if (!$user) {
-            /* Go log in, and then come back. */
-            common_set_returnto($_SERVER['REQUEST_URI']);
-            common_redirect(common_local_url('login'));
-            return false;
-        }
+        $user = $this->scoped->getUser();
 
         $this->content = $this->trimmed('content');
         $this->to = $this->trimmed('to');
@@ -117,76 +93,55 @@ class NewmessageAction extends Action
             if (!$this->other) {
                 // TRANS: Client error displayed trying to send a direct message to a non-existing user.
                 $this->clientError(_('No such user.'), 404);
-                return false;
             }
 
             if (!$user->mutuallySubscribed($this->other)) {
                 // TRANS: Client error displayed trying to send a direct message to a user while sender and
                 // TRANS: receiver are not subscribed to each other.
                 $this->clientError(_('You cannot send a message to this user.'), 404);
-                return false;
             }
         }
 
         return true;
     }
 
-    function saveNewMessage()
+    protected function handlePost()
     {
-        // CSRF protection
-
-        $token = $this->trimmed('token');
-        if (!$token || $token != common_session_token()) {
-            // TRANS: Client error displayed when the session token does not match or is not given.
-            $this->showForm(_('There was a problem with your session token. ' .
-                'Try again, please.'));
-            return;
-        }
+        parent::handlePost();
 
-        $user = common_current_user();
-        assert($user); // XXX: maybe an error instead...
+        assert($this->scoped); // XXX: maybe an error instead...
+        $user = $this->scoped->getUser();
 
         if (!$this->content) {
             // TRANS: Form validator error displayed trying to send a direct message without content.
-            $this->showForm(_('No content!'));
-            return;
+            $this->clientError(_('No content!'));
         } else {
             $content_shortened = $user->shortenLinks($this->content);
 
             if (Message::contentTooLong($content_shortened)) {
                 // TRANS: Form validation error displayed when message content is too long.
                 // TRANS: %d is the maximum number of characters for a message.
-                $this->showForm(sprintf(_m('That\'s too long. Maximum message size is %d character.',
+                $this->clientError(sprintf(_m('That\'s too long. Maximum message size is %d character.',
                                            'That\'s too long. Maximum message size is %d characters.',
                                            Message::maxContent()),
                                         Message::maxContent()));
-                return;
             }
         }
 
         if (!$this->other) {
             // TRANS: Form validation error displayed trying to send a direct message without specifying a recipient.
-            $this->showForm(_('No recipient specified.'));
-            return;
+            $this->clientError(_('No recipient specified.'));
         } else if (!$user->mutuallySubscribed($this->other)) {
             // TRANS: Client error displayed trying to send a direct message to a user while sender and
             // TRANS: receiver are not subscribed to each other.
             $this->clientError(_('You cannot send a message to this user.'), 404);
-            return;
         } else if ($user->id == $this->other->id) {
             // TRANS: Client error displayed trying to send a direct message to self.
             $this->clientError(_('Do not send a message to yourself; ' .
                 'just say it to yourself quietly instead.'), 403);
-            return;
         }
 
         $message = Message::saveNew($user->id, $this->other->id, $this->content, 'web');
-
-        if (is_string($message)) {
-            $this->showForm($message);
-            return;
-        }
-
         $message->notify();
 
         if ($this->boolean('ajax')) {

diff --git a/classes/Message.php b/classes/Message.php
index 7e8cd6245dd9f019bf39061c57575d6f698743f0..c5aefcc4e8d11bb832348157011d6165e4aab315 100644 (file)
--- a/classes/Message.php
+++ b/classes/Message.php
@@ -96,7 +96,7 @@ class Message extends Managed_DataObject
         if (!$result) {
             common_log_db_error($msg, 'INSERT', __FILE__);
             // TRANS: Message given when a message could not be stored on the server.
-            return _('Could not insert message.');
+            throw new ServerException(_('Could not insert message.'));
         }
 
         $orig = clone($msg);
@@ -107,7 +107,7 @@ class Message extends Managed_DataObject
         if (!$result) {
             common_log_db_error($msg, 'UPDATE', __FILE__);
             // TRANS: Message given when a message could not be updated on the server.
-            return _('Could not update message with new URI.');
+            throw new ServerException(_('Could not update message with new URI.'));
         }
 
         return $msg;

diff --git a/lib/action.php b/lib/action.php
index e4bd0746a59522498e3610e494496846ccde0e8a..09a15a4435a80d41985d64d2c1bc86347ebd7f41 100644 (file)
--- a/lib/action.php
+++ b/lib/action.php
@@ -1376,6 +1376,18 @@ class Action extends HTMLOutputter // lawsuit
         throw new ClientException($msg, $code);
     }
 
+    /**
+     * Redirect to login page (with returnto)
+     *
+     * @return nothing
+     */
+    function needLogin()
+    {
+        // this might be updated with a login check before redirecting
+        common_set_returnto($_SERVER['REQUEST_URI']);
+        common_redirect(common_local_url('login'));
+    }
+
     /**
      * Returns the current URL
      *

diff --git a/lib/command.php b/lib/command.php
index ae97b9facd31c3dd8801002d4901735a883940f2..e93b7fbfee62bb2237b9a69fdb7fb8e0cf8816dd 100644 (file)
--- a/lib/command.php
+++ b/lib/command.php
@@ -632,15 +632,15 @@ class MessageCommand extends Command
             $channel->error($this->user, _('Do not send a message to yourself; just say it to yourself quietly instead.'));
             return;
         }
-        $message = Message::saveNew($this->user->id, $other->id, $this->text, $channel->source());
-        if ($message) {
+        try {
+            $message = Message::saveNew($this->user->id, $other->id, $this->text, $channel->source());
             $message->notify();
             // TRANS: Message given have sent a direct message to another user.
             // TRANS: %s is the name of the other user.
             $channel->output($this->user, sprintf(_('Direct message to %s sent.'), $this->other));
-        } else {
+        } catch (Exception $e) {
             // TRANS: Error text shown sending a direct message fails with an unknown reason.
-            $channel->error($this->user, _('Error sending direct message.'));
+            $channel->error($this->user, $e->getMessage());
         }
     }
 }