Error when approve friend request

The main difference between empty-isset and current implementation that first check value for existence!
When you check array value by key $token = $_REQUEST['token']; and token does not exists in $_REQUEST php throws notice which show in development and thrashes php_error_log on production!!!

Next - empty() checks value but isset() checks existence - in current patch suppose better to check existence

About $user - broken only this logic branch:

function flag_friend_unfriend($event, $flag, $content_id, $account, $status) {

  // 'Denial' and 'Pending - Cancel?'
  if ($event == 'unflag') {
    if ($status == FLAG_FRIEND_APPROVAL) {
      // Denial - the content_id is actually the account param in this case
      $account = user_load(array('uid' => $content_id));
      // and the $user->uid is actually the content(_id) we're unflagging
      $content_id = $user->uid; //---------------------- this sets to NULL

      // Fire trigger
      module_invoke_all('flag_friend', 'deny', $user, $account); //-----------this $user is NULL
    }
    // remove any messages
    flag_friend_message($event, $flag, $content_id, $account->uid); //----possible this $content_id is NULL
    // unflag
    $flag->flag($event, $content_id, $account); //--------possible $content_id is NULL
  }

I dont know how it was working before the patch but it is really a BUG

This bug leads to flag_message table not cleared on friend/unfriend

About $token

code in flag.module set no token on confirm form! so notice throws everytime

function flag_flag_link(&$flag, $action, $content_id) {
  $token = flag_get_token($content_id);
  return array(
    'href' => "flag/". ($flag->link_type == 'confirm' ? 'confirm/' : '') ."$action/$flag->name/$content_id",
    'query' => drupal_get_destination() . ($flag->link_type == 'confirm' ? '' : '&token='. $token),
  );
}

After little debugging code I find why message was not cleared and bring error.

1) there was no call to flag_friend_message() in submit when user cancel own request
2) when someone aprove request message was stored in DB oposite to be cleared (so every second and next approve throws a error #435448: error message when adding friends)
3) flag_friend_unfriend_form() always determines wrong status

This patch is little complex but all this fixes should go together.

PS: here 2 fixed notices, as I describe before - they are very noisy when you debug.

About hunk #3

1) Take a look at flag_friend_form_flag_confirm_alter()

'flag_friend_message' added only when flag (first request) and approve (to send a message) all other states have no this key in form array so not error but notice is generated and isset() just checking "is message input in form" - this is a cases: pending and approve.

2) Next $status === FLAG_FRIEND_PENDING - because only pending and approve states come inside but only pending state should clear the message! If you leave this as it now (!== _BOTH) code will insert message with fcid=0 which throw an error "duplicate entry" (on approve)

3) elseif ($status === FLAG_FRIEND_PENDING) { additional branch for clearing {flag_message} when sender cancels own request.

Suppose we need someone with debugger who will take a debug view at this ATM. I've done this already - so please leave status of this issue "need review"

Maybe you have no time to dig into this piece of code too closely...

Still not agree with leaving

-    if ($status !== FLAG_FRIEND_BOTH) {
+    // Record message only if user pending request.
+    if ($status === FLAG_FRIEND_PENDING) {

This should be fired only in FLAG_FRIEND_PENDING but #message still not empty when decline - and this bring error.

This still not fixes a bug with inserting empty messages

As I write before - empty() does not check for existence - this is a case when user send invite or other side approve request (only in this states form has #message) so no matter is it empty or not we just need to send email and clear the message if PENDING not !=BOTH

Changing title to be more clear about patch