Creating a hook, allowing other modules to add functionality

+function privatemsg_module_invoke($op, &$array) {
+  foreach (module_list() as $module) {
+    $function = $module .'_privatemsg_' .$op;
+    if (function_exists($function)) {
+      $function($type, $array);
+    }
+  }
+}

I like where this is going. lets make a list for all possible places where we would want to place this API.

I would mind having things like 'pre_send' and 'after_send' but we have to be careful not to go overboard.

I already thought about this in #288183: Provide api function for other modules to send messages. and started to implement it (Haven't touched privatemsg.module, only as part of the api functions)

Is there a reason why "drupal_alter('privatemsg_message_insert', $message); wouldn't work ? => http://api.drupal.org/api/function/drupal_alter/6
Looks like re-inventing the wheel for me....

Some ideas I had about hooks (names are from http://api.drupal.org/api/function/hook_nodeapi)

- presave and insert, obvious...

- validate, I think that would be pretty useful, it would allow generic validation routines to hook in like messaging quota. Could also possibly replace/extend the current user block mechanism which is limited to one user at a time.

- delete, would for example allow to remove added tags. But would need that we have to load the message before deleting it...

I am unsure about the differences between our dedicated API and the use of drupal_alter(...).

Immediate thought that jumps out is that drupal_alter is used to alter data, not to signal an action. we could use drupal_alter before executing some privatemsgAPI.

validate op means different things when validating for display or for insert. deletes we have single message delete, thread delete. we have user blocks/unblocks. filter criterion (not sure about this one). what else?

From what I understand, drupal_alter is the same as module_invoke_all but with the ability to manipulate the parameters, meaning they are given by reference, that's the same as the current patch is doing it: privatemsg_module_invoke($op, &$array). All the other code of drupal_alter is just a hack to support more than one by reference parameters. If we don't need by reference, why not simply use module_invoke_all ?

I think, validate should be before the data is saved, handling of data to be displayed is already done with the filter.

Some more...

Naming: My idea was privatemsg_element_op, where element is the thing that is worked on... message, thread, maybe others..

- We (aka nbz) are already using "privatemsg_pm_controls", to add actions to messages like delete/block. Works fine, only thing is maybe to rename it, see above

- maybe we would need to have a hook for message and thread deletion, for example privatemsg_message_delete and privatemsg_thread_delete. However, this would make deleting messages quite expensive because it always needs a query to get the message and a second query to check if it is the last of that thread and if yes, to fire the privatemsg_thread_delete hook.

- validate: This is my current implementation for the API I'm working on...

  $errors = module_invoke_all('privatemsg_message_validate', $message);
  if (!empty($errors)) {
    foreach ($errors as $error) {
      watchdog('privatemsg', $error, FALSE, WATCHDOG_NOTICE);
    }
    return FALSE;
  }

So a module can hook in, validate whatever it want's and return a string or array (multiple errors). If atleast one error is returned, they are logged (remember, this is api, the form validation could display them via form_set_error ) and false is returned. Problem: Either we can't change $message (module_invoke_all), this would make it impossible to remove some recipients for example or we can't return anyting (drupal_alter), but we could use a specific array key (for example '__validate_errors') to handle validation errors.

Imho, filtering should be done as we are doing it now, by directly changing the sql query via query builder. filtering already loaded message would break paging and it also wouldn't be good for perfomance, if a user filters everything out except the last of his 1000 messages)

I think instead of storing validation errors in watchdog, those errors should be returned. validation api should understand that if a non empty array was returned then it means there are errors. returning those errors would allow user of the api to decide whether to drupal_Set_message or write to watchdog or what ever.

i dont like using watchdog for this case because its not really a thing that should be notified to site admin if some validation returns false, for example if user is blocked and some one is attempting to send message to him.

I'm going to close this one as duplicate of #288183: Provide api function for other modules to send messages..