notification method is_available() (possible) bug

Discuss general development subjects that are not specific to a particular version like the versioning control system we use or other infrastructure.
Post Reply
Senky
Extension Customisations
Extension Customisations
Posts: 312
Joined: Thu Jul 16, 2009 4:41 pm

notification method is_available() (possible) bug

Post by Senky » Sat Jan 12, 2019 10:09 am

Hi everyone,
I want to discuss usage of is_available() in notification methods. So far there is no method that uses auth class. So everything works normally, but I have added a new method which now looks like this:

Code: Select all

...
public function is_available()
{
	return $this->auth->acl_get('...');
}
...
So I wondered why it didn't send the notification sometimes and it turned out that is_available() is used in \phpbb\notification\manager::get_default_methods(). This method is used in almost all notification types to decide what notification methods will be used to notify users.

Now the problem is, that get_default_methods() uses sender's auth to decide what methods should receiver get. Therefore if sender is not allowed to use the method, receiver won't get the notification.

Can anyone tell me if this is really desired state and I should not be using auth in is_available() or it's a bug but no one cared yet because no one used auth yet?

User avatar
kasimi
Extension Customisations
Extension Customisations
Posts: 16
Joined: Thu May 24, 2012 1:54 pm

Re: notification method is_available() (possible) bug

Post by kasimi » Sat Jan 26, 2019 6:24 pm

This looks like by design to me. The auth instance you get from the container always belongs to the user of the current request. The notification methods simply weren't built with support for user-depending availability in mind. :|

Senky
Extension Customisations
Extension Customisations
Posts: 312
Joined: Thu Jul 16, 2009 4:41 pm

Re: notification method is_available() (possible) bug

Post by Senky » Mon Feb 18, 2019 12:24 pm

Oh I see that. But the semantics of the code clearly shows that receiver is the subject here. So I created the PR that at least somehow deals with it (by exposing receiver user ID):
https://github.com/phpbb/phpbb/pull/5511

Post Reply