From 2c68703923e94a27376622d96a1d5481807bfbf6 Mon Sep 17 00:00:00 2001 From: Zach Copley Date: Wed, 17 Nov 2010 21:53:56 +0000 Subject: [PATCH] Facebook: Gracefully handle disconnection --- lib/currentuserdesignaction.php | 5 + .../FacebookBridge/FacebookBridgePlugin.php | 36 ++-- .../actions/facebookdeauthorize.php | 76 +------- .../actions/facebooksettings.php | 167 +++++++++--------- plugins/FacebookBridge/lib/facebookclient.php | 136 ++++++++++++-- 5 files changed, 239 insertions(+), 181 deletions(-) diff --git a/lib/currentuserdesignaction.php b/lib/currentuserdesignaction.php index 012d30ad6f..e84c777685 100644 --- a/lib/currentuserdesignaction.php +++ b/lib/currentuserdesignaction.php @@ -88,4 +88,9 @@ class CurrentUserDesignAction extends Action return parent::getDesign(); } + function getCurrentUser() + { + return $this->cur; + } } + diff --git a/plugins/FacebookBridge/FacebookBridgePlugin.php b/plugins/FacebookBridge/FacebookBridgePlugin.php index c30ea15440..93427d5bdc 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 cb816fc54a..6813ccf1d2 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 = <<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 e511810369..b9fa7ba2af 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() - { + /* + * Show the settings form if he/she has a link to Facebook + * + * @return void + */ + 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' - ); - - $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.')); + + $msg = sprintf( + _m( + 'Disconnecting your Faceboook would make it impossible to ' + . 'log in! Please [set a password](%s) first.' + ), + common_local_url('passwordsettings') + ); + + $this->raw(common_markup_to_html($msg)); $this->elementEnd('p'); + } else { - $note = 'Keep your %s account but disconnect from Facebook. ' . - 'You\'ll use your %s password to log in.'; - - $site = common_config('site', 'name'); - - $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 33edf5c6b1..575208cacc 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(); - $result = $this->mailFacebookDisconnect(); + if (!$result) { - if (!$result) { + $msg = 'Unable to send email to notify %s (%d), fbuid %d ' + . 'about his/her Facebook link being removed.'; - $msg = 'Unable to send email to notify %s (%d), fbuid %s ' - . 'about his/her Facebook link being removed.'; + common_log( + LOG_WARNING, + sprintf( + $msg, + $this->user->nickname, + $this->user->id, + $fbuid + ), + __FILE__ + ); + } + + } else { + + $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 = <<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__ + ); + } } /*