Implement hook_flag_alter

aaron - April 16, 2009 - 18:26
Project:Flag
Version:6.x-2.x-dev
Component:User interface
Category:feature request
Priority:critical
Assigned:Unassigned
Status:closed
Description

For modules such as http://drupal.org/project/flag_form to work properly, they need to be able to modify a flag. For instance, in this particular case, I need to store information regarding a specific flag. For now, I have to do some heavy work-arounds since flag doesn't natively allow modification of its data.

This patch implements drupal_alter, so that any module can implement hook_flag_alter(&$flag) to do what it needs to do.

AttachmentSize
flag-alter.patch1.25 KB

#1

quicksketch - April 16, 2009 - 19:04
Status:needs review» needs work

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.

#2

aaron - April 16, 2009 - 19:50
Status:needs work» needs review

well, here it is with a loop then.

AttachmentSize
flag-alter.patch 654 bytes

#3

mooffie - April 20, 2009 - 01:57

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

#4

mooffie - April 20, 2009 - 02:49

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

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

#5

mooffie - April 20, 2009 - 03:16

$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.)

#6

mooffie - April 29, 2009 - 23:10

$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:

<?php
// 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.)

#7

opensanta - May 17, 2009 - 16:28
Component:Code» User interface

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

#8

quicksketch - May 18, 2009 - 19:11

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.

#9

quicksketch - September 14, 2009 - 14:21
Version:6.x-1.x-dev» 6.x-2.x-dev
Status:needs review» fixed

Committed #2 to the 2.x version.

#10

System Message - September 28, 2009 - 14:30
Status:fixed» closed

Automatically closed -- issue fixed for 2 weeks with no activity.

 
 

Drupal is a registered trademark of Dries Buytaert.