Implement hook_flag_alter

This patch doesn't seem correct. If possible, it'd be better to only call drupal_alter() in one place, though I admit it doesn't look like there's a real good place to do this currently, unless we add another loop at the bottom of the flag_get_flags() function to do the drupal_alter(). Either way though, the current location of the drupal_alter() calls is not correct, as the first call doesn't even get a real flag, it just gets a database row.

A more powerful approach is to let programmers alter the handlers' definitions. In other words, to put the following in flag_fetch_definition():

drupal_alter('flag_definitions', $cache);

(The "$cache" variable, in that funciton, should be renamed to "$definitions", for clarity.)

It's a more powerful approach because it lets you alter the behavior of flags. Not just their data.

For example, in your module you could then do:

function MYMODULE_alter_flag_definitions(&$definitions) {
  $definitions['node']['handler'] = 'a_better_node_handler';
}

class a_better_node_hanlder extends flag_node {

  function flag(...) {
    drupal_set_message("I'm flagging something!");
    parent::flag(...);
  }

  // You can override/augment any other method. 
}

It was my plan all along to add this feature, so:

+1

$definitions['node']['handler'] = 'a_better_node_handler';

Tomorrow, I hope, I'll write about one problem in this scheme.

$definitions['node']['handler'] = 'a_better_node_handler';

Tomorrow, I hope, I'll write about one problem in this scheme.

Oh, I'll make it short:

With the above scheme, only one module could override the handler class name.

(The real culprit is PHP's lousy object model, which provides us with very little power, if at all. It's beyond my understanding how some programmers believe PHP5 is somehow better than PHP3.)

$definitions['node']['handler'] = 'a_better_node_handler';

Tomorrow, I hope, I'll write about one problem in this scheme.

Oh, I'll make it short:

With the above scheme, only one module could override the handler class name.

(The real culprit is PHP's lousy object model, which [...]

I want to retract some of my criticism of PHP. PHP5 provides us with some features that enable us to implement a delegator (sometimes called a "proxy").

Oh, I'll make it short:

With the above scheme, only one module could override the handler class name.

So this is not true, because modules could install a delegator on $definitions['node']['handler']. When several modules do this simultanously, a chain will be formed, and everybody will be happy.

Here's the example from comment #3, but now revised to use a delegator:

// A base for a delegating object. Your object must have 
// a $delegated_object property that points to the delegated
// object.
//
class MyDelegator {

  function __call($method, $args) {
    return call_user_func_array(array($this->delegated_object, $method), $args);
  }
  
  function &__get($var) {
    if ($var == 'delegated_object') {
      return $this->delegated_object;
    } else {
      return $this->delegated_object->$var;
    }
  }

  function __set($var, $val) {
    if ($var == 'delegated_object') {
      $this->delegated_object = $val;
    } else {
      $this->delegated_object->$var = $val;
    }
  }
  
  function __isset($var) { return isset($this->delegated_object->$var); }
}

// Our own flag handler. It will replace Flag's own node handler
// (or some other module's handler) by way of delegation.
//
class a_better_node_hanlder extends MyDelegator {

  public static $delegated_class = NULL;
  
  function __construct() {
    $this->delegated_object = new self::$delegated_class;
  }
  
  //
  // Here's how to override the flag() method.
  //
  function flag(...) {
    drupal_set_message("I'm flagging something!");
    return $this->delegated_object->flag(...);
  }
  
}

// Implementation of hook_alter_flag_definitions().
// We are installing our own node flag handler here.
//
function MYMODULE_alter_flag_definitions(&$definitions) {
  a_better_node_handler::$delegated_class = $definitions['node']['handler'];
  $definitions['node']['handler'] = 'a_better_node_handler';
}

(The code was not tested, but it's largly correct.)

Marked #347082: Add hook_flag_alter() as a duplicate of this issue.

mooffie: any reason why we shouldn't add that patch as-is? Considering delegators are PHP 5 only, and current knowledge of how to use them is pretty minimal (self included). Contrary to hook_flag_alter(), which is a well-understood convention in Drupal.

I'm personally okay with #2.

Committed #2 to the 2.x version.