Use of hook_message_alter() per the canned example yields a PHP error and abend once the Notifications and Messaging modules are installed:

function MYMODULE_message_alter(&$messages) {
  $matches = $messages->match('/Text I am trying to remove/');
  $messages->remove($matches);
}

Result:

Fatal error: Call to undefined method stdClass::match() in /path/to/my/custom.module on line 13

A print_r of $messages reveals that the Messaging module seriously mucks with the $messages object, and likely overrides or clears out the match() method.

For now, I have changed my code to check for the object method ahead of its use:

  if (method_exists($messages, 'match')) {
    $matches = $messages->match('/Text I am trying to remove/');
    $messages->remove($matches);
  }

Please let me know if I can provide further analysis.

CommentFileSizeAuthor
#6 messages-alter-modify-hook-1234982-6.patch456 bytesthedavidmeister
#3 namespace-messages-alter-1234982-2.patch2.56 KBthedavidmeister
#1 namespace-messages-alter-1234982-1.patch3.27 KBthedavidmeister

Comments

thedavidmeister’s picture

thedavidmeister commented
StatusFileSize
new namespace-messages-alter-1234982-1.patch3.27 KB

Here's a patch of me doing a find and replace over the whole project for "$messages" with "$messages_alter".

This should resolve the namespace issue, but will obviously require that any existing custom implementations relying on this API are rewritten.

Obviously I'll need to actually test to make sure I didn't break something, but I doubt it would.

Seems strange that something like this wasn't done a long time ago, since there are now 3 versions of the MessagesAlter class distributed with the module.

On a side note, wouldn't it be better to work towards a new major release of the module if you're changing the API enough to release a new class?

BartonDrupal’s picture

BartonDrupal commented

Thanks for the patch. I must have been pretty busy. I didn't see this issue from back in July. I'm not a fan of find & replace solutions; although I'm sure your solution could work, I haven't read through your code. I know the issue deals with another contributed module using the hook_message_alter. These things happen from time to time. The fix would need to be to change that hook and then document the upgrade path for module developers that have used this module in the past, since a change like this will render their modules useless. I'm totally booked until wed. Hopefully this can wait a little bit longer :)

thedavidmeister’s picture

thedavidmeister commented
StatusFileSize
new namespace-messages-alter-1234982-2.patch2.56 KB

patch for version 1.2 of messages alter

BartonDrupal’s picture

BartonDrupal commented

The only line of code that actually would need to be changed is the line where this is at:

drupal_alter('message', $messages);

to

drupal_alter('status_message', $messages);

And that would call functions like this:

function MYMODULE_status_message_alter(&$message) { /** TODO: left as homework :) **/ }

And then of course document this so that people see it, otherwise, people will upgrade and get pissed that their old modules won't work. Ideally, I would add more code to alert the developer about what is going on. Installs that have the Notifications & Messaging modules wouldn't work anyway or would have hacked solutions since this module wouldn't work with those modules.

Either way, more needs to happen to make everyone happy.

thedavidmeister’s picture

thedavidmeister commented
Status: Active » Needs work

yes, well luckily I read through the issue queue before I started using the module so I had a fresh slate to work from :) I'm aware that many existing users wouldn't be so lucky.

also, you're probably right about what I did being overkill but it does make sense to me that the name of the object is the same as the name of the project.

the patch that I made works nicely for my use case. I'll just change the status to "needs work" as what I did does fix the problem, but there's more to it than that from an organizational perspective as you noted.

btw - props for the super fast response!

thedavidmeister’s picture

thedavidmeister commented
StatusFileSize
new messages-alter-modify-hook-1234982-6.patch456 bytes

here's the suggestion from #4 in patch form btw..

thedavidmeister’s picture

thedavidmeister commented

incidentally, my find and replace approach sucked in a fairly predictable way. I changed my setup slightly and hit problems, the patch in #6 is better but it does require you to rename your hook functions.

dstol’s picture

dstol
Primary language English
commented

This is related to #1955630: Name space collision with the messaging module.

Long story short, both modules declare the same hook.

thedavidmeister’s picture

thedavidmeister commented

#8 - yup, and the patch changes the hook from hook_message_alter() (bad!) to hook_status_message_alter() which is still a bit bad as it could potentially collide with http://drupal.org/project/status_message but it's way better than what it was.