diff --git flag.inc flag.inc index 731c863..a898e7e 100644 --- flag.inc +++ flag.inc @@ -527,20 +527,26 @@ class flag_flag { * The user on whose behalf to flag. Leave empty for the current user. * @param $skip_permission_check * Flag the item even if the $account user don't have permission to do so. + * @param $errors + * If the action cannot be carried out, this passed-by-reference variable + * will hold a list of error messages. + * * @return - * FALSE if some error occured (e.g., user has no permission, flag isn't - * applicable to the item, etc.), TRUE otherwise. + * FALSE if the action cannot be carried out (e.g., user has no permission, + * flag isn't applicable to the item, etc.), TRUE otherwise. */ - function flag($action, $content_id, $account = NULL, $skip_permission_check = FALSE) { + function flag($action, $content_id, $account = NULL, $skip_permission_check = FALSE, &$errors = array()) { + $errors = array(); if (!isset($account)) { $account = $GLOBALS['user']; } if (!$account) { + // @todo: This 'if' branch seems like a bug. Remove. return FALSE; } if (!$skip_permission_check) { if (!$this->access($content_id, $action, $account)) { - // User has no permission to flag/unflag this object. + $errors['access-denied'] = t('You are not allowed to flag, or unflag, this content.'); return FALSE; } } @@ -548,10 +554,16 @@ class flag_flag { // We are skipping permission checks. However, at a minimum we must make // sure the flag applies to this content type: if (!$this->applies_to_content_id($content_id)) { + $errors['content-type'] = t('This flag does not apply to this content type.'); return FALSE; } } + // @todo: Should we skip this if $skip_permission_check == TRUE ? + if (($errors = module_invoke_all('flag_validate', $action, $this, $content_id, $account))) { + return FALSE; + } + // Clear various caches; We don't want code running after us to report // wrong counts or false flaggings. flag_get_counts(NULL, NULL, TRUE); @@ -568,6 +580,7 @@ class flag_flag { $sid = flag_get_sid($uid); // Anonymous users must always have a session id. if ($sid == 0 && $account->uid == 0) { + $errors['session'] = t('Internal error: You are anonymous but you have no session ID.'); return FALSE; } } @@ -1082,7 +1095,7 @@ class flag_flag { * * For parameters docmentation, see theme_flag(). */ - function theme($action, $content_id, $after_flagging = FALSE) { + function theme($action, $content_id, $after_flagging = FALSE, $errors = array()) { static $js_added = array(); global $user; @@ -1103,7 +1116,7 @@ class flag_flag { } } - return theme($this->theme_suggestions(), $this, $action, $content_id, $after_flagging); + return theme($this->theme_suggestions(), $this, $action, $content_id, $after_flagging, $errors); } /** @@ -1237,9 +1250,9 @@ class flag_node extends flag_flag { return FALSE; } - function flag($action, $content_id, $account = NULL, $skip_permission_check = FALSE) { + function flag($action, $content_id, $account = NULL, $skip_permission_check = FALSE, &$errors = array()) { $content_id = $this->get_translation_id($content_id); - return parent::flag($action, $content_id, $account, $skip_permission_check); + return parent::flag($action, $content_id, $account, $skip_permission_check, $errors); } // Instead of overriding is_flagged() we override get_flagging_record(), diff --git flag.module flag.module index 0dc48ac..76de8db 100644 --- flag.module +++ flag.module @@ -658,6 +658,7 @@ function flag_activity_info() { */ function flag_page($action, $flag, $content_id) { global $user; + $errors = array(); // Shorten up the variables that affect the behavior of this page. $js = isset($_REQUEST['js']); @@ -668,33 +669,14 @@ function flag_page($action, $flag, $content_id) { // Check the flag token, then perform the flagging. if (!flag_check_token($token, $content_id)) { - $error = t('Bad token. You seem to have followed an invalid link.'); + $errors['token'] = t('Bad token. You seem to have followed an invalid link.'); } elseif ($user->uid == 0 && !$has_js) { - $error = t('You must have JavaScript and cookies enabled in your browser to flag content.'); + $errors['javascript'] = t('You must have JavaScript and cookies enabled in your browser to flag content.'); } else { - $result = $flag->flag($action, $content_id); - if (!$result) { - $error = t('You are not allowed to flag, or unflag, this content.'); - } - } - - // If an error was received, set a message and exit. - if (isset($error)) { - if ($js) { - drupal_set_header('Content-Type: text/javascript; charset=utf-8'); - print drupal_to_js(array( - 'status' => FALSE, - 'errorMessage' => $error, - )); - exit; - } - else { - drupal_set_message($error); - drupal_access_denied(); - return; - } + // We're getting the result via the passed-by-reference variable $errors. + $flag->flag($action, $content_id, NULL, FALSE, $errors); } // If successful, return data according to the request type. @@ -702,9 +684,9 @@ function flag_page($action, $flag, $content_id) { drupal_set_header('Content-Type: text/javascript; charset=utf-8'); $flag->link_type = 'toggle'; print drupal_to_js(array( - 'status' => TRUE, - 'newLink' => $flag->theme($flag->is_flagged($content_id) ? 'unflag' : 'flag', $content_id, TRUE), + 'newLink' => $flag->theme($flag->is_flagged($content_id) ? 'unflag' : 'flag', $content_id, TRUE, $errors), // Further information for the benefit of custom JavaScript event handlers: + 'flagSuccess' => !$errors, 'contentId' => $content_id, 'contentType' => $flag->content_type, 'flagName' => $flag->name, @@ -713,8 +695,22 @@ function flag_page($action, $flag, $content_id) { exit; } else { - drupal_set_message($flag->get_label($action . '_message', $content_id)); - drupal_goto(); + if ($errors) { + // If an error was received, set a message and exit. + foreach ($errors as $error) { + drupal_set_message($error, 'error'); + } + if (isset($errors['access-denied'])) { + drupal_access_denied(); + } + else { + drupal_goto(); + } + } + else { + drupal_set_message($flag->get_label($action . '_message', $content_id)); + drupal_goto(); + } } } @@ -748,15 +744,17 @@ function flag_confirm(&$form_state, $action, $flag, $content_id) { function flag_confirm_submit(&$form, &$form_state) { $action = $form_state['values']['action']; - $flag_name = $form_state['values']['flag_name']; + $flag = flag_get_flag($form_state['values']['flag_name']); $content_id = $form_state['values']['content_id']; + $errors = array(); - $result = flag($action, $flag_name, $content_id); - if (!$result) { - drupal_set_message(t('You are not allowed to flag, or unflag, this content.')); + $flag->flag($action, $content_id, NULL, FALSE, $errors); + if ($errors) { + foreach ($errors as $error) { + drupal_set_message($error, 'error'); + } } else { - $flag = flag_get_flag($flag_name); drupal_set_message($flag->get_label($action . '_message', $content_id)); } } @@ -1123,7 +1121,7 @@ function flag_theme() { return array( 'flag' => array( - 'arguments' => array('flag' => NULL, 'action' => NULL, 'content_id' => NULL, 'after_flagging' => FALSE), + 'arguments' => array('flag' => NULL, 'action' => NULL, 'content_id' => NULL, 'after_flagging' => FALSE, 'errors' => array()), 'template' => 'flag', 'pattern' => 'flag__', 'path' => $path, @@ -1144,9 +1142,17 @@ function flag_theme() { 'arguments' => array('element' => NULL), 'file' => 'includes/flag.rules_forms.inc', ), + 'flag_errors' => array( + 'arguments' => array('errors' => array()), + ), ); } +function theme_flag_errors($errors) { + // Note: by default we can't use
because it's not valid inside . + return join('
', $errors); +} + /** * A preprocess function for our theme('flag'). It generates the * variables needed there. @@ -1167,6 +1173,7 @@ function template_preprocess_flag(&$variables) { $flag =& $variables['flag']; $action = $variables['action']; $content_id = $variables['content_id']; + $errors = $variables['errors']; $flag_css_name = str_replace('_', '-', $flag->name); // Generate the link URL. @@ -1201,13 +1208,12 @@ function template_preprocess_flag(&$variables) { $variables['link_href'] = isset($link['href']) ? check_url(url($link['href'], $link)) : FALSE; $variables['link_text'] = isset($link['title']) ? $link['title'] : $flag->get_label($action . '_short', $content_id); $variables['link_title'] = isset($link['attributes']['title']) ? check_plain($link['attributes']['title']) : check_plain(strip_tags($flag->get_label($action . '_long', $content_id))); - $variables['last_action'] = ($action == 'flag' ? 'unflagged' : 'flagged'); + $variables['status'] = ($action == 'flag' ? 'unflagged' : 'flagged'); $variables['flag_wrapper_classes_array'] = array(); $variables['flag_wrapper_classes_array'][] = 'flag-wrapper'; $variables['flag_wrapper_classes_array'][] = 'flag-' . $flag_css_name; $variables['flag_wrapper_classes_array'][] = 'flag-' . $flag_css_name . '-' . $content_id; - $variables['flag_wrapper_classes'] = implode(' ', $variables['flag_wrapper_classes_array']); $variables['flag_classes_array'] = array(); $variables['flag_classes_array'][] = 'flag'; @@ -1216,17 +1222,37 @@ function template_preprocess_flag(&$variables) { if (isset($link['attributes']['class'])) { $variables['flag_classes_array'][] = $link['attributes']['class']; } + + $variables['message_classes_array'] = array(); if ($variables['after_flagging']) { - $inverse_action = ($action == 'flag' ? 'unflag' : 'flag'); - $variables['message_text'] = $flag->get_label($inverse_action . '_message', $content_id); - $variables['flag_classes_array'][] = $variables['last_action']; + $variables['message_classes_array'][] = 'flag-message'; + if ($errors) { + $variables['message_classes_array'][] = 'flag-failure-message'; + $variables['message_text'] = theme('flag_errors', $errors); + } + else { + $inverse_action = ($action == 'flag' ? 'unflag' : 'flag'); + $variables['message_classes_array'][] = 'flag-success-message'; + $variables['message_classes_array'][] = 'flag-' . $variables['status'] . '-message'; + $variables['message_text'] = $flag->get_label($inverse_action . '_message', $content_id); + $variables['flag_classes_array'][] = $variables['status']; + // By default we make our JS code remove, after a few seconds, only success messages. + $variables['message_classes_array'][] = 'flag-auto-remove'; + } } + else { + $variables['message_text'] = ''; + } + + $variables['flag_wrapper_classes'] = implode(' ', $variables['flag_wrapper_classes_array']); $variables['flag_classes'] = implode(' ', $variables['flag_classes_array']); + $variables['message_classes'] = join(' ', $variables['message_classes_array']); // Backward compatibility: if the user has an old flag.tpl.php, we don't want - // it to generate E_NOTICEs, so we make sure this variable exists there. + // it to generate E_NOTICEs, so we make sure these variables exist there. // @todo: Remove this sometime. $variables['setup'] = FALSE; + $variables['last_action'] = $variables['status']; } /** diff --git theme/flag.js theme/flag.js index 6f3cb79..06bbcdf 100644 --- theme/flag.js +++ theme/flag.js @@ -44,7 +44,7 @@ Drupal.flagLink = function(context) { Drupal.attachBehaviors($newLink.get(0)); $('.flag-message', $newLink).fadeIn(); - setTimeout(function(){ $('.flag-message', $newLink).fadeOut() }, 3000); + setTimeout(function(){ $('.flag-message.flag-auto-remove', $newLink).fadeOut() }, 3000); return $newLink.get(0); } @@ -76,20 +76,12 @@ Drupal.flagLink = function(context) { data: { js: true }, dataType: 'json', success: function (data) { - if (data.status) { - // Success. - data.link = $wrapper.get(0); - $.event.trigger('flagGlobalBeforeLinkUpdate', [data]); - if (!data.preventDefault) { // A handler may cancel updating the link. - data.link = updateLink(element, data.newLink); - } - $.event.trigger('flagGlobalAfterLinkUpdate', [data]); - } - else { - // Failure. - alert(data.errorMessage); - $wrapper.removeClass('flag-waiting'); + data.link = $wrapper.get(0); + $.event.trigger('flagGlobalBeforeLinkUpdate', [data]); + if (!data.preventDefault) { // A handler may cancel updating the link. + data.link = updateLink(element, data.newLink); } + $.event.trigger('flagGlobalAfterLinkUpdate', [data]); }, error: function (xmlhttp) { alert('An HTTP error '+ xmlhttp.status +' occurred.\n'+ element.href); diff --git theme/flag.tpl.php theme/flag.tpl.php index 089a569..3d140db 100644 --- theme/flag.tpl.php +++ theme/flag.tpl.php @@ -15,14 +15,15 @@ * - $flag_classes: A space-separated list of CSS classes that should be applied to the link. * * - $action: The action the link is about to carry out, either "flag" or "unflag". - * - $last_action: The action, as a passive English verb, either "flagged" or - * "unflagged", that led to the current status of the flag. + * - $status: The status of the item; either "flagged" or "unflagged". * * - $link_href: The URL for the flag link. * - $link_text: The text to show for the link. * - $link_title: The title attribute for the link. * * - $message_text: The long message to show after a flag action has been carried out. + * - $message_classes: A space-separated list of CSS classes that should be applied to + * the message. * - $after_flagging: This template is called for the link both before and after being * flagged. If displaying to the user immediately after flagging, this value * will be boolean TRUE. This is usually used in conjunction with immedate @@ -39,7 +40,7 @@ - +