Facebook: Gracefully handle disconnection

4 files changed, 234 insertions, 181 deletions

diff --git a/plugins/FacebookBridge/FacebookBridgePlugin.php b/plugins/FacebookBridge/FacebookBridgePlugin.php
index c30ea1544..93427d5bd 100644
--- a/plugins/FacebookBridge/FacebookBridgePlugin.php
+++ b/plugins/FacebookBridge/FacebookBridgePlugin.php
@@ -236,7 +236,8 @@ class FacebookBridgePlugin extends Plugin
     }
 
     /*
-     * Add a tab for user-level Facebook settings
+     * Add a tab for user-level Facebook settings if the user
+     * has a link to Facebook
      *
      * @param Action &action the current action
      *
@@ -247,17 +248,32 @@ class FacebookBridgePlugin extends Plugin
         if ($this->hasApplication()) {
             $action_name = $action->trimmed('action');
 
-            $action->menuItem(
-                common_local_url('facebooksettings'),
-                // TRANS: Menu item tab.
-                _m('MENU','Facebook'),
-                // TRANS: Tooltip for menu item "Facebook".
-                _m('Facebook settings'),
-                $action_name === 'facebooksettings'
-            );
+            // CurrentUserDesignAction stores the current user in $cur
+            $user = $action->getCurrentUser();
+
+            $flink = null;
+
+            if (!empty($user)) {
+                $flink = Foreign_link::getByUserID(
+                    $user->id,
+                    FACEBOOK_SERVICE
+                );
+            }
+
+            if (!empty($flink)) {
+
+                $action->menuItem(
+                    common_local_url('facebooksettings'),
+                    // TRANS: Menu item tab.
+                    _m('MENU','Facebook'),
+                    // TRANS: Tooltip for menu item "Facebook".
+                    _m('Facebook settings'),
+                    $action_name === 'facebooksettings'
+                );
+
+            }
         }
 
-        return true;
     }
 
     /*
diff --git a/plugins/FacebookBridge/actions/facebookdeauthorize.php b/plugins/FacebookBridge/actions/facebookdeauthorize.php
index cb816fc54..6813ccf1d 100644
--- a/plugins/FacebookBridge/actions/facebookdeauthorize.php
+++ b/plugins/FacebookBridge/actions/facebookdeauthorize.php
@@ -82,7 +82,7 @@ class FacebookdeauthorizeAction extends Action
                     LOG_WARNING,
                     sprintf(
                         'Unable to delete Facebook foreign link '
-                            . 'for %s (%d), fbuid %s',
+                            . 'for %s (%d), fbuid %d',
                         $user->nickname,
                         $user->id,
                         $fbuid
@@ -95,7 +95,7 @@ class FacebookdeauthorizeAction extends Action
             common_log(
                 LOG_INFO,
                 sprintf(
-                    'Facebook callback: %s (%d), fbuid %s has deauthorized '
+                    'Facebook callback: %s (%d), fbuid %d has deauthorized '
                         . 'the Facebook application.',
                     $user->nickname,
                     $user->id,
@@ -107,7 +107,7 @@ class FacebookdeauthorizeAction extends Action
             // Warn the user about being locked out of their account
             // if we can.
             if (empty($user->password) && !empty($user->email)) {
-                $this->emailWarn($user);
+                Facebookclient::emailWarn($user);
             } else {
                 common_log(
                     LOG_WARNING,
@@ -141,74 +141,4 @@ class FacebookdeauthorizeAction extends Action
         }
     }
 
-    /*
-     * Send the user an email warning that their account has been
-     * disconnected and he/she has no way to login and must contact
-     * the site administrator for help.
-     *
-     * @param User $user the deauthorizing user
-     *
-     */
-    function emailWarn($user)
-    {
-        $profile = $user->getProfile();
-
-        $siteName  = common_config('site', 'name');
-        $siteEmail = common_config('site', 'email');
-
-        if (empty($siteEmail)) {
-            common_log(
-                LOG_WARNING,
-                    "No site email address configured. Please set one."
-            );
-        }
-
-        common_switch_locale($user->language);
-
-        $subject = _m('Contact the %s administrator to retrieve your account');
-
-        $msg = <<<BODY
-Hi %1$s,
-
-We've noticed you have deauthorized the Facebook connection for your
-%2$s account.  You have not set a password for your %2$s account yet, so
-you will not be able to login. If you wish to continue using your %2$s
-account, please contact the site administrator (%3$s) to set a password.
-
-Sincerely,
-
-%2$s
-BODY;
-        $body = sprintf(
-            _m($msg),
-            $user->nickname,
-            $siteName,
-            $siteEmail
-        );
-
-        common_switch_locale();
-
-        if (mail_to_user($user, $subject, $body)) {
-            common_log(
-                LOG_INFO,
-                sprintf(
-                    'Sent account lockout warning to %s (%d)',
-                    $user->nickname,
-                    $user->id
-                ),
-                __FILE__
-            );
-        } else {
-            common_log(
-                LOG_WARNING,
-                sprintf(
-                    'Unable to send account lockout warning to %s (%d)',
-                    $user->nickname,
-                    $user->id
-                ),
-                __FILE__
-            );
-        }
-    }
-
 }
\ No newline at end of file
diff --git a/plugins/FacebookBridge/actions/facebooksettings.php b/plugins/FacebookBridge/actions/facebooksettings.php
index e51181036..b9fa7ba2a 100644
--- a/plugins/FacebookBridge/actions/facebooksettings.php
+++ b/plugins/FacebookBridge/actions/facebooksettings.php
@@ -2,7 +2,7 @@
 /**
  * StatusNet, the distributed open-source microblogging tool
  *
- * Settings for Facebook
+ *  Edit user settings for Facebook
  *
  * PHP version 5
  *
@@ -26,13 +26,12 @@
  * @license   http://www.fsf.org/licensing/licenses/agpl-3.0.html GNU Affero General Public License version 3.0
  * @link      http://status.net/
  */
-
 if (!defined('STATUSNET')) {
     exit(1);
 }
 
 /**
- * Settings for Facebook
+ * Edit user settings for Facebook
  *
  * @category Settings
  * @package  StatusNet
@@ -42,15 +41,20 @@ if (!defined('STATUSNET')) {
  *
  * @see      SettingsAction
  */
+class FacebooksettingsAction extends ConnectSettingsAction {
 
-class FacebooksettingsAction extends ConnectSettingsAction
-{
-    private $facebook;
+    private $facebook; // Facebook PHP-SDK client obj
     private $flink;
     private $user;
-    
-    function prepare($args)
-    {
+
+    /**
+     * For initializing members of the class.
+     *
+     * @param array $argarray misc. arguments
+     *
+     * @return boolean true
+     */
+    function prepare($args) {
         parent::prepare($args);
 
         $this->facebook = new Facebook(
@@ -62,13 +66,19 @@ class FacebooksettingsAction extends ConnectSettingsAction
         );
 
         $this->user = common_current_user();
-        $this->flink = Foreign_link::getByUserID($this->user->id, FACEBOOK_SERVICE);
+
+        $this->flink = Foreign_link::getByUserID(
+            $this->user->id,
+            FACEBOOK_SERVICE
+        );
 
         return true;
     }
 
-    function handlePost($args)
-    {
+    /*
+     * Check the sessions token and dispatch
+     */
+    function handlePost($args) {
         // CSRF protection
 
         $token = $this->trimmed('token');
@@ -86,50 +96,40 @@ class FacebooksettingsAction extends ConnectSettingsAction
         }
     }
 
-    function title()
-    {
+    /**
+     * Returns the page title
+     *
+     * @return string page title
+     */
+    function title() {
         // TRANS: Page title for Facebook settings.
         return _m('Facebook settings');
     }
-    
+
     /**
      * Instructions for use
      *
      * @return instructions for use
      */
-
-    function getInstructions()
-    {
+    function getInstructions() {
         return _('Facebook settings');
     }
 
-    function showContent()
-    {
-
-        if (empty($this->flink)) {
-
-            $this->element(
-                'p',
-                'instructions',
-                _m('There is no Facebook user connected to this account.')
-            );
-
-            $attrs = array(
-                'show-faces' => 'true',
-                'perms' => 'user_location,user_website,offline_access,publish_stream'
-            );
+    /*
+     * Show the settings form if he/she has a link to Facebook
+     *
+     * @return void
+     */
+    function showContent() {
 
-            $this->element('fb:login-button', $attrs);
-            
-
-        } else {
+        if (!empty($this->flink)) {
 
             $this->elementStart(
                 'form',
                 array(
                     'method' => 'post',
-                    'id'     => 'form_settings_facebook',
-                    'class'  => 'form_settings',
+                    'id' => 'form_settings_facebook',
+                    'class' => 'form_settings',
                     'action' => common_local_url('facebooksettings')
                 )
             );
@@ -140,22 +140,21 @@ class FacebooksettingsAction extends ConnectSettingsAction
 
             $this->elementStart('p', array('class' => 'facebook-user-display'));
 
-            $this->elementStart(
+            $this->element(
                 'fb:profile-pic',
-                array('uid' => $this->flink->foreign_id,
-                      'size' => 'small',
-                      'linked' => 'true',
-                      'facebook-logo' => 'true')
+                array(
+                    'uid' => $this->flink->foreign_id,
+                    'size' => 'small',
+                    'linked' => 'true',
+                    'facebook-logo' => 'true'
+                )
             );
-            $this->elementEnd('fb:profile-pic');
 
-            $this->elementStart(
+            $this->element(
                 'fb:name',
                 array('uid' => $this->flink->foreign_id, 'useyou' => 'false')
             );
 
-            $this->elementEnd('fb:name');
-
             $this->elementEnd('p');
 
             $this->elementStart('ul', 'form_data');
@@ -173,9 +172,9 @@ class FacebooksettingsAction extends ConnectSettingsAction
             $this->elementStart('li');
 
             $this->checkbox(
-                'replysync',
-                _m('Send "@" replies to Facebook.'),
-                ($this->flink) ? ($this->flink->noticesync & FOREIGN_NOTICE_SEND_REPLY) : true
+                    'replysync',
+                    _m('Send "@" replies to Facebook.'),
+                    ($this->flink) ? ($this->flink->noticesync & FOREIGN_NOTICE_SEND_REPLY) : true
             );
 
             $this->elementEnd('li');
@@ -183,10 +182,10 @@ class FacebooksettingsAction extends ConnectSettingsAction
             $this->elementStart('li');
 
             // TRANS: Submit button to save synchronisation settings.
-            $this->submit('save', _m('BUTTON','Save'));
+            $this->submit('save', _m('BUTTON', 'Save'));
 
             $this->elementEnd('li');
-        
+
             $this->elementEnd('ul');
 
             $this->elementStart('fieldset');
@@ -197,39 +196,44 @@ class FacebooksettingsAction extends ConnectSettingsAction
             if (empty($this->user->password)) {
 
                 $this->elementStart('p', array('class' => 'form_guide'));
-                // @todo FIXME: Bad i18n. Patchwork message in three parts.
-                // TRANS: Followed by a link containing text "set a password".
-                $this->text(_m('Disconnecting your Faceboook ' .
-                               'would make it impossible to log in! Please '));
-                $this->element('a',
-                    array('href' => common_local_url('passwordsettings')),
-                        // TRANS: Preceded by "Please " and followed by " first."
-                        _m('set a password'));
-                // TRANS: Preceded by "Please set a password".
-                $this->text(_m(' first.'));
-                $this->elementEnd('p');
-            } else {
 
-                $note = 'Keep your %s account but disconnect from Facebook. ' .
-                    'You\'ll use your %s password to log in.';
+                $msg = sprintf(
+                    _m(
+                        'Disconnecting your Faceboook would make it impossible to '
+                            . 'log in! Please [set a password](%s) first.'
+                    ),
+                    common_local_url('passwordsettings')
+                );
 
-                $site = common_config('site', 'name');
+                $this->raw(common_markup_to_html($msg));
+                $this->elementEnd('p');
+
+            } else {
 
-                $this->element('p', 'instructions',
-                    sprintf($note, $site, $site));
+                $msg = sprintf(
+                    _m(
+                        'Keep your %1$s account but disconnect from Facebook. ' .
+                        'You\'ll use your 1%$s password to log in.'
+                    ),
+                    common_config('site', 'name')
+                );
 
                 // TRANS: Submit button.
-                $this->submit('disconnect', _m('BUTTON','Disconnect'));
+                $this->submit('disconnect', _m('BUTTON', 'Disconnect'));
             }
 
             $this->elementEnd('fieldset');
 
             $this->elementEnd('form');
-        }
+         }
     }
 
-    function saveSettings()
-    {
+    /*
+     * Save the user's Facebook settings
+     *
+     * @return void
+     */
+    function saveSettings() {
 
         $noticesync = $this->boolean('noticesync');
         $replysync  = $this->boolean('replysync');
@@ -246,10 +250,14 @@ class FacebooksettingsAction extends ConnectSettingsAction
         }
     }
 
-    function disconnect()
-    {
-        $flink = Foreign_link::getByUserID($this->user->id, FACEBOOK_SERVICE);
-        $result = $flink->delete();
+    /*
+     * Disconnect the user's Facebook account - deletes the Foreign_link
+     * and shows the user a success message if all goes well.
+     */
+    function disconnect() {
+
+        $result = $this->flink->delete();
+        $this->flink = null;
 
         if ($result === false) {
             common_log_db_error($user, 'DELETE', __FILE__);
@@ -258,7 +266,6 @@ class FacebooksettingsAction extends ConnectSettingsAction
         }
 
         $this->showForm(_m('You have disconnected from Facebook.'), true);
-
     }
-}
 
+}
diff --git a/plugins/FacebookBridge/lib/facebookclient.php b/plugins/FacebookBridge/lib/facebookclient.php
index 33edf5c6b..575208cac 100644
--- a/plugins/FacebookBridge/lib/facebookclient.php
+++ b/plugins/FacebookBridge/lib/facebookclient.php
@@ -202,7 +202,7 @@ class Facebookclient
 
             common_debug(
                 sprintf(
-                    "Attempting use Graph API to post notice %d as a stream item for %s (%d), fbuid %s",
+                    "Attempting use Graph API to post notice %d as a stream item for %s (%d), fbuid %d",
                     $this->notice->id,
                     $this->user->nickname,
                     $this->user->id,
@@ -247,7 +247,7 @@ class Facebookclient
             common_log(
                 LOG_INFO,
                 sprintf(
-                    "Posted notice %d as a stream item for %s (%d), fbuid %s",
+                    "Posted notice %d as a stream item for %s (%d), fbuid %d",
                     $this->notice->id,
                     $this->user->nickname,
                     $this->user->id,
@@ -287,7 +287,7 @@ class Facebookclient
             } else {
 
                 $msg = 'Not sending notice %d to Facebook because user %s '
-                     . '(%d), fbuid %s,  does not have \'status_update\' '
+                     . '(%d), fbuid %d,  does not have \'status_update\' '
                      . 'or \'publish_stream\' permission.';
 
                 common_log(
@@ -330,7 +330,7 @@ class Facebookclient
 
         common_debug(
             sprintf(
-                'Checking for %s permission for user %s (%d), fbuid %s',
+                'Checking for %s permission for user %s (%d), fbuid %d',
                 $permission,
                 $this->user->nickname,
                 $this->user->id,
@@ -351,7 +351,7 @@ class Facebookclient
 
             common_debug(
                 sprintf(
-                    '%s (%d), fbuid %s has %s permission',
+                    '%s (%d), fbuid %d has %s permission',
                     $permission,
                     $this->user->nickname,
                     $this->user->id,
@@ -425,6 +425,12 @@ class Facebookclient
             );
             return true;
             break;
+
+         // @fixme: Facebook returns these 2xx permission errors sometimes
+         // FOR NO GOOD REASON AT ALL! It would be better to retry a few times
+         // over an extended period of time to instead of immediately
+         // disconnecting.
+
          case 200: // Permissions error
          case 250: // Updating status requires the extended permission status_update
             $this->disconnect();
@@ -485,7 +491,7 @@ class Facebookclient
 
         common_debug(
             sprintf(
-                "Attempting to post notice %d as a status update for %s (%d), fbuid %s",
+                "Attempting to post notice %d as a status update for %s (%d), fbuid %d",
                 $this->notice->id,
                 $this->user->nickname,
                 $this->user->id,
@@ -508,7 +514,7 @@ class Facebookclient
             common_log(
                 LOG_INFO,
                 sprintf(
-                    "Posted notice %s as a status update for %s (%d), fbuid %s",
+                    "Posted notice %s as a status update for %s (%d), fbuid %d",
                     $this->notice->id,
                     $this->user->nickname,
                     $this->user->id,
@@ -523,7 +529,7 @@ class Facebookclient
         } else {
 
             $msg = sprintf(
-                "Error posting notice %s as a status update for %s (%d), fbuid %s - error code: %s",
+                "Error posting notice %s as a status update for %s (%d), fbuid %d - error code: %s",
                 $this->notice->id,
                 $this->user->nickname,
                 $this->user->id,
@@ -544,7 +550,7 @@ class Facebookclient
 
         common_debug(
             sprintf(
-                'Attempting to post notice %d as stream item for %s (%d) fbuid %s',
+                'Attempting to post notice %d as stream item for %s (%d) fbuid %d',
                 $this->notice->id,
                 $this->user->nickname,
                 $this->user->id,
@@ -572,7 +578,7 @@ class Facebookclient
             common_log(
                 LOG_INFO,
                 sprintf(
-                    'Posted notice %d as a %s for %s (%d), fbuid %s',
+                    'Posted notice %d as a %s for %s (%d), fbuid %d',
                     $this->notice->id,
                     empty($fbattachment) ? 'stream item' : 'stream item with attachment',
                     $this->user->nickname,
@@ -585,7 +591,7 @@ class Facebookclient
         } else {
 
             $msg = sprintf(
-                'Could not post notice %d as a %s for %s (%d), fbuid %s - error code: %s',
+                'Could not post notice %d as a %s for %s (%d), fbuid %d - error code: %s',
                 $this->notice->id,
                 empty($fbattachment) ? 'stream item' : 'stream item with attachment',
                 $this->user->nickname,
@@ -694,7 +700,7 @@ class Facebookclient
         common_log(
             LOG_INFO,
             sprintf(
-                'Removing Facebook link for %s (%d), fbuid %s',
+                'Removing Facebook link for %s (%d), fbuid %d',
                 $this->user->nickname,
                 $this->user->id,
                 $fbuid
@@ -708,7 +714,7 @@ class Facebookclient
             common_log(
                 LOG_ERR,
                 sprintf(
-                    'Could not remove Facebook link for %s (%d), fbuid %s',
+                    'Could not remove Facebook link for %s (%d), fbuid %d',
                     $this->user->nickname,
                     $this->user->id,
                     $fbuid
@@ -719,13 +725,31 @@ class Facebookclient
         }
 
         // Notify the user that we are removing their Facebook link
+        if (!empty($this->user->email)) {
+            $result = $this->mailFacebookDisconnect();
+
+            if (!$result) {
+
+                $msg = 'Unable to send email to notify %s (%d), fbuid %d '
+                     . 'about his/her Facebook link being removed.';
 
-        $result = $this->mailFacebookDisconnect();
+                common_log(
+                    LOG_WARNING,
+                    sprintf(
+                        $msg,
+                        $this->user->nickname,
+                        $this->user->id,
+                        $fbuid
+                    ),
+                    __FILE__
+                );
+            }
 
-        if (!$result) {
+        } else {
 
-            $msg = 'Unable to send email to notify %s (%d), fbuid %s '
-                 . 'about his/her Facebook link being removed.';
+            $msg = 'Unable to send email to notify %s (%d), fbuid %d '
+                 . 'about his/her Facebook link being removed because the '
+                 . 'user has not set an email address.';
 
             common_log(
                 LOG_WARNING,
@@ -780,7 +804,83 @@ BODY;
 
         common_switch_locale();
 
-        return mail_to_user($this->user, $subject, $body);
+        $result = mail_to_user($this->user, $subject, $body);
+
+        if (empty($this->user->password)) {
+            $result = self::emailWarn($this->user);
+        }
+
+        return $result;
+    }
+
+    /*
+     * Send the user an email warning that their account has been
+     * disconnected and he/she has no way to login and must contact
+     * the site administrator for help.
+     *
+     * @param User $user the deauthorizing user
+     *
+     */
+    static function emailWarn($user)
+    {
+        $profile = $user->getProfile();
+
+        $siteName  = common_config('site', 'name');
+        $siteEmail = common_config('site', 'email');
+
+        if (empty($siteEmail)) {
+            common_log(
+                LOG_WARNING,
+                    "No site email address configured. Please set one."
+            );
+        }
+
+        common_switch_locale($user->language);
+
+        $subject = _m('Contact the %s administrator to retrieve your account');
+
+        $msg = <<<BODY
+Hi %1$s,
+
+We've noticed you have deauthorized the Facebook connection for your
+%2$s account.  You have not set a password for your %2$s account yet, so
+you will not be able to login. If you wish to continue using your %2$s
+account, please contact the site administrator (%3$s) to set a password.
+
+Sincerely,
+
+%2$s
+BODY;
+        $body = sprintf(
+            _m($msg),
+            $user->nickname,
+            $siteName,
+            $siteEmail
+        );
+
+        common_switch_locale();
+
+        if (mail_to_user($user, $subject, $body)) {
+            common_log(
+                LOG_INFO,
+                sprintf(
+                    'Sent account lockout warning to %s (%d)',
+                    $user->nickname,
+                    $user->id
+                ),
+                __FILE__
+            );
+        } else {
+            common_log(
+                LOG_WARNING,
+                sprintf(
+                    'Unable to send account lockout warning to %s (%d)',
+                    $user->nickname,
+                    $user->id
+                ),
+                __FILE__
+            );
+        }
     }
 
     /*